[gentoo-dev] [PATCHES] multilib-build & multilib-minimal EAPI 6 patchset

[gentoo-dev] [PATCHES] multilib-build & multilib-minimal EAPI 6 patchset

[Michał Górny]

Hi, everyone.

Here's a patchset for review. Changes:

1. eclass-generated variables are read-only,
2. missing error checks (||die) added,
3. 'unset -f' used to wipe local and temporary functions from the env,
4. deprecated function QA warnings turned into bans for EAPI 6,
5. eutils inherit fixes/updates.

Please review. I've tested it on a set of randomly selected packages.

[gentoo-dev] [PATCH 1/9] multilib-build.eclass: Mark eclass-generated variables read-only

[Michał Górny]

---
 eclass/multilib-build.eclass | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index ca0fd544..dd03553 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -37,7 +37,7 @@ inherit multibuild multilib
 # Please contact multilib before modifying this list. This way we can
 # ensure that every *preliminary* work is done and the multilib can be
 # extended safely.
-_MULTILIB_FLAGS=(
+declare -g -r _MULTILIB_FLAGS=(
 	abi_x86_32:x86,x86_fbsd,x86_freebsd,x86_linux,x86_macos,x86_solaris
 	abi_x86_64:amd64,amd64_fbsd,x64_freebsd,amd64_linux,x64_macos,x64_solaris
 	abi_x86_x32:x32
@@ -123,7 +123,7 @@ _multilib_build_set_globals() {
 	local usedeps=${flags[@]/%/(-)?}
 
 	IUSE=${flags[*]}
-	MULTILIB_USEDEP=${usedeps// /,}
+	declare -g -r MULTILIB_USEDEP=${usedeps// /,}
 }
 _multilib_build_set_globals
 
@@ -196,9 +196,10 @@ _multilib_multibuild_wrapper() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local ABI=${MULTIBUILD_VARIANT#*.}
-	local MULTILIB_ABI_FLAG=${MULTIBUILD_VARIANT%.*}
+	local -r MULTILIB_ABI_FLAG=${MULTIBUILD_VARIANT%.*}
 
 	multilib_toolchain_setup "${ABI}"
+	readonly ABI
 	"${@}"
 }
 
-- 
2.6.3

[gentoo-dev] [PATCH 2/9] multilib-build.eclass: Add missing error checks

[Michał Górny]

---
 eclass/multilib-build.eclass | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index dd03553..8e58a2b 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -262,19 +262,23 @@ multilib_for_best_abi() {
 # runs (if any). Dies if header files differ.
 multilib_check_headers() {
 	_multilib_header_cksum() {
-		[[ -d ${ED}usr/include ]] && \
-		find "${ED}"usr/include -type f \
-			-exec cksum {} + | sort -k2
+		set -o pipefail
+
+		if [[ -d ${ED}usr/include ]]; then
+			find "${ED}"usr/include -type f \
+				-exec cksum {} + | sort -k2
+		fi
 	}
 
-	local cksum=$(_multilib_header_cksum)
+	local cksum cksum_prev
 	local cksum_file=${T}/.multilib_header_cksum
+	cksum=$(_multilib_header_cksum) || die
 
 	if [[ -f ${cksum_file} ]]; then
-		local cksum_prev=$(< "${cksum_file}")
+		cksum_prev=$(< "${cksum_file}") || die
 
 		if [[ ${cksum} != ${cksum_prev} ]]; then
-			echo "${cksum}" > "${cksum_file}.new"
+			echo "${cksum}" > "${cksum_file}.new" || die
 
 			eerror "Header files have changed between ABIs."
 
@@ -288,7 +292,7 @@ multilib_check_headers() {
 			die "Header checksum mismatch, aborting."
 		fi
 	else
-		echo "${cksum}" > "${cksum_file}"
+		echo "${cksum}" > "${cksum_file}" || die
 	fi
 }
 
@@ -409,9 +413,9 @@ multilib_prepare_wrappers() {
 
 		if [[ -L ${root}/${f} ]]; then
 			# rewrite the symlink target
-			local target=$(readlink "${root}/${f}")
-			local target_dir
-			local target_fn=${target##*/}
+			local target
+			target=$(readlink "${root}/${f}") || die
+			local target_dir target_fn=${target##*/}
 
 			[[ ${target} == */* ]] && target_dir=${target%/*}
 
@@ -453,7 +457,7 @@ multilib_prepare_wrappers() {
 					if [[ ! -f ${ED}/tmp/multilib-include${f} ]]; then
 						dodir "/tmp/multilib-include${dir}"
 						# a generic template
-						cat > "${wrapper}" <<_EOF_
+						cat > "${wrapper}" <<_EOF_ || die
 /* This file is auto-generated by multilib-build.eclass
  * as a multilib-friendly wrapper. For the original content,
  * please see the files that are #included below.
-- 
2.6.3

Re: [gentoo-dev] [PATCH 2/9] multilib-build.eclass: Add missing error checks

[James Le Cuirot]

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

On Sun,  6 Dec 2015 19:57:47 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> diff --git a/eclass/multilib-build.eclass
> b/eclass/multilib-build.eclass index dd03553..8e58a2b 100644
> --- a/eclass/multilib-build.eclass
> +++ b/eclass/multilib-build.eclass
> @@ -262,19 +262,23 @@ multilib_for_best_abi() {
>  # runs (if any). Dies if header files differ.
>  multilib_check_headers() {
>  	_multilib_header_cksum() {
> -		[[ -d ${ED}usr/include ]] && \
> -		find "${ED}"usr/include -type f \
> -			-exec cksum {} + | sort -k2
> +		set -o pipefail
> +
> +		if [[ -d ${ED}usr/include ]]; then
> +			find "${ED}"usr/include -type f \
> +				-exec cksum {} + | sort -k2
> +		fi
>  	}

Possibly being dumb here but isn't "set -o pipefail" a global
operation? It will continue to take effect even after this function
ends?

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 2/9] multilib-build.eclass: Add missing error checks

[James Le Cuirot]

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

On Sun, 6 Dec 2015 20:23:56 +0000
James Le Cuirot <chewi@gentoo.org> wrote:

> Possibly being dumb here but isn't "set -o pipefail" a global
> operation? It will continue to take effect even after this function
> ends?

Oh never mind, it's an internal function that you're only calling in a
subshell. Moving along. :)

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

[gentoo-dev] [PATCH 3/9] multilib-build.eclass: Unset local functions after use

[Michał Górny]

---
 eclass/multilib-build.eclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index 8e58a2b..664cd43 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -273,6 +273,7 @@ multilib_check_headers() {
 	local cksum cksum_prev
 	local cksum_file=${T}/.multilib_header_cksum
 	cksum=$(_multilib_header_cksum) || die
+	unset -f _multilib_header_cksum
 
 	if [[ -f ${cksum_file} ]]; then
 		cksum_prev=$(< "${cksum_file}") || die
-- 
2.6.3

[gentoo-dev] [PATCH 4/9] multilib-build.eclass: Unset global-setting function after use

[Michał Górny]

---
 eclass/multilib-build.eclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index 664cd43..8a3ac7f 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -126,6 +126,7 @@ _multilib_build_set_globals() {
 	declare -g -r MULTILIB_USEDEP=${usedeps// /,}
 }
 _multilib_build_set_globals
+unset -f _multilib_build_set_globals
 
 # @FUNCTION: multilib_get_enabled_abis
 # @DESCRIPTION:
-- 
2.6.3

[gentoo-dev] [PATCH 5/9] multilib-build.eclass: Ban deprecated functions in EAPI 6

[Michał Górny]

---
 eclass/multilib-build.eclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index 8a3ac7f..49bebf3 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -246,6 +246,8 @@ multilib_parallel_foreach_abi() {
 multilib_for_best_abi() {
 	debug-print-function ${FUNCNAME} "${@}"
 
+	[[ ${EAPI} == [45] ]] || die "${FUNCNAME} is banned in EAPI ${EAPI}, use multilib_is_native_abi() instead"
+
 	eqawarn "QA warning: multilib_for_best_abi() function is deprecated and should"
 	eqawarn "not be used. The multilib_is_native_abi() check may be used instead."
 
@@ -580,6 +582,8 @@ multilib_is_native_abi() {
 multilib_build_binaries() {
 	debug-print-function ${FUNCNAME} "${@}"
 
+	[[ ${EAPI} == [45] ]] || die "${FUNCNAME} is banned in EAPI ${EAPI}, use multilib_is_native_abi() instead"
+
 	eqawarn "QA warning: multilib_build_binaries is deprecated. Please use the equivalent"
 	eqawarn "multilib_is_native_abi function instead."
 
-- 
2.6.3

[gentoo-dev] [PATCH 6/9] multilib-build.eclass: Add missing eutils inherit for eqawarn

[Michał Górny]

---
 eclass/multilib-build.eclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index 49bebf3..eb7bf9a 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -25,6 +25,7 @@ case ${EAPI:-0} in
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
+[[ ${EAPI} == [45] ]] && inherit eutils
 inherit multibuild multilib
 
 # @ECLASS-VARIABLE: _MULTILIB_FLAGS
-- 
2.6.3

[gentoo-dev] [PATCH 7/9] multilib-build.eclass: Enable EAPI 6

[Michał Górny]

---
 eclass/multilib-build.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/multilib-build.eclass b/eclass/multilib-build.eclass
index eb7bf9a..e9e5604 100644
--- a/eclass/multilib-build.eclass
+++ b/eclass/multilib-build.eclass
@@ -21,7 +21,7 @@ if [[ ! ${_MULTILIB_BUILD} ]]; then
 
 # EAPI=4 is required for meaningful MULTILIB_USEDEP.
 case ${EAPI:-0} in
-	4|5) ;;
+	[456]) ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
-- 
2.6.3

Re: [gentoo-dev] [PATCH 7/9] multilib-build.eclass: Enable EAPI 6

[Ulrich Mueller]

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

>>>>> On Sun, 6 Dec 2015, Michał Górny wrote:

>  # EAPI=4 is required for meaningful MULTILIB_USEDEP.
>  case ${EAPI:-0} in
> -	4|5) ;;
> +	[456]) ;;
>  	*) die "EAPI=${EAPI} is not supported" ;;
>  esac

Why not write this as 4|5|6) for better readability? It's not even
shorter with the brackets.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] [PATCH 7/9] multilib-build.eclass: Enable EAPI 6

[James Le Cuirot]

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

On Sun, 6 Dec 2015 22:32:30 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sun, 6 Dec 2015, Michał Górny wrote:
> 
> >  # EAPI=4 is required for meaningful MULTILIB_USEDEP.
> >  case ${EAPI:-0} in
> > -	4|5) ;;
> > +	[456]) ;;
> >  	*) die "EAPI=${EAPI} is not supported" ;;
> >  esac
> 
> Why not write this as 4|5|6) for better readability? It's not even
> shorter with the brackets.

Or [4-6]) perhaps? It would not get any longer for later versions. ;)

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

Re: [gentoo-dev] [PATCH 7/9] multilib-build.eclass: Enable EAPI 6

[Ulrich Mueller]

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

>>>>> On Sun, 6 Dec 2015, James Le Cuirot wrote:

>> >  # EAPI=4 is required for meaningful MULTILIB_USEDEP.
>> >  case ${EAPI:-0} in
>> > -	4|5) ;;
>> > +	[456]) ;;
>> >  	*) die "EAPI=${EAPI} is not supported" ;;
>> >  esac
>> 
>> Why not write this as 4|5|6) for better readability? It's not even
>> shorter with the brackets.

> Or [4-6]) perhaps? It would not get any longer for later versions. ;)

I'd rather avoid any syntax that suggests that EAPI names are single
characters.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

[gentoo-dev] [PATCH 8/9] multilib-minimal.eclass: Remove eutils in EAPI 6 (einstalldocs built-in)

[Michał Górny]

---
 eclass/multilib-minimal.eclass | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/eclass/multilib-minimal.eclass b/eclass/multilib-minimal.eclass
index a3b6d37..ca5e9ce 100644
--- a/eclass/multilib-minimal.eclass
+++ b/eclass/multilib-minimal.eclass
@@ -30,7 +30,8 @@ case ${EAPI:-0} in
 esac
 
 
-inherit eutils multilib-build
+[[ ${EAPI} == [45] ]] && inherit eutils
+inherit multilib-build
 
 EXPORT_FUNCTIONS src_configure src_compile src_test src_install
 
-- 
2.6.3

[gentoo-dev] [PATCH 9/9] multilib-minimal.eclass: Enable EAPI 6

[Michał Górny]

---
 eclass/multilib-minimal.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/multilib-minimal.eclass b/eclass/multilib-minimal.eclass
index ca5e9ce..c6f39e2 100644
--- a/eclass/multilib-minimal.eclass
+++ b/eclass/multilib-minimal.eclass
@@ -25,7 +25,7 @@
 
 # EAPI=4 is required for meaningful MULTILIB_USEDEP.
 case ${EAPI:-0} in
-	4|5) ;;
+	[456]) ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
-- 
2.6.3

Re: [gentoo-dev] [PATCHES] multilib-build & multilib-minimal EAPI 6 patchset

[Michał Górny]

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

On Sun,  6 Dec 2015 19:57:45 +0100
Michał Górny <mgorny@gentoo.org> wrote:

> Hi, everyone.
> 
> Here's a patchset for review. Changes:
> 
> 1. eclass-generated variables are read-only,
> 2. missing error checks (||die) added,
> 3. 'unset -f' used to wipe local and temporary functions from the env,
> 4. deprecated function QA warnings turned into bans for EAPI 6,
> 5. eutils inherit fixes/updates.
> 
> Please review. I've tested it on a set of randomly selected packages.

Applied after following ulm's suggestions.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

Re: [gentoo-dev] [PATCHES] multilib-build & multilib-minimal EAPI 6 patchset

[Doug Goldstein]

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

On 12/6/15 12:57 PM, Michał Górny wrote:
> Hi, everyone.
> 
> Here's a patchset for review. Changes:
> 
> 1. eclass-generated variables are read-only,
> 2. missing error checks (||die) added,
> 3. 'unset -f' used to wipe local and temporary functions from the env,
> 4. deprecated function QA warnings turned into bans for EAPI 6,
> 5. eutils inherit fixes/updates.
> 
> Please review. I've tested it on a set of randomly selected packages.
> 
> 

Its worth noting that you should probably be utilizing --cover-letter to
git format-patch to generate this.

-- 
Doug Goldstein


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