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

[Zac Medico]

On Sun, Jul 30, 2017 at 1:27 PM, Zac Medico  wrote:
> On Sat, Jul 29, 2017 at 10:50 PM, Michał Górny  wrote:
>> 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?
>
> Yeah, let's change it to BINPKG_COMPRESS and BINPKG_COMPRESS_FLAGS.

Pushed:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=2c7d38b9512609f6828cbba1066f2b3b2d9144bf
-- 
Thanks,
Zac

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

[Zac Medico]

On Sat, Jul 29, 2017 at 10:50 PM, Michał Górny  wrote:
> 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?

Yeah, let's change it to BINPKG_COMPRESS and BINPKG_COMPRESS_FLAGS.
-- 
Thanks,
Zac

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

[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

[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

[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

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

[Zac Medico]

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.
-- 
Thanks,
Zac

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

[Manuel Rüger]

This patch allows to set the compressor for binary packages via a
BINPKG_COMPRESSION variable. BINPKG_COMPRESSION_ARGS allows to specify
command-line arguments for the chosen compressor.
---
 bin/misc-functions.sh  |  6 ++-
 bin/quickpkg   | 62 --
 man/make.conf.5| 25 +
 pym/_emerge/BinpkgExtractorAsync.py| 43 +--
 pym/portage/dbapi/bintree.py   |  8 +--
 .../package/ebuild/_config/special_env_vars.py |  2 +-
 pym/portage/package/ebuild/doebuild.py | 34 ++--
 pym/portage/util/compression_probe.py  | 45 +---
 8 files changed, 186 insertions(+), 39 deletions(-)

diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index 58755a1e1..079369313 100755
--- a/bin/misc-functions.sh
+++ b/bin/misc-functions.sh
@@ -453,7 +453,7 @@ __dyn_package() {
# Make sure $PWD is not ${D} so that we don't leave gmon.out files
# in there in case any tools were built with -pg in CFLAGS.
 
-   cd "${T}"
+   cd "${T}" || die
 
if [[ -n ${PKG_INSTALL_MASK} ]] ; then
PROOT=${T}/packaging/
@@ -478,8 +478,10 @@ __dyn_package() {
[ -z "${PORTAGE_BINPKG_TMPFILE}" ] && \
die "PORTAGE_BINPKG_TMPFILE is unset"
mkdir -p "${PORTAGE_BINPKG_TMPFILE%/*}" || die "mkdir failed"
+   [ -z "${PORTAGE_COMPRESSION_COMMAND}" ] && \
+die "PORTAGE_COMPRESSION_COMMAND is unset"
tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
-   $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
+   $PORTAGE_COMPRESSION_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
assert "failed to pack binary package: '$PORTAGE_BINPKG_TMPFILE'"
PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}} \
"${PORTAGE_PYTHON:-/usr/bin/python}" 
"$PORTAGE_BIN_PATH"/xpak-helper.py recompose \
diff --git a/bin/quickpkg b/bin/quickpkg
index 4f26ee912..750400592 100755
--- a/bin/quickpkg
+++ b/bin/quickpkg
@@ -8,6 +8,7 @@ import argparse
 import errno
 import math
 import signal
+import subprocess
 import sys
 import tarfile
 
@@ -22,11 +23,13 @@ from portage.dbapi.dep_expand import dep_expand
 from portage.dep import Atom, use_reduce
 from portage.exception import (AmbiguousPackageName, InvalidAtom, InvalidData,
InvalidDependString, PackageSetNotFound, PermissionDenied)
-from portage.util import ConfigProtect, ensure_dirs, shlex_split, _xattr
+from portage.util import ConfigProtect, ensure_dirs, shlex_split, varexpand, 
_xattr
 xattr = _xattr.xattr
 from portage.dbapi.vartree import dblink, tar_contents
 from portage.checksum import perform_md5
 from portage._sets import load_default_config, SETPREFIX
+from portage.process import find_binary
+from portage.util.compression_probe import _compressors
 
 def quickpkg_atom(options, infos, arg, eout):
settings = portage.settings
@@ -50,16 +53,16 @@ def quickpkg_atom(options, infos, arg, eout):
" ".join(e.args[0]))
del e
infos["missing"].append(arg)
-   return
+   return 1
except (InvalidAtom, InvalidData):
eout.eerror("Invalid atom: %s" % (arg,))
infos["missing"].append(arg)
-   return
+   return 1
if atom[:1] == '=' and arg[:1] != '=':
# dep_expand() allows missing '=' but it's really invalid
eout.eerror("Invalid atom: %s" % (arg,))
infos["missing"].append(arg)
-   return
+   return 1
 
matches = vardb.match(atom)
pkgs_for_arg = 0
@@ -108,16 +111,16 @@ def quickpkg_atom(options, infos, arg, eout):
in settings.features))
def protect(filename):
if not confprot.isprotected(filename):
-   return False
+   return 1
if include_unmodified_config:
file_data = contents[filename]
if file_data[0] == "obj":
orig_md5 = 
file_data[2].lower()
cur_md5 = 
perform_md5(filename, calc_prelink=1)
if orig_md5 == cur_md5:
-   return False
+   return 1
excluded_config_files.append(filename)
-   return True
+