[PATCH v5 1/8] binman: add support for skipping file concatenation for mkimage
Simon Glass
sjg at chromium.org
Wed Aug 31 19:44:33 CEST 2022
Hi Quentin,
On Wed, 31 Aug 2022 at 10:39, Quentin Schulz
<quentin.schulz at theobroma-systems.com> wrote:
>
> Hi Simon,
>
> On 8/31/22 15:46, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Wed, 31 Aug 2022 at 03:25, Quentin Schulz
> > <quentin.schulz at theobroma-systems.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 8/31/22 05:15, Simon Glass wrote:
> >>> Hi Quentin,
> >>>
> >>> On Tue, 30 Aug 2022 at 11:54, Quentin Schulz
> >>> <quentin.schulz at theobroma-systems.com> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> On 8/30/22 17:56, Simon Glass wrote:
> >>>>> Hi Quentin,
> >>>>>
> >>>>> On Tue, 30 Aug 2022 at 03:57, Quentin Schulz
> >>>>> <quentin.schulz at theobroma-systems.com> wrote:
> [...]
> >>>> I do not understand how you found out coverage was not happy about my
> >>>> patchset. I have the same percentage reported from your branch or my
> >>>> local one. What am I missing?
> >>>>
> >>>> Regarding the content of the changed commits:
> >>>> testMkimageMultipleNoContent is not testing what is says it does?
> >>>> It's using multiple-data-files DT property which only impacts -d
> >>>> parameter of mkimage and the comment for the test is """Test using
> >>>> mkimage with -n and no data""".
> >>>>
> >>>> What exactly are you trying to test?
> >>>
> >>> 'binman test -T'
> >>>
> >>> I pushed your original patches to the try-rk4-orig branch. My changes
> >>> are in try-rk4.
> >>>
>
> I would change
> https://github.com/sjg20/u-boot/blob/try-rk4/tools/binman/ftest.py#L5917
> from
> """Test using mkimage with -n and no data"""
> to
> """Test passing multiple data files to mkimage with one data file having
> no content"""
> or something like that. Do you want me to send a v6 for this?
>
> Otherwise, looks fine to me :)
>
> With the bzip2 patch I just sent, I could see the line missing from unit
> tests.
>
> However, unit tests aren't happy on my PC.
>
> $ tools/binman/binman tool --list
> Name Version Description Path
> --------------- ----------- -------------------------
> ------------------------------
> gzip 1.11 gzip compression /usr/bin/gzip
> bzip2 1.0.8 bzip2 compression /usr/bin/bzip2
> cbfstool unknown Manipulate CBFS files
> /home/qschulz/work/upstream/coreboot/util/cbfstool/cbfstool
> fiptool Unknown vers Manipulate ATF FIP files
> /home/qschulz/work/upstream/trusted-firmware-a/tools/fiptool/fiptool
> futility v0.0.3050-18 Chromium OS firmware utili
> /home/qschulz/work/upstream/coreboot/util/futility/futility
> ifwitool unknown Manipulate Intel IFWI file
> /home/qschulz/work/upstream/coreboot/util/cbfstool/ifwitool
> lz4 v1.9.3 lz4 compression /usr/bin/lz4
> lzma_alone - lzma_alone compression (not found)
> lzop v1.04 lzo compression /usr/bin/lzop
> mkimage 2022.10-rc3- Generate image for U-Boot /usr/bin/mkimage
> xz 5.2.5 xz compression /usr/bin/xz
> zstd v1.5.2 zstd compression /usr/bin/zstd
> $ tools/binman/binman test -T
> ======================== Running binman tests ========================
> .........................................................sss.............................................F....Fs..............................................................................................F.................................................................................................................................................EEEEFF.................................FF.......FF............F................F.FFFF.................
> ======================================================================
> ERROR: testSymbols (binman.ftest.TestFunctional)
> Test binman can assign symbols embedded in U-Boot
> ----------------------------------------------------------------------
> KeyError: '_binman_sym_magic'
This could be a naming difference with the bintools on Fedora.
>
> ======================================================================
> ERROR: testSymbolsExpanded (binman.ftest.TestFunctional)
> Test binman can assign symbols in expanded entries
> ----------------------------------------------------------------------
> KeyError: '_binman_sym_magic'
>
> ======================================================================
> ERROR: testSymbolsNoDtb (binman.ftest.TestFunctional)
> Test binman can assign symbols embedded in U-Boot SPL
> ----------------------------------------------------------------------
> KeyError: '_binman_sym_magic'
>
> ======================================================================
> ERROR: testSymbolsSubsection (binman.ftest.TestFunctional)
> Test binman can assign symbols from a subsection
> ----------------------------------------------------------------------
> KeyError: '_binman_sym_magic'
>
> ======================================================================
> FAIL: testExtractAllEntries (binman.ftest.TestFunctional)
> Test extracting all entries
> ----------------------------------------------------------------------
> AssertionError: 969 != 0
>
> ======================================================================
> FAIL: testExtractCbfsCompressed (binman.ftest.TestFunctional)
> Test extracting CBFS compressed data
> ----------------------------------------------------------------------
> AssertionError: 969 != 0
That is a bit of a mystery.
>
> ======================================================================
> FAIL: testListCmd (binman.ftest.TestFunctional)
> Test listing the files in an image using an Fdtmap
> ----------------------------------------------------------------------
> AssertionError: Lists differ: ['Nam[463 chars]80 105 u-boot-dtb
> 80 3c9', [184 chars]bf8'] != ['Nam[463 chars]80 0
> u-boot-dtb 80 3c9', [184 chars]bf8']
>
> First differing element 7:
> ' u-boot-dtb 180 105 u-boot-dtb 80 3c9'
> ' u-boot-dtb 180 0 u-boot-dtb 80 3c9'
>
> Diff is 888 characters long. Set self.maxDiff to None to see it.
>
> ======================================================================
> FAIL: testSymbolsTplSection (binman.ftest.TestFunctional)
> Test binman can assign symbols embedded in U-Boot TPL in a section
> ----------------------------------------------------------------------
> AssertionError: b'\xff\xff\xff\xffBSYM\x04\x00\x00\x00
> \x00\x00\x00\x00\x00[36 chars]0klm' !=
> b'\xff\xff\xff\xff56780123456789abcdefghijklm'
>
> ======================================================================
> FAIL: testSymbolsTplSectionX86 (binman.ftest.TestFunctional)
> Test binman can assign symbols in a section with end-at-4gb
> ----------------------------------------------------------------------
> AssertionError: b'\xff\xff\xff\xffBSYM\x04\xff\xff\xff
> \xff\xff\xff\x00\x00[36 chars]0klm' !=
> b'\xff\xff\xff\xff56780123456789abcdefghijklm'
>
> ======================================================================
> FAIL: testBadSymbolSize (binman.elf_test.TestElf)
> Test that an attempt to use an 8-bit symbol are detected
> ----------------------------------------------------------------------
> AssertionError: ValueError not raised
>
> ======================================================================
> FAIL: testDebug (binman.elf_test.TestElf)
> Check that enabling debug in the elf module produced debug output
> ----------------------------------------------------------------------
> AssertionError: False is not true
>
> ======================================================================
> FAIL: testNoValue (binman.elf_test.TestElf)
> Test the case where we have no value for the symbol
> ----------------------------------------------------------------------
> AssertionError: b'BSYM\xff\xff\xff\xff\xff\xff\xff\xff\xff\[43
> chars]aaaa' != b'aaaaaaaaaaaaaaaaaaaaaaaaaaaa'
Probably the symbol table is not being read correctly in elf.py
>
> ======================================================================
> FAIL: testOutsideFile (binman.elf_test.TestElf)
> Test a symbol which extends outside the entry area is detected
> ----------------------------------------------------------------------
> AssertionError: ValueError not raised
>
> ======================================================================
> FAIL: test_cbfs_arch (binman.cbfs_util_test.TestCbfs)
> Test on non-x86 architecture
> ----------------------------------------------------------------------
> AssertionError: cbfstool produced a different result
There is no particular spec for what cbfstool does, so perhaps you
have a different version. Above it shows you have none at all,
actually.
>
> Stdout:
> diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
>
> ======================================================================
> FAIL: test_cbfs_offset (binman.cbfs_util_test.TestCbfs)
> Test a CBFS with files at particular offsets
> ----------------------------------------------------------------------
> AssertionError: cbfstool produced a different result
>
> Stdout:
> diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
>
> ======================================================================
> FAIL: test_cbfs_raw (binman.cbfs_util_test.TestCbfs)
> Test base handling of a Coreboot Filesystem (CBFS)
> ----------------------------------------------------------------------
> AssertionError: cbfstool produced a different result
>
> Stdout:
> diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
>
> ======================================================================
> FAIL: test_cbfs_raw_compress (binman.cbfs_util_test.TestCbfs)
> Test base handling of compressing raw files
> ----------------------------------------------------------------------
> AssertionError: b'' != b'compress xxxxxxxxxxxxxxxxxxxxxx data'
>
> ======================================================================
> FAIL: test_cbfs_raw_space (binman.cbfs_util_test.TestCbfs)
> Test files with unused space in the CBFS
> ----------------------------------------------------------------------
> AssertionError: cbfstool produced a different result
>
> Stdout:
> diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
>
> ======================================================================
> FAIL: test_cbfs_stage (binman.cbfs_util_test.TestCbfs)
> Tests handling of a Coreboot Filesystem (CBFS)
> ----------------------------------------------------------------------
> AssertionError: cbfstool produced a different result
>
> Stdout:
> diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
>
> ======================================================================
> SKIP: testCompUtilCompressions (binman.ftest.TestFunctional)
> Test compression algorithms
> ----------------------------------------------------------------------
> lzma_alone not available
>
> ======================================================================
> SKIP: testCompUtilPadding (binman.ftest.TestFunctional)
> Test padding of compression algorithms
> ----------------------------------------------------------------------
> lzma_alone not available
>
> ======================================================================
> SKIP: testCompUtilVersions (binman.ftest.TestFunctional)
> Test tool version of compression algorithms
> ----------------------------------------------------------------------
> lzma_alone not available
>
> ======================================================================
> SKIP: testExtractCbfsRaw (binman.ftest.TestFunctional)
> Test extracting CBFS compressed data without decompressing it
> ----------------------------------------------------------------------
> lzma_alone not available
These are pretty obvious
>
> ----------------------------------------------------------------------
> Ran 454 tests in 11.738s
>
> FAILED (failures=15, errors=4, skipped=4)
>
> 99%
> Name Stmts Miss Cover
> ---------------------------------------------------------------------------
> tools/binman/__init__.py 0 0 100%
> tools/binman/bintool.py 254 0 100%
> tools/binman/btool/btool_gzip.py 5 0 100%
> tools/binman/btool/bzip2.py 15 0 100%
> tools/binman/btool/cbfstool.py 24 0 100%
> tools/binman/btool/fiptool.py 22 0 100%
> tools/binman/btool/futility.py 24 0 100%
> tools/binman/btool/ifwitool.py 22 0 100%
> tools/binman/btool/lz4.py 28 0 100%
> tools/binman/btool/lzma_alone.py 34 3 91%
> tools/binman/btool/lzop.py 5 0 100%
> tools/binman/btool/mkimage.py 29 0 100%
> tools/binman/btool/xz.py 5 0 100%
> tools/binman/btool/zstd.py 5 0 100%
> tools/binman/cbfs_util.py 366 0 100%
> tools/binman/cmdline.py 73 0 100%
> tools/binman/control.py 342 0 100%
> tools/binman/elf.py 195 18 91%
> tools/binman/entry.py 483 0 100%
> tools/binman/etype/atf_bl31.py 5 0 100%
> tools/binman/etype/atf_fip.py 67 0 100%
> tools/binman/etype/blob.py 39 0 100%
> tools/binman/etype/blob_dtb.py 46 0 100%
> tools/binman/etype/blob_ext.py 11 0 100%
> tools/binman/etype/blob_ext_list.py 32 0 100%
> tools/binman/etype/blob_named_by_arg.py 9 0 100%
> tools/binman/etype/blob_phase.py 16 0 100%
> tools/binman/etype/cbfs.py 101 0 100%
> tools/binman/etype/collection.py 30 0 100%
> tools/binman/etype/cros_ec_rw.py 5 0 100%
> tools/binman/etype/fdtmap.py 62 0 100%
> tools/binman/etype/files.py 35 0 100%
> tools/binman/etype/fill.py 13 0 100%
> tools/binman/etype/fit.py 214 0 100%
> tools/binman/etype/fmap.py 34 0 100%
> tools/binman/etype/gbb.py 37 0 100%
> tools/binman/etype/image_header.py 53 0 100%
> tools/binman/etype/intel_cmc.py 4 0 100%
> tools/binman/etype/intel_descriptor.py 39 0 100%
> tools/binman/etype/intel_fit.py 12 0 100%
> tools/binman/etype/intel_fit_ptr.py 17 0 100%
> tools/binman/etype/intel_fsp.py 4 0 100%
> tools/binman/etype/intel_fsp_m.py 4 0 100%
> tools/binman/etype/intel_fsp_s.py 4 0 100%
> tools/binman/etype/intel_fsp_t.py 4 0 100%
> tools/binman/etype/intel_ifwi.py 67 0 100%
> tools/binman/etype/intel_me.py 4 0 100%
> tools/binman/etype/intel_mrc.py 6 0 100%
> tools/binman/etype/intel_refcode.py 6 0 100%
> tools/binman/etype/intel_vbt.py 4 0 100%
> tools/binman/etype/intel_vga.py 4 0 100%
> tools/binman/etype/mkimage.py 68 0 100%
> tools/binman/etype/opensbi.py 5 0 100%
> tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py 6 0 100%
> tools/binman/etype/pre_load.py 77 0 100%
> tools/binman/etype/scp.py 5 0 100%
> tools/binman/etype/section.py 376 12 97%
> tools/binman/etype/tee_os.py 5 0 100%
> tools/binman/etype/text.py 21 0 100%
> tools/binman/etype/u_boot.py 7 0 100%
> tools/binman/etype/u_boot_dtb.py 9 0 100%
> tools/binman/etype/u_boot_dtb_with_ucode.py 51 0 100%
> tools/binman/etype/u_boot_elf.py 19 0 100%
> tools/binman/etype/u_boot_env.py 27 0 100%
> tools/binman/etype/u_boot_expanded.py 4 0 100%
> tools/binman/etype/u_boot_img.py 7 0 100%
> tools/binman/etype/u_boot_nodtb.py 7 0 100%
> tools/binman/etype/u_boot_spl.py 11 0 100%
> tools/binman/etype/u_boot_spl_bss_pad.py 14 0 100%
> tools/binman/etype/u_boot_spl_dtb.py 9 0 100%
> tools/binman/etype/u_boot_spl_elf.py 7 0 100%
> tools/binman/etype/u_boot_spl_expanded.py 12 0 100%
> tools/binman/etype/u_boot_spl_nodtb.py 11 0 100%
> tools/binman/etype/u_boot_spl_with_ucode_ptr.py 8 0 100%
> tools/binman/etype/u_boot_tpl.py 11 0 100%
> tools/binman/etype/u_boot_tpl_bss_pad.py 14 0 100%
> tools/binman/etype/u_boot_tpl_dtb.py 9 0 100%
> tools/binman/etype/u_boot_tpl_dtb_with_ucode.py 8 0 100%
> tools/binman/etype/u_boot_tpl_elf.py 7 0 100%
> tools/binman/etype/u_boot_tpl_expanded.py 12 0 100%
> tools/binman/etype/u_boot_tpl_nodtb.py 11 0 100%
> tools/binman/etype/u_boot_tpl_with_ucode_ptr.py 12 0 100%
> tools/binman/etype/u_boot_ucode.py 33 0 100%
> tools/binman/etype/u_boot_with_ucode_ptr.py 42 0 100%
> tools/binman/etype/vblock.py 38 0 100%
> tools/binman/etype/x86_reset16.py 7 0 100%
> tools/binman/etype/x86_reset16_spl.py 7 0 100%
> tools/binman/etype/x86_reset16_tpl.py 7 0 100%
> tools/binman/etype/x86_start16.py 7 0 100%
> tools/binman/etype/x86_start16_spl.py 7 0 100%
> tools/binman/etype/x86_start16_tpl.py 7 0 100%
> tools/binman/fip_util.py 202 0 100%
> tools/binman/fmap_util.py 48 0 100%
> tools/binman/image.py 164 4 98%
> tools/binman/state.py 201 0 100%
> ---------------------------------------------------------------------------
> TOTAL 4539 37 99%
>
> To get a report in 'htmlcov/index.html', type: python3 -m coverage html
> Coverage error: 99%, but should be 100%
> ValueError: Test coverage failure
>
> I guess I just should send a new mail so the community can have a look
> at it? I sent two patches for low hanging fruits but I'm afraid I won't
> have time to look at those issues any time soon :/
Yes please resend your series, just with the extra test code I added.
>
> I'm running Fedora Server 36 x86_64 fully up-to-date if that helps.
>
> [...]
> >> I get 52% coverage only with that exact same branch, something's
> >> definitely wrong here in my setup. And I **definitely** do not have
> >> tools/binman/etype/mkimage.py listed in there.... Mmmmmm.
> >
> > You may need to get some bintools with 'binman tool -f missing'. But
>
> It expects apt package manager, I only have dnf :)
Patches welcome!
>
> Also, tries to download binaries from a Google Drive, no thank you :)
I don't like it either. Perhaps we could use a web site? I would
prefer to build from source, anyway.
>
> > in any case, you only need to worry about coverage in mkimage.py which
> > is what you changed.
> >
>
> That's fair. Thanks for fixing my patches, lemme know if you want a v6
> instead.
Yes please, my things were just an example to help.
>
> [...]
> >>
> >> I'll play with binman until I manage to get a coverage percentage equal
> >> to yours.
> >
> > OK, I'd appreciate a docs patch if you can produce one from your
> > efforts, or any feedback on how to make this automatic / easy.
> >
>
> I copied the errors I'm having right now a few paragraphs above but the
> biggest issue was bzip2 getting stuck and messing up the whole test
> suite. With it patched, I was able to see the line that was kept
> untested in my patches. Otherwise I believe your advice of running
> binman test -T was enough if that had worked, just bad luck :)
OK good.
Regards,
Simon
More information about the U-Boot
mailing list