kbuild: rust: move rust/target.json to scripts/
Commit Message
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(-)
Comments
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
@@ -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
#
@@ -1,6 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-target.json
bindings_generated.rs
bindings_helpers_generated.rs
exports_*_generated.h
@@ -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 '__*'
@@ -8,4 +8,5 @@
/recordmcount
/sign-file
/sorttable
+/target.json
/unifdef
@@ -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
@@ -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