diff mbox series

[1/2] kbuild: mark `rustc` (and others) invocations as recursive

Message ID 20240217002638.57373-1-ojeda@kernel.org
State New
Headers
Series [1/2] kbuild: mark `rustc` (and others) invocations as recursive |

Commit Message

Miguel Ojeda Feb. 17, 2024, 12:26 a.m. UTC 
  `rustc` (like Cargo) may take advantage of the jobserver at any time
(e.g. for backend parallelism, or eventually frontend too). In the kernel,
we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
warning is emitted by `rustc` when it cannot connect to the jobserver it
was passed (in many cases, but not all: compiling and `--print sysroot`
do, but `--version` does not). And given GNU Make always passes
the jobserver in the environment variable (even when a line is deemed
non-recursive), `rustc` will end up complaining about it (in particular
in Make 4.3 where there is only the simple pipe jobserver style).

One solution is to remove the jobserver from `MAKEFLAGS`. However, we
can mark the lines with calls to `rustc` (and Cargo) as recursive, which
looks simpler. This is what is now documented and recommended by `rustc`
and allows us to be ready for the time we may use parallelism inside
`rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.

Similarly, do the same for `rustdoc` and `cargo` calls.

Finally, there is one case that the solution does not cover, which is the
`$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
environment variable.

Link: https://github.com/rust-lang/rust/issues/120515
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Makefile               |  4 ++--
 rust/Makefile          | 48 +++++++++++++++++++++---------------------
 scripts/Makefile.build |  8 +++----
 scripts/Makefile.host  |  2 +-
 4 files changed, 31 insertions(+), 31 deletions(-)


base-commit: f090f0d0eea9666a96702b29bc9a64cbabee85c5
--
2.43.0

Comments

Masahiro Yamada Feb. 19, 2024, 9:52 a.m. UTC | #1 
On Sat, Feb 17, 2024 at 9:27 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> `rustc` (like Cargo) may take advantage of the jobserver at any time
> (e.g. for backend parallelism, or eventually frontend too). In the kernel,
> we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
> so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
> warning is emitted by `rustc` when it cannot connect to the jobserver it
> was passed (in many cases, but not all: compiling and `--print sysroot`
> do, but `--version` does not). And given GNU Make always passes
> the jobserver in the environment variable (even when a line is deemed
> non-recursive), `rustc` will end up complaining about it (in particular
> in Make 4.3 where there is only the simple pipe jobserver style).
>
> One solution is to remove the jobserver from `MAKEFLAGS`. However, we
> can mark the lines with calls to `rustc` (and Cargo) as recursive, which
> looks simpler. This is what is now documented and recommended by `rustc`
> and allows us to be ready for the time we may use parallelism inside
> `rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.
>
> Similarly, do the same for `rustdoc` and `cargo` calls.
>
> Finally, there is one case that the solution does not cover, which is the
> `$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
> environment variable.
>
> Link: https://github.com/rust-lang/rust/issues/120515
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>



Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Miguel Ojeda Feb. 29, 2024, 9:33 p.m. UTC | #2 
On Sat, Feb 17, 2024 at 1:26 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> `rustc` (like Cargo) may take advantage of the jobserver at any time
> (e.g. for backend parallelism, or eventually frontend too). In the kernel,
> we call `rustc` with `-Ccodegen-units=1` (and `-Zthreads` is 1 so far),
> so we do not expect parallelism. However, in the upcoming Rust 1.76.0, a
> warning is emitted by `rustc` when it cannot connect to the jobserver it
> was passed (in many cases, but not all: compiling and `--print sysroot`
> do, but `--version` does not). And given GNU Make always passes
> the jobserver in the environment variable (even when a line is deemed
> non-recursive), `rustc` will end up complaining about it (in particular
> in Make 4.3 where there is only the simple pipe jobserver style).
>
> One solution is to remove the jobserver from `MAKEFLAGS`. However, we
> can mark the lines with calls to `rustc` (and Cargo) as recursive, which
> looks simpler. This is what is now documented and recommended by `rustc`
> and allows us to be ready for the time we may use parallelism inside
> `rustc` (potentially now, if a user passes `-Zthreads`). Thus do so.
>
> Similarly, do the same for `rustdoc` and `cargo` calls.
>
> Finally, there is one case that the solution does not cover, which is the
> `$(shell ...)` call we have. Thus, for that one, set an empty `MAKEFLAGS`
> environment variable.
>
> Link: https://github.com/rust-lang/rust/issues/120515
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Applied (i.e. including the upgrade to 1.76.0) to `rust-next` --
thanks everyone!

[ Reworded to add link to PR documenting the recommendation. ]

Cheers,
Miguel
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 9869f57c3fb3..cbcdd8d9d0e3 100644
--- a/Makefile
+++ b/Makefile
@@ -1197,7 +1197,7 @@  prepare0: archprepare
 # All the preparing..
 prepare: prepare0
 ifdef CONFIG_RUST
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
+	+$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh
 	$(Q)$(MAKE) $(build)=rust
 endif

@@ -1707,7 +1707,7 @@  $(DOC_TARGETS):
 # "Is Rust available?" target
 PHONY += rustavailable
 rustavailable:
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh && echo "Rust is available!"
+	+$(Q)$(CONFIG_SHELL) $(srctree)/scripts/rust_is_available.sh && echo "Rust is available!"

 # Documentation target
 #
diff --git a/rust/Makefile b/rust/Makefile
index 9d2a16cc91cb..a78fcf4004b0 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -40,7 +40,7 @@  obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
 ifdef CONFIG_RUST

 # `$(rust_flags)` is passed in case the user added `--sysroot`.
-rustc_sysroot := $(shell $(RUSTC) $(rust_flags) --print sysroot)
+rustc_sysroot := $(shell MAKEFLAGS= $(RUSTC) $(rust_flags) --print sysroot)
 rustc_host_target := $(shell $(RUSTC) --version --verbose | grep -F 'host: ' | cut -d' ' -f2)
 RUST_LIB_SRC ?= $(rustc_sysroot)/lib/rustlib/src/rust/library

@@ -108,14 +108,14 @@  rustdoc-macros: private rustdoc_host = yes
 rustdoc-macros: private rustc_target_flags = --crate-type proc-macro \
     --extern proc_macro
 rustdoc-macros: $(src)/macros/lib.rs FORCE
-	$(call if_changed,rustdoc)
+	+$(call if_changed,rustdoc)

 rustdoc-core: private rustc_target_flags = $(core-cfgs)
 rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE
-	$(call if_changed,rustdoc)
+	+$(call if_changed,rustdoc)

 rustdoc-compiler_builtins: $(src)/compiler_builtins.rs rustdoc-core FORCE
-	$(call if_changed,rustdoc)
+	+$(call if_changed,rustdoc)

 # We need to allow `rustdoc::broken_intra_doc_links` because some
 # `no_global_oom_handling` functions refer to non-`no_global_oom_handling`
@@ -124,7 +124,7 @@  rustdoc-compiler_builtins: $(src)/compiler_builtins.rs rustdoc-core FORCE
 rustdoc-alloc: private rustc_target_flags = $(alloc-cfgs) \
     -Arustdoc::broken_intra_doc_links
 rustdoc-alloc: $(src)/alloc/lib.rs rustdoc-core rustdoc-compiler_builtins FORCE
-	$(call if_changed,rustdoc)
+	+$(call if_changed,rustdoc)

 rustdoc-kernel: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros=$(objtree)/$(obj)/libmacros.so \
@@ -132,7 +132,7 @@  rustdoc-kernel: private rustc_target_flags = --extern alloc \
 rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
     rustdoc-compiler_builtins rustdoc-alloc $(obj)/libmacros.so \
     $(obj)/bindings.o FORCE
-	$(call if_changed,rustdoc)
+	+$(call if_changed,rustdoc)

 quiet_cmd_rustc_test_library = RUSTC TL $<
       cmd_rustc_test_library = \
@@ -146,18 +146,18 @@  quiet_cmd_rustc_test_library = RUSTC TL $<
 		--crate-name $(subst rusttest-,,$(subst rusttestlib-,,$@)) $<

 rusttestlib-build_error: $(src)/build_error.rs rusttest-prepare FORCE
-	$(call if_changed,rustc_test_library)
+	+$(call if_changed,rustc_test_library)

 rusttestlib-macros: private rustc_target_flags = --extern proc_macro
 rusttestlib-macros: private rustc_test_library_proc = yes
 rusttestlib-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
-	$(call if_changed,rustc_test_library)
+	+$(call if_changed,rustc_test_library)

 rusttestlib-bindings: $(src)/bindings/lib.rs rusttest-prepare FORCE
-	$(call if_changed,rustc_test_library)
+	+$(call if_changed,rustc_test_library)

 rusttestlib-uapi: $(src)/uapi/lib.rs rusttest-prepare FORCE
-	$(call if_changed,rustc_test_library)
+	+$(call if_changed,rustc_test_library)

 quiet_cmd_rustdoc_test = RUSTDOC T $<
       cmd_rustdoc_test = \
@@ -189,7 +189,7 @@  quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $<
     $(src)/kernel/lib.rs $(obj)/kernel.o \
     $(objtree)/scripts/rustdoc_test_builder \
     $(objtree)/scripts/rustdoc_test_gen FORCE
-	$(call if_changed,rustdoc_test_kernel)
+	+$(call if_changed,rustdoc_test_kernel)

 # We cannot use `-Zpanic-abort-tests` because some tests are dynamic,
 # so for the moment we skip `-Cpanic=abort`.
@@ -254,21 +254,21 @@  quiet_cmd_rustsysroot = RUSTSYSROOT
 		$(objtree)/$(obj)/test/sysroot/lib/rustlib/$(rustc_host_target)/lib

 rusttest-prepare: FORCE
-	$(call if_changed,rustsysroot)
+	+$(call if_changed,rustsysroot)

 rusttest-macros: private rustc_target_flags = --extern proc_macro
 rusttest-macros: private rustdoc_test_target_flags = --crate-type proc-macro
 rusttest-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
-	$(call if_changed,rustc_test)
-	$(call if_changed,rustdoc_test)
+	+$(call if_changed,rustc_test)
+	+$(call if_changed,rustdoc_test)

 rusttest-kernel: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings --extern uapi
 rusttest-kernel: $(src)/kernel/lib.rs rusttest-prepare \
     rusttestlib-build_error rusttestlib-macros rusttestlib-bindings \
     rusttestlib-uapi FORCE
-	$(call if_changed,rustc_test)
-	$(call if_changed,rustc_test_library)
+	+$(call if_changed,rustc_test)
+	+$(call if_changed,rustc_test_library)

 ifdef CONFIG_CC_IS_CLANG
 bindgen_c_flags = $(c_flags)
@@ -396,7 +396,7 @@  quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@
 # Therefore, to get `libmacros.so` automatically recompiled when the compiler
 # version changes, we add `core.o` as a dependency (even if it is not needed).
 $(obj)/libmacros.so: $(src)/macros/lib.rs $(obj)/core.o FORCE
-	$(call if_changed_dep,rustc_procmacro)
+	+$(call if_changed_dep,rustc_procmacro)

 quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L $@
       cmd_rustc_library = \
@@ -435,36 +435,36 @@  $(obj)/core.o: private skip_flags = -Dunreachable_pub
 $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym))
 $(obj)/core.o: private rustc_target_flags = $(core-cfgs)
 $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs scripts/target.json FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/compiler_builtins.o: private rustc_objcopy = -w -W '__*'
 $(obj)/compiler_builtins.o: $(src)/compiler_builtins.rs $(obj)/core.o FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/alloc.o: private skip_clippy = 1
 $(obj)/alloc.o: private skip_flags = -Dunreachable_pub
 $(obj)/alloc.o: private rustc_target_flags = $(alloc-cfgs)
 $(obj)/alloc.o: $(src)/alloc/lib.rs $(obj)/compiler_builtins.o FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/compiler_builtins.o \
     $(obj)/bindings/bindings_generated.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/uapi.o: $(src)/uapi/lib.rs \
     $(obj)/compiler_builtins.o \
     $(obj)/uapi/uapi_generated.rs FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 $(obj)/kernel.o: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings --extern uapi
 $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
     $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
-	$(call if_changed_dep,rustc_library)
+	+$(call if_changed_dep,rustc_library)

 endif # CONFIG_RUST
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index dae447a1ad30..0fb7a785594c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -290,7 +290,7 @@  quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
       cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<

 $(obj)/%.o: $(src)/%.rs FORCE
-	$(call if_changed_dep,rustc_o_rs)
+	+$(call if_changed_dep,rustc_o_rs)

 quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
       cmd_rustc_rsi_rs = \
@@ -298,19 +298,19 @@  quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
 	command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@

 $(obj)/%.rsi: $(src)/%.rs FORCE
-	$(call if_changed_dep,rustc_rsi_rs)
+	+$(call if_changed_dep,rustc_rsi_rs)

 quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
       cmd_rustc_s_rs = $(rust_common_cmd) --emit=asm=$@ $<

 $(obj)/%.s: $(src)/%.rs FORCE
-	$(call if_changed_dep,rustc_s_rs)
+	+$(call if_changed_dep,rustc_s_rs)

 quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
       cmd_rustc_ll_rs = $(rust_common_cmd) --emit=llvm-ir=$@ $<

 $(obj)/%.ll: $(src)/%.rs FORCE
-	$(call if_changed_dep,rustc_ll_rs)
+	+$(call if_changed_dep,rustc_ll_rs)

 # Compile assembler sources (.S)
 # ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 08d83d9db31a..3c17e6ba421c 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -156,7 +156,7 @@  quiet_cmd_host-rust	= HOSTRUSTC $@
       cmd_host-rust	= \
 	$(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<
 $(host-rust): $(obj)/%: $(src)/%.rs FORCE
-	$(call if_changed_dep,host-rust)
+	+$(call if_changed_dep,host-rust)

 targets += $(host-csingle) $(host-cmulti) $(host-cobjs) \
 	   $(host-cxxmulti) $(host-cxxobjs) $(host-rust)