Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ian Rogers" <irogers@google.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Quentin Monnet" <qmo@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nicolas Schier" <nsc@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>
Cc: linux-perf-users@vger.kernel.org, bpf@vger.kernel.org,
	linux-kbuild@vger.kernel.org, gary@garyguo.net
Subject: Re: [RFC PATCH v1 1/3] tools build: Add Rust library build infrastructure and zlib-rs feature test
Date: Thu, 29 Oct 2026 16:57:04 +0000	[thread overview]
Message-ID: <sashiko-outbox-189694@kernel.org> (raw)
In-Reply-To: <20261029162958.3224280-2-irogers@google.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] tools build: sub-make PKG_CONFIG override blocks --static append
- [Medium] tools build: lib%.rlib pattern rule fails for subdirectory targets
- [Medium] tools build: missing rustc --target in Makefile.rust and early feature check
--

commit 15e2b90d3792008ff59190071c3b1973f7ab8c0b
Author: Ian Rogers <irogers@google.com>

tools build: Add Rust library build infrastructure and zlib-rs feature test

This patch adds shared Rust library build and feature detection infrastructure
under tools/build/ and tools/scripts/, along with a feature test for zlib-rs.
It supports locating Cargo crate sources or pkg-config packages, compiling
.rlib and .a targets with rustc, and resolving C compiler and linker flags.

> diff --git a/tools/build/Makefile.build b/tools/build/Makefile.build
> index ad69efdd4e85c..50c3eb57d34b8 100644
> --- a/tools/build/Makefile.build
> +++ b/tools/build/Makefile.build
[ ... ]
> @@ -117,6 +120,10 @@ $(OUTPUT)%.s: %.c FORCE
>  
>  # it's recommended to build a static rust library, when a foreight (to rust)
>  # linker is used.
> +$(OUTPUT)lib%.rlib: %.rs FORCE
> +	$(call rule_mkdir)
> +	$(call if_changed_dep,rustc_rlib_rs)

[Severity: Medium]
Will this pattern rule match .rlib targets located in subdirectories?

Later in tools/build/Makefile.build, targets in obj-y are prefixed with both
$(OUTPUT) and $(dir)/:

    objprefix    := $(subst ./,,$(OUTPUT)$(dir)/)
    obj-y        := $(addprefix $(objprefix),$(obj-y))

Other pattern rules in tools/build/Makefile.build place % immediately after
$(OUTPUT), such as $(OUTPUT)%.o: %.c FORCE and $(OUTPUT)%.a: %.rs FORCE, which
allows the stem % to match <subdir>/<name>.

Because $(OUTPUT)lib%.rlib places the literal prefix lib immediately after
$(OUTPUT), a target in a subdirectory expands to
$(OUTPUT)<subdir>/lib<name>.rlib. When $(OUTPUT) is non-empty, the pattern
contains a slash, so GNU Make matches against the full path and fails to match
$(OUTPUT)lib%.rlib whenever <subdir> is present. When $(OUTPUT) is empty,
directory-stripped matching sets the stem % to <name> without <subdir>/, so
the prerequisite %.rs looks for <name>.rs in the current directory rather than
in <subdir>/.

> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 4d37d11f674c5..c372600ae9e44 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -5,10 +5,11 @@ ifneq ($(OUTPUT),)
>    OUTPUT_FEATURES = $(OUTPUT)feature/
>    $(shell mkdir -p $(OUTPUT_FEATURES))
>  endif
> +RUST_OUTPUT ?= $(abspath $(or $(OUTPUT),$(CURDIR)))/
>  
>  feature_check = $(eval $(feature_check_code))
>  define feature_check_code
> -  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)" CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> +  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) RUST_OUTPUT="$(RUST_OUTPUT)" CC="$(CC)" CXX="$(CXX)" RUSTC="$(RUSTC)" $(if $(PKG_CONFIG),PKG_CONFIG="$(PKG_CONFIG)") CFLAGS="$(CLANG_CROSS_FLAGS) $(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(CLANG_CROSS_FLAGS) $(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" RUST_FLAGS="$(RUST_FLAGS)" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
>  endef
[ ... ]
> diff --git a/tools/build/Makefile.rust b/tools/build/Makefile.rust
> new file mode 100644
> index 0000000000000..cd9e0915b9298
> --- /dev/null
> +++ b/tools/build/Makefile.rust
> @@ -0,0 +1,149 @@
[ ... ]
> +ifeq ($(findstring -static,${LDFLAGS}),-static)
> +  ifeq ($(findstring --static,$(PKG_CONFIG)),)
> +    PKG_CONFIG += --static
> +  endif
> +endif

[Severity: Medium]
When LDFLAGS contains -static, does PKG_CONFIG += --static take effect if the
caller passes PKG_CONFIG on the sub-make command line?

Both feature_check_code in tools/build/Makefile.feature and
feature_rust_lib_code in tools/scripts/Makefile.include pass
$(if $(PKG_CONFIG),PKG_CONFIG="$(PKG_CONFIG)") as a command-line argument to
$(MAKE). When invoked from Makefiles that initialize PKG_CONFIG (such as
tools/perf/Makefile.perf or tools/tracing/rtla/Makefile.rtla), PKG_CONFIG has
command-line origin in the sub-make.

In GNU Make, ordinary non-override += appends to a command-line variable are
ignored, so PKG_CONFIG += --static in both tools/build/Makefile.rust and
tools/build/feature/Makefile:

    ifeq ($(findstring -static,${LDFLAGS}),-static)
      PKG_CONFIG += --static
    endif

is skipped during static builds, omitting transitive static libraries from
pkg-config queries. Should these use override PKG_CONFIG += --static?

[ ... ]
> +cmd_rustc_rlib = \
> +	$(rustc_target_envs) $(RUSTC) $(RUST_FLAGS) $(rustc_common_flags) $(rustc_target_flags) \
> +	$(call rlibs-to-externs,$^) --crate-type rlib --crate-name $(patsubst lib%.rlib,%,$(notdir $@)) \
> +	-o $@ $(filter %.rs,$^) > $(@:.rlib=.make.output) 2>&1
> +
> +cmd_rustc_staticlib = \
> +	$(rustc_target_envs) $(RUSTC) $(RUST_FLAGS) $(rustc_common_flags) $(rustc_target_flags) \
> +	$(call rlibs-to-externs,$^) --crate-type staticlib --crate-name $(patsubst lib%.a,%,$(notdir $@)) \
> +	-o $@ $(filter %.rs,$^) > $(@:.a=.make.output) 2>&1
[ ... ]
> diff --git a/tools/scripts/Makefile.arch b/tools/scripts/Makefile.arch
> index ed5f008d400ed..d5f25f3be555f 100644
> --- a/tools/scripts/Makefile.arch
> +++ b/tools/scripts/Makefile.arch
> @@ -50,3 +50,17 @@ ifeq ($(LP64), 1)
>  else
>    IS_64_BIT := 0
>  endif
> +
> +# Map kernel ARCH to rustc --target triples when cross-compiling tools/.
> +ifneq ($(CROSS_COMPILE),)
> +  RUST_TARGET_FLAGS_arm		:= arm-unknown-linux-gnueabi
> +  RUST_TARGET_FLAGS_arm64	:= aarch64-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_loongarch	:= loongarch64-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_m68k	:= m68k-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_mips	:= mipsel-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_powerpc	:= powerpc64le-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_riscv	:= riscv64gc-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_s390	:= s390x-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_x86		:= x86_64-unknown-linux-gnu
> +  RUST_TARGET_FLAGS_x86_64	:= x86_64-unknown-linux-gnu
> +endif

[Severity: Medium]
How does RUST_TARGET_FLAGS_$(ARCH) get passed to rustc when tools/build/feature
or tools/build/Makefile.rust builds libz_rs.a during cross-compilation?

tools/scripts/Makefile.arch defines RUST_TARGET_FLAGS_<arch> when CROSS_COMPILE
is set, but neither Makefile.arch, Makefile.include, Makefile.feature, nor
Makefile.rust adds --target=$(RUST_TARGET_FLAGS_$(ARCH)) to RUST_FLAGS or
rustc_common_flags.

When feature_check runs for zlib-rs without --target in RUST_FLAGS (for
example, in tools/build/Makefile.feature when FEATURE_TESTS=all is evaluated
before a caller sets RUST_FLAGS, or in standalone feature checks),
cmd_rustc_rlib and cmd_rustc_staticlib in tools/build/Makefile.rust compile
liblibc.rlib, libzlib_rs.rlib, and libz_rs.a for the host architecture. Linking
test-zlib-rs.bin with the cross-compiler $(CC) then fails, and because the
rules in tools/build/Makefile.rust do not rebuild when RUST_FLAGS changes, the
host-architecture archives remain cached in $(LIBZ_RS_OUTPUT) for subsequent
invocations in the same build directory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029162958.3224280-1-irogers@google.com?part=1

  reply	other threads:[~2026-10-29 16:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 16:29 [RFC PATCH v1 0/3] tools: Add Rust library build infrastructure and prefer zlib-rs Ian Rogers
2026-10-29 16:29 ` [RFC PATCH v1 1/3] tools build: Add Rust library build infrastructure and zlib-rs feature test Ian Rogers
2026-10-29 16:57   ` sashiko-bot [this message]
2026-10-29 16:29 ` [RFC PATCH v1 2/3] perf build: Prefer zlib-rs over zlib when available Ian Rogers
2026-10-29 16:50   ` sashiko-bot
2026-10-29 16:29 ` [RFC PATCH v1 3/3] bpftool: " Ian Rogers
2026-10-29 17:06   ` sashiko-bot
2026-10-29 17:32 ` [RFC PATCH v1 0/3] tools: Add Rust library build infrastructure and prefer zlib-rs Miguel Ojeda
2026-10-29 18:23   ` Ian Rogers

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=sashiko-outbox-189694@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acme@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=gary@garyguo.net \
    --cc=irogers@google.com \
    --cc=justinstitt@google.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morbo@google.com \
    --cc=namhyung@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nsc@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qmo@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox