Re: [PATCH v3 3/3] tools build: Correct bpf fixdep dependencies

From: Andrii Nakryiko
Date: Fri Jul 12 2024 - 15:38:50 EST


On Tue, Jul 9, 2024 at 1:43 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
>
> The dependencies in tools/lib/bpf/Makefile are incorrect. Before we
> recurse to build $(BPF_IN_STATIC), we need to build its 'fixdep'
> executable.
>
> I can't use the usual shortcut from Makefile.include:
>
> <target>: <sources> fixdep
>
> because its 'fixdep' target relies on $(OUTPUT), and $(OUTPUT) differs
> in the parent 'make' versus the child 'make' -- so I imitate it via
> open-coding.
>
> I tweak a few $(MAKE) invocations while I'm at it, because
> 1. I'm adding a new recursive make; and
> 2. these recursive 'make's print spurious lines about files that are "up
> to date" (which isn't normally a feature in Kbuild subtargets) or
> "jobserver not available" (see [1])
>
> I also need to tweak the assignment of the OUTPUT variable, so that
> relative path builds work. For example, for 'make tools/lib/bpf', OUTPUT
> is unset, and is usually treated as "cwd" -- but recursive make will
> change cwd and so OUTPUT has a new meaning. For consistency, I ensure
> OUTPUT is always an absolute path.
>
> And $(Q) gets a backup definition in tools/build/Makefile.include,
> because Makefile.include is sometimes included without
> tools/build/Makefile, so the "quiet command" stuff doesn't actually work
> consistently without it.
>
> After this change, top-level builds result in an empty grep result from:
>
> $ grep 'cannot find fixdep' $(find tools/ -name '*.cmd')
>
> [1] https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html
> If we're not using $(MAKE) directly, then we need to use more '+'.
>
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>

I almost gave my acked-by and tested-by, but then I noticed that this
leaves fixdep, staticobjs and sharedobjs directories as
to-be-committed files. Please check, something is off with .gitignore
or where those are put:

$ cd ~/linux/tools/lib/bpf
$ make -j90
$ git st
On branch master
Your branch is ahead of 'bpf-next/master' by 4 commits.
(use "git push" to publish your local commits)

Untracked files:
(use "git add <file>..." to include in what will be committed)
fixdep
sharedobjs/
staticobjs/

nothing added to commit but untracked files present (use "git add" to track)


Other than that the changes look good, but we should be leaving
uncommitted (and unignored) files around.


Also (in it's less the question to the author, but rather all the
maintainers involved), which kernel tree is this intended to go
through, seems like it was marked as "Not Applicable" for bpf, so I'm
wondering where is the proper destination?

> Changes in v3:
> - add Jiri's Acked-by
>
> Changes in v2:
> - also fix libbpf shared library rules
> - ensure OUTPUT is always set, and always an absolute path
> - add backup $(Q) definition in tools/build/Makefile.include
>
> tools/build/Makefile.include | 12 +++++++++++-
> tools/lib/bpf/Makefile | 14 ++++++++++++--
> 2 files changed, 23 insertions(+), 3 deletions(-)
>

[...]

> -$(BPF_IN_SHARED): force $(BPF_GENERATED)
> +$(SHARED_OBJDIR):
> + $(Q)mkdir -p $@
> +
> +$(STATIC_OBJDIR):
> + $(Q)mkdir -p $@

I'd probably combine the above two rules into one, but it's minor

[...]