[PATCH v4 0/2] automatically strip btool_ from btool names

[PATCH v4 0/2] automatically strip btool_ from btool names

[Quentin Schulz]

Commit daa2da754afe ("binman: btool: gzip: fix packer name so that binary can be
found") introduced some sort of work-around to make the gzip binary found by
bintool. However, this work-around would need to be applied to any btool whose
name is prefixed with btool_ (usually to avoid conflict with other python
modules).

Instead, let's handle this specific case directly inside bintool so that the
btools don't have to handle it themselves. This also reverts the now-unnecessary
work-around for gzip btool.

To: Simon Glass <sjg@chromium.org>
To: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Cc: Quentin Schulz <foss+uboot@0leil.net>
Cc: u-boot@lists.denx.de
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

---
Changes in v4:
- fix buildman test by enforcing class name to be Bintool<packer> even for
  Bintools in files whose filename is prefixed with btool_,
- Link to v3: https://lore.kernel.org/r/20220930-upstream-gzip-binman-v3-v3-0-17c99d6d87ac@theobroma-systems.com

---
Quentin Schulz (2):
      binman: bintool: remove btool_ prefix from btool names
      Revert "binman: btool: gzip: fix packer name so that binary can be found"

 tools/binman/bintool.py          | 3 ++-
 tools/binman/btool/btool_gzip.py | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)
---
base-commit: 898bd53e6a930080cee7cd7b1a09120c4dfd9467
change-id: 20220930-upstream-gzip-binman-v3-91b83bf16795

Best regards,
-- 
Quentin Schulz <quentin.schulz@theobroma-systems.com>

[PATCH v4 1/2] binman: bintool: remove btool_ prefix from btool names

[Quentin Schulz]

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

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.

Since a btool can have its btool_ prefix missing but its module and
binary presence on the system appropriately found, there's no need to
actually keep this prefix after listing all possible btools, so let's
remove it.

This fixes gzip btool by letting Bintool.find_bintool_class handle the
missing prefix and still return the correct class which is then init
with gzip name instead of btool_gzip.

Additionally, there was an issue with the cached module global variable.
The variable only stores the module and not the associated class name
when calling find_bintool_class.
This means that when caching the module on the first call to
find_bintool_class, class_name would be set to Bintoolbtool_gzip but the
module_name gzip only, adding the module in the gzip key in the module
dictionary. When hitting the cache on next calls, the gzip key would be
found, so its value (the module) is used. However the default class_name
(Bintoolgzip) is used, failing the getattr call.

Instead, let's enforce the same class name: Bintool<packer>, whatever
the filename it is contained in.

Cc: Quentin Schulz <foss+uboot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 tools/binman/bintool.py          | 3 ++-
 tools/binman/btool/btool_gzip.py | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
index a582d9d344..8fda13ff01 100644
--- a/tools/binman/bintool.py
+++ b/tools/binman/bintool.py
@@ -85,7 +85,6 @@ class Bintool:
                 try:
                     # Deal with classes which must be renamed due to conflicts
                     # with Python libraries
-                    class_name = f'Bintoolbtool_{module_name}'
                     module = importlib.import_module('binman.btool.btool_' +
                                                      module_name)
                 except ImportError:
@@ -137,6 +136,8 @@ class Bintool:
         names = [os.path.splitext(os.path.basename(fname))[0]
                  for fname in files]
         names = [name for name in names if name[0] != '_']
+        names = [name[6:] if name.startswith('btool_') else name
+                 for name in names]
         if include_testing:
             names.append('_testing')
         return sorted(names)
diff --git a/tools/binman/btool/btool_gzip.py b/tools/binman/btool/btool_gzip.py
index 70cbc19f04..a7ce6411cd 100644
--- a/tools/binman/btool/btool_gzip.py
+++ b/tools/binman/btool/btool_gzip.py
@@ -14,7 +14,7 @@ Documentation is available via::
 from binman import bintool
 
 # pylint: disable=C0103
-class Bintoolbtool_gzip(bintool.BintoolPacker):
+class Bintoolgzip(bintool.BintoolPacker):
     """Compression/decompression using the gzip algorithm
 
     This bintool supports running `gzip` to compress and decompress data, as

-- 
b4 0.10.1

[PATCH v4 2/2] Revert "binman: btool: gzip: fix packer name so that binary can be found"

[Quentin Schulz]

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

This reverts commit daa2da754afe1bac777f6cb0f05233e0de7b325d.

This commit is not needed anymore since the btool_ prefix is
automatically stripped by bintool.

Cc: Quentin Schulz <foss+uboot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 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 a7ce6411cd..0d75028120 100644
--- a/tools/binman/btool/btool_gzip.py
+++ b/tools/binman/btool/btool_gzip.py
@@ -27,5 +27,5 @@ class Bintoolgzip(bintool.BintoolPacker):
         man gzip
     """
     def __init__(self, name):
-        super().__init__("gzip", compress_args=[],
+        super().__init__(name, compress_args=[],
                          version_regex=r'gzip ([0-9.]+)')

-- 
b4 0.10.1

Re: [PATCH v4 1/2] binman: bintool: remove btool_ prefix from btool names

[Simon Glass]

On Mon, 7 Nov 2022 at 05:56, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> 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.
>
> Since a btool can have its btool_ prefix missing but its module and
> binary presence on the system appropriately found, there's no need to
> actually keep this prefix after listing all possible btools, so let's
> remove it.
>
> This fixes gzip btool by letting Bintool.find_bintool_class handle the
> missing prefix and still return the correct class which is then init
> with gzip name instead of btool_gzip.
>
> Additionally, there was an issue with the cached module global variable.
> The variable only stores the module and not the associated class name
> when calling find_bintool_class.
> This means that when caching the module on the first call to
> find_bintool_class, class_name would be set to Bintoolbtool_gzip but the
> module_name gzip only, adding the module in the gzip key in the module
> dictionary. When hitting the cache on next calls, the gzip key would be
> found, so its value (the module) is used. However the default class_name
> (Bintoolgzip) is used, failing the getattr call.
>
> Instead, let's enforce the same class name: Bintool<packer>, whatever
> the filename it is contained in.
>
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  tools/binman/bintool.py          | 3 ++-
>  tools/binman/btool/btool_gzip.py | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

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

Re: [PATCH v4 2/2] Revert "binman: btool: gzip: fix packer name so that binary can be found"

[Simon Glass]

On Mon, 7 Nov 2022 at 05:56, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> This reverts commit daa2da754afe1bac777f6cb0f05233e0de7b325d.
>
> This commit is not needed anymore since the btool_ prefix is
> automatically stripped by bintool.
>
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  tools/binman/btool/btool_gzip.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [PATCH v4 2/2] Revert "binman: btool: gzip: fix packer name so that binary can be found"

[Simon Glass]

On Mon, 7 Nov 2022 at 05:56, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> This reverts commit daa2da754afe1bac777f6cb0f05233e0de7b325d.
>
> This commit is not needed anymore since the btool_ prefix is
> automatically stripped by bintool.
>
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  tools/binman/btool/btool_gzip.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Applied to u-boot-dm, thanks!

Re: [PATCH v4 1/2] binman: bintool: remove btool_ prefix from btool names

[Simon Glass]

On Mon, 7 Nov 2022 at 05:56, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> 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.
>
> Since a btool can have its btool_ prefix missing but its module and
> binary presence on the system appropriately found, there's no need to
> actually keep this prefix after listing all possible btools, so let's
> remove it.
>
> This fixes gzip btool by letting Bintool.find_bintool_class handle the
> missing prefix and still return the correct class which is then init
> with gzip name instead of btool_gzip.
>
> Additionally, there was an issue with the cached module global variable.
> The variable only stores the module and not the associated class name
> when calling find_bintool_class.
> This means that when caching the module on the first call to
> find_bintool_class, class_name would be set to Bintoolbtool_gzip but the
> module_name gzip only, adding the module in the gzip key in the module
> dictionary. When hitting the cache on next calls, the gzip key would be
> found, so its value (the module) is used. However the default class_name
> (Bintoolgzip) is used, failing the getattr call.
>
> Instead, let's enforce the same class name: Bintool<packer>, whatever
> the filename it is contained in.
>
> Cc: Quentin Schulz <foss+uboot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  tools/binman/bintool.py          | 3 ++-
>  tools/binman/btool/btool_gzip.py | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

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

Applied to u-boot-dm, thanks!