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