diff mbox series

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

Message ID 20220831173936.150114-4-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>

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

Comments

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

Patch

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