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

[Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>]

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