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

2018-04-01 Thread Zac Medico 
On 04/01/2018 06:54 AM, Michał Górny wrote:
> 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.

I did this for consistency with the related loop in dyn_install, since
we might save values of other variables from the binary package
environment. In fact, I think we should record DOC_SYMLINKS_DIR here,
since it affects CONTENTS, much like INSTALL_MASK.

>> +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.

Maybe this only matters within the context of bug 364633 [1], and for
that I think we need to introduce a separate CONTENTS.INSTALL_MASK file
so that we can easily toggle collision-protect behavior to use
CONTENTS.INSTALL_MASK when desired.

[1] https://bugs.gentoo.org/364633
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

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

2018-04-01 Thread 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

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

2018-03-29 Thread Zac Medico 
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
+   x=$(echo -n ${!f})
+   [[ -n ${x} ]] && echo "${x}" > "${f}"
+   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'),
-- 
2.13.6