diff mbox series

spdx: Fix SPDX tasks not running when code changes

Message ID 20241122154939.2133845-1-JPEWhacker@gmail.com
State New
Headers show
Series spdx: Fix SPDX tasks not running when code changes | expand

Commit Message

Joshua Watt Nov. 22, 2024, 3:49 p.m. UTC
The SPDX code makes heavy use of python classes. While this works very
well, the bitbake dependency parser is unable to understand how to deal
with them, and thus changes to the class code do not cause rebuilds to
occur. To correct this, add the library files that include SPDX code as
file checksums for the SPDX tasks. If this method works well for SPDX,
we will look at implementing something similar in the bitbake dependency
parser that should allow correct checksums without having to explicitly
add them to each class.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 meta/classes-recipe/create-spdx-image-3.0.bbclass | 3 +++
 meta/classes-recipe/create-spdx-sdk-3.0.bbclass   | 2 ++
 meta/classes/create-spdx-3.0.bbclass              | 9 +++++++++
 3 files changed, 14 insertions(+)

Comments

Quentin Schulz Nov. 22, 2024, 4:22 p.m. UTC | #1
Hi Joshua,

On 11/22/24 4:49 PM, Joshua Watt via lists.openembedded.org wrote:
> The SPDX code makes heavy use of python classes. While this works very
> well, the bitbake dependency parser is unable to understand how to deal
> with them, and thus changes to the class code do not cause rebuilds to
> occur. To correct this, add the library files that include SPDX code as
> file checksums for the SPDX tasks. If this method works well for SPDX,
> we will look at implementing something similar in the bitbake dependency
> parser that should allow correct checksums without having to explicitly
> add them to each class.
> 

Oooooh that reminds me of this very issue happening at my previous company.

I'm wondering if we shouldn't just add this file-checksums dependency at 
the global level somehow? Since one needs to call addpylib 
layer-dir/path directory in order to be able to import those modules in 
tasks/functions. Maybe there's something we can hook there somehow? The 
issue being we would either blindly reparse/rebuild all recipes when 
those changes or we need a way to check which task(s) import those modules?

Another option could be to forbid addpylib on a conf file and force it 
inside recipes only, but I guess this means we cannot use those in 
inline python in conf files anymore? which we do use in 
meta/conf/bitbake.conf.

The change itself looks fine to me though, wish I had this idea at my 
previous company :)

Cheers,
Quentin

> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
>   meta/classes-recipe/create-spdx-image-3.0.bbclass | 3 +++
>   meta/classes-recipe/create-spdx-sdk-3.0.bbclass   | 2 ++
>   meta/classes/create-spdx-3.0.bbclass              | 9 +++++++++
>   3 files changed, 14 insertions(+)
> 
> diff --git a/meta/classes-recipe/create-spdx-image-3.0.bbclass b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> index 18e6cf6dfa2..51446162857 100644
> --- a/meta/classes-recipe/create-spdx-image-3.0.bbclass
> +++ b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> @@ -36,6 +36,7 @@ do_create_rootfs_spdx[sstate-inputdirs] = "${SPDXROOTFSDEPLOY}"
>   do_create_rootfs_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
>   do_create_rootfs_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_create_rootfs_spdx[cleandirs] += "${SPDXROOTFSDEPLOY}"
> +do_create_rootfs_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_rootfs_spdx_setscene() {
>       sstate_setscene(d)
> @@ -53,6 +54,7 @@ do_create_image_spdx[sstate-inputdirs] = "${SPDXIMAGEWORK}"
>   do_create_image_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
>   do_create_image_spdx[cleandirs] = "${SPDXIMAGEWORK}"
>   do_create_image_spdx[dirs] = "${SPDXIMAGEWORK}"
> +do_create_image_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_image_spdx_setscene() {
>       sstate_setscene(d)
> @@ -72,6 +74,7 @@ do_create_image_sbom_spdx[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
>   do_create_image_sbom_spdx[stamp-extra-info] = "${MACHINE_ARCH}"
>   do_create_image_sbom_spdx[cleandirs] = "${SPDXIMAGEDEPLOYDIR}"
>   do_create_image_sbom_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
> +do_create_image_sbom_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_image_sbom_spdx_setscene() {
>       sstate_setscene(d)
> diff --git a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> index ea01a21cc59..855fb3d09f9 100644
> --- a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> +++ b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> @@ -8,12 +8,14 @@
>   do_populate_sdk[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_populate_sdk[cleandirs] += "${SPDXSDKWORK}"
>   do_populate_sdk[postfuncs] += "sdk_create_sbom"
> +do_populate_sdk[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk = " sdk_host_create_spdx"
>   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk = " sdk_target_create_spdx"
>   
>   do_populate_sdk_ext[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_populate_sdk_ext[cleandirs] += "${SPDXSDKEXTWORK}"
>   do_populate_sdk_ext[postfuncs] += "sdk_ext_create_sbom"
> +do_populate_sdk_ext[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk-ext = " sdk_ext_host_create_spdx"
>   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk-ext = " sdk_ext_target_create_spdx"
>   
> diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> index bc23d2d2115..640f5490bda 100644
> --- a/meta/classes/create-spdx-3.0.bbclass
> +++ b/meta/classes/create-spdx-3.0.bbclass
> @@ -122,6 +122,13 @@ oe.spdx30_tasks.get_package_sources_from_debug[vardepsexclude] += "STAGING_KERNE
>   oe.spdx30_tasks.collect_dep_objsets[vardepsexclude] = "SPDX_MULTILIB_SSTATE_ARCHS"
>   
>   
> +# SPDX library code makes heavy use of classes, which bitbake cannot easily
> +# parse out dependencies. As such, the library code files that make use of
> +# classes are explicitly added as file checksum dependencies.
> +SPDX3_LIB_DEP_FILES = "\
> +    ${COREBASE}/meta/lib/oe/sbom30.py:True \
> +    ${COREBASE}/meta/lib/oe/spdx30.py:True \
> +    "
>   
>   python do_create_spdx() {
>       import oe.spdx30_tasks
> @@ -137,6 +144,7 @@ addtask do_create_spdx after \
>   SSTATETASKS += "do_create_spdx"
>   do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
>   do_create_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> +do_create_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_spdx_setscene () {
>       sstate_setscene(d)
> @@ -160,6 +168,7 @@ addtask do_create_package_spdx after do_create_spdx before do_build do_rm_work
>   SSTATETASKS += "do_create_package_spdx"
>   do_create_package_spdx[sstate-inputdirs] = "${SPDXRUNTIMEDEPLOY}"
>   do_create_package_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> +do_create_package_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_package_spdx_setscene () {
>       sstate_setscene(d)
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207626): https://lists.openembedded.org/g/openembedded-core/message/207626
> Mute This Topic: https://lists.openembedded.org/mt/109724577/6293953
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [quentin.schulz@cherry.de]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Quentin Schulz Nov. 22, 2024, 4:22 p.m. UTC | #2
Hi Joshua,

On 11/22/24 4:49 PM, Joshua Watt via lists.openembedded.org wrote:
> The SPDX code makes heavy use of python classes. While this works very
> well, the bitbake dependency parser is unable to understand how to deal
> with them, and thus changes to the class code do not cause rebuilds to
> occur. To correct this, add the library files that include SPDX code as
> file checksums for the SPDX tasks. If this method works well for SPDX,
> we will look at implementing something similar in the bitbake dependency
> parser that should allow correct checksums without having to explicitly
> add them to each class.
> 

Oooooh that reminds me of this very issue happening at my previous company.

I'm wondering if we shouldn't just add this file-checksums dependency at 
the global level somehow? Since one needs to call addpylib 
layer-dir/path directory in order to be able to import those modules in 
tasks/functions. Maybe there's something we can hook there somehow? The 
issue being we would either blindly reparse/rebuild all recipes when 
those changes or we need a way to check which task(s) import those modules?

Another option could be to forbid addpylib on a conf file and force it 
inside recipes only, but I guess this means we cannot use those in 
inline python in conf files anymore? which we do use in 
meta/conf/bitbake.conf.

The change itself looks fine to me though, wish I had this idea at my 
previous company :)

Cheers,
Quentin

> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
>   meta/classes-recipe/create-spdx-image-3.0.bbclass | 3 +++
>   meta/classes-recipe/create-spdx-sdk-3.0.bbclass   | 2 ++
>   meta/classes/create-spdx-3.0.bbclass              | 9 +++++++++
>   3 files changed, 14 insertions(+)
> 
> diff --git a/meta/classes-recipe/create-spdx-image-3.0.bbclass b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> index 18e6cf6dfa2..51446162857 100644
> --- a/meta/classes-recipe/create-spdx-image-3.0.bbclass
> +++ b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> @@ -36,6 +36,7 @@ do_create_rootfs_spdx[sstate-inputdirs] = "${SPDXROOTFSDEPLOY}"
>   do_create_rootfs_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
>   do_create_rootfs_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_create_rootfs_spdx[cleandirs] += "${SPDXROOTFSDEPLOY}"
> +do_create_rootfs_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_rootfs_spdx_setscene() {
>       sstate_setscene(d)
> @@ -53,6 +54,7 @@ do_create_image_spdx[sstate-inputdirs] = "${SPDXIMAGEWORK}"
>   do_create_image_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
>   do_create_image_spdx[cleandirs] = "${SPDXIMAGEWORK}"
>   do_create_image_spdx[dirs] = "${SPDXIMAGEWORK}"
> +do_create_image_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_image_spdx_setscene() {
>       sstate_setscene(d)
> @@ -72,6 +74,7 @@ do_create_image_sbom_spdx[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
>   do_create_image_sbom_spdx[stamp-extra-info] = "${MACHINE_ARCH}"
>   do_create_image_sbom_spdx[cleandirs] = "${SPDXIMAGEDEPLOYDIR}"
>   do_create_image_sbom_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
> +do_create_image_sbom_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_image_sbom_spdx_setscene() {
>       sstate_setscene(d)
> diff --git a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> index ea01a21cc59..855fb3d09f9 100644
> --- a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> +++ b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> @@ -8,12 +8,14 @@
>   do_populate_sdk[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_populate_sdk[cleandirs] += "${SPDXSDKWORK}"
>   do_populate_sdk[postfuncs] += "sdk_create_sbom"
> +do_populate_sdk[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk = " sdk_host_create_spdx"
>   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk = " sdk_target_create_spdx"
>   
>   do_populate_sdk_ext[recrdeptask] += "do_create_spdx do_create_package_spdx"
>   do_populate_sdk_ext[cleandirs] += "${SPDXSDKEXTWORK}"
>   do_populate_sdk_ext[postfuncs] += "sdk_ext_create_sbom"
> +do_populate_sdk_ext[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk-ext = " sdk_ext_host_create_spdx"
>   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk-ext = " sdk_ext_target_create_spdx"
>   
> diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> index bc23d2d2115..640f5490bda 100644
> --- a/meta/classes/create-spdx-3.0.bbclass
> +++ b/meta/classes/create-spdx-3.0.bbclass
> @@ -122,6 +122,13 @@ oe.spdx30_tasks.get_package_sources_from_debug[vardepsexclude] += "STAGING_KERNE
>   oe.spdx30_tasks.collect_dep_objsets[vardepsexclude] = "SPDX_MULTILIB_SSTATE_ARCHS"
>   
>   
> +# SPDX library code makes heavy use of classes, which bitbake cannot easily
> +# parse out dependencies. As such, the library code files that make use of
> +# classes are explicitly added as file checksum dependencies.
> +SPDX3_LIB_DEP_FILES = "\
> +    ${COREBASE}/meta/lib/oe/sbom30.py:True \
> +    ${COREBASE}/meta/lib/oe/spdx30.py:True \
> +    "
>   
>   python do_create_spdx() {
>       import oe.spdx30_tasks
> @@ -137,6 +144,7 @@ addtask do_create_spdx after \
>   SSTATETASKS += "do_create_spdx"
>   do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
>   do_create_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> +do_create_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_spdx_setscene () {
>       sstate_setscene(d)
> @@ -160,6 +168,7 @@ addtask do_create_package_spdx after do_create_spdx before do_build do_rm_work
>   SSTATETASKS += "do_create_package_spdx"
>   do_create_package_spdx[sstate-inputdirs] = "${SPDXRUNTIMEDEPLOY}"
>   do_create_package_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> +do_create_package_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
>   
>   python do_create_package_spdx_setscene () {
>       sstate_setscene(d)
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207626): https://lists.openembedded.org/g/openembedded-core/message/207626
> Mute This Topic: https://lists.openembedded.org/mt/109724577/6293953
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [quentin.schulz@cherry.de]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Joshua Watt Nov. 22, 2024, 4:24 p.m. UTC | #3
On Fri, Nov 22, 2024 at 9:22 AM Quentin Schulz <quentin.schulz@cherry.de> wrote:
>
> Hi Joshua,
>
> On 11/22/24 4:49 PM, Joshua Watt via lists.openembedded.org wrote:
> > The SPDX code makes heavy use of python classes. While this works very
> > well, the bitbake dependency parser is unable to understand how to deal
> > with them, and thus changes to the class code do not cause rebuilds to
> > occur. To correct this, add the library files that include SPDX code as
> > file checksums for the SPDX tasks. If this method works well for SPDX,
> > we will look at implementing something similar in the bitbake dependency
> > parser that should allow correct checksums without having to explicitly
> > add them to each class.
> >
>
> Oooooh that reminds me of this very issue happening at my previous company.
>
> I'm wondering if we shouldn't just add this file-checksums dependency at
> the global level somehow? Since one needs to call addpylib
> layer-dir/path directory in order to be able to import those modules in
> tasks/functions. Maybe there's something we can hook there somehow? The
> issue being we would either blindly reparse/rebuild all recipes when
> those changes or we need a way to check which task(s) import those modules?
>
> Another option could be to forbid addpylib on a conf file and force it
> inside recipes only, but I guess this means we cannot use those in
> inline python in conf files anymore? which we do use in
> meta/conf/bitbake.conf.

Ya, Richard and I talked about this on IRC. The plan is to do this as
sort of a PoC to fix SPDX for now, and if it works out we'll try to
figure out a way to make it more formal in bitbake dependency parser.
I have a few ideas for how to do that.

>
> The change itself looks fine to me though, wish I had this idea at my
> previous company :)

>
> Cheers,
> Quentin
>
> > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> > ---
> >   meta/classes-recipe/create-spdx-image-3.0.bbclass | 3 +++
> >   meta/classes-recipe/create-spdx-sdk-3.0.bbclass   | 2 ++
> >   meta/classes/create-spdx-3.0.bbclass              | 9 +++++++++
> >   3 files changed, 14 insertions(+)
> >
> > diff --git a/meta/classes-recipe/create-spdx-image-3.0.bbclass b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> > index 18e6cf6dfa2..51446162857 100644
> > --- a/meta/classes-recipe/create-spdx-image-3.0.bbclass
> > +++ b/meta/classes-recipe/create-spdx-image-3.0.bbclass
> > @@ -36,6 +36,7 @@ do_create_rootfs_spdx[sstate-inputdirs] = "${SPDXROOTFSDEPLOY}"
> >   do_create_rootfs_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> >   do_create_rootfs_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
> >   do_create_rootfs_spdx[cleandirs] += "${SPDXROOTFSDEPLOY}"
> > +do_create_rootfs_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >
> >   python do_create_rootfs_spdx_setscene() {
> >       sstate_setscene(d)
> > @@ -53,6 +54,7 @@ do_create_image_spdx[sstate-inputdirs] = "${SPDXIMAGEWORK}"
> >   do_create_image_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> >   do_create_image_spdx[cleandirs] = "${SPDXIMAGEWORK}"
> >   do_create_image_spdx[dirs] = "${SPDXIMAGEWORK}"
> > +do_create_image_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >
> >   python do_create_image_spdx_setscene() {
> >       sstate_setscene(d)
> > @@ -72,6 +74,7 @@ do_create_image_sbom_spdx[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
> >   do_create_image_sbom_spdx[stamp-extra-info] = "${MACHINE_ARCH}"
> >   do_create_image_sbom_spdx[cleandirs] = "${SPDXIMAGEDEPLOYDIR}"
> >   do_create_image_sbom_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
> > +do_create_image_sbom_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >
> >   python do_create_image_sbom_spdx_setscene() {
> >       sstate_setscene(d)
> > diff --git a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> > index ea01a21cc59..855fb3d09f9 100644
> > --- a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> > +++ b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
> > @@ -8,12 +8,14 @@
> >   do_populate_sdk[recrdeptask] += "do_create_spdx do_create_package_spdx"
> >   do_populate_sdk[cleandirs] += "${SPDXSDKWORK}"
> >   do_populate_sdk[postfuncs] += "sdk_create_sbom"
> > +do_populate_sdk[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk = " sdk_host_create_spdx"
> >   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk = " sdk_target_create_spdx"
> >
> >   do_populate_sdk_ext[recrdeptask] += "do_create_spdx do_create_package_spdx"
> >   do_populate_sdk_ext[cleandirs] += "${SPDXSDKEXTWORK}"
> >   do_populate_sdk_ext[postfuncs] += "sdk_ext_create_sbom"
> > +do_populate_sdk_ext[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >   POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk-ext = " sdk_ext_host_create_spdx"
> >   POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk-ext = " sdk_ext_target_create_spdx"
> >
> > diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> > index bc23d2d2115..640f5490bda 100644
> > --- a/meta/classes/create-spdx-3.0.bbclass
> > +++ b/meta/classes/create-spdx-3.0.bbclass
> > @@ -122,6 +122,13 @@ oe.spdx30_tasks.get_package_sources_from_debug[vardepsexclude] += "STAGING_KERNE
> >   oe.spdx30_tasks.collect_dep_objsets[vardepsexclude] = "SPDX_MULTILIB_SSTATE_ARCHS"
> >
> >
> > +# SPDX library code makes heavy use of classes, which bitbake cannot easily
> > +# parse out dependencies. As such, the library code files that make use of
> > +# classes are explicitly added as file checksum dependencies.
> > +SPDX3_LIB_DEP_FILES = "\
> > +    ${COREBASE}/meta/lib/oe/sbom30.py:True \
> > +    ${COREBASE}/meta/lib/oe/spdx30.py:True \
> > +    "
> >
> >   python do_create_spdx() {
> >       import oe.spdx30_tasks
> > @@ -137,6 +144,7 @@ addtask do_create_spdx after \
> >   SSTATETASKS += "do_create_spdx"
> >   do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
> >   do_create_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> > +do_create_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >
> >   python do_create_spdx_setscene () {
> >       sstate_setscene(d)
> > @@ -160,6 +168,7 @@ addtask do_create_package_spdx after do_create_spdx before do_build do_rm_work
> >   SSTATETASKS += "do_create_package_spdx"
> >   do_create_package_spdx[sstate-inputdirs] = "${SPDXRUNTIMEDEPLOY}"
> >   do_create_package_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
> > +do_create_package_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
> >
> >   python do_create_package_spdx_setscene () {
> >       sstate_setscene(d)
> >
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#207626): https://lists.openembedded.org/g/openembedded-core/message/207626
> > Mute This Topic: https://lists.openembedded.org/mt/109724577/6293953
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [quentin.schulz@cherry.de]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
Richard Purdie Nov. 22, 2024, 4:38 p.m. UTC | #4
On Fri, 2024-11-22 at 17:22 +0100, Quentin Schulz via lists.openembedded.org wrote:
> Hi Joshua,
> 
> On 11/22/24 4:49 PM, Joshua Watt via lists.openembedded.org wrote:
> > The SPDX code makes heavy use of python classes. While this works very
> > well, the bitbake dependency parser is unable to understand how to deal
> > with them, and thus changes to the class code do not cause rebuilds to
> > occur. To correct this, add the library files that include SPDX code as
> > file checksums for the SPDX tasks. If this method works well for SPDX,
> > we will look at implementing something similar in the bitbake dependency
> > parser that should allow correct checksums without having to explicitly
> > add them to each class.
> > 
> 
> Oooooh that reminds me of this very issue happening at my previous company.
> 
> I'm wondering if we shouldn't just add this file-checksums dependency at 
> the global level somehow? Since one needs to call addpylib 
> layer-dir/path directory in order to be able to import those modules in 
> tasks/functions. Maybe there's something we can hook there somehow? The 
> issue being we would either blindly reparse/rebuild all recipes when 
> those changes or we need a way to check which task(s) import those modules?
> 
> Another option could be to forbid addpylib on a conf file and force it 
> inside recipes only, but I guess this means we cannot use those in 
> inline python in conf files anymore? which we do use in 
> meta/conf/bitbake.conf.
> 
> The change itself looks fine to me though, wish I had this idea at my
> previous company :)

The challenge is that you only want the dependencies at the points of
usage. We could make bitbake just checksum all the files but it would
mean a world rebuild for any single change, even a whitespace one.
We're trying to be a little more clever than that.

In this case the python file is basically the class so it does make
more sense.

We might be able to teach the dependency code about classes as a large
object, that remains to be see. I'd not want to do it at the file level
though in general.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/classes-recipe/create-spdx-image-3.0.bbclass b/meta/classes-recipe/create-spdx-image-3.0.bbclass
index 18e6cf6dfa2..51446162857 100644
--- a/meta/classes-recipe/create-spdx-image-3.0.bbclass
+++ b/meta/classes-recipe/create-spdx-image-3.0.bbclass
@@ -36,6 +36,7 @@  do_create_rootfs_spdx[sstate-inputdirs] = "${SPDXROOTFSDEPLOY}"
 do_create_rootfs_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
 do_create_rootfs_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
 do_create_rootfs_spdx[cleandirs] += "${SPDXROOTFSDEPLOY}"
+do_create_rootfs_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 
 python do_create_rootfs_spdx_setscene() {
     sstate_setscene(d)
@@ -53,6 +54,7 @@  do_create_image_spdx[sstate-inputdirs] = "${SPDXIMAGEWORK}"
 do_create_image_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
 do_create_image_spdx[cleandirs] = "${SPDXIMAGEWORK}"
 do_create_image_spdx[dirs] = "${SPDXIMAGEWORK}"
+do_create_image_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 
 python do_create_image_spdx_setscene() {
     sstate_setscene(d)
@@ -72,6 +74,7 @@  do_create_image_sbom_spdx[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}"
 do_create_image_sbom_spdx[stamp-extra-info] = "${MACHINE_ARCH}"
 do_create_image_sbom_spdx[cleandirs] = "${SPDXIMAGEDEPLOYDIR}"
 do_create_image_sbom_spdx[recrdeptask] += "do_create_spdx do_create_package_spdx"
+do_create_image_sbom_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 
 python do_create_image_sbom_spdx_setscene() {
     sstate_setscene(d)
diff --git a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
index ea01a21cc59..855fb3d09f9 100644
--- a/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
+++ b/meta/classes-recipe/create-spdx-sdk-3.0.bbclass
@@ -8,12 +8,14 @@ 
 do_populate_sdk[recrdeptask] += "do_create_spdx do_create_package_spdx"
 do_populate_sdk[cleandirs] += "${SPDXSDKWORK}"
 do_populate_sdk[postfuncs] += "sdk_create_sbom"
+do_populate_sdk[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk = " sdk_host_create_spdx"
 POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk = " sdk_target_create_spdx"
 
 do_populate_sdk_ext[recrdeptask] += "do_create_spdx do_create_package_spdx"
 do_populate_sdk_ext[cleandirs] += "${SPDXSDKEXTWORK}"
 do_populate_sdk_ext[postfuncs] += "sdk_ext_create_sbom"
+do_populate_sdk_ext[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 POPULATE_SDK_POST_HOST_COMMAND:append:task-populate-sdk-ext = " sdk_ext_host_create_spdx"
 POPULATE_SDK_POST_TARGET_COMMAND:append:task-populate-sdk-ext = " sdk_ext_target_create_spdx"
 
diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
index bc23d2d2115..640f5490bda 100644
--- a/meta/classes/create-spdx-3.0.bbclass
+++ b/meta/classes/create-spdx-3.0.bbclass
@@ -122,6 +122,13 @@  oe.spdx30_tasks.get_package_sources_from_debug[vardepsexclude] += "STAGING_KERNE
 oe.spdx30_tasks.collect_dep_objsets[vardepsexclude] = "SPDX_MULTILIB_SSTATE_ARCHS"
 
 
+# SPDX library code makes heavy use of classes, which bitbake cannot easily
+# parse out dependencies. As such, the library code files that make use of
+# classes are explicitly added as file checksum dependencies.
+SPDX3_LIB_DEP_FILES = "\
+    ${COREBASE}/meta/lib/oe/sbom30.py:True \
+    ${COREBASE}/meta/lib/oe/spdx30.py:True \
+    "
 
 python do_create_spdx() {
     import oe.spdx30_tasks
@@ -137,6 +144,7 @@  addtask do_create_spdx after \
 SSTATETASKS += "do_create_spdx"
 do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
 do_create_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
+do_create_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 
 python do_create_spdx_setscene () {
     sstate_setscene(d)
@@ -160,6 +168,7 @@  addtask do_create_package_spdx after do_create_spdx before do_build do_rm_work
 SSTATETASKS += "do_create_package_spdx"
 do_create_package_spdx[sstate-inputdirs] = "${SPDXRUNTIMEDEPLOY}"
 do_create_package_spdx[sstate-outputdirs] = "${DEPLOY_DIR_SPDX}"
+do_create_package_spdx[file-checksums] += "${SPDX3_LIB_DEP_FILES}"
 
 python do_create_package_spdx_setscene () {
     sstate_setscene(d)