All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Schier <nsc@kernel.org>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Luis Augenstein <luis.augenstein@tngtech.com>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, kstewart@linuxfoundation.org,
	maximilian.huber@tngtech.com
Subject: Re: [PATCH 02/15] scripts/sbom: integrate script in make process
Date: Tue, 31 Mar 2026 18:04:54 +0200	[thread overview]
Message-ID: <acvwpv7ISJoYSttX@levanger> (raw)
In-Reply-To: <20260331153009.GA1103611@ax162>

On Tue, Mar 31, 2026 at 05:30:09PM +0200, Nathan Chancellor wrote:
> On Tue, Mar 31, 2026 at 07:15:35AM +0200, Greg KH wrote:
> > On Mon, Mar 30, 2026 at 10:32:00PM +0200, Luis Augenstein wrote:
> > > Hi Nathan,
> > > 
> > > thanks a lot for your recommendations.
> > > 
> > > > Does sbom-roots.txt need to be cleaned up as well?
> > > 
> > > This file is only required to pass the roots into the python script.
> > > We could also use a tmp file. Then we don't need to worry about clean
> > > up. Together with your other suggested changes something like this
> > > should work:
> > > 
> > > # 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))
> > >       cmd_sbom = roots_file=$$(mktemp); \
> 
> I think I would rather have a named file in objtree instead of one in
> /tmp, as we want all output to remain in the build folder.

+1

The common way in kbuild is using '$(tmp-target)'.

> 
> > >                  printf "%s\n" "$(KBUILD_IMAGE)" >"$$roots_file"; \
> > >                  $(if $(CONFIG_MODULES),sed 's/\.o$$/.ko/'
> > > $(objtree)/modules.order >> "$$roots_file";) \
> > >                  $(PYTHON3) $(srctree)/scripts/sbom/sbom.py \
> > >                      --src-tree $(abspath $(srctree)) \
> > >                      --obj-tree $(abspath $(objtree)) \
> > >                      --roots-file "$$roots_file" \
> > >                      --output-directory $(abspath $(objtree)) \
> > >                      --generate-spdx \
> > >                      --package-license "GPL-2.0 WITH Linux-syscall-note" \
> > >                      --package-version "$(KERNELVERSION)" \
> > >                      --write-output-on-error;
> > >                  rm -f "$$roots_file"
> 
> 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 it might become a bit cleaner if the roots file is a separate
target and the 'sbom' target simply depends on it.  But we can defer
that.

> 
> > > PHONY += sbom
> > > sbom: $(notdir $(KBUILD_IMAGE)) include/generated/autoconf.h $(if
> > > $(CONFIG_MODULES),modules modules.order)
> > > 	$(call cmd,sbom)
> > > 
> > > Note, I will also add the --write-output-on-error flag by default such
> > > that the .spdx.json documents are generated as much as possible even if
> > > some build commands are unknown to the parser.
> 
> Seems reasonable to me.

If sbom.py is unable to parse the build commands, does it exit with a
non-zero exit code, correct?  As 'cmd_sbom' is run within a 'set -e'
shell environment, the $(delete-on-interrupt) will delete $@, thus there
should be _no_ output on error, regardless of --write-output-on-error.
So, it might make sense to kill $(delete-on-interrupt) by intention; but
that doesn't feel good to me, as the intention of 'cmd' will be
intransparently.

Kind regards,
Nicolas

  reply	other threads:[~2026-03-31 16:05 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 [this message]
2026-04-01 11:09             ` Luis Augenstein
2026-04-02 20:57               ` Nicolas Schier
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=acvwpv7ISJoYSttX@levanger \
    --to=nsc@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.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.