[2/2] cve-check: hook cleanup to the BuildCompleted event, not CookerExit

Message ID 20220629151519.1179173-2-ross.burton@arm.com
State Accepted, archived
Commit fccdcfd301de281a427bfee48d8ff47fa07b7259
Headers show
Series [1/2] cups: ignore CVE-2022-26691 | expand

Commit Message

Ross Burton June 29, 2022, 3:15 p.m. UTC
The cve-check class writes temporary files to preserve state across the
build, and cleans them up in a CookerExit handler.

However, in memory-resident builds the cooker won't exit in between
builds, so the state isn't cleared and the CVE report generation fails:

NOTE: Generating JSON CVE summary
ERROR: Error adding the same package twice

Easily solved by hooking to BuildCompleted, instead of CookerExit.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 meta/classes/cve-check.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marta Rybczynska June 29, 2022, 3:19 p.m. UTC | #1
On Wed, Jun 29, 2022 at 5:15 PM Ross Burton <ross.burton@arm.com> wrote:

> The cve-check class writes temporary files to preserve state across the
> build, and cleans them up in a CookerExit handler.
>
> However, in memory-resident builds the cooker won't exit in between
> builds, so the state isn't cleared and the CVE report generation fails:
>
> NOTE: Generating JSON CVE summary
> ERROR: Error adding the same package twice
>
> Easily solved by hooking to BuildCompleted, instead of CookerExit.
>
> Signed-off-by: Ross Burton <ross.burton@arm.com>
>

Sean, could you check if it is your case too? I'll be adding a more verbose
error message
so that we know which package it comes from.

Regards,
Marta
Ross Burton June 29, 2022, 3:44 p.m. UTC | #2
> On 29 Jun 2022, at 16:19, Marta Rybczynska <rybczynska@gmail.com> wrote:
> Sean, could you check if it is your case too? I'll be adding a more verbose error message
> so that we know which package it comes from.

I actually think we should get rid of the index file entirely. Why can’t the big JSON file simply be all of the written JSON files merged together (by simply listing the directory contents)?

Ross
Marta Rybczynska June 30, 2022, 4:03 p.m. UTC | #3
On Wed, Jun 29, 2022 at 5:45 PM Ross Burton <ross.burton@arm.com> wrote:

>
> > On 29 Jun 2022, at 16:19, Marta Rybczynska <rybczynska@gmail.com> wrote:
> > Sean, could you check if it is your case too? I'll be adding a more
> verbose error message
> > so that we know which package it comes from.
>
> I actually think we should get rid of the index file entirely. Why can’t
> the big JSON file simply be all of the written JSON files merged together
> (by simply listing the directory contents)?
>
> Without the index file I was running into the issue of merging fragment
files from different builds
in the same directory (different images, the world build etc). I can see
the following solutions:
1. Move the fragment files to a separate directory and remove it at the
build end. It would work if nobody if using fragment files.
2. Extract the complete list of the packages build in a different way and
get fragment files using that different list.

Opinions?

Regards,
Marta

Patch

diff --git a/meta/classes/cve-check.bbclass b/meta/classes/cve-check.bbclass
index 50b9247f464..da7f93371c0 100644
--- a/meta/classes/cve-check.bbclass
+++ b/meta/classes/cve-check.bbclass
@@ -166,7 +166,7 @@  python cve_check_cleanup () {
 }
 
 addhandler cve_check_cleanup
-cve_check_cleanup[eventmask] = "bb.cooker.CookerExit"
+cve_check_cleanup[eventmask] = "bb.event.BuildCompleted"
 
 python cve_check_write_rootfs_manifest () {
     """