Re: [gentoo-portage-dev] [PATCH v2] Support different compressors for binary packages
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
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
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
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
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
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
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 + r