public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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


  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