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