linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kbuild: rust: move rust/target.json to scripts/
@ 2022-12-31  8:30 Masahiro Yamada
  2022-12-31 14:25 ` Miguel Ojeda
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2022-12-31  8:30 UTC (permalink / raw)
  To: linux-kbuild, Miguel Ojeda, rust-for-linux
  Cc: linux-kernel, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
	Gary Guo, Björn Roy Baron, Masahiro Yamada, Borislav Petkov,
	David Gow, Helge Deller, Kees Cook, Nathan Chancellor,
	Nick Desaulniers, Nicolas Schier, Palmer Dabbelt,
	Segher Boessenkool

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(-)

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
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kbuild: rust: move rust/target.json to scripts/
  2022-12-31  8:30 [PATCH] kbuild: rust: move rust/target.json to scripts/ Masahiro Yamada
@ 2022-12-31 14:25 ` Miguel Ojeda
  2022-12-31 14:56   ` Masahiro Yamada
  0 siblings, 1 reply; 6+ messages in thread
From: Miguel Ojeda @ 2022-12-31 14:25 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Miguel Ojeda, rust-for-linux, linux-kernel,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Borislav Petkov, David Gow, Helge Deller,
	Kees Cook, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Palmer Dabbelt, Segher Boessenkool

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kbuild: rust: move rust/target.json to scripts/
  2022-12-31 14:25 ` Miguel Ojeda
@ 2022-12-31 14:56   ` Masahiro Yamada
  2022-12-31 15:07     ` Miguel Ojeda
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2022-12-31 14:56 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: linux-kbuild, Miguel Ojeda, rust-for-linux, linux-kernel,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Borislav Petkov, David Gow, Helge Deller,
	Kees Cook, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Palmer Dabbelt, Segher Boessenkool

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



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kbuild: rust: move rust/target.json to scripts/
  2022-12-31 14:56   ` Masahiro Yamada
@ 2022-12-31 15:07     ` Miguel Ojeda
  2023-01-07  9:43       ` Masahiro Yamada
  0 siblings, 1 reply; 6+ messages in thread
From: Miguel Ojeda @ 2022-12-31 15:07 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Miguel Ojeda, rust-for-linux, linux-kernel,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Borislav Petkov, David Gow, Helge Deller,
	Kees Cook, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Palmer Dabbelt, Segher Boessenkool

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kbuild: rust: move rust/target.json to scripts/
  2022-12-31 15:07     ` Miguel Ojeda
@ 2023-01-07  9:43       ` Masahiro Yamada
  2023-01-07 14:45         ` Miguel Ojeda
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2023-01-07  9:43 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: linux-kbuild, Miguel Ojeda, rust-for-linux, linux-kernel,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Borislav Petkov, David Gow, Helge Deller,
	Kees Cook, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Palmer Dabbelt, Segher Boessenkool

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.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kbuild: rust: move rust/target.json to scripts/
  2023-01-07  9:43       ` Masahiro Yamada
@ 2023-01-07 14:45         ` Miguel Ojeda
  0 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2023-01-07 14:45 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Miguel Ojeda, rust-for-linux, linux-kernel,
	Alex Gaynor, Wedson Almeida Filho, Boqun Feng, Gary Guo,
	Björn Roy Baron, Borislav Petkov, David Gow, Helge Deller,
	Kees Cook, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Palmer Dabbelt, Segher Boessenkool

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-07 14:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-31  8:30 [PATCH] kbuild: rust: move rust/target.json to scripts/ Masahiro Yamada
2022-12-31 14:25 ` Miguel Ojeda
2022-12-31 14:56   ` Masahiro Yamada
2022-12-31 15:07     ` Miguel Ojeda
2023-01-07  9:43       ` Masahiro Yamada
2023-01-07 14:45         ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).