Re: [gentoo-portage-dev] [PATCH v2] Support different compressors for binary packages

2017-07-29 Thread Michał Górny 
On nie, 2017-07-30 at 00:56 +0200, Manuel Rüger wrote:
> Pushed as:
> https://gitweb.gentoo.org/proj/portage.git/commit/?id=cff2c0149142843316e1851c2e73bcec30f08471
> 
> Thanks for the patient reviews, Zac!
> 

I'm sorry for noticing this only now when I'm enabling it but we have
already:

PORTAGE_COMPRESS
PORTAGE_COMPRESS_FLAGS
^^

and you've added:

BINPKG_COMPRESSION
   ^^^
BINPKG_COMPRESSION_ARGS
   

Wouldn't it be better to at least try having consistent variable naming?

-- 
Best regards,
Michał Górny


signature.asc
Description: This is a digitally signed message part

Re: [gentoo-portage-dev] [PATCH v2] Support different compressors for binary packages

2017-07-29 Thread Zac Medico 
On Sat, Jul 29, 2017 at 3:56 PM, Manuel Rüger  wrote:
> Pushed as:
> https://gitweb.gentoo.org/proj/portage.git/commit/?id=cff2c0149142843316e1851c2e73bcec30f08471
>
> Thanks for the patient reviews, Zac!
>
> Cheers,
> Manuel

I've added an early warning for invalid BINPKG_COMPRESSION here:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=fa1ba6318e114cd9f3bb5b4e61ed3dab7521bc19

I guess it might be annoying to make it fatal, since people might have
BINPKG_COMPRESSION before their desired compressor is installed?
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH v2] Support different compressors for binary packages

2017-07-29 Thread Manuel Rüger 
Pushed as:
https://gitweb.gentoo.org/proj/portage.git/commit/?id=cff2c0149142843316e1851c2e73bcec30f08471

Thanks for the patient reviews, Zac!

Cheers,
Manuel

On 28.07.2017 18:24, Zac Medico wrote:
> The patch is looking really good now. Thanks for working on this!
> 
> On Fri, Jul 28, 2017 at 2:59 AM, Manuel Rüger  wrote:
>> @@ -518,6 +526,26 @@ def doebuild_environment(myebuild, mydo, myroot=None, 
>> settings=None,
>> mysettings["KV"] = ""
>> mysettings.backup_changes("KV")
>>
>> +   binpkg_compression = mysettings.get("BINPKG_COMPRESSION", 
>> "bzip2")
>> +   try:
>> +   compression = _compressors[binpkg_compression]
>> +   except KeyError as e:
>> +   writemsg("Warning: Invalid or unsupported 
>> compression method: %s" % e.args[0])
>> +   else:
>> +   try:
>> +   compression_binary = 
>> shlex_split(varexpand(compression["compress"], mydict=settings))[0]
>> +   except IndexError as e:
>> +   writemsg("Warning: Invalid or unsupported 
>> compression method: %s" % e.args[0])
>> +   else:
>> +   if find_binary(compression_binary) is None:
>> +   missing_package = 
>> compression["package"]
>> +   writemsg("Warning: File compression 
>> unsupported %s. Missing package: %s" % (binpkg_compression, missing_package))
> 
> It's going to be very helpful if we add some code to detect this case
> in emerge's action_build function, and exit unsuccessfully if the
> global BINPKG_COMPRESSION setting is invalid or the required package
> is missing. This will ensure that people don't build a bunch of binary
> packages, only to find out later that their BINPKG_COMPRESSION setting
> was ineffective.
> 




signature.asc
Description: OpenPGP digital signature