| Message ID | 20221231083028.1635698-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: rust: move rust/target.json to scripts/ | expand |
On Sat, Dec 31, 2022 at 9:30 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > scripts/ is a better place to generate files used treewide. Agreed, and its generator is in `scripts/` already anyway. > You do not need to add target.json to no-clean-files or MRPROER_FILES. Maybe adding something like "If moved to `scripts/`, then (...)" would make the sentence a bit more clear. (Also, typo: `MRPROPER`) > -$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs $(obj)/target.json FORCE > +$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs scripts/target.json FORCE Should this be `$(srctree)/scripts...` for clarity/consistency? (I see most instances like that elsewhere too) Cheers, Miguel
On Sat, Dec 31, 2022 at 11:25 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Dec 31, 2022 at 9:30 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > scripts/ is a better place to generate files used treewide. > > Agreed, and its generator is in `scripts/` already anyway. > > > You do not need to add target.json to no-clean-files or MRPROER_FILES. > > Maybe adding something like "If moved to `scripts/`, then (...)" would > make the sentence a bit more clear. OK, i will rephrase it in v2. > (Also, typo: `MRPROPER`) > > > -$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs $(obj)/target.json FORCE > > +$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs scripts/target.json FORCE > > Should this be `$(srctree)/scripts...` for clarity/consistency? (I see > most instances like that elsewhere too) No. scripts/target.json is a generated file. It is generated in objtree, not in srctree. > Cheers, > Miguel
On Sat, Dec 31, 2022 at 3:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > No. > scripts/target.json is a generated file. > It is generated in objtree, not in srctree. I meant `$(objtree)`, i.e. I meant if we should use a $(...)` prefix for clarity/consistency (even if it is just `.`). Happy New Year! Cheers, Miguel
On Sun, Jan 1, 2023 at 12:08 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Dec 31, 2022 at 3:57 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > No. > > scripts/target.json is a generated file. > > It is generated in objtree, not in srctree. > > I meant `$(objtree)`, i.e. I meant if we should use a $(...)` prefix > for clarity/consistency (even if it is just `.`). I usually do not add $(objtree)/. include/config/auto.conf is also a generated file. It is inconsistent to add $(objtree)/ to scripts/generate_rust_target, but not to include/config/auto.conf. (obj)/target.json: $(objtree)/scripts/generate_rust_target $(objtree)/include/config/auto.conf FORCE $(call filechk,rust_target) is annoying.
On Sat, Jan 7, 2023 at 10:43 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > I usually do not add $(objtree)/. > > include/config/auto.conf is also a generated file. > > It is inconsistent to add $(objtree)/ > to scripts/generate_rust_target, > but not to include/config/auto.conf. > > (obj)/target.json: $(objtree)/scripts/generate_rust_target > $(objtree)/include/config/auto.conf FORCE > $(call filechk,rust_target) > > is annoying. Being consistent sounds good to me, and I agree there are already a lot of `$`s around in `Makefile`s... :) In general, I tend to prefer explicit over implicit -- it would make non-prefixed paths less ambiguous on whether they are relative or anchored to the root. And I guess it could help reduce confusion, e.g. `arch/powerpc/boot/Makefile` mentions: # clean-files are relative to $(obj). Either way, it is fine. Thanks a lot for explaining the logic! I just sent a quick patch for Kbuild docs since I noticed it was outdated regarding `objtree` for `clean-files`. Cheers, Miguel
diff --git a/Makefile b/Makefile index 6b2a65f1aeaf..f9ec5bcacc60 100644 --- a/Makefile +++ b/Makefile @@ -569,7 +569,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \ -std=gnu11 KBUILD_CPPFLAGS := -D__KERNEL__ KBUILD_RUSTFLAGS := $(rust_common_flags) \ - --target=$(objtree)/rust/target.json \ + --target=$(objtree)/scripts/target.json \ -Cpanic=abort -Cembed-bitcode=n -Clto=n \ -Cforce-unwind-tables=n -Ccodegen-units=1 \ -Csymbol-mangling-version=v0 \ @@ -1593,7 +1593,7 @@ MRPROPER_FILES += include/config include/generated \ certs/x509.genkey \ vmlinux-gdb.py \ *.spec \ - rust/target.json rust/libmacros.so + rust/libmacros.so # clean - Delete most, but leave enough to build external modules # diff --git a/rust/.gitignore b/rust/.gitignore index 9bd1af8e05a1..168cb26a31b9 100644 --- a/rust/.gitignore +++ b/rust/.gitignore @@ -1,6 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -target.json bindings_generated.rs bindings_helpers_generated.rs exports_*_generated.h diff --git a/rust/Makefile b/rust/Makefile index c8941fec6955..9e33fa04b594 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -1,8 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -always-$(CONFIG_RUST) += target.json -no-clean-files += target.json - obj-$(CONFIG_RUST) += core.o compiler_builtins.o always-$(CONFIG_RUST) += exports_core_generated.h @@ -231,11 +228,6 @@ rusttest-kernel: $(src)/kernel/lib.rs rusttest-prepare \ $(call if_changed,rustc_test) $(call if_changed,rustc_test_library) -filechk_rust_target = $(objtree)/scripts/generate_rust_target < $< - -$(obj)/target.json: $(objtree)/include/config/auto.conf FORCE - $(call filechk,rust_target) - ifdef CONFIG_CC_IS_CLANG bindgen_c_flags = $(c_flags) else @@ -358,7 +350,7 @@ rust-analyzer: $(obj)/core.o: private skip_clippy = 1 $(obj)/core.o: private skip_flags = -Dunreachable_pub $(obj)/core.o: private rustc_target_flags = $(core-cfgs) -$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs $(obj)/target.json FORCE +$(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs scripts/target.json FORCE $(call if_changed_dep,rustc_library) $(obj)/compiler_builtins.o: private rustc_objcopy = -w -W '__*' diff --git a/scripts/.gitignore b/scripts/.gitignore index b7aec8eb1bd4..11bf3c075fb6 100644 --- a/scripts/.gitignore +++ b/scripts/.gitignore @@ -8,4 +8,5 @@ /recordmcount /sign-file /sorttable +/target.json /unifdef diff --git a/scripts/Makefile b/scripts/Makefile index 1575af84d557..0e0ae3c06ed7 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -10,8 +10,14 @@ hostprogs-always-$(CONFIG_BUILDTIME_TABLE_SORT) += sorttable hostprogs-always-$(CONFIG_ASN1) += asn1_compiler hostprogs-always-$(CONFIG_MODULE_SIG_FORMAT) += sign-file hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert -hostprogs-always-$(CONFIG_RUST) += generate_rust_target +always-$(CONFIG_RUST) += target.json +filechk_rust_target = $< < include/config/auto.conf + +$(obj)/target.json: scripts/generate_rust_target include/config/auto.conf FORCE + $(call filechk,rust_target) + +hostprogs += generate_rust_target generate_rust_target-rust := y HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include diff --git a/scripts/remove-stale-files b/scripts/remove-stale-files index cdbdde89a271..c71bf2f68360 100755 --- a/scripts/remove-stale-files +++ b/scripts/remove-stale-files @@ -27,3 +27,5 @@ rm -f arch/x86/purgatory/kexec-purgatory.c rm -f scripts/extract-cert rm -f scripts/kconfig/[gmnq]conf-cfg + +rm -f rust/target.json
scripts/ is a better place to generate files used treewide. You do not need to add target.json to no-clean-files or MRPROER_FILES. 'make clean' does not visit scripts/, but 'make mrproper' does. With target.json moved into scripts/, Kbuild will do the right thing. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Makefile | 4 ++-- rust/.gitignore | 1 - rust/Makefile | 10 +--------- scripts/.gitignore | 1 + scripts/Makefile | 8 +++++++- scripts/remove-stale-files | 2 ++ 6 files changed, 13 insertions(+), 13 deletions(-)