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

[Zac Medico <zmedico@gentoo.org>]

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