[gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Reinis Danne]

Hi!

I have cleaned up a bit alternatives-2.eclass to make it easier to review.
Also if someone knows any shortcommings of this eclass as it is, then point
it out, I'll try to address them.

I hope this can be soon moved to the main tree thus closing the divide between
science overlay and portage tree.

Reinis Danne (10):
  alternatives-2.eclass: Update copyright
  alternatives-2.eclass: Remove unused variables
  alternatives-2.eclass: Move EXPORT_FUNCTIONS to top
  alternatives-2.eclass: Remove commented-out code
  alternatives-2.eclass: Put commonly used path in local variable
  alternatives-2.eclass: Fix eclass name in comments
  alternatives-2.eclass: Add EAPI check
  alternatives-2.eclass: Add documentation comments
  alternatives-2.eclass: Minor comment changes
  alternatives-2.eclass: Use consistent quoting

 eclass/alternatives-2.eclass | 110 +++++++++++++++++++++++++++++++------------
 1 file changed, 79 insertions(+), 31 deletions(-)

-- 
1.8.5.3

[gentoo-science] [PATCH 01/10] alternatives-2.eclass: Update copyright

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 38a0328..5c42298 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -1,6 +1,6 @@
-# Copyright 2010-2013 Gentoo Foundation
+# Copyright 2010-2014 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
-#  $
+# $Header: $
 
 # Based in part upon 'alternatives.exlib' from Exherbo, which is:
 # Copyright 2008, 2009 Bo Ørsted Andresen
-- 
1.8.5.3

[gentoo-science] [PATCH 02/10] alternatives-2.eclass: Remove unused variables

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 5c42298..c466eed 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -23,7 +23,7 @@ alternatives_for() {
 	#echo alternatives_for "${@}"
 
 	(( $# >= 5 )) && (( ($#-3)%2 == 0)) || die "${FUNCNAME} requires exactly 3+N*2 arguments where N>=1"
-	local x dupl alternative=${1} provider=${2} importance=${3} index unique src target ret=0
+	local alternative=${1} provider=${2} importance=${3} index src target ret=0
 	shift 3
 
 	# make sure importance is a signed integer
@@ -139,7 +139,7 @@ alternatives-2_pkg_postinst() {
 }
 
 alternatives-2_pkg_prerm() {
-	local a alt provider p ignore
+	local a alt provider ignore
 	[[ -n ${REPLACED_BY_ID} ]] || ignore=" --ignore"
 	for a in "${ALTERNATIVES_PROVIDED[@]}"; do
 		alt="${a%:*}"
-- 
1.8.5.3

[gentoo-science] [PATCH 03/10] alternatives-2.eclass: Move EXPORT_FUNCTIONS to top

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index c466eed..a431d26 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -18,6 +18,8 @@ RDEPEND="${DEPEND}
 	!app-admin/eselect-cblas
 	!app-admin/eselect-lapack"
 
+EXPORT_FUNCTIONS pkg_postinst pkg_prerm
+
 # alternatives_for alternative provider importance source target [ source target [...]]
 alternatives_for() {
 	#echo alternatives_for "${@}"
@@ -156,5 +158,3 @@ alternatives-2_pkg_prerm() {
 		fi
 	done
 }
-
-EXPORT_FUNCTIONS pkg_postinst pkg_prerm
-- 
1.8.5.3

[gentoo-science] [PATCH 04/10] alternatives-2.eclass: Remove commented-out code

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index a431d26..98317dd 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -22,7 +22,6 @@ EXPORT_FUNCTIONS pkg_postinst pkg_prerm
 
 # alternatives_for alternative provider importance source target [ source target [...]]
 alternatives_for() {
-	#echo alternatives_for "${@}"
 
 	(( $# >= 5 )) && (( ($#-3)%2 == 0)) || die "${FUNCNAME} requires exactly 3+N*2 arguments where N>=1"
 	local alternative=${1} provider=${2} importance=${3} index src target ret=0
@@ -115,7 +114,6 @@ alternatives-2_pkg_postinst() {
 		if [[ ! -f "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" \
 			|| "$(source "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" &>/dev/null; echo "${VERSION}")" \
 				-ne "${module_version}" ]]; then
-			#einfo "Creating alternatives module for ${alt}"
 			if [[ ! -d ${EROOT%/}/usr/share/eselect/modules/auto ]]; then
 				install -d "${EROOT%/}"/usr/share/eselect/modules/auto || eerror "Could not create eselect modules dir"
 			fi
@@ -132,7 +130,6 @@ alternatives-2_pkg_postinst() {
 			EOF
 		fi
 
-		#echo eselect "${alt}" update "${provider}"
 		einfo "Creating ${provider} alternative module for ${alt}"
 		eselect "${alt}" update "${provider}"
 
@@ -146,8 +143,6 @@ alternatives-2_pkg_prerm() {
 	for a in "${ALTERNATIVES_PROVIDED[@]}"; do
 		alt="${a%:*}"
 		provider="${a#*:}"
-		#echo "Making sure ${alt} has a valid provider"
-		#echo eselect "${alt}" update${ignore} "${provider}"
 		eselect "${alt}" update${ignore} "${provider}" && continue
 		einfo "Removed ${provider} alternative module for ${alt}, current is $(eselect ${alt} show)"
 		if [[ $? -eq 2 ]]; then
-- 
1.8.5.3

[gentoo-science] [PATCH 05/10] alternatives-2.eclass: Put commonly used path in local variable

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 98317dd..4366601 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -108,16 +108,17 @@ cleanup_old_alternatives_module() {
 
 alternatives-2_pkg_postinst() {
 	local a alt provider module_version="20090908"
+	local EAUTO="${EROOT%/}/usr/share/eselect/modules/auto"
 	for a in "${ALTERNATIVES_PROVIDED[@]}"; do
 		alt="${a%:*}"
 		provider="${a#*:}"
-		if [[ ! -f "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" \
-			|| "$(source "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" &>/dev/null; echo "${VERSION}")" \
+		if [[ ! -f "${EAUTO}/${alt}.eselect" \
+			|| "$(source "${EAUTO}/${alt}.eselect" &>/dev/null; echo "${VERSION}")" \
 				-ne "${module_version}" ]]; then
-			if [[ ! -d ${EROOT%/}/usr/share/eselect/modules/auto ]]; then
-				install -d "${EROOT%/}"/usr/share/eselect/modules/auto || eerror "Could not create eselect modules dir"
+			if [[ ! -d ${EAUTO} ]]; then
+				install -d "${EAUTO}" || eerror "Could not create eselect modules dir"
 			fi
-			cat > "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" <<-EOF
+			cat > "${EAUTO}/${alt}.eselect" <<-EOF
 				# This module was automatically generated by alternatives.eclass
 				DESCRIPTION="Alternatives for ${alt}"
 				VERSION="${module_version}"
@@ -139,6 +140,7 @@ alternatives-2_pkg_postinst() {
 
 alternatives-2_pkg_prerm() {
 	local a alt provider ignore
+	local EAUTO="${EROOT%/}/usr/share/eselect/modules/auto"
 	[[ -n ${REPLACED_BY_ID} ]] || ignore=" --ignore"
 	for a in "${ALTERNATIVES_PROVIDED[@]}"; do
 		alt="${a%:*}"
@@ -147,9 +149,9 @@ alternatives-2_pkg_prerm() {
 		einfo "Removed ${provider} alternative module for ${alt}, current is $(eselect ${alt} show)"
 		if [[ $? -eq 2 ]]; then
 			einfo "Cleaning up unused alternatives module for ${alt}"
-			echo rm "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect"
-			rm "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" ||
-				eerror rm "${EROOT%/}/usr/share/eselect/modules/auto/${alt}.eselect" failed
+			echo rm "${EAUTO}/${alt}.eselect"
+			rm "${EAUTO}/${alt}.eselect" || \
+				eerror rm "${EAUTO}/${alt}.eselect" failed
 		fi
 	done
 }
-- 
1.8.5.3

[gentoo-science] [PATCH 06/10] alternatives-2.eclass: Fix eclass name in comments

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 4366601..9dc49af 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -8,7 +8,7 @@
 # Copyright 2009 David Leverton
 
 # If your package provides pkg_postinst or pkg_prerm phases, you need to be
-# sure you explicitly run alternatives_pkg_{postinst,prerm} where appropriate.
+# sure you explicitly run alternatives-2_pkg_{postinst,prerm} where appropriate.
 
 ALTERNATIVES_DIR="/etc/env.d/alternatives"
 
@@ -119,7 +119,7 @@ alternatives-2_pkg_postinst() {
 				install -d "${EAUTO}" || eerror "Could not create eselect modules dir"
 			fi
 			cat > "${EAUTO}/${alt}.eselect" <<-EOF
-				# This module was automatically generated by alternatives.eclass
+				# This module was automatically generated by alternatives-2.eclass
 				DESCRIPTION="Alternatives for ${alt}"
 				VERSION="${module_version}"
 				MAINTAINER="eselect@gentoo.org"
-- 
1.8.5.3

[gentoo-science] [PATCH 07/10] alternatives-2.eclass: Add EAPI check

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 9dc49af..5128e11 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -10,6 +10,17 @@
 # If your package provides pkg_postinst or pkg_prerm phases, you need to be
 # sure you explicitly run alternatives-2_pkg_{postinst,prerm} where appropriate.
 
+case "${EAPI:-0}" in
+	0|1|2|3)
+		die "Unsupported EAPI=${EAPI:-0} (too old) for ${ECLASS}"
+		;;
+	4|5)
+		;;
+	*)
+		die "Unsupported EAPI=${EAPI} (unknown) for ${ECLASS}"
+		;;
+esac
+
 ALTERNATIVES_DIR="/etc/env.d/alternatives"
 
 DEPEND=">=app-admin/eselect-1.4-r100"
-- 
1.8.5.3

[gentoo-science] [PATCH 08/10] alternatives-2.eclass: Add documentation comments

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index 5128e11..e15edfa 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -7,6 +7,12 @@
 # Copyright 2008, 2009 Mike Kelly
 # Copyright 2009 David Leverton
 
+# @ECLASS: alternatives-2
+# @MAINTAINER:
+# Gentoo Science Project <sci@gentoo.org>
+# @BLURB: Manage alternative implementations.
+# @DESCRIPTION:
+#
 # If your package provides pkg_postinst or pkg_prerm phases, you need to be
 # sure you explicitly run alternatives-2_pkg_{postinst,prerm} where appropriate.
 
@@ -21,8 +27,6 @@ case "${EAPI:-0}" in
 		;;
 esac
 
-ALTERNATIVES_DIR="/etc/env.d/alternatives"
-
 DEPEND=">=app-admin/eselect-1.4-r100"
 RDEPEND="${DEPEND}
 	!app-admin/eselect-blas
@@ -31,9 +35,24 @@ RDEPEND="${DEPEND}
 
 EXPORT_FUNCTIONS pkg_postinst pkg_prerm
 
-# alternatives_for alternative provider importance source target [ source target [...]]
-alternatives_for() {
+# @ECLASS-VARIABLE: ALTERNATIVES_DIR
+# @INTERNAL
+# @DESCRIPTION:
+# Alternatives directory with symlinks managed by eselect.
+ALTERNATIVES_DIR="/etc/env.d/alternatives"
 
+# @FUNCTION: alternatives_for
+# @USAGE: alternative provider importance source target [source target [...]]
+# @DESCRIPTION:
+# Set up alternative provider.
+#
+# EXAMPLE:
+# @CODE
+# alternatives_for cblas atlas 0 \
+#     /usr/$(get_libdir)/pkgconfig/cblas.pc atlas-cblas.pc \
+#     /usr/include/cblas.h atlas/cblas.h
+# @CODE
+alternatives_for() {
 	(( $# >= 5 )) && (( ($#-3)%2 == 0)) || die "${FUNCNAME} requires exactly 3+N*2 arguments where N>=1"
 	local alternative=${1} provider=${2} importance=${3} index src target ret=0
 	shift 3
@@ -106,6 +125,10 @@ alternatives_for() {
 	[[ ${ret} -eq 0 ]] || die "Errors detected for ${provider}, provided for ${alternative}"
 }
 
+# @FUNCTION: cleanup_old_alternatives_module
+# @USAGE: alternative
+# @DESCRIPTION:
+# Remove old alternatives module.
 cleanup_old_alternatives_module() {
 	local alt=${1} old_module="${EROOT%/}/usr/share/eselect/modules/${alt}.eselect"
 	if [[ -f "${old_module}" && "$(source "${old_module}" &>/dev/null; echo "${ALTERNATIVE}")" == "${alt}" ]]; then
@@ -117,6 +140,11 @@ cleanup_old_alternatives_module() {
 	fi
 }
 
+# @FUNCTION: alternatives-2_pkg_postinst
+# @DESCRIPTION:
+# Create eselect modules for all provided alternatives.
+#
+# Provided alternatives are set up using alternatives_for().
 alternatives-2_pkg_postinst() {
 	local a alt provider module_version="20090908"
 	local EAUTO="${EROOT%/}/usr/share/eselect/modules/auto"
@@ -149,6 +177,11 @@ alternatives-2_pkg_postinst() {
 	done
 }
 
+# @FUNCTION: alternatives-2_pkg_prerm
+# @DESCRIPTION:
+# Remove autogenerated eselect modules for all alternatives.
+#
+# Provided alternatives are set up using alternatives_for().
 alternatives-2_pkg_prerm() {
 	local a alt provider ignore
 	local EAUTO="${EROOT%/}/usr/share/eselect/modules/auto"
-- 
1.8.5.3

[gentoo-science] [PATCH 09/10] alternatives-2.eclass: Minor comment changes

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index e15edfa..d9896f1 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -57,22 +57,26 @@ alternatives_for() {
 	local alternative=${1} provider=${2} importance=${3} index src target ret=0
 	shift 3
 
-	# make sure importance is a signed integer
+	# Make sure importance is a signed integer
 	if [[ -n ${importance} ]] && ! [[ ${importance} =~ ^[0-9]+(\.[0-9]+)*$ ]]; then
 		eerror "Invalid importance (${importance}) detected"
 		((ret++))
 	fi
 
+	# Create alternative provider subdirectories under ALTERNATIVES_DIR if needed
 	[[ -d "${ED}${ALTERNATIVES_DIR}/${alternative}/${provider}" ]] || dodir "${ALTERNATIVES_DIR}/${alternative}/${provider}"
 
-	# keep track of provided alternatives for use in pkg_{postinst,prerm}. keep a mapping between importance and
-	# provided alternatives and make sure the former is set to only one value
+	# Keep track of provided alternatives for use in pkg_{postinst,prerm}.
+	# Keep a mapping between importance and provided alternatives
+	# and make sure the former is set to only one value.
 	if ! has "${alternative}:${provider}" "${ALTERNATIVES_PROVIDED[@]}"; then
+		# Add new provider and set its importance
 		index=${#ALTERNATIVES_PROVIDED[@]}
 		ALTERNATIVES_PROVIDED+=( "${alternative}:${provider}" )
 		ALTERNATIVES_IMPORTANCE[index]=${importance}
 		[[ -n ${importance} ]] && echo "${importance}" > "${ED}${ALTERNATIVES_DIR}/${alternative}/${provider}/_importance"
 	else
+		# Set importance for existing provider
 		for((index=0;index<${#ALTERNATIVES_PROVIDED[@]};index++)); do
 			if [[ ${alternative}:${provider} == ${ALTERNATIVES_PROVIDED[index]} ]]; then
 				if [[ -n ${ALTERNATIVES_IMPORTANCE[index]} ]]; then
@@ -88,6 +92,7 @@ alternatives_for() {
 		done
 	fi
 
+	# Process source-target pairs
 	while (( $# >= 2 )); do
 		src=${1//+(\/)/\/}; target=${2//+(\/)/\/}
 		if [[ ${src} != /* ]]; then
@@ -107,8 +112,9 @@ alternatives_for() {
 			dodir "${ALTERNATIVES_DIR}/${alternative}/${provider}${src%/*}"
 			dosym "${reltarget}" "${ALTERNATIVES_DIR}/${alternative}/${provider}${src}"
 
-			# say ${ED}/sbin/init exists and links to /bin/systemd (which doesn't exist yet)
-			# the -e test will fail, so check for -L also
+			# The -e test will fail if existing symlink points to non-existing target,
+			# so check for -L also.
+			# Say ${ED}/sbin/init exists and links to /bin/systemd (which doesn't exist yet).
 			if [[ -e ${ED}${src} || -L ${ED}${src} ]]; then
 				local fulltarget=${target}
 				[[ ${fulltarget} != /* ]] && fulltarget=${src%/*}/${fulltarget}
@@ -122,6 +128,7 @@ alternatives_for() {
 		shift 2
 	done
 
+	# Stop if there were any errors
 	[[ ${ret} -eq 0 ]] || die "Errors detected for ${provider}, provided for ${alternative}"
 }
 
-- 
1.8.5.3

[gentoo-science] [PATCH 10/10] alternatives-2.eclass: Use consistent quoting

[Reinis Danne]

---
 eclass/alternatives-2.eclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/eclass/alternatives-2.eclass b/eclass/alternatives-2.eclass
index d9896f1..9b1d8b2 100644
--- a/eclass/alternatives-2.eclass
+++ b/eclass/alternatives-2.eclass
@@ -141,7 +141,7 @@ cleanup_old_alternatives_module() {
 	if [[ -f "${old_module}" && "$(source "${old_module}" &>/dev/null; echo "${ALTERNATIVE}")" == "${alt}" ]]; then
 		local version="$(source "${old_module}" &>/dev/null; echo "${VERSION}")"
 		if [[ "${version}" == "0.1" || "${version}" == "20080924" ]]; then
-			echo rm "${old_module}"
+			echo "rm ${old_module}"
 			rm "${old_module}" || eerror "rm ${old_module} failed"
 		fi
 	fi
@@ -200,9 +200,9 @@ alternatives-2_pkg_prerm() {
 		einfo "Removed ${provider} alternative module for ${alt}, current is $(eselect ${alt} show)"
 		if [[ $? -eq 2 ]]; then
 			einfo "Cleaning up unused alternatives module for ${alt}"
-			echo rm "${EAUTO}/${alt}.eselect"
+			echo "rm ${EAUTO}/${alt}.eselect"
 			rm "${EAUTO}/${alt}.eselect" || \
-				eerror rm "${EAUTO}/${alt}.eselect" failed
+				eerror "rm ${EAUTO}/${alt}.eselect failed"
 		fi
 	done
 }
-- 
1.8.5.3

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[justin]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

On 20/01/14 18:53, Reinis Danne wrote:
> Hi!
> 
> I have cleaned up a bit alternatives-2.eclass to make it easier to review.
> Also if someone knows any shortcommings of this eclass as it is, then point
> it out, I'll try to address them.
> 
> I hope this can be soon moved to the main tree thus closing the divide between
> science overlay and portage tree.
> 
> Reinis Danne (10):
>   alternatives-2.eclass: Update copyright
>   alternatives-2.eclass: Remove unused variables
>   alternatives-2.eclass: Move EXPORT_FUNCTIONS to top
>   alternatives-2.eclass: Remove commented-out code
>   alternatives-2.eclass: Put commonly used path in local variable
>   alternatives-2.eclass: Fix eclass name in comments
>   alternatives-2.eclass: Add EAPI check
>   alternatives-2.eclass: Add documentation comments
>   alternatives-2.eclass: Minor comment changes
>   alternatives-2.eclass: Use consistent quoting
> 
>  eclass/alternatives-2.eclass | 110 +++++++++++++++++++++++++++++++------------
>  1 file changed, 79 insertions(+), 31 deletions(-)
> 

Thanks for all your work.

Sebastien, Dennis, Ulrich and everybody else, let's gather a list of the
remaining issue to resolve.


Reinis, could you branch the sci overlay on github and and push your
changes there. Everybody, let's stick to clear commit msgs for the
eclass so that the review goes easy.

Justin

P.s. Sorry guys for CCing you directly. I am not sure if you are all
following the list.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Sébastien Fabbro]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

On Mon, Jan 20, 2014 at 9:53 AM, Reinis Danne <rei4dan@gmail.com> wrote:

>
> I have cleaned up a bit alternatives-2.eclass to make it easier to review.
> Also if someone knows any shortcommings of this eclass as it is, then point
> it out, I'll try to address them.
>
> I hope this can be soon moved to the main tree thus closing the divide
> between
> science overlay and portage tree.
>


Your changes are mostly minor and only apply to the eclass, so I have no
comments. You should commit them to the overlay, code reviews on the github
review panel are more practical than on the mailing list.
One important thing the creation of eselect modules could be done directly
from eselect rather than the eclass, keeping the eclass much more simple,
and allowing users to create their own providers easily. I may work on it.
I had some local changes of eselect and I added the repo [1] to the github
gentoo-science, that you can test by unmasking eselect-9999.

Known issues of the alternatives framework:
* when having more than 1 provider, updating a  package that provided the
eselected provider, will not re-eselect the same provider unless it was the
first on the list
* possible residual orphan files
* given the number of bugs, we should keep the linking to the reference
names libraries, so we could eselect providers without re-compiling all
reverse dependencies. We could do this in the open sourced providers by
changing the soname of the libraries we compile, and in the binary ones
(mkl,amcl...) with a link script generated library.


[1] https://github.com/gentoo-science/eselect

Sebastien

[-- Attachment #2: Type: text/html, Size: 2252 bytes --]

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Reinis Danne]

On Tue, Jan 21, 2014 at 07:51:34AM +0100, justin wrote:
> On 20/01/14 18:53, Reinis Danne wrote:
> > Hi!
> > 
> > I have cleaned up a bit alternatives-2.eclass to make it easier to review.
> > Also if someone knows any shortcommings of this eclass as it is, then point
> > it out, I'll try to address them.
> > 
> > I hope this can be soon moved to the main tree thus closing the divide between
> > science overlay and portage tree.
> > 
> > Reinis Danne (10):
> >   alternatives-2.eclass: Update copyright
> >   alternatives-2.eclass: Remove unused variables
> >   alternatives-2.eclass: Move EXPORT_FUNCTIONS to top
> >   alternatives-2.eclass: Remove commented-out code
> >   alternatives-2.eclass: Put commonly used path in local variable
> >   alternatives-2.eclass: Fix eclass name in comments
> >   alternatives-2.eclass: Add EAPI check
> >   alternatives-2.eclass: Add documentation comments
> >   alternatives-2.eclass: Minor comment changes
> >   alternatives-2.eclass: Use consistent quoting
> > 
> >  eclass/alternatives-2.eclass | 110 +++++++++++++++++++++++++++++++------------
> >  1 file changed, 79 insertions(+), 31 deletions(-)
> > 
> 
> Thanks for all your work.
> 
> Sebastien, Dennis, Ulrich and everybody else, let's gather a list of the
> remaining issue to resolve.
> 
> 
> Reinis, could you branch the sci overlay on github and and push your
> changes there. Everybody, let's stick to clear commit msgs for the
> eclass so that the review goes easy.

They are in alternatives-2 branch on
https://github.com/Reinis/sci

Added also:
alternatives-2.eclass: Fix einfo message in pkg_postinst
alternatives-2.eclass: Fix check for uninstalling
alternatives-2.eclass: Don't source eselect modules

With that it keeps provider selected if it is reinstalled/updated
and avoids recreating the eselect module every single time.


Reinis

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Reinis Danne]

On Tue, Jan 21, 2014 at 09:04:56AM -0800, Sébastien Fabbro wrote:
> On Mon, Jan 20, 2014 at 9:53 AM, Reinis Danne <rei4dan@gmail.com> wrote:
> 
> >
> > I have cleaned up a bit alternatives-2.eclass to make it easier to review.
> > Also if someone knows any shortcommings of this eclass as it is, then point
> > it out, I'll try to address them.
> >
> > I hope this can be soon moved to the main tree thus closing the divide
> > between
> > science overlay and portage tree.
> >
> 
> 
> Your changes are mostly minor and only apply to the eclass, so I have no
> comments. You should commit them to the overlay, code reviews on the github
> review panel are more practical than on the mailing list.
> One important thing the creation of eselect modules could be done directly
> from eselect rather than the eclass, keeping the eclass much more simple,
> and allowing users to create their own providers easily. I may work on it.
> I had some local changes of eselect and I added the repo [1] to the github
> gentoo-science, that you can test by unmasking eselect-9999.

How the eselect module creation could work? Something like this?

~# eselect module create blas
~# eselect blas add atlas importance src dest [src dest [..]]
...
~# eselect blas remove atlas
~# eselect delete blas

> 
> Known issues of the alternatives framework:
> * when having more than 1 provider, updating a  package that provided the
> eselected provider, will not re-eselect the same provider unless it was the
> first on the list

Fixed by "alternatives-2.eclass: Fix check for uninstalling".

> * possible residual orphan files

Haven't observed any after my patch series applied.

> * given the number of bugs, we should keep the linking to the reference
> names libraries, so we could eselect providers without re-compiling all
> reverse dependencies. We could do this in the open sourced providers by
> changing the soname of the libraries we compile, and in the binary ones
> (mkl,amcl...) with a link script generated library.

Yes, this is ok and AFAICT it is like that now in the overlay.

> 
> 
> [1] https://github.com/gentoo-science/eselect
> 
> Sebastien

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Sébastien Fabbro]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Tue, Jan 21, 2014 at 11:26 AM, Reinis Danne <rei4dan@gmail.com> wrote:

>
> How the eselect module creation could work? Something like this?
>
> ~# eselect module create blas
> ~# eselect blas add atlas importance src dest [src dest [..]]
> ...
> ~# eselect blas remove atlas
> ~# eselect delete blas
>


That is the idea. Last one should be "eselect module delete blas".


> > * given the number of bugs, we should keep the linking to the reference
> > names libraries, so we could eselect providers without re-compiling all
> > reverse dependencies. We could do this in the open sourced providers by
> > changing the soname of the libraries we compile, and in the binary ones
> > (mkl,amcl...) with a link script generated library.
>
> Yes, this is ok and AFAICT it is like that now in the overlay.
>
>
It is not. Let's take blas for example. Install openblas and
blas-reference, then eselect blas reference. Then install lapack-reference,
then remove blas-reference. You will have to re-install lapack-reference,
because of a missing reference to librefblas.so. If openblas had the
libblas.so soname and link, you would not have to. I'm actually not sure
whether it is a side effect of our as-needed policy.

We need to change all blas ebuilds to make sure they have the same soname,
and add to alternatives_for blas the libraries to link. For blas-reference,
eigen, atlas and openblas, this is just a matter of a link flag. For acml
and mkl, this is trickier, thus the gen_usr_ldscript from toolchain-funcs
eclass idea. If the ldscript stuff is not much overhead, we could apply it
to all.

Sebastien

[-- Attachment #2: Type: text/html, Size: 2269 bytes --]

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[François Bissey]

On 2014-01-22 09:04, Sébastien Fabbro wrote:
> On Tue, Jan 21, 2014 at 11:26 AM, Reinis Danne <rei4dan@gmail.com>
> wrote:
> 
>> How the eselect module creation could work? Something like this?
>> 
>> ~# eselect module create blas
>> ~# eselect blas add atlas importance src dest [src dest [..]]
>> ...
>> ~# eselect blas remove atlas
>> ~# eselect delete blas
> 
>  
> 
> That is the idea. Last one should be "eselect module delete blas".
>  
> 
>>> * given the number of bugs, we should keep the linking to the
>> reference
>>> names libraries, so we could eselect providers without
>> re-compiling all
>>> reverse dependencies. We could do this in the open sourced
>> providers by
>>> changing the soname of the libraries we compile, and in the
>> binary ones
>>> (mkl,amcl...) with a link script generated library.
>> 
>> Yes, this is ok and AFAICT it is like that now in the overlay.
> 
>  
> It is not. Let's take blas for example. Install openblas and
> blas-reference, then eselect blas reference. Then install
> lapack-reference, then remove blas-reference. You will have to
> re-install lapack-reference, because of a missing reference to
> librefblas.so. If openblas had the libblas.so soname and link, you
> would not have to. I'm actually not sure whether it is a side effect
> of our as-needed policy.
>  

Just to make sure the message is clear, I use openblas:
readelf -d /usr//lib64/libreflapack.so

Dynamic section at offset 0x4f9e08 contains 26 entries:
   Tag        Type                         Name/Value
  0x0000000000000001 (NEEDED)             Shared library: 
[libopenblas.so.0]
  0x0000000000000001 (NEEDED)             Shared library: 
[libgfortran.so.3]
  0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
  0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
  0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
  0x000000000000000e (SONAME)             Library soname: 
[libreflapack.so]

So the soname is embedded in the shared object and the linker will look 
for it
at runtime. Remove openblas and my reflapack is broken in spite of the 
fact that
I have 2 other blas on the system.

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[justin]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On 21/01/14 18:04, Sébastien Fabbro wrote:
> * given the number of bugs, we should keep the linking to the reference
> names libraries, so we could eselect providers without re-compiling all
> reverse dependencies. We could do this in the open sourced providers by
> changing the soname of the libraries we compile, and in the binary ones
> (mkl,amcl...) with a link script generated library.

I don't get this point. Why do we need to play around with sonames?
Doesn't this bring more problems and maintainer burden then letting the
consumer recompile reverse dependencies? Are the libs all ABI compatible?

Jusitn


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[François Bissey]

On 2014-01-22 10:21, justin wrote:
> On 21/01/14 18:04, Sébastien Fabbro wrote:
>> * given the number of bugs, we should keep the linking to the 
>> reference
>> names libraries, so we could eselect providers without re-compiling 
>> all
>> reverse dependencies. We could do this in the open sourced providers 
>> by
>> changing the soname of the libraries we compile, and in the binary 
>> ones
>> (mkl,amcl...) with a link script generated library.
> 
> I don't get this point. Why do we need to play around with sonames?
> Doesn't this bring more problems and maintainer burden then letting the
> consumer recompile reverse dependencies? Are the libs all ABI 
> compatible?
> 
All libs are supposed to be ABI compatible. For soname look at my 
previous post.
If you want to interchange libblas on the fly you need to have the same 
soname
for the library.

Francois

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Reinis Danne]

On Tue, Jan 21, 2014 at 12:04:43PM -0800, Sébastien Fabbro wrote:
> On Tue, Jan 21, 2014 at 11:26 AM, Reinis Danne <rei4dan@gmail.com> wrote:
> 
> > > * given the number of bugs, we should keep the linking to the reference
> > > names libraries, so we could eselect providers without re-compiling all
> > > reverse dependencies. We could do this in the open sourced providers by
> > > changing the soname of the libraries we compile, and in the binary ones
> > > (mkl,amcl...) with a link script generated library.
> >
> > Yes, this is ok and AFAICT it is like that now in the overlay.
> >
> >
> It is not. Let's take blas for example. Install openblas and
> blas-reference, then eselect blas reference. Then install lapack-reference,
> then remove blas-reference. You will have to re-install lapack-reference,
> because of a missing reference to librefblas.so. If openblas had the
> libblas.so soname and link, you would not have to. I'm actually not sure
> whether it is a side effect of our as-needed policy.
> 
> We need to change all blas ebuilds to make sure they have the same soname,
> and add to alternatives_for blas the libraries to link. For blas-reference,
> eigen, atlas and openblas, this is just a matter of a link flag. For acml
> and mkl, this is trickier, thus the gen_usr_ldscript from toolchain-funcs
> eclass idea. If the ldscript stuff is not much overhead, we could apply it
> to all.
> 
> Sebastien

Ok, that is different issue. The question is whether we want,
e.g., eselect blas be build time only or also runtime setting.

If we use provider specific sonames, then the program will use
whatever provider was selected at build time even if different
provider is set afterwards. Thus it is build time only setting,
with the caveat that package has to be rebuilt if selected
provider is removed (preserved-libs should catch this).

⬑ This is how it is now in the overlay.

If we provide generic sonames for alternatives (symlink or
ldscript), then program should use whichever provider is
currently selected. Thus it is runtime setting, with the caveat
that all packages can use only currently selected provider (user
has to keep track of selected provider).

Symlinks can be provided with the existing alternatives-2.eclass
if ebuilds choose to do so. Only change for generic pc file would
be not to point to selected provider's pc file, but to give
settings for generic libs (which is symlink to currently set). In
this case generic pc file generation should be added to eclass?

Is there any reason why it has not been used?

I don't know how often people actually do use specific providers
with different applications or how often they switch between
them?

As long as there are no programs with different preferred
providers, the generic libs would be ok. But if there are such
programs, then provider specific libs should be used. This could
already be done by picking provider specific pc file (and
optional USE-flags for providers in ebuilds).

AFAIK, benchmarking is the only use for generic libs (no need to
reinstall for every provider) and provider selection rarely
changes otherwise. Are there examples of opposite?

Could this be different for different alternatives (blas, cblas,
lapack, ...)?


Reinis

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[Sébastien Fabbro]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On Tue, Jan 21, 2014 at 1:30 PM, Reinis Danne <rei4dan@gmail.com> wrote:

>
>
> Ok, that is different issue. The question is whether we want,
> e.g., eselect blas be build time only or also runtime setting.
>
> If we use provider specific sonames, then the program will use
> whatever provider was selected at build time even if different
> provider is set afterwards. Thus it is build time only setting,
> with the caveat that package has to be rebuilt if selected
> provider is removed (preserved-libs should catch this).
>
> ⬑ This is how it is now in the overlay.
>

One other advantage of keeping the generic names is also that maintainer
would not have to modify the ebuild for packages to link with the selected
provider. Most blas dependent packages assume there is /usr/lib/libblas.so.

Some applications (I can see armadillo, scamp) actually use the
atlas-specific routines clapack_* routines.
We could aim at keeping both the library soname and filename as close to
upstream, and add an option to the alternative framework to create a
ldscript with a generic soname and filename, turned on for blas and cblas
at least.

As a user, I don't particularly enjoy rebuilding octave, ROOT, R, or the
scipy stack. @preserved-rebuild is more a hack than a feature.
I don't know how many people actually switch providers, besides a first
benchmark test. My guess is probably not many given the burden of
re-compiling all the reverse dependencies and the previous fragility of the
system.

Sebastien

[-- Attachment #2: Type: text/html, Size: 2125 bytes --]

Re: [gentoo-science] [PATCH 00/10] alternatives-2.eclass updates

[justin]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On 21/01/14 23:46, Sébastien Fabbro wrote:
> Some applications (I can see armadillo, scamp) actually use the
> atlas-specific routines clapack_* routines. 
> We could aim at keeping both the library soname and filename as close to
> upstream, and add an option to the alternative framework to create a
> ldscript with a generic soname and filename, turned on for blas and
> cblas at least.


I would really like to see that ldscript to be present. it would solve
many propblems including linking in distutils against mkl.



> 
> As a user, I don't particularly enjoy rebuilding octave, ROOT, R, or the
> scipy stack. @preserved-rebuild is more a hack than a feature.
> I don't know how many people actually switch providers, besides a first
> benchmark test. My guess is probably not many given the burden of
> re-compiling all the reverse dependencies and the previous fragility of
> the system.

But I still don't see the point of equalizing the soname. One advantage
I see from separate sonames is that you can link different applications
to different implementations. Of course this would require recompilation
of revdeps in case you remove one from your system. But I would argue
that switching/removing an implementation is a rare case for the average
user. Most will pick one or take the default aka reference
implementation and never touch it again.


Justin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]