[gentoo-dev] [PATCH 0/3] llvm.eclass: Fix prepending path and add tests

[gentoo-dev] [PATCH 0/3] llvm.eclass: Fix prepending path and add tests

[Michał Górny]

Hi,

Here's a quick patch series that aims to fix prepending PATH to stop
overriding distcc/ccache/etc.  While at it, it adds some tests
to the eclass.  Please review.


Michał Górny (3):
  llvm.eclass: Add initial tests
  llvm.eclass: Remove remnants of slot :0 support
  llvm.eclass: Fix prepending LLVM path before system paths

 eclass/llvm.eclass   |  39 ++++++++----
 eclass/tests/llvm.sh | 138 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 13 deletions(-)
 create mode 100755 eclass/tests/llvm.sh

-- 
2.26.2

[gentoo-dev] [PATCH 1/3] llvm.eclass: Add initial tests

[Michał Górny]

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/tests/llvm.sh | 138 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100755 eclass/tests/llvm.sh

diff --git a/eclass/tests/llvm.sh b/eclass/tests/llvm.sh
new file mode 100755
index 000000000000..bb8d5fc998e8
--- /dev/null
+++ b/eclass/tests/llvm.sh
@@ -0,0 +1,138 @@
+#!/bin/bash
+# Copyright 1999-2020 Gentoo Authors
+# Distributed under the terms of the GNU General Public License v2
+
+EAPI=7
+source tests-common.sh
+
+inherit llvm
+
+# llvm_check_deps override to disable has_version use.
+# in: ${LLVM_SLOT}
+# returns 0 if installed (i.e. == LLVM_INSTALLED_SLOT), 1 otherwise
+llvm_check_deps() {
+	[[ ${LLVM_SLOT} == ${LLVM_INSTALLED_SLOT} ]]
+}
+
+# check_prefix <expected> [<args>...]
+# Check output of `get_llvm_prefix <args>...`.
+check_prefix() {
+	local expected=${1}
+	shift
+
+	tbegin "get_llvm_prefix ${*}; inst=${LLVM_INSTALLED_SLOT} -> ${expected}"
+	prefix=$(get_llvm_prefix "${@}")
+	[[ ${prefix} == ${expected} ]] ||
+		eerror "got: ${prefix} != exp: ${expected}"
+	tend ${?}
+}
+
+# check_setup_path <expected>
+# Check PATH after pkg_setup.
+check_setup_path() {
+	local expected=${1}
+	shift
+
+	tbegin "pkg_setup; max=${LLVM_MAX_SLOT}; inst=${LLVM_INSTALLED_SLOT} -> PATH=${expected}"
+	path=$(llvm_pkg_setup; echo "${PATH}")
+	[[ ${path} == ${expected} ]] ||
+		eerror "got: ${path} != exp: ${expected}"
+	tend ${?}
+}
+
+
+EAPI=7
+BROOT=/broot
+SYSROOT=/sysroot
+ESYSROOT=/sysroot/eprefix
+ROOT=/root
+EROOT=/root/eprefix
+
+ebegin "Testing check_setup_path without max slot"
+eindent
+	LLVM_INSTALLED_SLOT=11 \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/11
+	LLVM_INSTALLED_SLOT=10 \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/10
+eoutdent
+
+ebegin "Testing check_setup_path with max slot"
+eindent
+	LLVM_INSTALLED_SLOT=1* \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/11 11
+	LLVM_INSTALLED_SLOT=1* \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/10 10
+	LLVM_INSTALLED_SLOT=10 \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/10 11
+eoutdent
+
+ebegin "Testing check_setup_path option switches"
+eindent
+	LLVM_INSTALLED_SLOT=11 \
+	check_prefix /broot/usr/lib/llvm/11 -b
+	LLVM_INSTALLED_SLOT=11 \
+	check_prefix /sysroot/eprefix/usr/lib/llvm/11 -d
+eoutdent
+
+ebegin "Testing check_setup_path EAPI 6 API"
+eindent
+	EAPI=6 \
+	LLVM_INSTALLED_SLOT=11 \
+	check_prefix /usr/lib/llvm/11 -d
+eoutdent
+
+BASEPATH=/usr/lib/ccache/bin:/usr/bin:/usr/sbin:/bin:/sbin
+
+# TODO: cross support?
+ESYSROOT=
+
+ebegin "Testing pkg_setup with all installed LLVM versions in PATH"
+eindent
+	LLVM_MAX_SLOT=11 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH}:/usr/lib/llvm/11/bin \
+	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}:/usr/lib/llvm/11/bin"
+
+	LLVM_MAX_SLOT=10 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH}:/usr/lib/llvm/11/bin:/usr/lib/llvm/10/bin \
+	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/11/bin:/usr/lib/llvm/10/bin"
+
+	LLVM_MAX_SLOT=11 \
+	LLVM_INSTALLED_SLOT=10 \
+	PATH=${BASEPATH}:/usr/lib/llvm/10/bin \
+	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/10/bin"
+eoutdent
+
+ebegin "Testing pkg_setup with the other LLVM version in PATH"
+eindent
+	LLVM_MAX_SLOT=11 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH}:/usr/lib/llvm/10/bin \
+	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}:/usr/lib/llvm/10/bin"
+
+	LLVM_MAX_SLOT=10 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH}:/usr/lib/llvm/11/bin \
+	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/11/bin"
+eoutdent
+
+ebegin "Testing pkg_setup with LLVM missing from PATH"
+eindent
+	LLVM_MAX_SLOT=11 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH} \
+	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}"
+
+	LLVM_MAX_SLOT=10 \
+	LLVM_INSTALLED_SLOT=1* \
+	PATH=${BASEPATH} \
+	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}"
+
+	LLVM_MAX_SLOT=11 \
+	LLVM_INSTALLED_SLOT=10 \
+	PATH=${BASEPATH} \
+	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}"
+eoutdent
+
+texit
-- 
2.26.2

[gentoo-dev] [PATCH 2/3] llvm.eclass: Remove remnants of slot :0 support

[Michał Górny]

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/llvm.eclass | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/eclass/llvm.eclass b/eclass/llvm.eclass
index a2da231b035a..4f968dc39f87 100644
--- a/eclass/llvm.eclass
+++ b/eclass/llvm.eclass
@@ -71,6 +71,10 @@ EXPORT_FUNCTIONS pkg_setup
 
 if [[ ! ${_LLVM_ECLASS} ]]; then
 
+# make sure that the versions installing straight into /usr/bin
+# are uninstalled
+DEPEND="!!sys-devel/llvm:0"
+
 # @ECLASS-VARIABLE: LLVM_MAX_SLOT
 # @DEFAULT_UNSET
 # @DESCRIPTION:
@@ -173,13 +177,6 @@ get_llvm_prefix() {
 		die "${FUNCNAME}: invalid max_slot=${max_slot}"
 	fi
 
-	# fallback to :0
-	# assume it's always <= 4 (the lower max_slot allowed)
-	if has_version ${hv_switch} "sys-devel/llvm:0"; then
-		echo "${prefix}/usr"
-		return
-	fi
-
 	die "No LLVM slot${1:+ <= ${1}} found installed!"
 }
 
@@ -202,12 +199,7 @@ llvm_pkg_setup() {
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
 		local llvm_prefix=$(get_llvm_prefix "${LLVM_MAX_SLOT}")
-
-		# do not prepend /usr/bin, it's not necessary and breaks other
-		# prepends, https://bugs.gentoo.org/622866
-		if [[ ${llvm_prefix} != ${EPREFIX}/usr ]]; then
-			export PATH=${llvm_prefix}/bin:${PATH}
-		fi
+		export PATH=${llvm_prefix}/bin:${PATH}
 	fi
 }
 
-- 
2.26.2

[gentoo-dev] [PATCH 3/3] llvm.eclass: Fix prepending LLVM path before system paths

[Michał Górny]

Do not prepend LLVM path before system path, in particular before
ccache/distcc paths.  Instead, prepend it before the first LLVM version
found in PATH, or append to the end if no LLVM is found in PATH.

Closes: https://bugs.gentoo.org/627726
Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/llvm.eclass   | 25 +++++++++++++++++++++++--
 eclass/tests/llvm.sh | 16 ++++++++--------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/eclass/llvm.eclass b/eclass/llvm.eclass
index 4f968dc39f87..61b34d4985eb 100644
--- a/eclass/llvm.eclass
+++ b/eclass/llvm.eclass
@@ -198,8 +198,29 @@ llvm_pkg_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
-		local llvm_prefix=$(get_llvm_prefix "${LLVM_MAX_SLOT}")
-		export PATH=${llvm_prefix}/bin:${PATH}
+		local llvm_path=$(get_llvm_prefix "${LLVM_MAX_SLOT}")/bin
+		local IFS=:
+		local split_path=( ${PATH} )
+		local new_path=()
+		local x added=
+
+		# prepend new path before first LLVM version found
+		for x in "${split_path[@]}"; do
+			if [[ ${x} == */usr/lib/llvm/*/bin ]]; then
+				if [[ ${x} != ${llvm_path} ]]; then
+					new_path+=( "${llvm_path}" )
+				elif [[ ${added} && ${x} == ${llvm_path} ]]; then
+					# deduplicate
+					continue
+				fi
+				added=1
+			fi
+			new_path+=( "${x}" )
+		done
+		# ...or to the end of PATH
+		[[ ${added} ]] || new_path+=( "${llvm_path}" )
+
+		export PATH=${new_path[*]}
 	fi
 }
 
diff --git a/eclass/tests/llvm.sh b/eclass/tests/llvm.sh
index bb8d5fc998e8..8527d81765dd 100755
--- a/eclass/tests/llvm.sh
+++ b/eclass/tests/llvm.sh
@@ -91,17 +91,17 @@ eindent
 	LLVM_MAX_SLOT=11 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH}:/usr/lib/llvm/11/bin \
-	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}:/usr/lib/llvm/11/bin"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/11/bin"
 
 	LLVM_MAX_SLOT=10 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH}:/usr/lib/llvm/11/bin:/usr/lib/llvm/10/bin \
-	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/11/bin:/usr/lib/llvm/10/bin"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/10/bin:/usr/lib/llvm/11/bin"
 
 	LLVM_MAX_SLOT=11 \
 	LLVM_INSTALLED_SLOT=10 \
 	PATH=${BASEPATH}:/usr/lib/llvm/10/bin \
-	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/10/bin"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/10/bin"
 eoutdent
 
 ebegin "Testing pkg_setup with the other LLVM version in PATH"
@@ -109,12 +109,12 @@ eindent
 	LLVM_MAX_SLOT=11 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH}:/usr/lib/llvm/10/bin \
-	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}:/usr/lib/llvm/10/bin"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/11/bin:/usr/lib/llvm/10/bin"
 
 	LLVM_MAX_SLOT=10 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH}:/usr/lib/llvm/11/bin \
-	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}:/usr/lib/llvm/11/bin"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/10/bin:/usr/lib/llvm/11/bin"
 eoutdent
 
 ebegin "Testing pkg_setup with LLVM missing from PATH"
@@ -122,17 +122,17 @@ eindent
 	LLVM_MAX_SLOT=11 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH} \
-	check_setup_path "/usr/lib/llvm/11/bin:${BASEPATH}"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/11/bin"
 
 	LLVM_MAX_SLOT=10 \
 	LLVM_INSTALLED_SLOT=1* \
 	PATH=${BASEPATH} \
-	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/10/bin"
 
 	LLVM_MAX_SLOT=11 \
 	LLVM_INSTALLED_SLOT=10 \
 	PATH=${BASEPATH} \
-	check_setup_path "/usr/lib/llvm/10/bin:${BASEPATH}"
+	check_setup_path "${BASEPATH}:/usr/lib/llvm/10/bin"
 eoutdent
 
 texit
-- 
2.26.2