Re: [PATCH bpf-next v7 8/9] selftests/bpf: move shared build definitions into Makefile.buildvars

> commit 79d6c61a930a7bcff9e424734672a09c86fbdcf2
> Author: Mykola Lysenko <[email protected]>
> Date:   Sun Aug 23 12:42:40 2026 -0700
>
>     selftests/bpf: move shared build definitions into Makefile.buildvars
>
>     Move the build definitions a test runner needs - tree layout and
>     tool locations, flag assembly, feature probes, signing key paths, the
>     permissive-mode helpers - into Makefile.buildvars, in their current
>     order, as preparation for building each test runner instance in its
>     own sub-make: the definitions become includable by more than one
>     makefile. No rules or recipes are changed.
>
>     The include sits directly after ../lib.mk, ahead of the
>     ../../../build/Makefile.feature evaluation, so the definitions are in
>     place for everything the top Makefile parses afterwards - in particular
>     the LLVM feature probe captures srctree and PKG_CONFIG from their new
>     location, and the top Makefile's remaining uses of both are all
>     recipe-time expansions. Two evaluation-time changes follow from
>     the new position and a third is made on the way, each commented in
>     place: CFLAGS and LDFLAGS are
>     reassembled around a snapshot taken before lib.mk, keeping the
>     pre-split order;
>     CLANG_SYS_INCLUDES becomes an immediate assignment (its inputs are
>     final by this point, and expanding it forks shells); and the
>     CLANG_HAS_ARENA_ASAN probe now sees CLANG as resolved from
>     LLVM=<suffix-or-path> rather than the bare 'clang' default it saw
>     above the include, while the CLANG_CPUV4 probe, needed before lib.mk,
>     keeps the old behavior.
>
>     BPF_GCC, TEST_KMODS and the knobs the runner never reads (SKIP_*,
>     submake_extras, TEST_KMOD_TARGETS, the VMLINUX_BTF block) stay in the
>     Makefile - the first two are needed before lib.mk is included, where
>     Makefile.buildvars cannot yet be, and reach the runners from there once
>     a later patch adds them: TEST_KMODS exported in the environment, BPF_GCC
>     as the bpf_gcc flavor's compiler parameter. Definitions that reach no
>     runner, directly or as a sub-make parameter, also stay - except
>     HOST_INCLUDE_DIR, which moves with the conditional block it shares
>     with the host build directories - and the four that derive from
>     Makefile.buildvars values sit in a block right after the include.
>
>     Suggested-by: Eduard Zingerman <[email protected]>
>     Assisted-by: Claude:claude-fable-5 shellcheck
>     Signed-off-by: Mykola Lysenko <[email protected]>

This isn't a bug, but the second paragraph has some lines that look
unfinished mid-paragraph ("place: CFLAGS and LDFLAGS are" at 29
columns, then "pre-split order;" at 16 columns), as if the text was
edited but not rewrapped. Would it help to reflow those lines?

The third paragraph walks through which variables moved and which
stayed ("BPF_GCC, TEST_KMODS and the knobs the runner never reads...
also stay - except HOST_INCLUDE_DIR..."), restating what the diff shows.
The key detail - that BPF_GCC and TEST_KMODS must be defined before
../lib.mk - is there, but it's somewhat buried in the inventory. Would
trimming the list and keeping just the reason make this clearer?

> diff --git a/tools/testing/selftests/bpf/Makefile.buildvars b/tools/testing/selftests/bpf/Makefile.buildvars
> new file mode 100644
> index 0000000000000..dc0be9ee72bc0
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/Makefile.buildvars
> @@ -0,0 +1,184 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Shared toolchain, path and flag definitions for the BPF selftests
> +# build, included by both Makefile and Makefile.runner. Definitions
> +# follow the order they had in the Makefile before the split.
> +
> +TOOLSDIR := $(abspath ../../..)

[ ... ]

> +# Before the split these definitions sat above the ../lib.mk include:
> +# they appended to the flags inherited from the environment, and
> +# lib.mk's additions (USERCFLAGS/USERLDFLAGS among them) came last.
> +# This file is parsed after lib.mk at the top level and in its place
> +# in the runner sub-makes, so the same order is rebuilt here. The
> +# includer snapshots the inherited flags in INHERITED_CFLAGS and

This isn't a bug, but the header and several comments are written from
the patch's point of view rather than as documentation of the current
state. The header says definitions "follow the order they had in the
Makefile before the split", and this block starts with "Before the
split these definitions sat above the ../lib.mk include". A reader
arriving later has no "before" to compare against.

The header also says this file is "included by both Makefile and
Makefile.runner", but Makefile.runner doesn't exist until the next
commit. If someone lands here during a bisect, that sentence is
inaccurate.

Would these comments read better stated as the current invariant ("lib.mk
appends to CFLAGS/LDFLAGS; this file's additions must come first, so
the includer snapshots them in INHERITED_*"), and should the header
mention Makefile.runner only once it exists?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32662817569