Re: [gentoo-portage-dev] [PATCH] INSTALL_MASK: honor install time config for binary packages (bug 651952)

[Michał Górny]

W dniu czw, 29.03.2018 o godzinie 15∶34 -0700, użytkownik Zac Medico
napisał:
> For binary packages, honor the INSTALL_MASK configuration that
> exists at install time, since it might differ from the build time
> setting.
>
> Fixes: 3416876c0ee7 ("{,PKG_}INSTALL_MASK: python implementation")
> Bug: https://bugs.gentoo.org/651952
> ---
>  bin/misc-functions.sh        | 23 +++++++++++++++++++++++
>  bin/phase-functions.sh       | 10 +---------
>  pym/portage/dbapi/vartree.py |  5 +++++
>  3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
> index 26f589915..a6330ee93 100755
> --- a/bin/misc-functions.sh
> +++ b/bin/misc-functions.sh
> @@ -323,6 +323,29 @@ postinst_qa_check() {
>  	done < <(printf "%s\0" "${qa_checks[@]}" | LC_ALL=C sort -u -z)
>  }
>
> +preinst_mask() {
> +	# Remove man pages, info pages, docs if requested. This is
> +	# implemented in bash in order to respect INSTALL_MASK settings
> +	# from bashrc.
> +	local f x
> +	for f in man info doc; do
> +		if has no${f} ${FEATURES}; then
> +			INSTALL_MASK+=" /usr/share/${f}"
> +		fi
> +	done
> +
> +	# Store modified variables in build-info.
> +	cd "${PORTAGE_BUILDDIR}"/build-info || die
> +	set -f
> +
> +	IFS=$' \t\n\r'
> +	for f in INSTALL_MASK; do

This loop along with the whole indirection is entirely pointless, given
that you're processing exactly one variable.

> +		x=$(echo -n ${!f})
> +		[[ -n ${x} ]] && echo "${x}" > "${f}"

There's probably no point in this [[ -n ... ]], as that:

a. requires you to special-handle missing INSTALL_MASK file, while it's
easier to just ensure that it's there (and I think you requested
the same thing from me before you rewritten my commit into breakage),

b. makes it impossible to distinguish packages from before INSTALL_MASK
storing was added from those where it is empty.

> +	done
> +	set +f
> +}
> +
>  preinst_sfperms() {
>  	if [ -z "${D}" ]; then
>  		 eerror "${FUNCNAME}: D is unset"
> diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
> index bdae68f79..3de8d01b5 100644
> --- a/bin/phase-functions.sh
> +++ b/bin/phase-functions.sh
> @@ -661,14 +661,6 @@ __dyn_install() {
>  	set -f
>  	local f x
>
> -	# remove man pages, info pages, docs if requested
> -	for f in man info doc; do
> -		if has no${f} ${FEATURES} && \
> -			! has "/usr/share/${f}" ${INSTALL_MASK}; then
> -			INSTALL_MASK+=" /usr/share/${f}"
> -		fi
> -	done
> -
>  	IFS=$' \t\n\r'
>  	for f in CATEGORY DEFINED_PHASES FEATURES INHERITED IUSE \
>  		PF PKGUSE SLOT KEYWORDS HOMEPAGE DESCRIPTION \
> @@ -676,7 +668,7 @@ __dyn_install() {
>  		CXXFLAGS EXTRA_ECONF EXTRA_EINSTALL EXTRA_MAKE \
>  		LDFLAGS LIBCFLAGS LIBCXXFLAGS QA_CONFIGURE_OPTIONS \
>  		QA_DESKTOP_FILE QA_PREBUILT PROVIDES_EXCLUDE REQUIRES_EXCLUDE \
> -		INSTALL_MASK PKG_INSTALL_MASK; do
> +		PKG_INSTALL_MASK; do
>
>  		x=$(echo -n ${!f})
>  		[[ -n $x ]] && echo "$x" > $f
> diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
> index 378d42dc0..a136c38f1 100644
> --- a/pym/portage/dbapi/vartree.py
> +++ b/pym/portage/dbapi/vartree.py
> @@ -3846,6 +3846,11 @@ class dblink(object):
>  		# be useful to avoid collisions in some scenarios.
>  		# We cannot detect if this is needed or not here as INSTALL_MASK can be
>  		# modified by bashrc files.
> +		phase = MiscFunctionsProcess(background=False,
> +			commands=["preinst_mask"], phase="preinst",
> +			scheduler=self._scheduler, settings=self.settings)
> +		phase.start()
> +		phase.wait()
>  		try:
>  			with io.open(_unicode_encode(os.path.join(inforoot, "INSTALL_MASK"),
>  				encoding=_encodings['fs'], errors='strict'),

-- 
Best regards,
Michał Górny