[PATCH v2 1/7] binman: bintool: move version check implementation into bintool class

[PATCH v2 1/7] binman: bintool: move version check implementation into bintool class

[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
@@ -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
-- 
2.37.2

[PATCH v2 2/7] binman: btool: lz4: use Bintool.version

[Quentin Schulz]

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
-- 
2.37.2

Re: [PATCH v2 2/7] binman: btool: lz4: use Bintool.version

[Simon Glass]

On Wed, 31 Aug 2022 at 11:39, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> 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(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 3/7] binman: btool: mkimage: use Bintool.version

[Quentin Schulz]

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
-- 
2.37.2

Re: [PATCH v2 3/7] binman: btool: mkimage: use Bintool.version

[Simon Glass]

On Wed, 31 Aug 2022 at 11:39, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> 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(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 4/7] binman: bintool: parametrize parameter to pass to binary for returning version

[Quentin Schulz]

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
-- 
2.37.2

Re: [PATCH v2 4/7] binman: bintool: parametrize parameter to pass to binary for returning version

[Simon Glass]

Hi Quentin,

Fix spelling for parametrize

On Wed, 31 Aug 2022 at 11:39, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> 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'):

version_args?

It is shorter

>          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
> --
> 2.37.2
>

Regards,
Simon

Re: [PATCH v2 4/7] binman: bintool: parametrize parameter to pass to binary for returning version

[Stefan Herbrechtsmeier]

Hi,

Am 01.09.2022 um 04:27 schrieb Simon Glass:
> Hi Quentin,
> 
> Fix spelling for parametrize
> 
> On Wed, 31 Aug 2022 at 11:39, Quentin Schulz <foss+uboot@0leil.net> wrote:
>>
>> 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'):
> 
> version_args?
> 
> It is shorter

And it will be consistent to the compress_args and decompress_args of 
the BintoolPacker class.

> 
>>           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

Regards
   Stefan

[PATCH v2 5/7] binman: btool: fiptool: use Bintool.version

[Quentin Schulz]

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

Bintool.version can now be passed the binary argument to return the
version text, so there's no need to override it in fiptool anymore.

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

added in v2

 tools/binman/btool/fiptool.py | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/tools/binman/btool/fiptool.py b/tools/binman/btool/fiptool.py
index c6d71cebc2..c80f8275c4 100644
--- a/tools/binman/btool/fiptool.py
+++ b/tools/binman/btool/fiptool.py
@@ -49,7 +49,7 @@ class Bintoolfiptool(bintool.Bintool):
         https://trustedfirmware-a.readthedocs.io/en/latest/getting_started/tools-build.html?highlight=fiptool#building-and-using-the-fip-tool
     """
     def __init__(self, name):
-        super().__init__(name, 'Manipulate ATF FIP files')
+        super().__init__(name, 'Manipulate ATF FIP files', r'^(.*)$', 'version')
 
     def info(self, fname):
         """Get info on a FIP image
@@ -112,12 +112,3 @@ class Bintoolfiptool(bintool.Bintool):
             'fiptool',
             'tools/fiptool/fiptool')
         return result
-
-    def version(self):
-        """Version handler for fiptool
-
-        Returns:
-            str: Version number of fiptool
-        """
-        out = self.run_cmd('version').strip()
-        return out or super().version()
-- 
2.37.2

Re: [PATCH v2 5/7] binman: btool: fiptool: use Bintool.version

[Simon Glass]

On Wed, 31 Aug 2022 at 11:40, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Bintool.version can now be passed the binary argument to return the
> version text, so there's no need to override it in fiptool anymore.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>
> added in v2
>
>  tools/binman/btool/fiptool.py | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 6/7] binman: btool: futility: use Bintool.version

[Quentin Schulz]

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

Bintool.version can now be passed the binary argument to return the
version text, so there's no need to override it in futility anymore.

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

added in v2

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

diff --git a/tools/binman/btool/futility.py b/tools/binman/btool/futility.py
index 8d00966a9d..75a05c2ac6 100644
--- a/tools/binman/btool/futility.py
+++ b/tools/binman/btool/futility.py
@@ -69,7 +69,7 @@ class Bintoolfutility(bintool.Bintool):
         https://chromium.googlesource.com/chromiumos/platform/vboot/+/refs/heads/main/_vboot_reference/README
     """
     def __init__(self, name):
-        super().__init__(name, 'Chromium OS firmware utility')
+        super().__init__(name, 'Chromium OS firmware utility', r'^(.*)$', 'version')
 
     def gbb_create(self, fname, sizes):
         """Create a new Google Binary Block
@@ -165,14 +165,3 @@ class Bintoolfutility(bintool.Bintool):
         fname, tmpdir = self.fetch_from_drive(
             '1hdsInzsE4aJbmBeJ663kYgjOQyW1I-E0')
         return fname, tmpdir
-
-    def version(self):
-        """Version handler for futility
-
-        Returns:
-            str: Version string for futility
-        """
-        out = self.run_cmd('version').strip()
-        if not out:
-            return super().version()
-        return out
-- 
2.37.2

Re: [PATCH v2 6/7] binman: btool: futility: use Bintool.version

[Simon Glass]

On Wed, 31 Aug 2022 at 11:40, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Bintool.version can now be passed the binary argument to return the
> version text, so there's no need to override it in futility anymore.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>
> added in v2
>
>  tools/binman/btool/futility.py | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 7/7] binman: bintool: bzip2: fix version function on non-Debian-based systems

[Quentin Schulz]

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

Upstream bzip2 1.0.x actually is stuck when running bzip2 -V and
redirecting the output. This is fixed in Debian for about a decade
already in
https://git.launchpad.net/ubuntu/+source/bzip2/tree/debian/patches/20-legacy.patch?h=ubuntu/jammy
and in bzip2 1.1.x (no release yet, see
https://gitlab.com/bzip2/bzip2/-/commit/65179284ceddc43e6388bf4ed8c2d85cf16e1b2f
).

Fedora notably does not have such a patch.

Since bzip2 --help actually prints the version number too, let's use it
instead so that binman works fine on (hopefully) all distributions.

Fixes: 45aa2798008c ("binman: Add bzip2 bintool")
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

v2:
 - use version_parameters from Bintoolpacker class instead of overriding
 version method,

 tools/binman/btool/bzip2.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/btool/bzip2.py b/tools/binman/btool/bzip2.py
index 9be87a621f..078f637dda 100644
--- a/tools/binman/btool/bzip2.py
+++ b/tools/binman/btool/bzip2.py
@@ -27,4 +27,4 @@ class Bintoolbzip2(bintool.BintoolPacker):
         man bzip2
     """
     def __init__(self, name):
-        super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)')
+        super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)', version_parameters='--help')
-- 
2.37.2

Re: [PATCH v2 7/7] binman: bintool: bzip2: fix version function on non-Debian-based systems

[Simon Glass]

On Wed, 31 Aug 2022 at 11:40, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> Upstream bzip2 1.0.x actually is stuck when running bzip2 -V and
> redirecting the output. This is fixed in Debian for about a decade
> already in
> https://git.launchpad.net/ubuntu/+source/bzip2/tree/debian/patches/20-legacy.patch?h=ubuntu/jammy
> and in bzip2 1.1.x (no release yet, see
> https://gitlab.com/bzip2/bzip2/-/commit/65179284ceddc43e6388bf4ed8c2d85cf16e1b2f
> ).
>
> Fedora notably does not have such a patch.
>
> Since bzip2 --help actually prints the version number too, let's use it
> instead so that binman works fine on (hopefully) all distributions.
>
> Fixes: 45aa2798008c ("binman: Add bzip2 bintool")
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>
> v2:
>  - use version_parameters from Bintoolpacker class instead of overriding
>  version method,
>
>  tools/binman/btool/bzip2.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

version_args again

Also something in this series breaks code coverage, I think in
mkimage.py - so can you take a look at that? You may just need to
delete code, or update the tests, not sure.

>
> diff --git a/tools/binman/btool/bzip2.py b/tools/binman/btool/bzip2.py
> index 9be87a621f..078f637dda 100644
> --- a/tools/binman/btool/bzip2.py
> +++ b/tools/binman/btool/bzip2.py
> @@ -27,4 +27,4 @@ class Bintoolbzip2(bintool.BintoolPacker):
>          man bzip2
>      """
>      def __init__(self, name):
> -        super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)')
> +        super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)', version_parameters='--help')
> --
> 2.37.2
>

Regards,
Siumon

Re: [PATCH v2 1/7] binman: bintool: move version check implementation into bintool class

[Simon Glass]

On Wed, 31 Aug 2022 at 11:39, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> 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(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 1/7] binman: bintool: move version check implementation into bintool class

[Stefan Herbrechtsmeier]

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