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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.