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

[Simon Glass]

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>
> Add a bintools base class for packers which compression / decompression
> entry contents.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>
> ---
>
> Changes in v2:
> - Added
>
>  tools/binman/bintool.py | 94 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>

Reviewed-by: Simon Glass <sjg at 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 at 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