[OE-core] [PATCH v6 0/5] wic: bugfixes & --fixed-size support, tests, oeqa: enhancements

[Maciej Borzęcki]

On Tue, Dec 20, 2016 at 3:04 PM, Ed Bartosh <ed.bartosh at linux.intel.com> wrote:
> On Mon, Dec 19, 2016 at 12:20:56PM +0100, Maciej Borzecki wrote:
>> v6 of a patch series previously posted here [1].
>>
>> Changes since v5:
>>
>> * `oe-selftest: enforce en_US.UTF-8 locale` was merged to master, hence it's no
>>   longer part of this series
>>
>> * rebased on top of master (d0d260b0b85790ceb136dd6b0445e8e33d038f5e)
>>
>
> The patchset looks good to me. Thank you.
>
> Couple of questions:
> - did you test if Lars' change
>   https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=db08ffee0ad1451f3bf710f4d1b623938ba9aefb
>   works with your patchset?

Yes. The series was rebased on top of
d0d260b0b85790ceb136dd6b0445e8e33d038f5e, which includes Lars' patch.
The tests passed on all MACHINES I've used (though I'm not sure if
scenario addressed in that patch is covered by relevant test cases).

I believe I have also hit a weird bug in BitbakeVars._parse_line() while
testing with this patch and meta-mender. I Hope you can confirm it.

>From a quick investigation I have determined that _parse_line() may
interpret arbitrary lines that are of 'key=value' format as legitimate
bitbake variables. In my case, IMAGE_CMD_sdimg defined in
mender-sdimg.bbclass includes the following piece:

    IMAGE_CMD_sdimg() {
        ...
        REMAINING_SIZE=$(expr ${MENDER_STORAGE_TOTAL_SIZE_MB} - \
        ${MENDER_BOOT_PART_SIZE_MB} - \
        ${MENDER_DATA_PART_SIZE_MB} - \
        ${MENDER_PARTITIONING_OVERHEAD_MB} - \
        $boot_env_size_mb)
        ROOTFS_SIZE=$(expr $REMAINING_SIZE / 2)
        ...
    }

This will obviously be seen in `bitbake -e output`. Looking at code
here:
https://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/scripts/lib/wic/utils/oe/misc.py?id=db08ffee0ad1451f3bf710f4d1b623938ba9aefb#n151
the line will be split at '=', then whitespace will be stripped from
both key and value. So a line (taken from bitbake -e):

    ROOTFS_SIZE=\$(expr \$REMAINING_SIZE / 2)

Will define a variable ROOTFS_SIZE with value '\\$(expr \\$REMAINING_SIZE / 2)'.

It's a pure coincidence that Lars' patch queries ROOTFS_SIZE which also
appears in bitbake -e output under exactly the same name.

IMO, when parsing env output, we should expect variable name to start
right at the beginning of a line, IOW skip the line if key[0].isspace() == True

>
> - Regarding oe-selftest fixes, wich machine did you run oe-selftest on?
>   It looks to me that a lot of test cases can be run on your machine
>   if we use another wks.

I've run it on beaglebone, vexpress-qemu, qemux86 and qemuarm. Agree
with your observation though. I think that at least these TCs could be
made machine agnostic:

- test_build_artifacts
- test_build_image_name
- test_compress_*
- test_gpt_image
- test_wic_image_type
- test_wrong_compressor
- test_rootfs_*



>
>> [1]. http://lists.openembedded.org/pipermail/openembedded-core/2016-November/129187.html
>>
>> Maciej Borzecki (5):
>>   oeqa/utils/commands.py: allow use of binaries from native sysroot
>>   wic: add --fixed-size wks option
>>   wic: selftest: avoid COMPATIBLE_HOST issues
>>   wic: selftest: do not assume bzImage kernel image
>>   wic: selftest: add tests for --fixed-size partition flags
>>
>>  meta/lib/oeqa/selftest/wic.py          | 117 +++++++++++++++++++++++++++++++--
>>  meta/lib/oeqa/utils/commands.py        |   9 ++-
>>  scripts/lib/wic/help.py                |  14 +++-
>>  scripts/lib/wic/imager/direct.py       |   2 +-
>>  scripts/lib/wic/ksparser.py            |  41 ++++++++++--
>>  scripts/lib/wic/partition.py           |  88 +++++++++++++++----------
>>  scripts/lib/wic/utils/partitionedfs.py |   2 +-
>>  7 files changed, 224 insertions(+), 49 deletions(-)
>

Regards,
-- 
Maciej Borzecki
RnDity