| Message ID | 20240808221138.873750-3-ojeda@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kbuild: rust: add `RUSTC_VERSION` and reconfig/rebuild support | expand |
On Fri, Aug 9, 2024 at 7:12 AM Miguel Ojeda <ojeda@kernel.org> wrote: > > `CC_VERSION_TEXT` is defined as: > > CC_VERSION_TEXT = $(subst $(pound),,$(shell LC_ALL=C $(CC) --version 2>/dev/null | head -n 1)) > > Make `RUSTC_VERSION_TEXT` closer to that, i.e. add `LC_ALL=C` and `| > head -n 1` in case it matters in the future, and for consistency. Even if "rustc --version" starts to print multiple lines, it will not cause an immediate problem. $(shell ... ) (both Makefile and Kconfig) replaces a new line with a space. CONFIG_RUSTC_VERSION_TEXT is not broken in any way. It would be just longer than we would expect. > > This reduces the difference in the next commit. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > --- > init/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/init/Kconfig b/init/Kconfig > index 47e2c3227b99..2f974f412374 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -1926,7 +1926,7 @@ config RUST > config RUSTC_VERSION_TEXT > string > depends on RUST > - default "$(shell,$(RUSTC) --version 2>/dev/null)" > + default "$(shell,LC_ALL=C $(RUSTC) --version 2>/dev/null | head -n 1)" > > config BINDGEN_VERSION_TEXT > string > -- > 2.46.0 >
On Fri, Aug 9, 2024 at 7:53 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Even if "rustc --version" starts to print multiple lines, > it will not cause an immediate problem. > > $(shell ... ) (both Makefile and Kconfig) replaces a new line > with a space. > > CONFIG_RUSTC_VERSION_TEXT is not broken in any way. > It would be just longer than we would expect. +1, I will update the commit message when I apply it (or if you prefer a v2 if you are taking it, or if you want to drop this commit, please let me know!). Thanks for taking a look, Masahiro. Cheers, Miguel
On Friday, August 9th, 2024 at 00:11, Miguel Ojeda <ojeda@kernel.org> wrote: > `CC_VERSION_TEXT` is defined as: > > CC_VERSION_TEXT = $(subst $(pound),,$(shell LC_ALL=C $(CC) --version 2>/dev/null | head -n 1)) > > Make `RUSTC_VERSION_TEXT` closer to that, i.e. add `LC_ALL=C` and `| > head -n 1` in case it matters in the future, and for consistency. Cargo depends on the rustc version string not getting localized. Or to be precise it depends on the version string being fixed for a given rustc version, which would not be the case if the value of LC_ALL could change the version string. If the version string changes, cargo will rebuild everything from scratch. There is also not really anything to localize in the non-verbose version string. I guess setting LC_ALL doesn't hurt either though. > > This reduces the difference in the next commit. > > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > --- > init/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/init/Kconfig b/init/Kconfig > index 47e2c3227b99..2f974f412374 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -1926,7 +1926,7 @@ config RUST > config RUSTC_VERSION_TEXT > string > depends on RUST > - default "$(shell,$(RUSTC) --version 2>/dev/null)" > + default "$(shell,LC_ALL=C $(RUSTC) --version 2>/dev/null | head -n 1)" > > config BINDGEN_VERSION_TEXT > string > -- > 2.46.0
On Sat, Aug 10, 2024 at 3:44 PM Björn Roy Baron <bjorn3_gh@protonmail.com> wrote: > > Cargo depends on the rustc version string not getting localized. Or to be precise it depends on the version string being fixed for a given rustc version, which would not be the case if the value of LC_ALL could change the version string. If the version string changes, cargo will rebuild everything from scratch. There is also not really anything to localize in the non-verbose version string. I guess setting LC_ALL doesn't hurt either though. Thanks Björn -- yeah, I agree it works either way (also for `head` as Masahiro mentioned). I made this commit to make it as close as possible to the C one, since in the next patch it gets moved to another place where both commands go together. It is reasonable to argue that it makes it more complex and slower, so I am happy dropping it. Cheers, Miguel
diff --git a/init/Kconfig b/init/Kconfig index 47e2c3227b99..2f974f412374 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1926,7 +1926,7 @@ config RUST config RUSTC_VERSION_TEXT string depends on RUST - default "$(shell,$(RUSTC) --version 2>/dev/null)" + default "$(shell,LC_ALL=C $(RUSTC) --version 2>/dev/null | head -n 1)" config BINDGEN_VERSION_TEXT string
`CC_VERSION_TEXT` is defined as: CC_VERSION_TEXT = $(subst $(pound),,$(shell LC_ALL=C $(CC) --version 2>/dev/null | head -n 1)) Make `RUSTC_VERSION_TEXT` closer to that, i.e. add `LC_ALL=C` and `| head -n 1` in case it matters in the future, and for consistency. This reduces the difference in the next commit. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> --- init/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)