Track the missing dependencies on SPDX documents by quaresmajose · Pull Request #1109 · foundriesio/meta-lmp

quaresmajose · 2023-04-18T15:16:56Z

This handler will generate an ERROR when there are 'do_deploy' dependencies without setting the DEPENDS.
One of the side effect of this issues is SPDX documents not including the correct dependencies.

/
| ERROR: lmp-boot-firmware-0-r0 do_install: Task 'do_install' depends on 'virtual/bootloader:do_deploy' but 'virtual/bootloader' is not in DEPENDS
| ERROR: lmp-boot-firmware-0-r0 do_install: Task 'do_install' depends on 'virtual/trusted-firmware-a:do_deploy' but 'virtual/trusted-firmware-a' is not in DEPENDS
\

Explicit adds the dependencies of the 'do_deploy' in the task[depends].
With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image.

The following example is for the lmp-base-console-image on stm32mp15-disco machine:

/
| grep -E "u-boot|tf-a" deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "documentNamespace": "http://spdx.org/spdxdoc/recipe-tf-a-fio-546ca65d-3ceb-5456-93cd-e8acffa32a17",
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "filename": "recipe-tf-a-fio.spdx.json",
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "documentNamespace": "http://spdx.org/spdxdoc/recipe-tf-a-tools-native-52687ea6-9f82-5a2d-a8ab-f4e90f31e6fd",
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "filename": "recipe-tf-a-tools-native.spdx.json",
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "documentNamespace": "http://spdx.org/spdxdoc/recipe-u-boot-fio-cfb5b3c2-fc87-5d33-a223-5b46ae9bb0f5",
| deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json:      "filename": "recipe-u-boot-fio.spdx.json",
\

ricardosalveti · 2023-04-18T15:23:14Z

Please extend the commit messages to avoid patches with just the subject line (add more details as part of the commit body message).

quaresmajose · 2023-04-18T17:49:51Z

I reduce this PR only with the fix and move the refactor to a new one.

MrCry0 · 2023-04-18T18:34:24Z

in the commit msg: s/combinad/combined/ ?

quaresmajose · 2023-04-18T18:45:06Z

in the commit msg: s/combinad/combined/ ?

fixed! thanks

MrCry0

LGTM

angolini

LGTM

angolini · 2023-04-18T20:05:02Z

+python () {
+    # we need to set the DEPENDS as well to produce valid SPDX documents
+    for tasks in d.getVarFlag('do_install', 'depends').split():
+        d.appendVar('DEPENDS', " %s" % tasks.split(":")[0])


Setting DEPENDS usually goes on the top of the file. Is there any reason it must be at the bottom in this case?

Nothing special, only because it is using a non common anonymous python.

quaresmajose · 2023-04-21T15:11:14Z

validated on foundriesio/lmp-manifest#318

ricardosalveti · 2023-04-24T21:41:21Z

+    for depend in depends:
+        recipe, task = depend.split(':')
+        if task == 'do_deploy' and recipe not in d.getVar('DEPENDS'):
+            bb.error("Task '%s' depends on '%s' but '%s' is not in DEPENDS" % (taskname, depend, recipe))


I think it is better for us to warn by default instead of calling an error here, as this might have side effects in customer related recipes and cause certain builds to fail.

While we do want to have SPDX in a correct form, we shouldn't just yet enforce this on every recipe (we can have as a warn first and on another release move to error, for example).

I put a new commit 25a255e wih this that is simpler to reference later.

…n depends This handler will generate an ERROR when there are 'do_deploy' dependencies without setting the DEPENDS. One of the side effect of this issues is SPDX documents not including the correct dependencies. / | ERROR: lmp-boot-firmware-0-r0 do_install: Task 'do_install' depends on 'virtual/bootloader:do_deploy' but 'virtual/bootloader' is not in DEPENDS | ERROR: lmp-boot-firmware-0-r0 do_install: Task 'do_install' depends on 'virtual/trusted-firmware-a:do_deploy' but 'virtual/trusted-firmware-a' is not in DEPENDS \ Signed-off-by: Jose Quaresma <[email protected]>

Setting DEPENDS create dependency loops so skip the check Signed-off-by: Jose Quaresma <[email protected]>

… task[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. The following example is for the lmp-base-console-image on stm32mp15-disco machine: / | grep -E "u-boot|tf-a" deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "documentNamespace": "http://spdx.org/spdxdoc/recipe-tf-a-fio-546ca65d-3ceb-5456-93cd-e8acffa32a17", | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "filename": "recipe-tf-a-fio.spdx.json", | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "documentNamespace": "http://spdx.org/spdxdoc/recipe-tf-a-tools-native-52687ea6-9f82-5a2d-a8ab-f4e90f31e6fd", | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "filename": "recipe-tf-a-tools-native.spdx.json", | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "documentNamespace": "http://spdx.org/spdxdoc/recipe-u-boot-fio-cfb5b3c2-fc87-5d33-a223-5b46ae9bb0f5", | deploy/images/stm32mp15-disco/lmp-base-console-image-stm32mp15-disco.spdx.index.json: "filename": "recipe-u-boot-fio.spdx.json", \ Signed-off-by: Jose Quaresma <[email protected]>

…pends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…nce on task[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…sk[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…k[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

… on task[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…nds] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

… on task[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…epends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…o_deploy' presence on task[depends] Explicit adds the dependencies of the 'do_deploy' in the task[depends]. With this patch the missed dependencies are now in the SPDX documents of the package as well as in the final SPDX of the combined image. Signed-off-by: Jose Quaresma <[email protected]>

…f an error It is better for us to warn by default instead of calling an error here, as this might have side effects in customer related recipes and cause certain builds to fail. While we do want to have SPDX in a correct form, we shouldn't just yet enforce this on every recipe (we can have as a warn first and on another release move to error, for example). Signed-off-by: Jose Quaresma <[email protected]>

Signed-off-by: Jose Quaresma <[email protected]>

ricardosalveti

LGTM

quaresmajose requested a review from a team April 18, 2023 15:16

ricardosalveti reviewed Apr 18, 2023

View reviewed changes

Comment thread meta-lmp-base/recipes-bsp/lmp-boot-firmware/lmp-boot-firmware.bb Outdated

quaresmajose force-pushed the lmp-boot-firmware branch from cca557e to fa73062 Compare April 18, 2023 17:47

quaresmajose force-pushed the lmp-boot-firmware branch 2 times, most recently from 5ef29b2 to 9576f23 Compare April 18, 2023 18:44

MrCry0 approved these changes Apr 18, 2023

View reviewed changes

angolini approved these changes Apr 18, 2023

View reviewed changes

quaresmajose force-pushed the lmp-boot-firmware branch 2 times, most recently from 0abef67 to ee4404e Compare April 19, 2023 10:42

quaresmajose marked this pull request as draft April 19, 2023 22:50

quaresmajose force-pushed the lmp-boot-firmware branch 3 times, most recently from 83a8b45 to c1eb5f5 Compare April 21, 2023 11:33

quaresmajose mentioned this pull request Apr 21, 2023

base: lmp-staging: add an handler to warn about 'do_deploy' bad usage #1112

Closed

quaresmajose force-pushed the lmp-boot-firmware branch 2 times, most recently from 91f39ab to 48dd890 Compare April 21, 2023 15:05

quaresmajose marked this pull request as ready for review April 21, 2023 15:08

quaresmajose changed the title ~~Lmp boot firmware: track the missing dependencies on SPDX combined image~~ Track the missing dependencies on SPDX documents Apr 21, 2023

quaresmajose force-pushed the lmp-boot-firmware branch from 48dd890 to c6be850 Compare April 21, 2023 19:10

ricardosalveti reviewed Apr 24, 2023

View reviewed changes

Comment thread meta-lmp-base/classes/lmp-staging.bbclass Outdated

ricardosalveti reviewed Apr 24, 2023

View reviewed changes

Comment thread meta-lmp-base/recipes-sota/ostree-kernel-initramfs/ostree-kernel-initramfs_%.bbappend Outdated

quaresmajose force-pushed the lmp-boot-firmware branch 2 times, most recently from 0783ed6 to 0ba3026 Compare April 25, 2023 09:25

quaresmajose added 14 commits May 8, 2023 14:45

base: u-boot-fio-common: skip checking 'do_deploy' in depends

7adf7c9

Setting DEPENDS create dependency loops so skip the check Signed-off-by: Jose Quaresma <[email protected]>

bsp: u-boot-xlnx: skip checking 'do_deploy' in depends

85de3c6

Setting DEPENDS create dependency loops so skip the check Signed-off-by: Jose Quaresma <[email protected]>

base: ostree-kernel-initramfs: the default dependencies is not needed

3b12a43

Signed-off-by: Jose Quaresma <[email protected]>

quaresmajose force-pushed the lmp-boot-firmware branch from 0ba3026 to 3b12a43 Compare May 8, 2023 14:45

ricardosalveti approved these changes May 9, 2023

View reviewed changes

ricardosalveti merged commit 6e74f8c into foundriesio:main May 9, 2023

quaresmajose deleted the lmp-boot-firmware branch May 9, 2023 09:19

Conversation

quaresmajose commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ricardosalveti commented Apr 18, 2023

Uh oh!

quaresmajose commented Apr 18, 2023

Uh oh!

MrCry0 commented Apr 18, 2023

Uh oh!

quaresmajose commented Apr 18, 2023

Uh oh!

MrCry0 left a comment

Choose a reason for hiding this comment

Uh oh!

angolini left a comment

Choose a reason for hiding this comment

Uh oh!

angolini Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

quaresmajose Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quaresmajose commented Apr 21, 2023

Uh oh!

Uh oh!

ricardosalveti Apr 24, 2023

Choose a reason for hiding this comment

Uh oh!

quaresmajose Apr 25, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ricardosalveti left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

quaresmajose commented Apr 18, 2023 •

edited

Loading

quaresmajose Apr 19, 2023 •

edited

Loading