Re: [gentoo-portage-dev] [PATCH] Support all install-qa-check.d locations and overrides

[Zac Medico]

On 11/22/2014 03:07 PM, Michał Górny wrote:
> Update the install-qa-check.d handling code to conform to GLEP 65.
> Collect files from all defined locations, order them lexically by name
> and run each uniquely named script once. Make scripts in higher priority
> locations override lower priority locations properly.
> ---
>  bin/misc-functions.sh | 65 ++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
> index 6e6fcb4..632d24e 100755
> --- a/bin/misc-functions.sh
> +++ b/bin/misc-functions.sh
> @@ -163,40 +163,61 @@ prepcompress() {
>  }
>
>  install_qa_check() {
> -	local f i qa_var x
> +	local d f i qa_var x paths qa_checks=() checks_run=()
>  	if ! ___eapi_has_prefix_variables; then
>  		local EPREFIX= ED=${D}
>  	fi
>
>  	cd "${ED}" || die "cd failed"
>
> -	# Run QA checks from install-qa-check.d.
> -	# Note: checks need to be run *before* stripping.
> -	local f
> -	# TODO: handle nullglob-like
> -	for f in "${PORTAGE_BIN_PATH}"/install-qa-check.d/*; do
> -		# Run in a subshell to treat it like external script,
> -		# but use 'source' to pass all variables through.
> -		(
> -			source "${f}" || eerror "Post-install QA check ${f##*/} failed to run"
> +	# Collect the paths for QA checks, highest prio first.
> +	paths=(
> +		# sysadmin overrides
> +		"${EPREFIX}"/usr/local/lib/install-qa-check.d
> +		# system-wide package installs
> +		"${EPREFIX}"/usr/lib/install-qa-check.d
> +	)

For "cross-prefix" support, the EPREFIX usage here seems wrong. I think
we should use PORTAGE_OVERRIDE_EPREFIX instead of EPREFIX. For
reference, see has_version --host-root code in bin/phase-helpers.sh.

> +
> +	# Now repo-specific checks.
> +	# (yes, PORTAGE_ECLASS_LOCATIONS contains repo paths...)
> +	for d in "${PORTAGE_ECLASS_LOCATIONS[@]}"; do
> +		paths+=(
> +			"${d}"/metadata/install-qa-check.d
>  		)
>  	done
>
> -	# Run QA checks from repositories
> -	# (yes, PORTAGE_ECLASS_LOCATIONS contains repo paths...)
> -	local repo_location
> -	for repo_location in "${PORTAGE_ECLASS_LOCATIONS[@]}"; do
> -		for f in "${repo_location}"/metadata/install-qa-check.d/*; do
> -			if [[ -f ${f} ]]; then
> -				(
> -					# allow inheriting eclasses
> -					_IN_INSTALL_QA_CHECK=1
> -					source "${f}" || eerror "Post-install QA check ${f##*/} failed to run"
> -				)
> -			fi
> +	paths+=(
> +		# Portage built-in checks
> +		"${EPREFIX}"/usr/lib/portage/install-qa-check.d

EPREFIX -> PORTAGE_OVERRIDE_EPREFIX

> +		"${PORTAGE_BIN_PATH}"/install-qa-check.d
> +	)
> +
> +	# Collect file names of QA checks. We need them early to support
> +	# overrides properly.
> +	for d in "${paths[@]}"; do
> +		for f in "${d}"/*; do
> +			[[ -f ${f} ]] && qa_checks+=( "${f##*/}" )
>  		done
>  	done
>
> +	# Now we need to sort the filenames lexically, and process
> +	# them in order.
> +	while IFS= read -r -d '' f; do

I'm not sure how IFS is relevant here. Please clarify.

> +		# Find highest priority file matching the basename.
> +		for d in "${paths[@]}"; do
> +			[[ -f ${d}/${f} ]] && break
> +		done
> +
> +		# Run in a subshell to treat it like external script,
> +		# but use 'source' to pass all variables through.
> +		(
> +			# Allow inheriting eclasses.
> +			# XXX: we want this only in repository-wide checks.
> +			_IN_INSTALL_QA_CHECK=1
> +			source "${d}/${f}" || eerror "Post-install QA check ${f} failed to run"
> +		)
> +	done < <(printf "%s\0" "${qa_checks[@]}" | LC_ALL=C sort -u -z)
> +
>  	export STRIP_MASK
>  	prepall
>  	___eapi_has_docompress && prepcompress
>


-- 
Thanks,
Zac