From: Quentin Schulz quentin.schulz@theobroma-systems.com

Bintool.version already contains everything required to get the version out of lz4 binary so let's not override it with its own implementation.

Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com ---

added in v2

tools/binman/btool/lz4.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/tools/binman/btool/lz4.py b/tools/binman/btool/lz4.py index f09c5c8904..dc9e37921a 100644 --- a/tools/binman/btool/lz4.py +++ b/tools/binman/btool/lz4.py @@ -76,7 +76,7 @@ class Bintoollz4(bintool.Bintool): man lz4 """ def __init__(self, name): - super().__init__(name, 'lz4 compression') + super().__init__(name, 'lz4 compression', r'.* (v[0-9.]*),.*')

def compress(self, indata): """Compress data with lz4 @@ -126,15 +126,3 @@ class Bintoollz4(bintool.Bintool): if method != bintool.FETCH_BIN: return None return self.apt_install('lz4') - - def version(self): - """Version handler - - Returns: - str: Version number of lz4 - """ - out = self.run_cmd('-V').strip() - if not out: - return super().version() - m_version = re.match(r'.* (v[0-9.]*),.*', out) - return m_version.group(1) if m_version else out

From: Quentin Schulz quentin.schulz@theobroma-systems.com

Bintool.version already contains everything required to get the version out of mkimage binary so let's not override it with its own implementation.

Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com ---

added in v2

tools/binman/btool/mkimage.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py index c85bfe053c..e08e3ef709 100644 --- a/tools/binman/btool/mkimage.py +++ b/tools/binman/btool/mkimage.py @@ -18,7 +18,7 @@ class Bintoolmkimage(bintool.Bintool): Support is provided for fetching this on Debian-like systems, using apt. """ def __init__(self, name): - super().__init__(name, 'Generate image for U-Boot') + super().__init__(name, 'Generate image for U-Boot', r'mkimage version (.*)')

# pylint: disable=R0913 def run(self, reset_timestamp=False, output_fname=None, external=False, @@ -66,15 +66,3 @@ class Bintoolmkimage(bintool.Bintool): if method != bintool.FETCH_BIN: return None return self.apt_install('u-boot-tools') - - def version(self): - """Version handler for mkimage - - Returns: - str: Version string for mkimage - """ - out = self.run(version=True).strip() - if not out: - return super().version() - m_version = re.match(r'mkimage version (.*)', out) - return m_version.group(1) if m_version else out

From: Quentin Schulz quentin.schulz@theobroma-systems.com

The code to check the version is very similar between binaries, the most likely only needed variables are the regex to find the version (already supported) and the parameter to pass to the binary so that it prints this version (e.g. --version, -V or similar).

Let's make it a parameter of Bintool so that code duplication can be avoided for simple changes.

Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com ---

added in v2

tools/binman/bintool.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index a156ffb550..77608cec23 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -53,10 +53,11 @@ class Bintool: # List of bintools to regard as missing missing_list = []

- def __init__(self, name, desc, version_regex=None): + def __init__(self, name, desc, version_regex=None, version_parameters='-V'): self.name = name self.desc = desc self.version_regex = version_regex + self.version_parameters = version_parameters

@staticmethod def find_bintool_class(btype): @@ -476,7 +477,7 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the

import re

- result = self.run_cmd_result('-V') + result = self.run_cmd_result(self.version_parameters) out = result.stdout.strip() if not out: out = result.stderr.strip() @@ -507,9 +508,9 @@ class BintoolPacker(Bintool): """ def __init__(self, name, compression=None, compress_args=None, decompress_args=None, fetch_package=None, - version_regex=r'(v[0-9.]+)'): + version_regex=r'(v[0-9.]+)', version_parameters='-V'): desc = '%s compression' % (compression if compression else name) - super().__init__(name, desc, version_regex) + super().__init__(name, desc, version_regex, version_parameters) if compress_args is None: compress_args = ['--compress'] self.compress_args = compress_args

Hi Quentin,

Am 31.08.2022 um 19:39 schrieb Quentin Schulz:

From: Quentin Schulz quentin.schulz@theobroma-systems.com

Version checking has nothing specific to compression/decompression tools so let's move it to the Bintool class.

Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com

added in v2

tools/binman/bintool.py | 46 ++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index ec30ceff74..a156ffb550 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -53,9 +53,10 @@ class Bintool: # List of bintools to regard as missing missing_list = []

• def __init__(self, name, desc):

• def __init__(self, name, desc, version_regex=None): self.name = name self.desc = desc

•    self.version_regex = version_regex
  
  @staticmethod
  def find_bintool_class(btype):
  

@@ -464,16 +465,27 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the print(f"No method to fetch bintool '{self.name}'") return False

• # pylint: disable=R0201 def version(self):

•    """Version handler for a bintool
  

•    This should be implemented by the base class
  

•    """Version handler
  
      Returns:
  

•        str: Version string for this bintool
  

•        str: Version number
      """
  

•    return 'unknown'
  

•    if self.version_regex is None:
  

•        return 'unknown'
  

•    import re
  

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

•    out = result.stdout.strip()
  

•    if not out:
  

•        out = result.stderr.strip()
  

•    if not out:
  

•        return 'unknown'
  

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

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

• class BintoolPacker(Bintool): """Tool which compression / decompression entry contents

@@ -497,7 +509,7 @@ class BintoolPacker(Bintool): decompress_args=None, fetch_package=None, version_regex=r'(v[0-9.]+)'): desc = '%s compression' % (compression if compression else name)

•    super().__init__(name, desc)
  

•    super().__init__(name, desc, version_regex)
      if compress_args is None:
          compress_args = ['--compress']
      self.compress_args = compress_args
  

Please remove the redundant self.version_regex assignment at the bottom of this function.

@@ -557,21 +569,3 @@ class BintoolPacker(Bintool): if method != FETCH_BIN: return None return self.apt_install(self.fetch_package)

• def version(self):

•    """Version handler
  

•    Returns:
  

•        str: Version number
  

•    """
  

•    import re
  

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

•    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
  

Regards Stefan