Re: [PATCH v2 1/1] kbuild: record real-prereqs in .cmd files

[Nicolas Schier]

First of all, sorry for the very long delay.  I finally had a first view
and tests on the patch, but I haven't tested in depth yet.  Some
first-round feedback:

> Record $(real-prereqs), the non-phony prerequisites of the target, in a
> new metadata field:
> 
>     make_prereqs_<target> := <prerequisites>
> 
> Write the field from both cmd_and_savecmd and cmd_and_fixdep.
>
> Update scripts/make_fit.py to read only savedcmd_* instead of parsing the
> complete .cmd file.

This sounds like a good thing; but I can't find that in the diff.

While at it, you changed the handling of deps in fixdep -- I think in a
good way.  I'd like to see this in three steps:

  * Limit make_fit's parsing to savedcmd_
  * Update fixdeps handling of 'deps_'
  * Add 'make_prereqs_<target>' support to Kbuild.include and fixdep

> Ignore make_prereqs_* in KernelSbom.

I know this makes sense to fit the every-interface-needs-a-user rule,
but I don't like to add a dummy consumer here to match that rule.

Why do you defer the actual implementation in the SBOM scripts?  From my
point of view, the introduction of 'make_prereqs_<target>' looks good
and I don't see a reason for waiting with the corresponding SBOM
implementation.

> 
> Link: https://lore.kernel.org/r/a01233b9-23a2-4666-91ed-f1cf030dcb9f@xxxxxxxxxxx
> Assisted-by: Cursor:GPT-5.6 Sol

The new rule wants:

    Assisted-by: LLM 

instead, cp. https://git.kernel.org/stable/c/816d9992d9ed434ec52cfbd63080d518e535a41b

> Co-developed-by: Maximilian Huber <maximilian.huber@xxxxxxxxxxx>
> Signed-off-by: Maximilian Huber <maximilian.huber@xxxxxxxxxxx>
> Signed-off-by: Luis Augenstein <luis.augenstein@xxxxxxxxxxx>
> ---
>  scripts/Kbuild.include                  |   9 +-
>  scripts/basic/fixdep.c                  | 159 ++++++++++++++++++++----
>  scripts/make_fit.py                     |   2 +-
>  scripts/sbom/sbom/cmd_graph/cmd_file.py |   4 +
>  4 files changed, 148 insertions(+), 26 deletions(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 8c311b997e24..6daa244ba0ef 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -181,6 +181,9 @@ endif
>  # (needed for the shell)
>  make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
>  
> +# prerequisites to record in .cmd files, excluding those covered in deps_*
> +cmd-prereqs = $(call escsq,$(filter-out $(deps_$@), $(real-prereqs)))
> +
>  # Find any prerequisites that are newer than target or that do not exist.
>  # PHONY targets skipped in both cases.
>  # If there is no prerequisite other than phony targets, $(newer-prereqs) becomes
> @@ -198,14 +201,16 @@ if_changed = $(if $(if-changed-cond),$(cmd_and_savecmd),@:)
>  
>  cmd_and_savecmd =                                                            \
>  	$(cmd);                                                              \
> -	printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd
> +	printf '%s\n\n%s\n' 'savedcmd_$@ := $(make-cmd)'                    \
> +		'make_prereqs_$@ := $(cmd-prereqs)' > $(dot-target).cmd

Just bike-shedding:  I think the main points are tiny bit easier to read
this way:

 cmd_and_savecmd =							\
 	$(cmd);								\
	printf '%s\n'							\
		'savedcmd_$@ := $(make-cmd)' ''				\
		'make_prereqs_$@ := $(cmd-prereqs)' > $(dot-target).cmd

Take it or just keep your version, as you like.




All in all, this looks good to me. Thanks

-- 
Nicolas