diff mbox series

[v2,03/10] binman: Simplify comp_util class

Message ID 20220808105125.21356-3-stefan.herbrechtsmeier-oss@weidmueller.com
State Superseded
Delegated to: Simon Glass
Headers show
Series [v2,01/10] binman: Skip elf tests if python elftools is not available | expand

Commit Message

Stefan Herbrechtsmeier Aug. 8, 2022, 10:51 a.m. UTC 
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Simplify the comp_util class by remove duplicate code as preparation for
additional compress algorithms.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

(no changes since v1)

 tools/binman/cbfs_util_test.py |  2 +-
 tools/binman/comp_util.py      | 46 +++++++++++++++++++++++-----------
 tools/binman/ftest.py          |  2 +-
 3 files changed, 33 insertions(+), 17 deletions(-)

Comments

Simon Glass Aug. 13, 2022, 2:59 p.m. UTC | #1 
Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Simplify the comp_util class by remove duplicate code as preparation for
> additional compress algorithms.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
>
> (no changes since v1)
>
>  tools/binman/cbfs_util_test.py |  2 +-
>  tools/binman/comp_util.py      | 46 +++++++++++++++++++++++-----------
>  tools/binman/ftest.py          |  2 +-
>  3 files changed, 33 insertions(+), 17 deletions(-)

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

Please see below

>
> diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
> index f86b295149..44ebd04278 100755
> --- a/tools/binman/cbfs_util_test.py
> +++ b/tools/binman/cbfs_util_test.py
> @@ -50,7 +50,7 @@ class TestCbfs(unittest.TestCase):
>          cls.cbfstool = bintool.Bintool.create('cbfstool')
>          cls.have_cbfstool = cls.cbfstool.is_present()
>
> -        cls.have_lz4 = comp_util.HAVE_LZ4
> +        cls.have_lz4 = comp_util.is_present('lz4')
>
>      @classmethod
>      def tearDownClass(cls):
> diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
> index 269bbf7975..faa08a7f8a 100644
> --- a/tools/binman/comp_util.py
> +++ b/tools/binman/comp_util.py
> @@ -1,5 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  # Copyright 2022 Google LLC
> +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
> +# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>  #
>  """Utilities to compress and decompress data"""
>
> @@ -8,12 +10,23 @@ import tempfile
>  from binman import bintool
>  from patman import tools
>
> -LZ4 = bintool.Bintool.create('lz4')
> -HAVE_LZ4 = LZ4.is_present()
> +# Supported compressions
> +COMPRESSIONS = ['lz4', 'lzma']

How about ALGOS ?

'Compressions' is a bit of an odd word.

>
> -LZMA_ALONE = bintool.Bintool.create('lzma_alone')
> -HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
> +bintools = {}
>
> +def _get_tool_name(algo):

function comment

> +    names = {'lzma': 'lzma_alone'}
> +    return names.get(algo, algo)
> +
> +def _get_tool(algo):

function comment

> +    global bintools
> +    name = _get_tool_name(algo)
> +    tool = bintools.get(name)
> +    if not tool:
> +        tool = bintool.Bintool.create(name)
> +        bintools[name] = tool
> +    return tool
>
>  def compress(indata, algo):
>      """Compress some data using a given algorithm
> @@ -33,13 +46,12 @@ def compress(indata, algo):
>      """
>      if algo == 'none':
>          return indata
> -    if algo == 'lz4':
> -        data = LZ4.compress(indata)
> -    # cbfstool uses a very old version of lzma
> -    elif algo == 'lzma':
> -        data = LZMA_ALONE.compress(indata)
> -    else:
> +    if algo not in COMPRESSIONS:
>          raise ValueError("Unknown algorithm '%s'" % algo)
> +
> +    tool = _get_tool(algo)
> +    data = tool.compress(indata)
> +
>      return data
>
>  def decompress(indata, algo):
> @@ -60,10 +72,14 @@ def decompress(indata, algo):
>      """
>      if algo == 'none':
>          return indata
> -    if algo == 'lz4':
> -        data = LZ4.decompress(indata)
> -    elif algo == 'lzma':
> -        data = LZMA_ALONE.decompress(indata)
> -    else:
> +    if algo not in COMPRESSIONS:
>          raise ValueError("Unknown algorithm '%s'" % algo)
> +
> +    tool = _get_tool(algo)
> +    data = tool.decompress(indata)
> +
>      return data
> +
> +def is_present(algo):
> +     tool = _get_tool(algo)
> +     return tool.is_present()
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 057b4e28b7..96c15cff77 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -212,7 +212,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('tee.elf',
>              tools.read_file(cls.ElfTestFile('elf_sections')))
>
> -        cls.have_lz4 = comp_util.HAVE_LZ4
> +        cls.have_lz4 = comp_util.is_present('lz4')
>
>      @classmethod
>      def tearDownClass(cls):
> --
> 2.30.2
>

Regards,
Simon
diff mbox series

Patch

diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index f86b295149..44ebd04278 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -50,7 +50,7 @@  class TestCbfs(unittest.TestCase):
         cls.cbfstool = bintool.Bintool.create('cbfstool')
         cls.have_cbfstool = cls.cbfstool.is_present()
 
-        cls.have_lz4 = comp_util.HAVE_LZ4
+        cls.have_lz4 = comp_util.is_present('lz4')
 
     @classmethod
     def tearDownClass(cls):
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 269bbf7975..faa08a7f8a 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -1,5 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0+
 # Copyright 2022 Google LLC
+# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
 #
 """Utilities to compress and decompress data"""
 
@@ -8,12 +10,23 @@  import tempfile
 from binman import bintool
 from patman import tools
 
-LZ4 = bintool.Bintool.create('lz4')
-HAVE_LZ4 = LZ4.is_present()
+# Supported compressions
+COMPRESSIONS = ['lz4', 'lzma']
 
-LZMA_ALONE = bintool.Bintool.create('lzma_alone')
-HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
+bintools = {}
 
+def _get_tool_name(algo):
+    names = {'lzma': 'lzma_alone'}
+    return names.get(algo, algo)
+
+def _get_tool(algo):
+    global bintools
+    name = _get_tool_name(algo)
+    tool = bintools.get(name)
+    if not tool:
+        tool = bintool.Bintool.create(name)
+        bintools[name] = tool
+    return tool
 
 def compress(indata, algo):
     """Compress some data using a given algorithm
@@ -33,13 +46,12 @@  def compress(indata, algo):
     """
     if algo == 'none':
         return indata
-    if algo == 'lz4':
-        data = LZ4.compress(indata)
-    # cbfstool uses a very old version of lzma
-    elif algo == 'lzma':
-        data = LZMA_ALONE.compress(indata)
-    else:
+    if algo not in COMPRESSIONS:
         raise ValueError("Unknown algorithm '%s'" % algo)
+
+    tool = _get_tool(algo)
+    data = tool.compress(indata)
+
     return data
 
 def decompress(indata, algo):
@@ -60,10 +72,14 @@  def decompress(indata, algo):
     """
     if algo == 'none':
         return indata
-    if algo == 'lz4':
-        data = LZ4.decompress(indata)
-    elif algo == 'lzma':
-        data = LZMA_ALONE.decompress(indata)
-    else:
+    if algo not in COMPRESSIONS:
         raise ValueError("Unknown algorithm '%s'" % algo)
+
+    tool = _get_tool(algo)
+    data = tool.decompress(indata)
+
     return data
+
+def is_present(algo):
+     tool = _get_tool(algo)
+     return tool.is_present()
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 057b4e28b7..96c15cff77 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -212,7 +212,7 @@  class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('tee.elf',
             tools.read_file(cls.ElfTestFile('elf_sections')))
 
-        cls.have_lz4 = comp_util.HAVE_LZ4
+        cls.have_lz4 = comp_util.is_present('lz4')
 
     @classmethod
     def tearDownClass(cls):