Re: [PATCH v3] meta/classes: generate bill of material from image

[Quirin Gylstorff <quirin.gylstorff@siemens.com>]

On 8/12/19 10:04 AM, Claudius Heine wrote:
> Hi Quirin,
> 
> On 09/08/2019 12.30, [ext] Q. Gylstorff wrote:
>> From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
>>
>> To create products it is necessary to have a list
>> of used packages for clearance and to security monitoring.
>> To get a simple list of packages use dpkg-query and generate
>> a list with the following pattern:
>>
>> source name| source version | binary package name | binary version
>>
>> The list is stored in ${IMAGE_FULLNAME}.rootfs.manifest
>>
>> Remove the feature with:
>> ROOTFS_FEATURES_remove = "generate-manifest"
>>
>> Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
>> ---
>> Changes:
>> v3:
>> Add list of manifest for buildchroot manifest
>> This list can be exdent to add additional output generators
>> v2:
>> use FEATURE instead of own variable
>>
>> .../image-package-list-extension.bbclass      | 66 +++++++++++++++++++
>>   meta/classes/image.bbclass                    |  3 +-
>>   2 files changed, 68 insertions(+), 1 deletion(-)
>>   create mode 100644 meta/classes/image-package-list-extension.bbclass
>>
>> diff --git a/meta/classes/image-package-list-extension.bbclass 
>> b/meta/classes/image-package-list-extension.bbclass
>> new file mode 100644
>> index 0000000..11896f1
>> --- /dev/null
>> +++ b/meta/classes/image-package-list-extension.bbclass
>> @@ -0,0 +1,66 @@
>> +# This software is a part of ISAR.
>> +# Copyright (C) Siemens AG, 2019
>> +#
>> +# SPDX-License-Identifier: MIT
>> +
>> +MANIFESTS ?= "target build"
>> +MANIFEST_build[rootfs] ?= "/var/lib/dpkg"
>> +MANIFEST_target[rootfs] ?= "${PP_ROOTFS}/var/lib/dpkg"
> 
> Have you planned additional flags for this?
> 
> Currently I think that this mechanism is a bit of an overkill for just 
> two variables. But since you touched this now and are the second users 
> of this, I have further comments ;).
> 

One Idea was to add use this to add additional generators. If this is 
not a use case anymore than it is overkill.

>> +
>> +def gen_manifests_array(d, listname, entryname, flags, verb_flags=None):
>> +    from itertools import chain
>> +
>> +    entries = (d.getVar(listname, True) or "").split()
>> +    return " ".join(
>> +        ":".join(
>> +            chain(
>> +                (entry,),
>> +                (
>> +                    (",".join(
>> +                        (
>> +                            d.getVarFlag(entryname + "_" + entry, 
>> flag, True) or ""
>> +                        ).split()
>> +                    ) if flag not in (verb_flags or []) else (
>> +                        d.getVarFlag(entryname + "_" + entry, flag, 
>> True) or ""
>> +                    )).replace(":","=")
>> +                    for flag in flags
>> +                ),
>> +            )
>> +        )
>> +        for entry in entries
> 
> Is this a 1:1 copy from image-account-extension.bbclass? If so, then can 
> you put those implementations together to avoid code duplication?
> 
> I also have to say, I am not 100% happy with that code. While it is very 
> efficient, it might be not that maintainable compared to simple loops 
> that build the output string using string operators or StringIO. Those 
> are probably easier to understand for people not that fluent in python.
> 
>> +    )
>> +
>> +
>> +IMAGE_MANIFESTS =+ "${@gen_manifests_array(d, 'MANIFESTS', 
>> 'MANIFEST', ['rootfs'])}"
>> +
>> +image_generate_manifest[dirs] = "${DEPLOY_DIR_IMAGE}"
>> +image_generate_manifest() {
>> +    image_do_mounts
>> +    list='${@" ".join(d.getVar('IMAGE_MANIFESTS', True).split())} '
>> +    while true; do
>> +        list_rest="${list#*:* }"
>> +        entry="${list%%${list_rest}}"
>> +        list="${list_rest}"
>> +
>> +        if [ -z "${entry}" ]; then
>> +            break
>> +        fi
>> +        # Add colon to the end of the entry and remove trailing space:
>> +        entry="${entry% }:"
>> +
>> +        # Decode entries:
>> +        name="${entry%%:*}"
>> +        entry="${entry#${name}:}"
>> +
>> +        rootfs="${entry%%:*}"
>> +        entry="${entry#${rootfs}:}"
> 
> I also see here some ways to avoid code duplication. Maybe generate this 
> shell code in a python function >
> Something like:
> 
>      ${@gen_shell_list_processing('IMAGE_MANIFESTS', ['name', 'rootfs'], 
> """
>          sudo -E chroot ${BUILDCHROOT_DIR} \
>          /usr/bin/dpkg-query --admindir="$rootfs" \
>          -f 
> '${source:Package}|${source:Version}|${binary:Package}|${Version}\n' -W\
>          ${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}."$name".manifest
> """)}
> 
> Not sure about the shell part. BB seems to not handle line breaks in 
> inline python very well.
> 
> To move it outside of the inline would also be possible:
> 
>      ${@gen_shell_list_processing_start('IMAGE_MANIFESTS', ['name', 
> 'rootfs'])}
>          sudo -E chroot ${BUILDCHROOT_DIR} \
>          /usr/bin/dpkg-query --admindir="$rootfs" \
>          -f 
> '${source:Package}|${source:Version}|${binary:Package}|${Version}\n' -W\
>          ${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}."$name".manifest
>      ${@gen_shell_list_processing_end()}
> 
> gen_shell_list_processing_end would just return 'done;' but it makes is 
> more flexible in the future... maybe, IDK. But having 'done' directly 
> there would also be ok.
> 
>

I will test it

>> +
>> +
>> +        sudo -E chroot ${BUILDCHROOT_DIR} \
>> +        /usr/bin/dpkg-query --admindir="$rootfs" \
>> +        -f 
>> '${source:Package}|${source:Version}|${binary:Package}|${Version}\n' 
>> -W > \
>> +        ${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}."$name".manifest
> 
> Does this also work if you just specify 'dpkg-query' without '/usr/bin/'?
> 
> Cheers,
> Claudius
> 
>> +    done
>> +}
>> +ROOTFS_POSTPROCESS_COMMAND += 
>> "${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest', 
>> 'image_generate_manifest', '', d)}"
>> +
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index ec6bd39..60dd9fb 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -58,7 +58,7 @@ image_do_mounts() {
>>   }
>>   ROOTFSDIR = "${IMAGE_ROOTFS}"
>> -ROOTFS_FEATURES += "copy-package-cache clean-package-cache 
>> finalize-rootfs"
>> +ROOTFS_FEATURES += "copy-package-cache clean-package-cache 
>> finalize-rootfs generate-manifest"
>>   ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}"
>>   inherit rootfs
>> @@ -68,6 +68,7 @@ inherit image-tools-extension
>>   inherit image-postproc-extension
>>   inherit image-locales-extension
>>   inherit image-account-extension
>> +inherit image-package-list-extension
>>   # Extra space for rootfs in MB
>>   ROOTFS_EXTRA ?= "64"
>>
> 

Regards
Quirin