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

Move the compressed data header handling into the dtb blob entry class and make it optional. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.

If needed the header could be enabled with the following attribute beside the compress attribute: prepend = "length";

The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger that the compressed contents. Regarding the commit "this is necessary to cope with a compressed device tree being updated in such a way that it shrinks after the entry size is already set (an obscure case)". This case need to be fixed without influence any compressed data by itself.

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

---

Changes in v2:

• Reworked to make the feature optional.

tools/binman/cbfs_util.py | 8 ++--- tools/binman/comp_util.py | 11 ++---- tools/binman/entries.rst | 4 +++ tools/binman/entry.py | 2 +- tools/binman/etype/blob_dtb.py | 21 ++++++++++++ tools/binman/ftest.py | 34 ++++++++++++++++--- .../test/235_compress_prepend_length_dtb.dts | 17 ++++++++++ 7 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts

I've been through this and I think it is a good change. We can use your technique (property in the blob_dtb etype) to deal with any future problems that come up.

But I would like to split this patch into three:

1. Add your blob_dtb property and the test 2. Change all uses of compress()/decompress() to call with add with_header=False 3. Drop the with_header arg from comp_util.py

A few more tweaks below.

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9cad03886f..a1836f4ad3 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -241,9 +241,9 @@ class CbfsFile(object): """Handle decompressing data if necessary""" indata = self.data if self.compress == COMPRESS_LZ4:

•        data = comp_util.decompress(indata, 'lz4', with_header=False)
  

•        data = comp_util.decompress(indata, 'lz4')
     elif self.compress == COMPRESS_LZMA:
  

•        data = comp_util.decompress(indata, 'lzma', with_header=False)
  

•        data = comp_util.decompress(indata, 'lzma')
     else:
         data = indata
     self.memlen = len(data)
  

@@ -362,9 +362,9 @@ class CbfsFile(object): elif self.ftype == TYPE_RAW: orig_data = data if self.compress == COMPRESS_LZ4:

•            data = comp_util.compress(orig_data, 'lz4', with_header=False)
  

•            data = comp_util.compress(orig_data, 'lz4')
         elif self.compress == COMPRESS_LZMA:
  

•            data = comp_util.compress(orig_data, 'lzma', with_header=False)
  

•            data = comp_util.compress(orig_data, 'lzma')
         self.memlen = len(orig_data)
         self.data_len = len(data)
         attr = struct.pack(ATTR_COMPRESSION_FORMAT,
  

diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index dc76adab35..269bbf7975 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -3,7 +3,6 @@ # """Utilities to compress and decompress data"""

-import struct import tempfile

from binman import bintool @@ -16,7 +15,7 @@ LZMA_ALONE = bintool.Bintool.create('lzma_alone') HAVE_LZMA_ALONE = LZMA_ALONE.is_present()

-def compress(indata, algo, with_header=True): +def compress(indata, algo): """Compress some data using a given algorithm

 Note that for lzma this uses an old version of the algorithm, not that

@@ -41,12 +40,9 @@ def compress(indata, algo, with_header=True): data = LZMA_ALONE.compress(indata) else: raise ValueError("Unknown algorithm '%s'" % algo)

• if with_header:

•    hdr = struct.pack('<I', len(data))
  

•    data = hdr + data
  

  return data

-def decompress(indata, algo, with_header=True): +def decompress(indata, algo): """Decompress some data using a given algorithm

 Note that for lzma this uses an old version of the algorithm, not that

@@ -64,9 +60,6 @@ def decompress(indata, algo, with_header=True): """ if algo == 'none': return indata

• if with_header:

•    data_len = struct.unpack('<I', indata[:4])[0]
  

•    indata = indata[4:4 + data_len]
  

  if algo == 'lz4': data = LZ4.decompress(indata) elif algo == 'lzma':

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index ae4305c99e..8e722426d3 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -208,6 +208,10 @@ This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module.

+Additional Properties / Entry arguments:

• • prepend: Header type to use:

•    none: No header
  

•    length: 32-bit length header
  
  
  

Entry: blob-ext: Externally built binary blob diff --git a/tools/binman/entry.py b/tools/binman/entry.py index a07a588864..8cbfd43af9 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1069,7 +1069,7 @@ features to produce new behaviours. indata: Data to compress

     Returns:

•        Compressed data (first word is the compressed size)
  

•        Compressed data
     """
     self.uncomp_data = indata
     if self.compress != 'none':
  

diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 4159e3032a..652b8abd8f 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -7,6 +7,8 @@

from binman.entry import Entry from binman.etype.blob import Entry_blob +from dtoc import fdt_util +import struct

# This is imported if needed state = None @@ -17,6 +19,9 @@ class Entry_blob_dtb(Entry_blob): This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module.

• Additional attributes:

•    prepend: Header used (e.g. 'length'), 'none' if none
  

  """ def __init__(self, section, etype, node): # Put this here to allow entry-docs and help to work without libfdt

@@ -25,6 +30,12 @@ class Entry_blob_dtb(Entry_blob):

     super().__init__(section, etype, node)

•    self.prepend = 'none'
  

None ?

• def ReadNode(self):

•    super().ReadNode()
  

•    self.prepend = fdt_util.GetString(self._node, 'prepend', 'none')
  

Can you drop the 'none' so that it uses None instead?

Aso we should check for a valid value here - e.g. it must be 'length' and not something else, otherwise self.Raise()

• def ObtainContents(self): """Get the device-tree from the list held by the 'state' module""" self._filename = self.GetDefaultFilename()

@@ -35,6 +46,9 @@ class Entry_blob_dtb(Entry_blob): """Re-read the DTB contents so that we get any calculated properties""" _, indata = state.GetFdtContents(self.GetFdtEtype()) data = self.CompressData(indata)

•    if self.prepend == 'length':
  

•        hdr = struct.pack('<I', len(data))
  

•        data = hdr + data
     return self.ProcessContentsUpdate(data)
  
  

  def GetFdtEtype(self):

@@ -50,6 +64,13 @@ class Entry_blob_dtb(Entry_blob): fname = self.GetDefaultFilename() return {self.GetFdtEtype(): [self, fname]}

• def ReadData(self, decomp=True, alt_format=None):

•    data = super().ReadData(decomp, alt_format)
  

•    if self.prepend == 'length':
  

•        data_len = struct.unpack('<I', data[:4])[0]
  

•        data = data[4:4 + data_len]
  

•    return data
  

• def WriteData(self, data, decomp=True): ok = super().WriteData(data, decomp)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index da9aa9e679..057b4e28b7 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2536,6 +2536,28 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)

• def testCompressPrependLengthDtb(self):

•    """Test that compress of device-tree files with length header is
  

•    supported
  

•    """
  

•    data = self.data = self._DoReadFileRealDtb('235_compress_prepend_length_dtb.dts')
  

•    self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
  

•    dtb_data = data[len(U_BOOT_DATA):]
  

•    comp_data_len = struct.unpack('<I', dtb_data[:4])[0]
  

•    comp_data = dtb_data[4:4 + comp_data_len]
  

•    orig = self._decompress(comp_data)
  

•    dtb = fdt.Fdt.FromData(orig)
  

•    dtb.Scan()
  

•    props = self._GetPropTree(dtb, ['size', 'uncomp-size'])
  

•    expected = {
  

•        'u-boot:size': len(U_BOOT_DATA),
  

•        'u-boot-dtb:uncomp-size': len(orig),
  

•        'u-boot-dtb:size': len(dtb_data),
  

•        'size': len(data),
  

•        }
  

•    self.assertEqual(expected, props)
  

• def testCbfsUpdateFdt(self): """Test that we can update the device tree with CBFS offset/size info""" self._CheckLz4()

@@ -2856,7 +2878,7 @@ class TestFunctional(unittest.TestCase): def testExtractCbfsRaw(self): """Test extracting CBFS compressed data without decompressing it""" data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False)

•    dtb = comp_util.decompress(data, 'lzma', with_header=False)
  

•    dtb = comp_util.decompress(data, 'lzma')
     self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
  
  

  def testExtractBadEntry(self):

@@ -4427,15 +4449,17 @@ class TestFunctional(unittest.TestCase): rest = base[len(U_BOOT_DATA):]

     # Check compressed data

•    section1 = self._decompress(rest)
     expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4')
  

•    self.assertEquals(expect1, rest[:len(expect1)])
  

•    data1 = rest[:len(expect1)]
  

•    section1 = self._decompress(data1)
  

•    self.assertEquals(expect1, data1)
     self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1)
     rest1 = rest[len(expect1):]
  
  

•    section2 = self._decompress(rest1)
     expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4')
  

•    self.assertEquals(expect2, rest1[:len(expect2)])
  

•    data2 = rest1[:len(expect2)]
  

•    section2 = self._decompress(data2)
  

•    self.assertEquals(expect2, data2)
     self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2)
     rest2 = rest1[len(expect2):]
  
  

diff --git a/tools/binman/test/235_compress_prepend_length_dtb.dts b/tools/binman/test/235_compress_prepend_length_dtb.dts new file mode 100644 index 0000000000..a5abc60477 --- /dev/null +++ b/tools/binman/test/235_compress_prepend_length_dtb.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

•   #address-cells = <1>;
  

•   #size-cells = <1>;
  

•   binman {
  

•           u-boot {
  

•           };
  

•           u-boot-dtb {
  

•                   compress = "lz4";
  

•                   prepend = "length";
  

•           };
  

•   };
  

+};

2.30.2

Regards, Simon

Hi Stefan,

On Mon, 15 Aug 2022 at 01:09, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:

Hi Simon,

Am 13.08.2022 um 16:59 schrieb Simon Glass:

...

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier

[..]

...

...

# This is imported if needed state = None @@ -17,6 +19,9 @@ class Entry_blob_dtb(Entry_blob): This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module.

• Additional attributes:

•    prepend: Header used (e.g. 'length'), 'none' if none
  """
  def __init__(self, section, etype, node):
      # Put this here to allow entry-docs and help to work without libfdt
  

@@ -25,6 +30,12 @@ class Entry_blob_dtb(Entry_blob):

      super().__init__(section, etype, node)

•    self.prepend = 'none'
  

None ?

I copy this from the compress attribute. You only need one check to support a missing value or a 'none' value. But I don't need this check and can use None.

OK I see. The idea there was that people might want to explicitly say 'none'. I;m not sure how use that is, particularly with prepend, but I'm OK with either way.

...

...

• def ReadNode(self):

•    super().ReadNode()
  

•    self.prepend = fdt_util.GetString(self._node, 'prepend', 'none')
  

Can you drop the 'none' so that it uses None instead?

Is 'none' a valid entry? Do we need to distinguish between 'none' and an invalid value?

Eventually we do...but for now bad things happen. See the TODO in binman for some of that.

...

Aso we should check for a valid value here - e.g. it must be 'length' and not something else, otherwise self.Raise()

Okay. I will remove the 'none' and only support 'length'.

As above, up to you. I had forgotten about the compress thing.

Regards, Simon

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

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

Add common test functions to test all supported compressions.

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

---

Instead of the for loop it is possible to use Parameterized [1] testing.

[1] https://github.com/wolever/parameterized

Changes in v2:

• Added

tools/binman/ftest.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

nits below

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 96c15cff77..c9b67c48d6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5248,6 +5248,30 @@ fdt fdtmap Extract the devicetree blob from the fdtmap comp_util.decompress(b'1234', 'invalid') self.assertIn("Unknown algorithm 'invalid'", str(e.exception))

• def testCompressions(self):

•    """Test compression algorithms"""
  

•    for algo in comp_util.COMPRESSIONS:
  

•        data = comp_util.compress(COMPRESS_DATA, algo)
  

•        self.assertNotEqual(COMPRESS_DATA, data)
  

•        orig = comp_util.decompress(data, algo)
  

•        self.assertEquals(COMPRESS_DATA, orig)
  

• def testVersions(self):

•    """Test tool version of compression algorithms"""
  

•    for algo in comp_util.COMPRESSIONS:
  

•        tool = comp_util._get_tool(algo)
  

•        version = tool.version()
  

•        print('%s - %s' % (algo, version))
  

•        self.assertRegex(version, '^v?[0-9]+[0-9.]*')
  

• def testPadding(self):

•    """Test padding of compression algorithms"""
  

•    for algo in comp_util.COMPRESSIONS:
  

•        data = comp_util.compress(COMPRESS_DATA, algo)
  

•        data = data + bytes([0]) * 64
  

tools.get_bytes(0, 64)

•        orig = comp_util.decompress(data, algo)
  

•        self.assertEquals(COMPRESS_DATA, orig)
  

• def testBintoolDocs(self): """Test for creation of bintool documentation""" with test_util.capture_sys_output() as (stdout, stderr):

-- 2.30.2

Regards, Simon

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

Add a bintools base class for packers which compression / decompression entry contents.

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

---

Changes in v2:

• Added

tools/binman/bintool.py | 94 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

The one strange thing about this is that we don't support these compression tools being missing, as we do with other tools. If 'lz4' is needed and not present, binman just fails. This predates your change, but I just noticed it.

I think this is OK though, at least for now. We could handle a missing algo by returning empty data and marking the entry as 'missing', but I don't see a great need for it at present. The compression tools are widely available and easy to install.

nits below

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 8435b29749..1d22962790 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.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 # """Base class for all bintools

@@ -464,3 +466,95 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the str: Version string for this bintool """ return 'unknown'

+class BintoolPacker(Bintool):

• """Tool which compression / decompression entry contents
• This is a bintools base class for compression / decompression packer
• """
• def __init__(self, name, compression=None, compress_args=None,

•             decompress_args=None, fetch_package=None,
  

•             version_regex=r'(v[0-9.]+)'):
  

Can you document these?

•    desc = '%s compression' % (compression if compression else name)
  

•    super().__init__(name, desc)
  

•    if compress_args is None:
  

•        compress_args = ['--compress']
  

•    self.compress_args = compress_args
  

•    if decompress_args is None:
  

•        decompress_args = ['--decompress']
  

•    self.decompress_args = decompress_args
  

•    if fetch_package is None:
  

•        fetch_package = name
  

•    self.fetch_package = fetch_package
  

•    self.version_regex = version_regex
  

• def compress(self, indata):

•    """Compress data
  

•    Args:
  

•        indata (bytes): Data to compress
  

•    Returns:
  

•        bytes: Compressed data
  

•    """
  

•    with tempfile.NamedTemporaryFile(prefix='comp.tmp',
  

•                                     dir=tools.get_output_dir()) as tmp:
  

•        tools.write_file(tmp.name, indata)
  

•        args = self.compress_args + ['--stdout', tmp.name]
  

•        return self.run_cmd(*args, binary=True)
  

• def decompress(self, indata):

•    """Decompress data
  

•    Args:
  

•        indata (bytes): Data to decompress
  

•    Returns:
  

•        bytes: Decompressed data
  

•    """
  

•    with tempfile.NamedTemporaryFile(prefix='decomp.tmp',
  

•                                     dir=tools.get_output_dir()) as inf:
  

•        tools.write_file(inf.name, indata)
  

•        args = self.decompress_args + ['--stdout', inf.name]
  

•        return self.run_cmd(*args, binary=True)
  

• def fetch(self, method):

•    """Fetch handler
  

•    This installs the gzip package using the apt utility.
  

•    Args:
  

•        method (FETCH_...): Method to use
  

•    Returns:
  

•        True if the file was fetched and now installed, None if a method
  

•        other than FETCH_BIN was requested
  

•    Raises:
  

•        Valuerror: Fetching could not be completed
  

•    """
  

•    if method != FETCH_BIN:
  

•        return None
  

•    pkg = self.fetch_package if self.fetch_package else self.name
  

•    return self.apt_install(pkg)
  

• def version(self):

•    """Version handler
  

•    Returns:
  

•        str: Version number
  

•    """
  

•    import re
  

•    result = self.run_cmd_result('-V')
  

•    if not result:
  

•        return super().version()
  

•    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
  

-- 2.30.2

Regards, Simon