From daa2da754afe1bac777f6cb0f05233e0de7b325d Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Wed, 31 Aug 2022 17:55:14 +0200 Subject: [PATCH 1/8] binman: btool: gzip: fix packer name so that binary can be found The binary is looked on the system by the suffix of the packer class. This means binman was looking for btool_gzip on the system and not gzip. Therefore, let's pass "gzip" as the name so that it can be found and used. Fixes: 0f369d79925a ("binman: Add gzip bintool") Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- tools/binman/btool/btool_gzip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/btool/btool_gzip.py b/tools/binman/btool/btool_gzip.py index 7bea300b5d..70cbc19f04 100644 --- a/tools/binman/btool/btool_gzip.py +++ b/tools/binman/btool/btool_gzip.py @@ -27,5 +27,5 @@ class Bintoolbtool_gzip(bintool.BintoolPacker): man gzip """ def __init__(self, name): - super().__init__(name, compress_args=[], + super().__init__("gzip", compress_args=[], version_regex=r'gzip ([0-9.]+)') From 723a63eeec3ee22d63006dcae593d395698fa55c Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:37 +0200 Subject: [PATCH 2/8] binman: bintool: move version check implementation into bintool class Version checking has nothing specific to compression/decompression tools so let's move it to the Bintool class. Signed-off-by: Quentin Schulz Reviewed-by: Simon Glass --- tools/binman/bintool.py | 43 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index ec30ceff74..ef2bdeb696 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 - Returns: str: Version string for this bintool """ - 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 @@ -507,7 +519,6 @@ class BintoolPacker(Bintool): if fetch_package is None: fetch_package = name self.fetch_package = fetch_package - self.version_regex = version_regex def compress(self, indata): """Compress data @@ -557,21 +568,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 From f17219ad42b47921864bef25d9e058959ef87265 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:38 +0200 Subject: [PATCH 3/8] binman: btool: lz4: use Bintool.version 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 Reviewed-by: Simon Glass --- 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 65e2c14d5a5a406decae07e2b5a2a74e7bd69c68 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:39 +0200 Subject: [PATCH 4/8] binman: btool: mkimage: use Bintool.version 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 Reviewed-by: Simon Glass --- tools/binman/btool/mkimage.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/tools/binman/btool/mkimage.py b/tools/binman/btool/mkimage.py index c85bfe053c..da5f344162 100644 --- a/tools/binman/btool/mkimage.py +++ b/tools/binman/btool/mkimage.py @@ -18,11 +18,11 @@ 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, - pad=None, version=False): + pad=None): """Run mkimage Args: @@ -44,8 +44,6 @@ class Bintoolmkimage(bintool.Bintool): args.append('-t') if output_fname: args += ['-F', output_fname] - if version: - args.append('-V') return self.run_cmd(*args) def fetch(self, method): @@ -66,15 +64,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 e440843448d27f2cc5a6446decd1bcbaae3b1533 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:40 +0200 Subject: [PATCH 5/8] binman: bintool: parametrize args to pass to binary for returning version 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 args 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 --- 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 ef2bdeb696..032179a99d 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_args='-V'): self.name = name self.desc = desc self.version_regex = version_regex + self.version_args = version_args @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_args) 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_args='-V'): desc = '%s compression' % (compression if compression else name) - super().__init__(name, desc, version_regex) + super().__init__(name, desc, version_regex, version_args) if compress_args is None: compress_args = ['--compress'] self.compress_args = compress_args From 4508fb9a7784b691bc6807bb3f8b79d07a15fd26 Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:41 +0200 Subject: [PATCH 6/8] binman: btool: fiptool: use Bintool.version 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 Reviewed-by: Simon Glass --- 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() From 9c9678632765678e607a88f0ebe7f5260ad6fdaa Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:42 +0200 Subject: [PATCH 7/8] binman: btool: futility: use Bintool.version 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 Reviewed-by: Simon Glass --- 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 From 7ac6842316ad1d412a49f88d8668923b40b09b5f Mon Sep 17 00:00:00 2001 From: Quentin Schulz Date: Thu, 1 Sep 2022 17:51:43 +0200 Subject: [PATCH 8/8] binman: bintool: bzip2: fix version function on non-Debian-based systems 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 Reviewed-by: Simon Glass --- 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..c3897d63ac 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_args='--help')