[PATCH] Support different (de)compressors for binary packages

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Support different (de)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 that compressor.

---
 bin/binpkg-helper.py                  | 91
+++++++++++++++++++++++++++++++++++
 bin/misc-functions.sh                 |  9 +++-
 bin/quickpkg                          | 67 +++++++++++++++++++-------
 man/make.conf.5                       | 25 ++++++++++
 pym/_emerge/BinpkgExtractorAsync.py   | 43 +++++++++++++++--
 pym/portage/dbapi/bintree.py          |  8 +--
 pym/portage/util/compression_probe.py | 45 ++++++++++++++---
 7 files changed, 253 insertions(+), 35 deletions(-)
 create mode 100755 bin/binpkg-helper.py

diff --git a/bin/binpkg-helper.py b/bin/binpkg-helper.py
new file mode 100755
index 000000000..b603747cf
--- /dev/null
+++ b/bin/binpkg-helper.py
@@ -0,0 +1,91 @@
+#!/usr/bin/python -b
+# Copyright 2009-2017 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import argparse
+import sys
+
+import portage
+portage._internal_caller = True
+from portage import os
+from portage import cpv_getkey
+from portage.process import find_binary
+from portage.util import (
+ shlex_split,
+ varexpand,
+ )
+from portage.util.compression_probe import _compressors
+
+def command_compressioncmd(args):
+ settings = portage.settings
+ usage = "usage: compressioncmd ${CATEGORY}/${P}\n"
+
+ if len(args) != 1:
+ sys.stderr.write(usage)
+ sys.stderr.write("One argument is required, got %s\n" % len(args))
+ return 1
+
+ cpv = args[0]
+ binpkg_compression = settings.get("BINPKG_COMPRESSION", "bzip2")
+ try:
+ compression = _compressors[binpkg_compression]
+ except KeyError as e:
+ sys.stderr.write("Invalid or unsupported compression method: %s" %
e.args[0])
+ return 1
+ # Fallback to bzip2 for the package providing the decompressor
+ # This solves bootstrapping a client without the decompressor
+
+ if cpv_getkey(cpv) is None:
+ sys.stderr.write("The argument must be in the ${CATEGORY}/${PN}-${PV}
format. Provided argument was: %s\n" % cpv)
+ return 1
+
+ if cpv_getkey(cpv) == compression["package"]:
+ compression = _compressors["bzip2"]
+ try:
+ compression_binary = shlex_split(varexpand(compression["compress"],
mydict=settings))[0]
+ except IndexError as e:
+ sys.stderr.write("Invalid or unsupported compression method: %s" %
e.args[0])
+ return 1
+ if find_binary(compression_binary) is None:
+ missing_package = compression["package"]
+ sys.stderr.write("File compression unsupported %s. Missing package:
%s" % (binpkg_compression, missing_package))
+ return 1
+ cmd = [varexpand(x, mydict=settings) for x in
shlex_split(compression["compress"])]
+ # Filter empty elements
+ cmd = [x for x in cmd if x != ""]
+
+ print(' '.join(cmd))
+ return os.EX_OK
+
+def main(argv):
+
+ if argv and isinstance(argv[0], bytes):
+ for i, x in enumerate(argv):
+ argv[i] = portage._unicode_decode(x, errors='strict')
+
+ valid_commands = ('compressioncmd',)
+ description = "Returns the compression command"
+ usage = "usage: %s COMMAND [args]" % \
+ os.path.basename(argv[0])
+
+ parser = argparse.ArgumentParser(description=description, usage=usage)
+ options, args = parser.parse_known_args(argv[1:])
+
+ if not args:
+ parser.error("missing command argument")
+
+ command = args[0]
+
+ if command not in valid_commands:
+ parser.error("invalid command: '%s'" % command)
+
+ if command == 'compressioncmd':
+ rval = command_compressioncmd(args[1:])
+ else:
+ raise AssertionError("invalid command: '%s'" % command)
+
+ return rval
+
+if __name__ == "__main__":
+ rval = main(sys.argv[:])
+ sys.exit(rval)
diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index 58755a1e1..0ba0db226 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,13 @@ __dyn_package() {
  [ -z "${PORTAGE_BINPKG_TMPFILE}" ] && \
  die "PORTAGE_BINPKG_TMPFILE is unset"
  mkdir -p "${PORTAGE_BINPKG_TMPFILE%/*}" || die "mkdir failed"
+
COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
\
+ "${PORTAGE_PYTHON:-/usr/bin/python}"
"$PORTAGE_BIN_PATH"/binpkg-helper.py \
+ compressioncmd ${CATEGORY}/${P})
+ [ -z "${COMPRESSION_COMMAND}" ] && \
+ die "Failed to get COMPRESSION_COMMAND"
  tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
- $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
+ $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..a7f7ec74f 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

@@ -16,17 +17,20 @@ if
osp.isfile(osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))),
".porta
  sys.path.insert(0,
osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))), "pym"))
 import portage
 portage._internal_caller = True
+from portage import cpv_getkey
 from portage import os
 from portage import xpak
 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 +54,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 +112,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
+ return os.EX_OK
  existing_metadata = dict(zip(fix_metadata_keys,
  vardb.aux_get(cpv, fix_metadata_keys)))
  category, pf = portage.catsplit(cpv)
@@ -134,12 +138,36 @@ def quickpkg_atom(options, infos, arg, eout):
  binpkg_tmpfile = os.path.join(bintree.pkgdir,
  cpv + ".tbz2." + str(os.getpid()))
  ensure_dirs(os.path.dirname(binpkg_tmpfile))
- # The tarfile module will write pax headers holding the
- # xattrs only if PAX_FORMAT is specified here.
- tar = tarfile.open(binpkg_tmpfile, "w:bz2",
- format=tarfile.PAX_FORMAT if xattrs else tarfile.DEFAULT_FORMAT)
- tar_contents(contents, root, tar, protect=protect, xattrs=xattrs)
- tar.close()
+ binpkg_compression = settings.get("BINPKG_COMPRESSION", "bzip2")
+ try:
+ compression = _compressors[binpkg_compression]
+ except KeyError as e:
+ eout.eerror("Invalid or unsupported compression method: %s" %
e.args[0])
+ return 1
+ # Fallback to bzip2 for the package providing the decompressor
+ # This solves bootstrapping a client without the decompressor
+ if cpv_getkey(cpv) == compression["package"]:
+ compression = _compressors["bzip2"]
+ try:
+ compression_binary = shlex_split(varexpand(compression["compress"],
mydict=settings))[0]
+ except IndexError as e:
+ eout.eerror("Invalid or unsupported compression method: %s" %
e.args[0])
+ return 1
+ if find_binary(compression_binary) is None:
+ missing_package = compression["package"]
+ eout.eerror("File compression unsupported %s. Missing package: %s"
% (binpkg_compression, missing_package))
+ return 1
+ cmd = [varexpand(x, mydict=settings) for x in
shlex_split(compression["compress"])]
+ # Filter empty elements that make Popen fail
+ cmd = [x for x in cmd if x != ""]
+ with open(binpkg_tmpfile, "wb") as fobj:
+ proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=fobj)
+ # The tarfile module will write pax headers holding the
+ # xattrs only if PAX_FORMAT is specified here.
+ with tarfile.open(mode="w|",format=tarfile.PAX_FORMAT if xattrs
else tarfile.DEFAULT_FORMAT, fileobj=proc.stdin) as tar:
+ tar_contents(contents, root, tar, protect=protect, xattrs=xattrs)
+ proc.stdin.close()
+ proc.wait()
  xpak.tbz2(binpkg_tmpfile).recompose_mem(xpdata)
  finally:
  if have_lock:
@@ -154,16 +182,20 @@ def quickpkg_atom(options, infos, arg, eout):
  eout.eerror(str(e))
  del e
  eout.eerror("Failed to create package: '%s'" % binpkg_path)
+ return 1
  else:
  eout.eend(0)
  infos["successes"].append((cpv, s.st_size))
  infos["config_files_excluded"] += len(excluded_config_files)
  for filename in excluded_config_files:
  eout.ewarn("Excluded config: '%s'" % filename)
+ return os.EX_OK
  if not pkgs_for_arg:
  eout.eerror("Could not find anything " + \
  "to match '%s'; skipping" % arg)
  infos["missing"].append(arg)
+ return 1
+ return os.EX_OK

 def quickpkg_set(options, infos, arg, eout):
  eroot = portage.settings['EROOT']
@@ -179,7 +211,7 @@ def quickpkg_set(options, infos, arg, eout):
  if not set in sets:
  eout.eerror("Package set not found: '%s'; skipping" % (arg,))
  infos["missing"].append(arg)
- return
+ return 1

  try:
  atoms = setconfig.getSetAtoms(set)
@@ -187,10 +219,11 @@ def quickpkg_set(options, infos, arg, eout):
  eout.eerror("Failed to process package set '%s' because " % set +
  "it contains the non-existent package set '%s'; skipping" % e)
  infos["missing"].append(arg)
- return
-
+ return 1
+ retval = os.EX_OK
  for atom in atoms:
- quickpkg_atom(options, infos, atom, eout)
+ retval |= quickpkg_atom(options, infos, atom, eout)
+ return retval


 def quickpkg_extended_atom(options, infos, atom, eout):
diff --git a/man/make.conf.5 b/man/make.conf.5
index aea189e4a..8e0d04967 100644
--- a/man/make.conf.5
+++ b/man/make.conf.5
@@ -110,6 +110,31 @@ ACCEPT_RESTRICT="*"
 ACCEPT_RESTRICT="* -bindist"
 .fi
 .TP
+\fBBINPKG_COMPRESSION\fR = \fI"compression"\fR
+This variable is used to determine the compression used for \fIbinary
+packages\fR. Supported settings and compression algorithms are: bzip2,
gzip,
+lz4, lzip, lzop, xz, zstd.
+.br
+Defaults to "bzip2".
+.br
+.I Example:
+.nf
+# Set it to use lz4:
+BINPKG_COMPRESSION="lz4"
+.fi
+.TP
+\fBBINPKG_COMPRESSION_ARGS\fR = \fI"arguments for compression command"\fR
+This variable is used to add additional arguments to the compression
command
+selected by \fBBINPKG_COMPRESSION\fR.
+.br
+Defaults to "".
+.br
+.I Example:
+.nf
+# Set it to use compression level 9:
+BINPKG_COMPRESSION_ARGS="-9"
+.fi
+.TP
 .B CBUILD
 This variable is passed by the \fIebuild scripts\fR to the \fIconfigure\fR
 as \fI\-\-build=${CBUILD}\fR only if it is defined.  Do not set this
yourself
diff --git a/pym/_emerge/BinpkgExtractorAsync.py
b/pym/_emerge/BinpkgExtractorAsync.py
index 0bf3c74c9..e85f4ecac 100644
--- a/pym/_emerge/BinpkgExtractorAsync.py
+++ b/pym/_emerge/BinpkgExtractorAsync.py
@@ -6,8 +6,15 @@
 from _emerge.SpawnProcess import SpawnProcess
 import portage
 from portage.localization import _
-from portage.util.compression_probe import (compression_probe,
- _decompressors)
+from portage.util.compression_probe import (
+ compression_probe,
+ _compressors,
+)
+from portage.process import find_binary
+from portage.util import (
+ shlex_split,
+ varexpand,
+)
 import signal
 import subprocess

@@ -28,8 +35,11 @@ def _start(self):
  tar_options.append(portage._shell_quote("--xattrs-exclude=%s" % x))
  tar_options = " ".join(tar_options)

- decomp_cmd = _decompressors.get(
- compression_probe(self.pkg_path))
+ decomp = _compressors.get(compression_probe(self.pkg_path))
+ if decomp is not None:
+ decomp_cmd = decomp.get("decompress")
+ else:
+ decomp_cmd = None
  if decomp_cmd is None:
  self.scheduler.output("!!! %s\n" %
  _("File compression header unrecognized: %s") %
@@ -39,6 +49,31 @@ def _start(self):
  self._async_wait()
  return

+ try:
+ decompression_binary = shlex_split(varexpand(decomp_cmd,
mydict=self.env))[0]
+ except IndexError:
+ decompression_binary = ""
+
+ if find_binary(decompression_binary) is None:
+ # Try alternative command if it exists
+ if
_compressors.get(compression_probe(self.pkg_path)).get("decompress_alt"):
+ decomp_cmd = _compressors.get(
+ compression_probe(self.pkg_path)).get("decompress_alt")
+ try:
+ decompression_binary = shlex_split(varexpand(decomp_cmd,
mydict=self.env))[0]
+ except IndexError:
+ decompression_binary = ""
+
+ if find_binary(decompression_binary) is None:
+ missing_package =
_compressors.get(compression_probe(self.pkg_path)).get("package")
+ self.scheduler.output("!!! %s\n" %
+ _("File compression unsupported %s.\n Command was: %s.\n Maybe
missing package: %s") %
+ (self.pkg_path, varexpand(decomp_cmd, mydict=self.env),
missing_package), log_path=self.logfile,
+ background=self.background, level=logging.ERROR)
+ self.returncode = 1
+ self._async_wait()
+ return
+
  # Add -q to decomp_cmd opts, in order to avoid "trailing garbage
  # after EOF ignored" warning messages due to xpak trailer.
  # SIGPIPE handling (128 + SIGPIPE) should be compatible with
diff --git a/pym/portage/dbapi/bintree.py b/pym/portage/dbapi/bintree.py
index ca90ba8f9..c833968c2 100644
--- a/pym/portage/dbapi/bintree.py
+++ b/pym/portage/dbapi/bintree.py
@@ -141,7 +141,6 @@ def aux_get(self, mycpv, wants, myrepo=None):
  return [aux_cache.get(x, "") for x in wants]
  mysplit = mycpv.split("/")
  mylist = []
- tbz2name = mysplit[1]+".tbz2"
  if not self.bintree._remotepkgs or \
  not self.bintree.isremote(mycpv):
  try:
@@ -1448,9 +1447,10 @@ def _allocate_filename_multi(self, cpv):
  @staticmethod
  def _parse_build_id(filename):
  build_id = -1
- hyphen = filename.rfind("-", 0, -6)
+ suffixlen = len(".xpak")
+ hyphen = filename.rfind("-", 0, -(suffixlen + 1))
  if hyphen != -1:
- build_id = filename[hyphen+1:-5]
+ build_id = filename[hyphen+1:-suffixlen]
  try:
  build_id = long(build_id)
  except ValueError:
@@ -1497,7 +1497,7 @@ def gettbz2(self, pkgname):
  if self._remote_has_index:
  rel_url = self._remotepkgs[instance_key].get("PATH")
  if not rel_url:
- rel_url = pkgname+".tbz2"
+ rel_url = pkgname + ".tbz2"
  remote_base_uri = self._remotepkgs[instance_key]["BASE_URI"]
  url = remote_base_uri.rstrip("/") + "/" + rel_url.lstrip("/")
  else:
diff --git a/pym/portage/util/compression_probe.py
b/pym/portage/util/compression_probe.py
index 754621016..b15200044 100644
--- a/pym/portage/util/compression_probe.py
+++ b/pym/portage/util/compression_probe.py
@@ -11,14 +11,43 @@
 from portage import _encodings, _unicode_encode
 from portage.exception import FileNotFound, PermissionDenied

-_decompressors = {
- "bzip2": "${PORTAGE_BUNZIP2_COMMAND:-${PORTAGE_BZIP2_COMMAND} -d}",
- "gzip": "gzip -d",
- "lz4": "lz4 -d",
- "lzip": "lzip -d",
- "lzop": "lzop -d",
- "xz": "xz -d",
- "zstd": "zstd -d",
+_compressors = {
+ "bzip2": {
+ "compress": "${PORTAGE_BZIP2_COMMAND} ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "${PORTAGE_BUNZIP2_COMMAND}",
+ "decompress_alt": "${PORTAGE_BZIP2_COMMAND} -d",
+ "package": "app-arch/bzip2",
+ },
+ "gzip": {
+ "compress": "gzip ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "gzip -d",
+ "package": "app-arch/gzip",
+ },
+ "lz4": {
+ "compress": "lz4 ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "lz4 -d",
+ "package": "app-arch/lz4",
+ },
+ "lzip": {
+ "compress": "lzip ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "lzip -d",
+ "package": "app-arch/lzip",
+ },
+ "lzop": {
+ "compress": "lzop ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "lzop -d",
+ "package": "app-arch/lzop",
+ },
+ "xz": {
+ "compress": "xz ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "xz -d",
+ "package": "app-arch/xz-utils",
+ },
+ "zstd": {
+ "compress": "zstd ${BINPKG_COMPRESSION_ARGS}",
+ "decompress": "zstd -d",
+ "package": "app-arch/zstd",
+ },
 }

 _compression_re = re.compile(b'^(' +

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support different (de)compressors for binary packages

Zac Medico-2
On Fri, Jun 30, 2017 at 2:49 AM, Manuel Rüger <[hidden email]> wrote:

> +
> COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
> \
> +               "${PORTAGE_PYTHON:-/usr/bin/python}"
> "$PORTAGE_BIN_PATH"/binpkg-helper.py \
> +               compressioncmd ${CATEGORY}/${P})
> +       [ -z "${COMPRESSION_COMMAND}" ] && \
> +               die "Failed to get COMPRESSION_COMMAND"
>         tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
> -               $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
> +               $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}"

If all that we really need is COMPRESSION_COMMAND, then the helper
script is overkill. We should just pass a variable from
doebuild_environment function. The variable name must be prefixed with
PORTAGE_.

Also, note that your mail client wrapped lines in this patch.
--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support different (de)compressors for binary packages

Manuel Rüger
On 04.07.2017 21:29, Zac Medico wrote:

> On Fri, Jun 30, 2017 at 2:49 AM, Manuel Rüger <[hidden email]> wrote:
>> +
>> COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
>> \
>> +               "${PORTAGE_PYTHON:-/usr/bin/python}"
>> "$PORTAGE_BIN_PATH"/binpkg-helper.py \
>> +               compressioncmd ${CATEGORY}/${P})
>> +       [ -z "${COMPRESSION_COMMAND}" ] && \
>> +               die "Failed to get COMPRESSION_COMMAND"
>>         tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
>> -               $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
>> +               $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}"
>
> If all that we really need is COMPRESSION_COMMAND, then the helper
> script is overkill. We should just pass a variable from
> doebuild_environment function. The variable name must be prefixed with
> PORTAGE_.
>
> Also, note that your mail client wrapped lines in this patch.
>
Thanks for the review Zac!

We'd need a bit more as the COMPRESSION_COMMAND depends on
${CATEGORY}/${P} here in order to avoid a catch22 when using a
decompressor that is set to something a standard install doesn't include.

Assume all binpkgs are set to be compressed with zstd, the patch makes
sure an app-arch/zstd binpkg will still be compressed with bzip2.

Mails were sent with Thunderbird, I'll set up git send-mail next time.


signature.asc (650 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support different (de)compressors for binary packages

Zac Medico-2
On Tue, Jul 4, 2017 at 2:33 PM, Manuel Rüger <[hidden email]> wrote:

> On 04.07.2017 21:29, Zac Medico wrote:
>> On Fri, Jun 30, 2017 at 2:49 AM, Manuel Rüger <[hidden email]> wrote:
>>> +
>>> COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
>>> \
>>> +               "${PORTAGE_PYTHON:-/usr/bin/python}"
>>> "$PORTAGE_BIN_PATH"/binpkg-helper.py \
>>> +               compressioncmd ${CATEGORY}/${P})
>>> +       [ -z "${COMPRESSION_COMMAND}" ] && \
>>> +               die "Failed to get COMPRESSION_COMMAND"
>>>         tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
>>> -               $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
>>> +               $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}"
>>
>> If all that we really need is COMPRESSION_COMMAND, then the helper
>> script is overkill. We should just pass a variable from
>> doebuild_environment function. The variable name must be prefixed with
>> PORTAGE_.
>>
>> Also, note that your mail client wrapped lines in this patch.
>>
>
> Thanks for the review Zac!
>
> We'd need a bit more as the COMPRESSION_COMMAND depends on
> ${CATEGORY}/${P} here in order to avoid a catch22 when using a
> decompressor that is set to something a standard install doesn't include.
>
> Assume all binpkgs are set to be compressed with zstd, the patch makes
> sure an app-arch/zstd binpkg will still be compressed with bzip2.

Inside the doebuild_environment function, there's a mycpv, mypv, and
cat variables available to use, and it's also possible to access
relevant package.env variables via mysettings.

> Mails were sent with Thunderbird, I'll set up git send-mail next time.

Okay, thanks.
--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support different (de)compressors for binary packages

Michał Górny-5
In reply to this post by Manuel Rüger
On wto, 2017-07-04 at 23:33 +0200, Manuel Rüger wrote:

> On 04.07.2017 21:29, Zac Medico wrote:
> > On Fri, Jun 30, 2017 at 2:49 AM, Manuel Rüger <[hidden email]> wrote:
> > > +
> > > COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
> > > \
> > > +               "${PORTAGE_PYTHON:-/usr/bin/python}"
> > > "$PORTAGE_BIN_PATH"/binpkg-helper.py \
> > > +               compressioncmd ${CATEGORY}/${P})
> > > +       [ -z "${COMPRESSION_COMMAND}" ] && \
> > > +               die "Failed to get COMPRESSION_COMMAND"
> > >         tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
> > > -               $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
> > > +               $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}"
> >
> > If all that we really need is COMPRESSION_COMMAND, then the helper
> > script is overkill. We should just pass a variable from
> > doebuild_environment function. The variable name must be prefixed with
> > PORTAGE_.
> >
> > Also, note that your mail client wrapped lines in this patch.
> >
>
> Thanks for the review Zac!
>
> We'd need a bit more as the COMPRESSION_COMMAND depends on
> ${CATEGORY}/${P} here in order to avoid a catch22 when using a
> decompressor that is set to something a standard install doesn't include.
>
> Assume all binpkgs are set to be compressed with zstd, the patch makes
> sure an app-arch/zstd binpkg will still be compressed with bzip2.
And app-arch/bzip2 binpackage?

In other words, that's the wrong approach. If I set binpackages to use
X, I want them to use X, not 'use X if someone thought it correct to use
X'. Just do what the user says to do. If you try to outsmart him, you're
building another Windows.

--
Best regards,
Michał Górny

signature.asc (1007 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support different (de)compressors for binary packages

Zac Medico-2
On Wed, Jul 5, 2017 at 1:03 AM, Michał Górny <[hidden email]> wrote:

> On wto, 2017-07-04 at 23:33 +0200, Manuel Rüger wrote:
>> On 04.07.2017 21:29, Zac Medico wrote:
>> > On Fri, Jun 30, 2017 at 2:49 AM, Manuel Rüger <[hidden email]> wrote:
>> > > +
>> > > COMPRESSION_COMMAND=$(PYTHONPATH=${PORTAGE_PYTHONPATH:-${PORTAGE_PYM_PATH}}
>> > > \
>> > > +               "${PORTAGE_PYTHON:-/usr/bin/python}"
>> > > "$PORTAGE_BIN_PATH"/binpkg-helper.py \
>> > > +               compressioncmd ${CATEGORY}/${P})
>> > > +       [ -z "${COMPRESSION_COMMAND}" ] && \
>> > > +               die "Failed to get COMPRESSION_COMMAND"
>> > >         tar $tar_options -cf - $PORTAGE_BINPKG_TAR_OPTS -C "${PROOT}" . | \
>> > > -               $PORTAGE_BZIP2_COMMAND -c > "$PORTAGE_BINPKG_TMPFILE"
>> > > +               $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}"
>> >
>> > If all that we really need is COMPRESSION_COMMAND, then the helper
>> > script is overkill. We should just pass a variable from
>> > doebuild_environment function. The variable name must be prefixed with
>> > PORTAGE_.
>> >
>> > Also, note that your mail client wrapped lines in this patch.
>> >
>>
>> Thanks for the review Zac!
>>
>> We'd need a bit more as the COMPRESSION_COMMAND depends on
>> ${CATEGORY}/${P} here in order to avoid a catch22 when using a
>> decompressor that is set to something a standard install doesn't include.
>>
>> Assume all binpkgs are set to be compressed with zstd, the patch makes
>> sure an app-arch/zstd binpkg will still be compressed with bzip2.
>
> And app-arch/bzip2 binpackage?
>
> In other words, that's the wrong approach. If I set binpackages to use
> X, I want them to use X, not 'use X if someone thought it correct to use
> X'. Just do what the user says to do. If you try to outsmart him, you're
> building another Windows.

Yeah, people can use package.env if they want per-package compression settings.
--
Thanks,
Zac