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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4Bzb6-DLL966XKyMhe+nmpvdqYVrzfmfkAiDdFHNyD0qGWw@mail.gmail.com>
Date: Fri, 12 Jul 2024 12:38:28 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Brian Norris <briannorris@...omium.org>
Cc: Arnaldo Carvalho de Melo <acme@...hat.com>, Namhyung Kim <namhyung@...nel.org>, 
	Ian Rogers <irogers@...gle.com>, Thomas Richter <tmricht@...ux.ibm.com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org, 
	Masahiro Yamada <masahiroy@...nel.org>, Jiri Olsa <jolsa@...nel.org>
Subject: Re: [PATCH v3 3/3] tools build: Correct bpf fixdep dependencies

On Tue, Jul 9, 2024 at 1:43 PM Brian Norris <briannorris@...omium.org> 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@...omium.org>
> Acked-by: Jiri Olsa <jolsa@...nel.org>
> ---
>

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

[...]