[gentoo-portage-dev] [PATCH 1/3] portage.package.ebuild.config: Move FEATURES=no* handling there

[Michał Górny]

Move the code responsible for adding additional paths to INSTALL_MASK
into portage.package.ebuild.config.
---
 bin/misc-functions.sh| 13 -
 pym/portage/package/ebuild/config.py | 10 ++
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index 7643af7b5..6a5c2ea05 100755
--- a/bin/misc-functions.sh
+++ b/bin/misc-functions.sh
@@ -375,20 +375,7 @@ preinst_mask() {
# in there in case any tools were built with -pg in CFLAGS.
cd "${T}"
 
-   # remove man pages, info pages, docs if requested
-   local f
-   for f in man info doc; do
-   if has no${f} $FEATURES; then
-   INSTALL_MASK="${INSTALL_MASK} /usr/share/${f}"
-   fi
-   done
-
install_mask "${ED}" "${INSTALL_MASK}"
-
-   # remove share dir if unnessesary
-   if has nodoc $FEATURES || has noman $FEATURES || has noinfo $FEATURES; 
then
-   rmdir "${ED%/}/usr/share" &> /dev/null
-   fi
 }
 
 preinst_sfperms() {
diff --git a/pym/portage/package/ebuild/config.py 
b/pym/portage/package/ebuild/config.py
index 3dc64a067..3f575fcaf 100644
--- a/pym/portage/package/ebuild/config.py
+++ b/pym/portage/package/ebuild/config.py
@@ -2465,6 +2465,16 @@ class config(object):
myflags.difference_update(self.usemask)
self.configlist[-1]["USE"]= " ".join(sorted(myflags))
 
+   # Prepare the final value of INSTALL_MASK
+   install_mask = self.get("INSTALL_MASK", '').split()
+   if 'nodoc' in self.features:
+   install_mask.append("/usr/share/doc")
+   if 'noinfo' in self.features:
+   install_mask.append("/usr/share/info")
+   if 'noman' in self.features:
+   install_mask.append("/usr/share/man")
+   self["INSTALL_MASK"] = ' '.join(install_mask)
+
if self.mycpv is None:
# Generate global USE_EXPAND variables settings that are
# consistent with USE, for display by emerge --info. For
-- 
2.16.2

Re: [gentoo-portage-dev] [PATCH 1/3] portage.package.ebuild.config: Move FEATURES=no* handling there

[Michał Górny]

Dnia 12 czerwca 2016 09:28:14 CEST, Zac Medico  napisał(a):
>On 06/12/2016 12:19 AM, Zac Medico wrote:
>> On 05/21/2016 11:56 PM, Michał Górny wrote:
>>> diff --git a/pym/portage/package/ebuild/config.py
>b/pym/portage/package/ebuild/config.py
>>> index 45b7d08..fcc7ce5 100644
>>> --- a/pym/portage/package/ebuild/config.py
>>> +++ b/pym/portage/package/ebuild/config.py
>>> @@ -1773,6 +1773,16 @@ class config(object):
>>> # setcpv triggers lazy instantiation of things like 
>>> _use_manager.
>>> _eapi_cache.clear()
>>>  
>>> +   # Prepare the final value of INSTALL_MASK
>>> +   install_mask = self["INSTALL_MASK"].split()
>>> +   if 'nodoc' in self.features:
>>> +   install_mask.append("/usr/share/doc")
>>> +   if 'noinfo' in self.features:
>>> +   install_mask.append("/usr/share/info")
>>> +   if 'noman' in self.features:
>>> +   install_mask.append("/usr/share/man")
>>> +   self["INSTALL_MASK"] = ' '.join(install_mask)
>>> +
>>> def _grab_pkg_env(self, penv, container, protected_keys=None):
>>> if protected_keys is None:
>>> protected_keys = ()
>>>
>> 
>> I'm concerned that these values can be appended more than once,
>causing
>> the variable to contain duplicate values, because setcpv only calls
>> reset when the has_changed variable is True. So, we only want to call
>> this code when has_changed is True. In fact, this code can go
>> immediately after the reset call here:
>> 
>>  if has_changed:
>>  self.reset(keeping_pkg=1)
>> 
>
>Actually, it's more tricky than that, because we need to account for
>both global FEATURES settings and package.env FEATURES settings, and my
>above statements do not account for the global settings.
>
>Also need to consider the case where these features are enabled
>globally, and then *disabled* via package.env!

Then maybe we should just ban them and tell people to use INSTALL_MASK directly 
instead. Their behavior is quite unclear anyway, considering later possible 
exclusions.


-- 
Best regards,
Michał Górny (by phone)

Re: [gentoo-portage-dev] [PATCH 1/3] portage.package.ebuild.config: Move FEATURES=no* handling there

[Zac Medico]

On 06/12/2016 12:19 AM, Zac Medico wrote:
> On 05/21/2016 11:56 PM, Michał Górny wrote:
>> diff --git a/pym/portage/package/ebuild/config.py 
>> b/pym/portage/package/ebuild/config.py
>> index 45b7d08..fcc7ce5 100644
>> --- a/pym/portage/package/ebuild/config.py
>> +++ b/pym/portage/package/ebuild/config.py
>> @@ -1773,6 +1773,16 @@ class config(object):
>>  # setcpv triggers lazy instantiation of things like 
>> _use_manager.
>>  _eapi_cache.clear()
>>  
>> +# Prepare the final value of INSTALL_MASK
>> +install_mask = self["INSTALL_MASK"].split()
>> +if 'nodoc' in self.features:
>> +install_mask.append("/usr/share/doc")
>> +if 'noinfo' in self.features:
>> +install_mask.append("/usr/share/info")
>> +if 'noman' in self.features:
>> +install_mask.append("/usr/share/man")
>> +self["INSTALL_MASK"] = ' '.join(install_mask)
>> +
>>  def _grab_pkg_env(self, penv, container, protected_keys=None):
>>  if protected_keys is None:
>>  protected_keys = ()
>>
> 
> I'm concerned that these values can be appended more than once, causing
> the variable to contain duplicate values, because setcpv only calls
> reset when the has_changed variable is True. So, we only want to call
> this code when has_changed is True. In fact, this code can go
> immediately after the reset call here:
> 
>   if has_changed:
>   self.reset(keeping_pkg=1)
> 

Actually, it's more tricky than that, because we need to account for
both global FEATURES settings and package.env FEATURES settings, and my
above statements do not account for the global settings.

Also need to consider the case where these features are enabled
globally, and then *disabled* via package.env!
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] portage.package.ebuild.config: Move FEATURES=no* handling there

[Zac Medico]

On 05/21/2016 11:56 PM, Michał Górny wrote:
> diff --git a/pym/portage/package/ebuild/config.py 
> b/pym/portage/package/ebuild/config.py
> index 45b7d08..fcc7ce5 100644
> --- a/pym/portage/package/ebuild/config.py
> +++ b/pym/portage/package/ebuild/config.py
> @@ -1773,6 +1773,16 @@ class config(object):
>   # setcpv triggers lazy instantiation of things like 
> _use_manager.
>   _eapi_cache.clear()
>  
> + # Prepare the final value of INSTALL_MASK
> + install_mask = self["INSTALL_MASK"].split()
> + if 'nodoc' in self.features:
> + install_mask.append("/usr/share/doc")
> + if 'noinfo' in self.features:
> + install_mask.append("/usr/share/info")
> + if 'noman' in self.features:
> + install_mask.append("/usr/share/man")
> + self["INSTALL_MASK"] = ' '.join(install_mask)
> +
>   def _grab_pkg_env(self, penv, container, protected_keys=None):
>   if protected_keys is None:
>   protected_keys = ()
> 

I'm concerned that these values can be appended more than once, causing
the variable to contain duplicate values, because setcpv only calls
reset when the has_changed variable is True. So, we only want to call
this code when has_changed is True. In fact, this code can go
immediately after the reset call here:

if has_changed:
self.reset(keeping_pkg=1)
-- 
Thanks,
Zac