diff mbox series

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

Message ID 20220831173936.150114-1-foss+uboot@0leil.net
State Superseded
Delegated to: Simon Glass
Headers show
Series [v2,1/7] binman: bintool: move version check implementation into bintool class | expand

Commit Message

Quentin Schulz Aug. 31, 2022, 5:39 p.m. UTC
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(-)

Comments

Simon Glass Sept. 1, 2022, 2:27 a.m. UTC | #1
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>
Stefan Herbrechtsmeier Sept. 1, 2022, 6:17 a.m. UTC | #2
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
diff mbox series

Patch

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