Re: [PATCH v2 05/10] binman: Add BintoolPacker class to bintool

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Add a bintools base class for packers which compression / decompression entry contents.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

Changes in v2:

• Added

tools/binman/bintool.py | 94 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

The one strange thing about this is that we don't support these compression tools being missing, as we do with other tools. If 'lz4' is needed and not present, binman just fails. This predates your change, but I just noticed it.

I think this is OK though, at least for now. We could handle a missing algo by returning empty data and marking the entry as 'missing', but I don't see a great need for it at present. The compression tools are widely available and easy to install.

nits below

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 8435b29749..1d22962790 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright 2022 Google LLC +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com # """Base class for all bintools

@@ -464,3 +466,95 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the str: Version string for this bintool """ return 'unknown'

+class BintoolPacker(Bintool):

• """Tool which compression / decompression entry contents
• This is a bintools base class for compression / decompression packer
• """
• def __init__(self, name, compression=None, compress_args=None,

•             decompress_args=None, fetch_package=None,
  

•             version_regex=r'(v[0-9.]+)'):
  

Can you document these?

•    desc = '%s compression' % (compression if compression else name)
  

•    super().__init__(name, desc)
  

•    if compress_args is None:
  

•        compress_args = ['--compress']
  

•    self.compress_args = compress_args
  

•    if decompress_args is None:
  

•        decompress_args = ['--decompress']
  

•    self.decompress_args = decompress_args
  

•    if fetch_package is None:
  

•        fetch_package = name
  

•    self.fetch_package = fetch_package
  

•    self.version_regex = version_regex
  

• def compress(self, indata):

•    """Compress data
  

•    Args:
  

•        indata (bytes): Data to compress
  

•    Returns:
  

•        bytes: Compressed data
  

•    """
  

•    with tempfile.NamedTemporaryFile(prefix='comp.tmp',
  

•                                     dir=tools.get_output_dir()) as tmp:
  

•        tools.write_file(tmp.name, indata)
  

•        args = self.compress_args + ['--stdout', tmp.name]
  

•        return self.run_cmd(*args, binary=True)
  

• def decompress(self, indata):

•    """Decompress data
  

•    Args:
  

•        indata (bytes): Data to decompress
  

•    Returns:
  

•        bytes: Decompressed data
  

•    """
  

•    with tempfile.NamedTemporaryFile(prefix='decomp.tmp',
  

•                                     dir=tools.get_output_dir()) as inf:
  

•        tools.write_file(inf.name, indata)
  

•        args = self.decompress_args + ['--stdout', inf.name]
  

•        return self.run_cmd(*args, binary=True)
  

• def fetch(self, method):

•    """Fetch handler
  

•    This installs the gzip package using the apt utility.
  

•    Args:
  

•        method (FETCH_...): Method to use
  

•    Returns:
  

•        True if the file was fetched and now installed, None if a method
  

•        other than FETCH_BIN was requested
  

•    Raises:
  

•        Valuerror: Fetching could not be completed
  

•    """
  

•    if method != FETCH_BIN:
  

•        return None
  

•    pkg = self.fetch_package if self.fetch_package else self.name
  

•    return self.apt_install(pkg)
  

• def version(self):

•    """Version handler
  

•    Returns:
  

•        str: Version number
  

•    """
  

•    import re
  

•    result = self.run_cmd_result('-V')
  

•    if not result:
  

•        return super().version()
  

•    out = result.stdout.strip()
  

•    if not out:
  

•        out = result.stderr.strip()
  

•    if not out:
  

•        return super().version()
  

•    m_version = re.search(self.version_regex, out)
  

•    return m_version.group(1) if m_version else out
  

-- 2.30.2

Regards, Simon