From: Nicolas Schier <nsc@kernel.org>
To: Luis Augenstein <luis.augenstein@tngtech.com>
Cc: Nathan Chancellor <nathan@kernel.org>,
"linux-kbuild >> \"linux-kbuild@vger.kernel.org\""
<linux-kbuild@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"kstewart@linuxfoundation.org" <kstewart@linuxfoundation.org>,
Maximilian Huber <maximilian.huber@tngtech.com>
Subject: Re: [PATCH 02/15] scripts/sbom: integrate script in make process
Date: Thu, 2 Apr 2026 22:57:14 +0200 [thread overview]
Message-ID: <ac7YKs79GPfxvw5T@levanger> (raw)
In-Reply-To: <900ee659-3663-44a7-806d-93f995224f76@tngtech.com>
On Wed, Apr 01, 2026 at 01:09:55PM +0200, Luis Augenstein wrote:
> Hi everyone,
>
> > If sbom.py is unable to parse the build commands, does it exit with a
> > non-zero exit code, correct?
>
> yes correct. The current behavior is to always parse as much as
> possible, collect all problems, print an error summary in the end and
> exit with a non-zero exit code.
>
> >> The cmd macro uses 'set -e', so consider moving this up and making it
> >>
> >> trap "rm -rf $$roots_file" EXIT; \
> >>
> >> like try-run in scripts/Makefile.compiler does to ensure it is always
> >> cleaned up.
> >
> > hm, well. Yes, this should do as expected, but please be aware that
> > this also kills the $(delete-on-interrupt) which is part of $(cmd) and
> > removes $@ in case of error or interruption by installing a trap --
> > which will be overwritten. See also below.
>
> I think $(delete-on-interrupt) only operates on non-phony targets. Since
> our target is $@=sbom instead of the generated .spdx.json files
> $(delete-on-interrupt) currently has no effect and wouldn't really be
> "killed by intention" by introducing a trap within cmd_sbom.
ah yes, correct.
> > there should be _no_ output on error, regardless of
> --write-output-on-error.
>
> If this is a general convention we should follow, then we maybe want to
> return a zero exit code in sbom.py in case of errors when
> --write-output-on-error is set, effectively treating errors as warnings?
> Alternatively, we could keep not using --write-output-on-error by
> default. Or we don't follow the convention and write the output
> *.spdx.json files in case of errors.
I don't have a strong opnion on that. But I think it is important to
keep make exiting with a non-zero exit code if a target fails. As you
described above, that is the way it is with your current patch set
version; thus I am happy with that behaviour.
> I am not sure what's the most appropriate behavior here. However, we
> know that people will very likely encounter cases with commands unknown
> to the parser. It's not very useful to simply deny generating the entire
> documents, because in many cases the sbom will still be quite usable
> although less complete. For example, if the parsing error occurs close
> to the leaf nodes it cuts off only a small part of the dependency graph
> which still retains most of the information.
Ack.
> Tim Bird apparently already encountered this problem which lead him to
> manually add the --write-output-on-error flag.
> https://birdcloud.org/bc/Linux_Kernel_Missing_SPDX_ID_lines_from_build_SBOMs
>
>
> > The cmd macro uses 'set -e', so consider moving this up and making it
> >
> > trap "rm -rf $$roots_file" EXIT; \
> >
> > like try-run in scripts/Makefile.compiler does to ensure it is always
> > cleaned up.
> and
> > The common way in kbuild is using '$(tmp-target)'.
>
> This would be the new version then:
>
> # Script to generate .spdx.json SBOM documents describing the build
> #
> ---------------------------------------------------------------------------
>
> ifdef building_out_of_srctree
> sbom_targets := sbom-source.spdx.json
> endif
> sbom_targets += sbom-build.spdx.json sbom-output.spdx.json
> quiet_cmd_sbom = GEN $(notdir $(sbom_targets))
If all filenames in $(sbom_targets) are w/o path components, the
$(notdir) is obsolete.
> cmd_sbom = roots_file="$(tmp-target)"; \
> trap "rm -rf $$roots_file" EXIT; \
I don't see a good reason in the trap here, now that the roots file is a
kernel build temp file (ignored by git, removed by make clean). Thus
I'd rather recommend to remove the 'trap' line as well as the roots_file
variable but use $(tmp-target) instead in all three places.
Thanks!
Kind regards
Nicolas
next prev parent reply other threads:[~2026-04-02 20:58 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-10 20:54 [PATCH v4 00/15] add SPDX SBOM generation script Luis Augenstein
2026-02-10 20:54 ` [PATCH 01/15] scripts/sbom: add documentation Luis Augenstein
2026-02-10 20:54 ` [PATCH 02/15] scripts/sbom: integrate script in make process Luis Augenstein
2026-03-30 9:50 ` Nathan Chancellor
2026-03-30 20:32 ` Luis Augenstein
2026-03-31 5:15 ` Greg KH
2026-03-31 15:30 ` Nathan Chancellor
2026-03-31 16:04 ` Nicolas Schier
2026-04-01 11:09 ` Luis Augenstein
2026-04-02 20:57 ` Nicolas Schier [this message]
2026-04-01 11:12 ` Luis Augenstein
2026-02-10 20:54 ` [PATCH 03/15] scripts/sbom: setup sbom logging Luis Augenstein
2026-02-10 20:54 ` [PATCH 04/15] scripts/sbom: add command parsers Luis Augenstein
2026-02-10 20:54 ` [PATCH 05/15] scripts/sbom: add cmd graph generation Luis Augenstein
2026-02-10 20:54 ` [PATCH 06/15] scripts/sbom: add additional dependency sources for cmd graph Luis Augenstein
2026-02-10 20:54 ` [PATCH 07/15] scripts/sbom: add SPDX classes Luis Augenstein
2026-02-10 20:54 ` [PATCH 08/15] scripts/sbom: add JSON-LD serialization Luis Augenstein
2026-02-10 20:54 ` [PATCH 09/15] scripts/sbom: add shared SPDX elements Luis Augenstein
2026-02-10 20:54 ` [PATCH 10/15] scripts/sbom: collect file metadata Luis Augenstein
2026-02-10 20:54 ` [PATCH 11/15] scripts/sbom: add SPDX output graph Luis Augenstein
2026-02-10 20:54 ` [PATCH 12/15] scripts/sbom: add SPDX source graph Luis Augenstein
2026-02-10 20:54 ` [PATCH 13/15] scripts/sbom: add SPDX build graph Luis Augenstein
2026-02-10 20:54 ` [PATCH 14/15] scripts/sbom: add unit tests for command parsers Luis Augenstein
2026-02-10 20:54 ` [PATCH 15/15] scripts/sbom: add unit tests for SPDX-License-Identifier parsing Luis Augenstein
2026-03-23 13:39 ` [PATCH v4 00/15] add SPDX SBOM generation script Greg KH
2026-03-29 6:29 ` Greg KH
2026-03-30 5:50 ` Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ac7YKs79GPfxvw5T@levanger \
--to=nsc@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=kstewart@linuxfoundation.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luis.augenstein@tngtech.com \
--cc=maximilian.huber@tngtech.com \
--cc=nathan@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox