[PATCH v4 01/13] binman: Skip elf tests if python elftools is not available
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Skip tests which requires python elftools if the tool is not available.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com Reviewed-by: Simon Glass sjg@chromium.org
---
(no changes since v2)
Changes in v2: - Added
tools/binman/elf_test.py | 14 ++++++++++++++ tools/binman/ftest.py | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 5a51c64cfe..75b867c2be 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -122,6 +122,8 @@ class TestElf(unittest.TestCase):
def testOutsideFile(self): """Test a symbol which extends outside the entry area is detected""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(10) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms') @@ -147,6 +149,8 @@ class TestElf(unittest.TestCase): Only 32 and 64 bits are supported, since we need to store an offset into the image. """ + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(10) section = FakeSection() elf_fname =self.ElfTestFile('u_boot_binman_syms_size') @@ -161,6 +165,8 @@ class TestElf(unittest.TestCase): This should produce -1 values for all thress symbols, taking up the first 16 bytes of the image. """ + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(28) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') @@ -172,6 +178,8 @@ class TestElf(unittest.TestCase):
def testDebug(self): """Check that enabling debug in the elf module produced debug output""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') try: tout.init(tout.DEBUG) entry = FakeEntry(24) @@ -281,6 +289,8 @@ class TestElf(unittest.TestCase):
def test_read_segments_bad_data(self): """Test for read_loadable_segments() with an invalid ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('embed_data') with self.assertRaises(ValueError) as e: elf.read_loadable_segments(tools.get_bytes(100, 100)) @@ -288,6 +298,8 @@ class TestElf(unittest.TestCase):
def test_get_file_offset(self): """Test GetFileOffset() gives the correct file offset for a symbol""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('embed_data') syms = elf.GetSymbols(fname, ['embed']) addr = syms['embed'].address @@ -314,6 +326,8 @@ class TestElf(unittest.TestCase):
def test_get_symbol_from_address(self): """Test GetSymbolFromAddress()""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('elf_sections') sym_name = 'calculate' syms = elf.GetSymbols(fname, [sym_name]) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index fa1f421c05..da9aa9e679 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4807,6 +4807,8 @@ class TestFunctional(unittest.TestCase):
def testUpdateFdtInElf(self): """Test that we can update the devicetree in an ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed') outfile = os.path.join(self._indir, 'u-boot.out') begin_sym = 'dtb_embed_begin' @@ -4858,6 +4860,8 @@ class TestFunctional(unittest.TestCase):
def testUpdateFdtInElfNoSyms(self): """Test that missing symbols are detected with --update-fdt-in-elf""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed') outfile = '' begin_sym = 'wrong_begin' @@ -4871,6 +4875,8 @@ class TestFunctional(unittest.TestCase):
def testUpdateFdtInElfTooSmall(self): """Test that an over-large dtb is detected with --update-fdt-in-elf""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed_sm') outfile = os.path.join(self._indir, 'u-boot.out') begin_sym = 'dtb_embed_begin' @@ -5344,6 +5350,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElf(self): """Test an image with an FIT with an split-elf operation""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry_args = { 'of-list': 'test-fdt1 test-fdt2', 'default-dt': 'test-fdt2', @@ -5421,6 +5429,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfBadElf(self): """Test a FIT split-elf operation with an invalid ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') TestFunctional._MakeInputFile('bad.elf', tools.get_bytes(100, 100)) entry_args = { 'of-list': 'test-fdt1 test-fdt2', @@ -5440,6 +5450,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfBadDirective(self): """Test a FIT split-elf invalid fit,xxx directive in an image node""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self._check_bad_fit('227_fit_bad_dir.dts') self.assertIn( "Node '/binman/fit': subnode 'images/@atf-SEQ': Unknown directive 'fit,something'", @@ -5447,6 +5459,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfBadDirectiveConfig(self): """Test a FIT split-elf with invalid fit,xxx directive in config""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self._check_bad_fit('228_fit_bad_dir_config.dts') self.assertEqual( "Node '/binman/fit': subnode 'configurations/@config-SEQ': Unknown directive 'fit,config'", @@ -5470,6 +5484,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfMissing(self): """Test an split-elf FIT with a missing ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self.checkFitSplitElf(allow_missing=True) self.assertRegex( err, @@ -5477,6 +5493,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfFaked(self): """Test an split-elf FIT with faked ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self.checkFitSplitElf(allow_missing=True, allow_fake_blobs=True) self.assertRegex( err,
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add an optional length header attribute to the device tree blob entry class based on the compressed data header from the utilities to compress and decompress data.
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 than 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
---
(no changes since v3)
Changes in v3: - Move comp_util.py changes (drop with_header) into separate commits.
Changes in v2: - Reworked to make the feature optional.
tools/binman/entries.rst | 3 +++ tools/binman/etype/blob_dtb.py | 24 +++++++++++++++++++ tools/binman/ftest.py | 22 +++++++++++++++++ .../test/235_compress_prepend_length_dtb.dts | 17 +++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 782bff1f8d..c9336d1bb4 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -216,6 +216,9 @@ 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: + length: 32-bit length header
.. _etype_blob_ext: diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 4159e3032a..4fd2ecda83 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') """ def __init__(self, section, etype, node): # Put this here to allow entry-docs and help to work without libfdt @@ -25,6 +30,15 @@ class Entry_blob_dtb(Entry_blob):
super().__init__(section, etype, node)
+ self.prepend = None + + def ReadNode(self): + super().ReadNode() + self.prepend = fdt_util.GetString(self._node, 'prepend') + if self.prepend and self.prepend not in ['length']: + self.Raise("Invalid prepend in '%s': '%s'" % + (self._node.name, self.prepend)) + def ObtainContents(self): """Get the device-tree from the list held by the 'state' module""" self._filename = self.GetDefaultFilename() @@ -35,6 +49,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 +67,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..1b468d8e6d 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() 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"; + }; + }; +};
Hi Stefan,
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add an optional length header attribute to the device tree blob entry class based on the compressed data header from the utilities to compress and decompress data.
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 than 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
(no changes since v3)
This looks fine except that it drops test coverage (binman test -T should be 100%). Please can you check it?
Regards, Simon
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Disable the compressed data header of the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger than the compressed contents.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Added
tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 3 ++- tools/binman/ftest.py | 18 +++++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e3767aefa7..a45aeeaa59 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1079,7 +1079,7 @@ features to produce new behaviours. self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata) - data = comp_util.compress(indata, self.compress) + data = comp_util.compress(indata, self.compress, with_header=False) return data
@classmethod diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b91..0a92bf2386 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -777,7 +777,8 @@ class Entry_section(Entry): data = parent_data[offset:offset + child.size] if decomp: indata = data - data = comp_util.decompress(indata, child.compress) + data = comp_util.decompress(indata, child.compress, + with_header=False) if child.uncomp_size: tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" % (child.GetPath(), len(indata), child.compress, diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1b468d8e6d..d082442bec 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1967,7 +1967,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()
def _decompress(self, data): - return comp_util.decompress(data, 'lz4') + return comp_util.decompress(data, 'lz4', with_header=False)
def testCompress(self): """Test compression of blobs""" @@ -4449,15 +4449,19 @@ 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)]) + expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4', + with_header=False) + 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)]) + expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4', + with_header=False) + data2 = rest1[:len(expect2)] + section2 = self._decompress(data2) + self.assertEquals(expect2, data2) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):]
Hi Stefan,
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Disable the compressed data header of the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger than the compressed contents.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Added
tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 3 ++- tools/binman/ftest.py | 18 +++++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-)
This seems strange to me.
I would expect this patch to make every call set with_header to False, then the next patch remove the parameters. That would allow later bisecting to see where a problem occurs.
But at present this patch and the next seem to be mixed together.
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e3767aefa7..a45aeeaa59 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1079,7 +1079,7 @@ features to produce new behaviours. self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata)
data = comp_util.compress(indata, self.compress)
data = comp_util.compress(indata, self.compress, with_header=False) return data@classmethod
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b91..0a92bf2386 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -777,7 +777,8 @@ class Entry_section(Entry): data = parent_data[offset:offset + child.size] if decomp: indata = data
data = comp_util.decompress(indata, child.compress)
data = comp_util.decompress(indata, child.compress,with_header=False) if child.uncomp_size: tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" % (child.GetPath(), len(indata), child.compress,diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1b468d8e6d..d082442bec 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1967,7 +1967,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()
def _decompress(self, data):
return comp_util.decompress(data, 'lz4')
return comp_util.decompress(data, 'lz4', with_header=False)def testCompress(self): """Test compression of blobs"""
@@ -4449,15 +4449,19 @@ 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)])
expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4',with_header=False)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)])
expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4',with_header=False)data2 = rest1[:len(expect2)]section2 = self._decompress(data2)self.assertEquals(expect2, data2) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):]-- 2.30.2
Regards, Simon
Hi Simon,
Am 16.08.2022 um 13:48 schrieb Simon Glass:
Hi Stefan,
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Disable the compressed data header of the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger than the compressed contents.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
Added
tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 3 ++- tools/binman/ftest.py | 18 +++++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-)
This seems strange to me.
I would expect this patch to make every call set with_header to False, then the next patch remove the parameters. That would allow later bisecting to see where a problem occurs.
But at present this patch and the next seem to be mixed together.
I skipped the following calls because it doesn't matter: comp_util.compress(b'', 'invalid') comp_util.decompress(b'1234', 'invalid')
Do I miss a call?
The cbfs_util.py file doesn't use the header.
Hi Stefan,
On Tue, 16 Aug 2022 at 06:03, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
Hi Simon,
Am 16.08.2022 um 13:48 schrieb Simon Glass:
Hi Stefan,
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Disable the compressed data header of the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger than the compressed contents.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
Added
tools/binman/entry.py | 2 +- tools/binman/etype/section.py | 3 ++- tools/binman/ftest.py | 18 +++++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-)
This seems strange to me.
I would expect this patch to make every call set with_header to False, then the next patch remove the parameters. That would allow later bisecting to see where a problem occurs.
But at present this patch and the next seem to be mixed together.
I skipped the following calls because it doesn't matter: comp_util.compress(b'', 'invalid') comp_util.decompress(b'1234', 'invalid')
I still think it is worth it, because the default is True.
Do I miss a call?
Everything should pass False in this patch I think.
The cbfs_util.py file doesn't use the header.
OK
Regards, SImon
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Remove the obsolete compressed data header handling from the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com ---
(no changes since v3)
Changes in v3: - Added
tools/binman/cbfs_util.py | 8 ++++---- tools/binman/comp_util.py | 11 ++--------- tools/binman/entry.py | 4 ++-- tools/binman/etype/section.py | 3 +-- tools/binman/ftest.py | 10 ++++------ 5 files changed, 13 insertions(+), 23 deletions(-)
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/entry.py b/tools/binman/entry.py index a45aeeaa59..9ec5811b46 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1074,12 +1074,12 @@ 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': self.uncomp_size = len(indata) - data = comp_util.compress(indata, self.compress, with_header=False) + data = comp_util.compress(indata, self.compress) return data
@classmethod diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 0a92bf2386..bd67238b91 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -777,8 +777,7 @@ class Entry_section(Entry): data = parent_data[offset:offset + child.size] if decomp: indata = data - data = comp_util.decompress(indata, child.compress, - with_header=False) + data = comp_util.decompress(indata, child.compress) if child.uncomp_size: tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" % (child.GetPath(), len(indata), child.compress, diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index d082442bec..057b4e28b7 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1967,7 +1967,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()
def _decompress(self, data): - return comp_util.decompress(data, 'lz4', with_header=False) + return comp_util.decompress(data, 'lz4')
def testCompress(self): """Test compression of blobs""" @@ -2878,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): @@ -4449,16 +4449,14 @@ class TestFunctional(unittest.TestCase): rest = base[len(U_BOOT_DATA):]
# Check compressed data - expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4', - with_header=False) + expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') data1 = rest[:len(expect1)] section1 = self._decompress(data1) self.assertEquals(expect1, data1) self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):]
- expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4', - with_header=False) + expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') data2 = rest1[:len(expect2)] section2 = self._decompress(data2) self.assertEquals(expect2, data2)
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 v3)
Changes in v3: - Rename COMPRESSIONS to ALGORITHMS - Rework existing comments - Add _get_tool_name function comment - Add _get_tool function comment
tools/binman/cbfs_util_test.py | 2 +- tools/binman/comp_util.py | 101 +++++++++++++++++++++++---------- tools/binman/ftest.py | 2 +- 3 files changed, 72 insertions(+), 33 deletions(-)
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..00761d44cc 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -1,69 +1,108 @@ # 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""" +"""Utilities to compress and decompress data + +This supports the following compression algorithm: + none + lz4 + lzma + +Note that for lzma this uses an old version of the algorithm, not that +provided by xz. + +This requires the following tools: + lz4 + lzma_alone + +It also requires an output directory to be previously set up, by calling +PrepareOutputDir(). +"""
import tempfile
from binman import bintool from patman import tools
-LZ4 = bintool.Bintool.create('lz4') -HAVE_LZ4 = LZ4.is_present() +# Supported compression algorithms +ALGORITHMS = ['lz4', 'lzma']
-LZMA_ALONE = bintool.Bintool.create('lzma_alone') -HAVE_LZMA_ALONE = LZMA_ALONE.is_present() +bintools = {}
+def _get_tool_name(algo): + """Get the tool name of a compression algorithm
-def compress(indata, algo): - """Compress some data using a given algorithm + Args: + algo (str): Algorithm to use
- Note that for lzma this uses an old version of the algorithm, not that - provided by xz. + Returns: + str: Tool name + """ + names = {'lzma': 'lzma_alone'} + return names.get(algo, algo) + +def _get_tool(algo): + """Get the bintool object of a compression algorithm
- This requires 'lz4' and 'lzma_alone' tools. It also requires an output - directory to be previously set up, by calling PrepareOutputDir(). + The function creates new bintool object on demand per compression algorithm + and save it in a global bintools dictionary. + + Args: + algo (str): Algorithm to use + + Returns: + A bintool object for the compression algorithm + """ + 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
Args: indata (bytes): Input data to compress - algo (str): Algorithm to use ('none', 'lz4' or 'lzma') + algo (str): Algorithm to use
Returns: bytes: Compressed data """ 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 ALGORITHMS: raise ValueError("Unknown algorithm '%s'" % algo) + + tool = _get_tool(algo) + data = tool.compress(indata) + return data
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 - provided by xz. - - This requires 'lz4' and 'lzma_alone' tools. It also requires an output - directory to be previously set up, by calling PrepareOutputDir(). - Args: indata (bytes): Input data to decompress - algo (str): Algorithm to use ('none', 'lz4' or 'lzma') + algo (str): Algorithm to use
Returns: - (bytes) Compressed data + bytes: Decompressed data """ 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 ALGORITHMS: 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):
On Tue, 16 Aug 2022 at 02:42, 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 v3)
Changes in v3:
- Rename COMPRESSIONS to ALGORITHMS
- Rework existing comments
- Add _get_tool_name function comment
- Add _get_tool function comment
tools/binman/cbfs_util_test.py | 2 +- tools/binman/comp_util.py | 101 +++++++++++++++++++++++---------- tools/binman/ftest.py | 2 +- 3 files changed, 72 insertions(+), 33 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
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 Reviewed-by: Simon Glass sjg@chromium.org
--- Instead of the for loop it is possible to use Parameterized [1] testing.
[1] https://github.com/wolever/parameterized
(no changes since v3)
Changes in v3: - Use 'tools.get_bytes(0, 64)' instead of 'bytes([0]) * 64' - Check if tool is present - Rename tests
Changes in v2: - Added
tools/binman/ftest.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 96c15cff77..bc17107735 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5248,6 +5248,37 @@ fdt fdtmap Extract the devicetree blob from the fdtmap comp_util.decompress(b'1234', 'invalid') self.assertIn("Unknown algorithm 'invalid'", str(e.exception))
+ def _checkCompUtil(self, algo): + if not comp_util.is_present(algo): + self.skipTest('%s not available' % comp_util._get_tool_name(algo)) + + def testCompUtilCompressions(self): + """Test compression algorithms""" + for algo in comp_util.ALGORITHMS: + self._checkCompUtil(algo) + data = comp_util.compress(COMPRESS_DATA, algo) + self.assertNotEqual(COMPRESS_DATA, data) + orig = comp_util.decompress(data, algo) + self.assertEquals(COMPRESS_DATA, orig) + + def testCompUtilVersions(self): + """Test tool version of compression algorithms""" + for algo in comp_util.ALGORITHMS: + self._checkCompUtil(algo) + tool = comp_util._get_tool(algo) + version = tool.version() + print('%s - %s' % (algo, version)) + self.assertRegex(version, '^v?[0-9]+[0-9.]*') + + def testCompUtilPadding(self): + """Test padding of compression algorithms""" + for algo in comp_util.ALGORITHMS: + self._checkCompUtil(algo) + data = comp_util.compress(COMPRESS_DATA, algo) + data = data + 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):
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
---
(no changes since v3)
Changes in v3: - Document class properties
Changes in v2: - Added
tools/binman/bintool.py | 107 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 8435b29749..2eaad418f2 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,108 @@ 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 + + Properties: + name: Name of packer tool + compression: Compression type (COMPRESS_...), value of 'name' property + if none + compress_args: List of positional args provided to tool for compress, + ['--compress'] if none + decompress_args: List of positional args provided to tool for + decompress, ['--decompress'] if none + fetch_package: Name of the tool installed using the apt, value of 'name' + property if none + version_regex: Regular expressions to extract the version from tool + version output, '(v[0-9.]+)' if none + """ + def __init__(self, name, compression=None, compress_args=None, + decompress_args=None, fetch_package=None, + version_regex=r'(v[0-9.]+)'): + 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
On Tue, 16 Aug 2022 at 02:42, 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
(no changes since v3)
Changes in v3:
- Document class properties
Changes in v2:
- Added
tools/binman/bintool.py | 107 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add bzip2 bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Rebase
Changes in v2: - Added
tools/binman/btool/bzip2.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/bzip2.py
diff --git a/tools/binman/btool/bzip2.py b/tools/binman/btool/bzip2.py new file mode 100644 index 0000000000..9be87a621f --- /dev/null +++ b/tools/binman/btool/bzip2.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for bzip2 + +bzip2 allows compression and decompression of files. + +Documentation is available via:: + + man bzip2 +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolbzip2(bintool.BintoolPacker): + """Compression/decompression using the bzip2 algorithm + + This bintool supports running `bzip2` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man bzip2 + """ + def __init__(self, name): + super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)') diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 00761d44cc..6ec371b145 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -7,6 +7,7 @@
This supports the following compression algorithm: none + bzip2 lz4 lzma
@@ -14,6 +15,7 @@ Note that for lzma this uses an old version of the algorithm, not that provided by xz.
This requires the following tools: + bzip2 lz4 lzma_alone
@@ -27,7 +29,7 @@ from binman import bintool from patman import tools
# Supported compression algorithms -ALGORITHMS = ['lz4', 'lzma'] +ALGORITHMS = ['bzip2', 'lz4', 'lzma']
bintools = {}
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add bzip2 bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Rebase
Changes in v2:
- Added
tools/binman/btool/bzip2.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/bzip2.py
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add gzip bintool to binman to support on-the-fly compression of Linux kernel images and FPGA bitstreams. The SPL basic fitImage implementation supports only gzip decompression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Rebase
Changes in v2: - Added
tools/binman/btool/gzip.py | 31 +++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/gzip.py
diff --git a/tools/binman/btool/gzip.py b/tools/binman/btool/gzip.py new file mode 100644 index 0000000000..0d75028120 --- /dev/null +++ b/tools/binman/btool/gzip.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for gzip + +gzip allows compression and decompression of files. + +Documentation is available via:: + + man gzip +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolgzip(bintool.BintoolPacker): + """Compression/decompression using the gzip algorithm + + This bintool supports running `gzip` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man gzip + """ + def __init__(self, name): + super().__init__(name, compress_args=[], + version_regex=r'gzip ([0-9.]+)') diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 6ec371b145..19627e490c 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -8,6 +8,7 @@ This supports the following compression algorithm: none bzip2 + gzip lz4 lzma
@@ -16,6 +17,7 @@ provided by xz.
This requires the following tools: bzip2 + gzip lz4 lzma_alone
@@ -29,7 +31,7 @@ from binman import bintool from patman import tools
# Supported compression algorithms -ALGORITHMS = ['bzip2', 'lz4', 'lzma'] +ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma']
bintools = {}
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add gzip bintool to binman to support on-the-fly compression of Linux kernel images and FPGA bitstreams. The SPL basic fitImage implementation supports only gzip decompression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Rebase
Changes in v2:
- Added
tools/binman/btool/gzip.py | 31 +++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/gzip.py
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add lzop bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Rebase
Changes in v2: - Added
tools/binman/btool/lzop.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 6 ++++-- 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/lzop.py
diff --git a/tools/binman/btool/lzop.py b/tools/binman/btool/lzop.py new file mode 100644 index 0000000000..f6903b4db7 --- /dev/null +++ b/tools/binman/btool/lzop.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for lzop + +lzop allows compression and decompression of files. + +Documentation is available via:: + + man lzop +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoollzop(bintool.BintoolPacker): + """Compression/decompression using the lzop algorithm + + This bintool supports running `lzop` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man lzop + """ + def __init__(self, name): + super().__init__(name, 'lzo', compress_args=[]) diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 19627e490c..d8fdd5c7a2 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -11,6 +11,7 @@ This supports the following compression algorithm: gzip lz4 lzma + lzo
Note that for lzma this uses an old version of the algorithm, not that provided by xz. @@ -20,6 +21,7 @@ This requires the following tools: gzip lz4 lzma_alone + lzop
It also requires an output directory to be previously set up, by calling PrepareOutputDir(). @@ -31,7 +33,7 @@ from binman import bintool from patman import tools
# Supported compression algorithms -ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma'] +ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo']
bintools = {}
@@ -44,7 +46,7 @@ def _get_tool_name(algo): Returns: str: Tool name """ - names = {'lzma': 'lzma_alone'} + names = {'lzma': 'lzma_alone', 'lzo': 'lzop'} return names.get(algo, algo)
def _get_tool(algo):
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add lzop bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Rebase
Changes in v2:
- Added
tools/binman/btool/lzop.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 6 ++++-- 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/lzop.py
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add xz bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Rebase
Changes in v2: - Added
tools/binman/btool/xz.py | 31 +++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/xz.py
diff --git a/tools/binman/btool/xz.py b/tools/binman/btool/xz.py new file mode 100644 index 0000000000..e2b413d18b --- /dev/null +++ b/tools/binman/btool/xz.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for xz + +xz allows compression and decompression of files. + +Documentation is available via:: + + man xz +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolxz(bintool.BintoolPacker): + """Compression/decompression using the xz algorithm + + This bintool supports running `xz` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man xz + """ + def __init__(self, name): + super().__init__(name, fetch_package='xz-utils', + version_regex=r'xz (XZ Utils) ([0-9.]+)') diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index d8fdd5c7a2..8c0fe5078f 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -12,6 +12,7 @@ This supports the following compression algorithm: lz4 lzma lzo + xz
Note that for lzma this uses an old version of the algorithm, not that provided by xz. @@ -22,6 +23,7 @@ This requires the following tools: lz4 lzma_alone lzop + xz
It also requires an output directory to be previously set up, by calling PrepareOutputDir(). @@ -33,7 +35,7 @@ from binman import bintool from patman import tools
# Supported compression algorithms -ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo'] +ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz']
bintools = {}
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add xz bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Rebase
Changes in v2:
- Added
tools/binman/btool/xz.py | 31 +++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tools/binman/btool/xz.py
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add zstd bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
(no changes since v3)
Changes in v3: - Rebase
Changes in v2: - Added
tools/binman/btool/zstd.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- tools/binman/etype/blob_dtb.py | 4 ++++ tools/binman/ftest.py | 3 ++- 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/zstd.py
diff --git a/tools/binman/btool/zstd.py b/tools/binman/btool/zstd.py new file mode 100644 index 0000000000..299bd37126 --- /dev/null +++ b/tools/binman/btool/zstd.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for zstd + +zstd allows compression and decompression of files. + +Documentation is available via:: + + man zstd +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolzstd(bintool.BintoolPacker): + """Compression/decompression using the zstd algorithm + + This bintool supports running `zstd` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man zstd + """ + def __init__(self, name): + super().__init__(name) diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index 8c0fe5078f..6b4ab646e0 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -13,6 +13,7 @@ This supports the following compression algorithm: lzma lzo xz + zstd
Note that for lzma this uses an old version of the algorithm, not that provided by xz. @@ -24,6 +25,7 @@ This requires the following tools: lzma_alone lzop xz + zstd
It also requires an output directory to be previously set up, by calling PrepareOutputDir(). @@ -35,7 +37,7 @@ from binman import bintool from patman import tools
# Supported compression algorithms -ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz'] +ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz', 'zstd']
bintools = {}
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 4fd2ecda83..fab79e43cc 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -48,6 +48,10 @@ class Entry_blob_dtb(Entry_blob): def ProcessContents(self): """Re-read the DTB contents so that we get any calculated properties""" _, indata = state.GetFdtContents(self.GetFdtEtype()) + + if self.compress == 'zstd' and self.prepend != 'length': + self.Raise('The zstd compression requires a length header') + data = self.CompressData(indata) if self.prepend == 'length': hdr = struct.pack('<I', len(data)) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index bc17107735..a360ebeef5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5272,7 +5272,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testCompUtilPadding(self): """Test padding of compression algorithms""" - for algo in comp_util.ALGORITHMS: + # Skip zstd because it doesn't support padding + for algo in [a for a in comp_util.ALGORITHMS if a != 'zstd']: self._checkCompUtil(algo) data = comp_util.compress(COMPRESS_DATA, algo) data = data + tools.get_bytes(0, 64)
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Add zstd bintool to binman to support on-the-fly compression.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
(no changes since v3)
Changes in v3:
- Rebase
Changes in v2:
- Added
tools/binman/btool/zstd.py | 30 ++++++++++++++++++++++++++++++ tools/binman/comp_util.py | 4 +++- tools/binman/etype/blob_dtb.py | 4 ++++ tools/binman/ftest.py | 3 ++- 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/zstd.py
Reviewed-by: Simon Glass sjg@chromium.org
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Handle missing compression tools by returning empty data and marking the entry as 'missing'.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
Changes in v4: - Add missing 236_compress_dtb_missing_bintool.dts file
Changes in v3: - Added
tools/binman/entry.py | 4 ++++ tools/binman/ftest.py | 8 ++++++++ .../test/236_compress_dtb_missing_bintool.dts | 16 ++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 tools/binman/test/236_compress_dtb_missing_bintool.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 9ec5811b46..c86b757a4e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1078,7 +1078,11 @@ features to produce new behaviours. """ self.uncomp_data = indata if self.compress != 'none': + if not comp_util.is_present(self.compress): + self.missing = True + return b'' self.uncomp_size = len(indata) + data = comp_util.compress(indata, self.compress) return data
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a360ebeef5..eac7ccb087 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2557,6 +2557,14 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)
+ def testCompressMissingBintool(self): + """Test that compress of device-tree files with missing bintool is + supported + """ + data = self.data = self._DoReadFileRealDtb('236_compress_dtb_missing_bintool.dts') + self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)]) + dtb_data = data[len(U_BOOT_DATA):] + self.assertEqual(0, len(dtb_data))
def testCbfsUpdateFdt(self): """Test that we can update the device tree with CBFS offset/size info""" diff --git a/tools/binman/test/236_compress_dtb_missing_bintool.dts b/tools/binman/test/236_compress_dtb_missing_bintool.dts new file mode 100644 index 0000000000..e7ce1b893d --- /dev/null +++ b/tools/binman/test/236_compress_dtb_missing_bintool.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + u-boot-dtb { + compress = "_testing"; + }; + }; +};
Hi Stefan,
On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier stefan.herbrechtsmeier-oss@weidmueller.com wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Handle missing compression tools by returning empty data and marking the entry as 'missing'.
Great to see this.
This is actually a bit more subtle, see below.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Changes in v4:
- Add missing 236_compress_dtb_missing_bintool.dts file
Changes in v3:
- Added
tools/binman/entry.py | 4 ++++ tools/binman/ftest.py | 8 ++++++++ .../test/236_compress_dtb_missing_bintool.dts | 16 ++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 tools/binman/test/236_compress_dtb_missing_bintool.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 9ec5811b46..c86b757a4e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1078,7 +1078,11 @@ features to produce new behaviours. """ self.uncomp_data = indata if self.compress != 'none':
if not comp_util.is_present(self.compress):self.missing = True
We must check self.GetAllowMissing(). If that is True then we can do this. If False then we cannot. Hmm binman needs to fail if a bintool is missing, unless --allow-missing is given.
Also you should call self.record_missing_bintool() here, instead of setting self.missing (which refers to a missing blob).
BTW you also need to record the binbool somewhere with self.AddBintools() in Entry:
def AddBintools(self, btools): self.comp_bintool = self.AddBintool(btools, self.compress)
That way you will get the right message, which is 'has missing bintools'
You then need to check for that stdout in your test, e.g. as is done in testGbbMissing()
Finally, be careful to keep code coverage at 100%.
return b'' self.uncomp_size = len(indata)data = comp_util.compress(indata, self.compress) return datadiff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a360ebeef5..eac7ccb087 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2557,6 +2557,14 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)
- def testCompressMissingBintool(self):
"""Test that compress of device-tree files with missing bintool issupported
Please add new tests at the end (so things are roughly in order of test-file number). Also the test desc should fit on one like (e.g. drop the 'Test that' text.
"""data = self.data = self._DoReadFileRealDtb('236_compress_dtb_missing_bintool.dts')
Can you drop self.data ?
self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])dtb_data = data[len(U_BOOT_DATA):]self.assertEqual(0, len(dtb_data))
def testCbfsUpdateFdt(self): """Test that we can update the device tree with CBFS offset/size info"""diff --git a/tools/binman/test/236_compress_dtb_missing_bintool.dts b/tools/binman/test/236_compress_dtb_missing_bintool.dts new file mode 100644 index 0000000000..e7ce1b893d --- /dev/null +++ b/tools/binman/test/236_compress_dtb_missing_bintool.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+/ {
#address-cells = <1>;#size-cells = <1>;binman {u-boot {};u-boot-dtb {compress = "_testing";};};+};
2.30.2
Regards, Simon
participants (2)
-
Simon Glass -
Stefan Herbrechtsmeier