| Message ID | 20221231064203.1623793-2-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: fix dep-file processing for rust | expand |
On Sat, 31 Dec 2022 15:41:58 +0900 Masahiro Yamada <masahiroy@kernel.org> wrote: > diff --git a/scripts/Makefile.host b/scripts/Makefile.host > index da133780b751..4434cdbf7b8e 100644 > --- a/scripts/Makefile.host > +++ b/scripts/Makefile.host > @@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \ > $(HOSTCFLAGS_$(target-stem).o) > _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \ > $(HOSTCXXFLAGS_$(target-stem).o) > -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ > - $(HOSTRUSTFLAGS_$(target-stem)) > +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ > + $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile) > > # $(objtree)/$(obj) for including generated headers from checkin source files > ifeq ($(KBUILD_EXTMOD),) > @@ -97,7 +97,6 @@ endif > > hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) > hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) > -hostrust_flags = $(_hostrust_flags) Would it be better to have `--emit=dep-info=$(depfile)` added here instead so that it mimics c/cxx flags? > > ##### > # Compile programs on the host > @@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE > # host-rust -> Executable > quiet_cmd_host-rust = HOSTRUSTC $@ > cmd_host-rust = \ > - $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \ > - --out-dir=$(obj)/ $<; \ > - mv $(obj)/$(target-stem).d $(depfile); \ > + $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \ > sed -i '/^\#/d' $(depfile) > $(host-rust): $(obj)/%: $(src)/%.rs FORCE > $(call if_changed_dep,host-rust) Best, Gary
On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \ > samples/rust/rust_minimal.s samples/rust/rust_minimal.ll Yeah, we were testing the single targets, but not multiple at once, thanks! > + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ Perhaps a newline here to avoid the lengthy line? > hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) > hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) > -hostrust_flags = $(_hostrust_flags) This was originally meant to be consistent with C and C++ indeed, but if you prefer less variables, I guess it is fine, in which case, should we update the C/C++ side too (in another series)? Reviewed-by: Miguel Ojeda <ojeda@kernel.org> Tested-by: Miguel Ojeda <ojeda@kernel.org> Cheers, Miguel
Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> On Sat Dec 31, 2022 at 7:41 AM CET, Masahiro Yamada wrote: > In Kbuild, two different rules must not write to the same file, but > it happens when compiling rust source files. > > For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following: > > $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \ > samples/rust/rust_minimal.s samples/rust/rust_minimal.ll > [snip] > RUSTC [M] samples/rust/rust_minimal.o > RUSTC [M] samples/rust/rust_minimal.rsi > RUSTC [M] samples/rust/rust_minimal.s > RUSTC [M] samples/rust/rust_minimal.ll > mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory > make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1 > make[3]: *** Waiting for unfinished jobs.... > mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory > make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1 > mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory > make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1 > make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2 > make[1]: *** [scripts/Makefile.build:504: samples] Error 2 > make: *** [Makefile:2008: .] Error 2 > > The reason for the error is that 4 threads running in parallel creates > and renames the same file path, samples/rust/rust_minimal.d. > > This does not happen when compiling C or assembly files because we > explicitly specify the dependency filename by using the preprocessor > option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target. > > Currently, rustc is only given --out-dir and the list of emitted types. > So, all the rust build rules output the dep-info into the default > <CRATE_NAME>.d, causing the conflict. > > Fortunately, the --emit option is able to specify the output path > individually, with the form --emit=<type>=<path>. > > Add --emit=dep-info=$(depfile) to the common command part. Also, remove > the redundant --out-dir because we specify the output path for each type. > > The code gets much cleaner because we do not need to rename *.d files. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > rust/Makefile | 10 ++++------ > scripts/Makefile.build | 14 +++++++------- > scripts/Makefile.host | 9 +++------ > 3 files changed, 14 insertions(+), 19 deletions(-) > > diff --git a/rust/Makefile b/rust/Makefile > index ff70c4c916f8..0e2a32f4b3e9 100644 > --- a/rust/Makefile > +++ b/rust/Makefile > @@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE > quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@ > cmd_rustc_procmacro = \ > $(RUSTC_OR_CLIPPY) $(rust_common_flags) \ > - --emit=dep-info,link --extern proc_macro \ > - --crate-type proc-macro --out-dir $(objtree)/$(obj) \ > + --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \ > + --crate-type proc-macro \ > --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \ > - mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \ > sed -i '/^\#/d' $(depfile) > > # Procedural macros can only be used with the `rustc` that compiled it. > @@ -348,10 +347,9 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L > OBJTREE=$(abspath $(objtree)) \ > $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \ > $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \ > - --emit=dep-info,obj,metadata --crate-type rlib \ > - --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \ > + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ > + --crate-type rlib -L$(objtree)/$(obj) \ > --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \ > - mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \ > sed -i '/^\#/d' $(depfile) \ > $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build > index a0d5c6cca76d..40de20246e50 100644 > --- a/scripts/Makefile.build > +++ b/scripts/Makefile.build > @@ -285,11 +285,11 @@ rust_common_cmd = \ > -Zcrate-attr=no_std \ > -Zcrate-attr='feature($(rust_allowed_features))' \ > --extern alloc --extern kernel \ > - --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \ > - --crate-name $(basename $(notdir $@)) > + --crate-type rlib -L $(objtree)/rust/ \ > + --crate-name $(basename $(notdir $@)) \ > + --emit=dep-info=$(depfile) > > rust_handle_depfile = \ > - mv $(obj)/$(basename $(notdir $@)).d $(depfile); \ > sed -i '/^\#/d' $(depfile) > > # `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit > @@ -302,7 +302,7 @@ rust_handle_depfile = \ > > quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ > cmd_rustc_o_rs = \ > - $(rust_common_cmd) --emit=dep-info,obj $<; \ > + $(rust_common_cmd) --emit=obj=$@ $<; \ > $(rust_handle_depfile) > > $(obj)/%.o: $(src)/%.rs FORCE > @@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE > > quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ > cmd_rustc_rsi_rs = \ > - $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \ > + $(rust_common_cmd) -Zunpretty=expanded $< >$@; \ > command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \ > $(rust_handle_depfile) > > @@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE > > quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ > cmd_rustc_s_rs = \ > - $(rust_common_cmd) --emit=dep-info,asm $<; \ > + $(rust_common_cmd) --emit=asm=$@ $<; \ > $(rust_handle_depfile) > > $(obj)/%.s: $(src)/%.rs FORCE > @@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE > > quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ > cmd_rustc_ll_rs = \ > - $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \ > + $(rust_common_cmd) --emit=llvm-ir=$@ $<; \ > $(rust_handle_depfile) > > $(obj)/%.ll: $(src)/%.rs FORCE > diff --git a/scripts/Makefile.host b/scripts/Makefile.host > index da133780b751..4434cdbf7b8e 100644 > --- a/scripts/Makefile.host > +++ b/scripts/Makefile.host > @@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \ > $(HOSTCFLAGS_$(target-stem).o) > _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \ > $(HOSTCXXFLAGS_$(target-stem).o) > -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ > - $(HOSTRUSTFLAGS_$(target-stem)) > +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ > + $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile) > > # $(objtree)/$(obj) for including generated headers from checkin source files > ifeq ($(KBUILD_EXTMOD),) > @@ -97,7 +97,6 @@ endif > > hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) > hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) > -hostrust_flags = $(_hostrust_flags) > > ##### > # Compile programs on the host > @@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE > # host-rust -> Executable > quiet_cmd_host-rust = HOSTRUSTC $@ > cmd_host-rust = \ > - $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \ > - --out-dir=$(obj)/ $<; \ > - mv $(obj)/$(target-stem).d $(depfile); \ > + $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \ > sed -i '/^\#/d' $(depfile) > $(host-rust): $(obj)/%: $(src)/%.rs FORCE > $(call if_changed_dep,host-rust) > -- > 2.34.1
On Wed, Jan 4, 2023 at 5:45 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Dec 31, 2022 at 7:42 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \ > > samples/rust/rust_minimal.s samples/rust/rust_minimal.ll > > Yeah, we were testing the single targets, but not multiple at once, thanks! > > > + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ > > Perhaps a newline here to avoid the lengthy line? OK, I will wrap it in v2. > > > hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) > > hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) > > -hostrust_flags = $(_hostrust_flags) > > This was originally meant to be consistent with C and C++ indeed, but > if you prefer less variables, I guess it is fine, in which case, > should we update the C/C++ side too (in another series)? Yup, we could do this with less variables. I will send a clean up. > Reviewed-by: Miguel Ojeda <ojeda@kernel.org> > Tested-by: Miguel Ojeda <ojeda@kernel.org> > > Cheers, > Miguel
diff --git a/rust/Makefile b/rust/Makefile index ff70c4c916f8..0e2a32f4b3e9 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -331,10 +331,9 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@ cmd_rustc_procmacro = \ $(RUSTC_OR_CLIPPY) $(rust_common_flags) \ - --emit=dep-info,link --extern proc_macro \ - --crate-type proc-macro --out-dir $(objtree)/$(obj) \ + --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \ + --crate-type proc-macro \ --crate-name $(patsubst lib%.so,%,$(notdir $@)) $<; \ - mv $(objtree)/$(obj)/$(patsubst lib%.so,%,$(notdir $@)).d $(depfile); \ sed -i '/^\#/d' $(depfile) # Procedural macros can only be used with the `rustc` that compiled it. @@ -348,10 +347,9 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L OBJTREE=$(abspath $(objtree)) \ $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \ $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \ - --emit=dep-info,obj,metadata --crate-type rlib \ - --out-dir $(objtree)/$(obj) -L$(objtree)/$(obj) \ + --emit=dep-info=$(depfile) --emit=obj=$@ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ + --crate-type rlib -L$(objtree)/$(obj) \ --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \ - mv $(objtree)/$(obj)/$(patsubst %.o,%,$(notdir $@)).d $(depfile); \ sed -i '/^\#/d' $(depfile) \ $(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index a0d5c6cca76d..40de20246e50 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -285,11 +285,11 @@ rust_common_cmd = \ -Zcrate-attr=no_std \ -Zcrate-attr='feature($(rust_allowed_features))' \ --extern alloc --extern kernel \ - --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \ - --crate-name $(basename $(notdir $@)) + --crate-type rlib -L $(objtree)/rust/ \ + --crate-name $(basename $(notdir $@)) \ + --emit=dep-info=$(depfile) rust_handle_depfile = \ - mv $(obj)/$(basename $(notdir $@)).d $(depfile); \ sed -i '/^\#/d' $(depfile) # `--emit=obj`, `--emit=asm` and `--emit=llvm-ir` imply a single codegen unit @@ -302,7 +302,7 @@ rust_handle_depfile = \ quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ cmd_rustc_o_rs = \ - $(rust_common_cmd) --emit=dep-info,obj $<; \ + $(rust_common_cmd) --emit=obj=$@ $<; \ $(rust_handle_depfile) $(obj)/%.o: $(src)/%.rs FORCE @@ -310,7 +310,7 @@ $(obj)/%.o: $(src)/%.rs FORCE quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ cmd_rustc_rsi_rs = \ - $(rust_common_cmd) --emit=dep-info -Zunpretty=expanded $< >$@; \ + $(rust_common_cmd) -Zunpretty=expanded $< >$@; \ command -v $(RUSTFMT) >/dev/null && $(RUSTFMT) $@; \ $(rust_handle_depfile) @@ -319,7 +319,7 @@ $(obj)/%.rsi: $(src)/%.rs FORCE quiet_cmd_rustc_s_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ cmd_rustc_s_rs = \ - $(rust_common_cmd) --emit=dep-info,asm $<; \ + $(rust_common_cmd) --emit=asm=$@ $<; \ $(rust_handle_depfile) $(obj)/%.s: $(src)/%.rs FORCE @@ -327,7 +327,7 @@ $(obj)/%.s: $(src)/%.rs FORCE quiet_cmd_rustc_ll_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@ cmd_rustc_ll_rs = \ - $(rust_common_cmd) --emit=dep-info,llvm-ir $<; \ + $(rust_common_cmd) --emit=llvm-ir=$@ $<; \ $(rust_handle_depfile) $(obj)/%.ll: $(src)/%.rs FORCE diff --git a/scripts/Makefile.host b/scripts/Makefile.host index da133780b751..4434cdbf7b8e 100644 --- a/scripts/Makefile.host +++ b/scripts/Makefile.host @@ -84,8 +84,8 @@ _hostc_flags = $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS) \ $(HOSTCFLAGS_$(target-stem).o) _hostcxx_flags = $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \ $(HOSTCXXFLAGS_$(target-stem).o) -_hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ - $(HOSTRUSTFLAGS_$(target-stem)) +hostrust_flags = $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \ + $(HOSTRUSTFLAGS_$(target-stem)) --emit=dep-info=$(depfile) # $(objtree)/$(obj) for including generated headers from checkin source files ifeq ($(KBUILD_EXTMOD),) @@ -97,7 +97,6 @@ endif hostc_flags = -Wp,-MMD,$(depfile) $(_hostc_flags) hostcxx_flags = -Wp,-MMD,$(depfile) $(_hostcxx_flags) -hostrust_flags = $(_hostrust_flags) ##### # Compile programs on the host @@ -149,9 +148,7 @@ $(host-cxxobjs): $(obj)/%.o: $(src)/%.cc FORCE # host-rust -> Executable quiet_cmd_host-rust = HOSTRUSTC $@ cmd_host-rust = \ - $(HOSTRUSTC) $(hostrust_flags) --emit=dep-info,link \ - --out-dir=$(obj)/ $<; \ - mv $(obj)/$(target-stem).d $(depfile); \ + $(HOSTRUSTC) $(hostrust_flags) --emit=link=$@ $<; \ sed -i '/^\#/d' $(depfile) $(host-rust): $(obj)/%: $(src)/%.rs FORCE $(call if_changed_dep,host-rust)
In Kbuild, two different rules must not write to the same file, but it happens when compiling rust source files. For example, set CONFIG_SAMPLE_RUST_MINIMAL=m and run the following: $ make -j$(nproc) samples/rust/rust_minimal.o samples/rust/rust_minimal.rsi \ samples/rust/rust_minimal.s samples/rust/rust_minimal.ll [snip] RUSTC [M] samples/rust/rust_minimal.o RUSTC [M] samples/rust/rust_minimal.rsi RUSTC [M] samples/rust/rust_minimal.s RUSTC [M] samples/rust/rust_minimal.ll mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory make[3]: *** [scripts/Makefile.build:334: samples/rust/rust_minimal.ll] Error 1 make[3]: *** Waiting for unfinished jobs.... mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory make[3]: *** [scripts/Makefile.build:309: samples/rust/rust_minimal.o] Error 1 mv: cannot stat 'samples/rust/rust_minimal.d': No such file or directory make[3]: *** [scripts/Makefile.build:326: samples/rust/rust_minimal.s] Error 1 make[2]: *** [scripts/Makefile.build:504: samples/rust] Error 2 make[1]: *** [scripts/Makefile.build:504: samples] Error 2 make: *** [Makefile:2008: .] Error 2 The reason for the error is that 4 threads running in parallel creates and renames the same file path, samples/rust/rust_minimal.d. This does not happen when compiling C or assembly files because we explicitly specify the dependency filename by using the preprocessor option, -Wp,-MMD,$(depfile). $(depfile) is a unique path for each target. Currently, rustc is only given --out-dir and the list of emitted types. So, all the rust build rules output the dep-info into the default <CRATE_NAME>.d, causing the conflict. Fortunately, the --emit option is able to specify the output path individually, with the form --emit=<type>=<path>. Add --emit=dep-info=$(depfile) to the common command part. Also, remove the redundant --out-dir because we specify the output path for each type. The code gets much cleaner because we do not need to rename *.d files. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- rust/Makefile | 10 ++++------ scripts/Makefile.build | 14 +++++++------- scripts/Makefile.host | 9 +++------ 3 files changed, 14 insertions(+), 19 deletions(-)