Following commit e88ca24319e4 ("kbuild: consolidate warning flags
in scripts/Makefile.extrawarn"), move `-Dwarnings` handling into
`Makefile.extrawarn` like C's `-Werror`.
No functional change intended.
Signed-off-by: Miguel Ojeda <[email protected]>
---
Makefile | 3 ---
scripts/Makefile.extrawarn | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 763b6792d3d5..fba567a55607 100644
--- a/Makefile
+++ b/Makefile
@@ -842,9 +842,6 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong
KBUILD_CFLAGS += $(stackp-flags-y)
-KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
-KBUILD_RUSTFLAGS += $(KBUILD_RUSTFLAGS-y)
-
ifdef CONFIG_FRAME_POINTER
KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
KBUILD_RUSTFLAGS += -Cforce-frame-pointers=y
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 3ce5d503a6da..48114e91c386 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -26,6 +26,9 @@ endif
KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
+KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
+KBUILD_RUSTFLAGS += $(KBUILD_RUSTFLAGS-y)
+
KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
ifdef CONFIG_CC_IS_CLANG
--
2.45.1
Make all Rust code (i.e. not just kernel code) respect `CONFIG_WERROR`,
so that we keep everything warning clean.
In particular, this affects targets in `rust/` (`RUSTDOC H`, `RUSTC TL`,
`RUSTDOC T`, `RUSTC T` and `RUSTC P`), plus host programs and any others
we may add later.
Signed-off-by: Miguel Ojeda <[email protected]>
---
This one requires the `rusttest` warning cleanup at
https://lore.kernel.org/rust-for-linux/[email protected]/
Makefile | 4 ++--
scripts/Makefile.extrawarn | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index fba567a55607..2d0ea441cb9c 100644
--- a/Makefile
+++ b/Makefile
@@ -469,7 +469,7 @@ export rust_common_flags := --edition=2021 \
KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS)
-KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
+KBUILD_HOSTRUSTFLAGS = $(rust_common_flags) -O -Cstrip=debuginfo \
-Zallow-features= $(HOSTRUSTFLAGS)
KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
@@ -560,7 +560,7 @@ KBUILD_CFLAGS += -fno-PIE
KBUILD_CFLAGS += -fno-strict-aliasing
KBUILD_CPPFLAGS := -D__KERNEL__
-KBUILD_RUSTFLAGS := $(rust_common_flags) \
+KBUILD_RUSTFLAGS = $(rust_common_flags) \
-Cpanic=abort -Cembed-bitcode=n -Clto=n \
-Cforce-unwind-tables=n -Ccodegen-units=1 \
-Csymbol-mangling-version=v0 \
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 48114e91c386..990890821889 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -26,8 +26,8 @@ endif
KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
-KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
-KBUILD_RUSTFLAGS += $(KBUILD_RUSTFLAGS-y)
+rust_common_flags-$(CONFIG_WERROR) += -Dwarnings
+rust_common_flags += $(rust_common_flags-y)
KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
--
2.45.1
With `W=e`, kernel C targets error out on warnings.
Add support for the same feature for Rust code, but take the opportunity
to apply it for every Rust target (i.e. not just kernel code), so that
it behaves like having set `CONFIG_WERROR`.
Signed-off-by: Miguel Ojeda <[email protected]>
---
scripts/Makefile.extrawarn | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 990890821889..214b5edce4f2 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -206,5 +206,6 @@ endif
ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),)
KBUILD_CFLAGS += -Werror
+rust_common_flags += -Dwarnings
endif
--
2.45.1
On Sun, May 19, 2024 at 11:12 PM Miguel Ojeda <[email protected]> wrote:
>
> Following commit e88ca24319e4 ("kbuild: consolidate warning flags
> in scripts/Makefile.extrawarn"), move `-Dwarnings` handling into
> `Makefile.extrawarn` like C's `-Werror`.
>
> No functional change intended.
>
> Signed-off-by: Miguel Ojeda <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
On Sun, May 19, 2024 at 11:13 PM Miguel Ojeda <[email protected]> wrote:
>
> With `W=e`, kernel C targets error out on warnings.
>
> Add support for the same feature for Rust code, but take the opportunity
> to apply it for every Rust target (i.e. not just kernel code), so that
> it behaves like having set `CONFIG_WERROR`.
>
> Signed-off-by: Miguel Ojeda <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
On Mon, May 20, 2024 at 6:12 AM Miguel Ojeda <[email protected]> wrote:
>
> Make all Rust code (i.e. not just kernel code) respect `CONFIG_WERROR`,
> so that we keep everything warning clean.
Rust started to do something different from C.
KBUILD_HOSTCFLAGS is not affected by any CONFIG option.
The reason is because HOSTCC is needed for building Kconfig.
If the flags for HOSTCC is changed by a CONFIG option,
it would be a chicken-egg problem.
Also, some host programs might be compiled even
without .config at all. (e.g. scripts/unifdef)
I know Rust will not become a part of the core infrastructure
of the build system, but IMHO, host programs should not be
affected by any CONFIG option.
I do not like this patch.
>
> In particular, this affects targets in `rust/` (`RUSTDOC H`, `RUSTC TL`,
> `RUSTDOC T`, `RUSTC T` and `RUSTC P`), plus host programs and any others
> we may add later.
>
> Signed-off-by: Miguel Ojeda <[email protected]>
> ---
> This one requires the `rusttest` warning cleanup at
> https://lore.kernel.org/rust-for-linux/[email protected]/
>
> Makefile | 4 ++--
> scripts/Makefile.extrawarn | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index fba567a55607..2d0ea441cb9c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -469,7 +469,7 @@ export rust_common_flags := --edition=2021 \
>
> KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) $(HOSTCFLAGS)
> KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS)
> -KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> +KBUILD_HOSTRUSTFLAGS = $(rust_common_flags) -O -Cstrip=debuginfo \
> -Zallow-features= $(HOSTRUSTFLAGS)
> KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS)
> KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
> @@ -560,7 +560,7 @@ KBUILD_CFLAGS += -fno-PIE
> KBUILD_CFLAGS += -fno-strict-aliasing
>
> KBUILD_CPPFLAGS := -D__KERNEL__
> -KBUILD_RUSTFLAGS := $(rust_common_flags) \
> +KBUILD_RUSTFLAGS = $(rust_common_flags) \
> -Cpanic=abort -Cembed-bitcode=n -Clto=n \
> -Cforce-unwind-tables=n -Ccodegen-units=1 \
> -Csymbol-mangling-version=v0 \
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 48114e91c386..990890821889 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -26,8 +26,8 @@ endif
>
> KBUILD_CPPFLAGS-$(CONFIG_WERROR) += -Werror
> KBUILD_CPPFLAGS += $(KBUILD_CPPFLAGS-y)
> -KBUILD_RUSTFLAGS-$(CONFIG_WERROR) += -Dwarnings
> -KBUILD_RUSTFLAGS += $(KBUILD_RUSTFLAGS-y)
> +rust_common_flags-$(CONFIG_WERROR) += -Dwarnings
> +rust_common_flags += $(rust_common_flags-y)
>
> KBUILD_CFLAGS-$(CONFIG_CC_NO_ARRAY_BOUNDS) += -Wno-array-bounds
>
> --
> 2.45.1
--
Best Regards
Masahiro Yamada
On Tue, May 21, 2024 at 6:15 AM Masahiro Yamada <[email protected]> wrote:
>
> Rust started to do something different from C.
>
> KBUILD_HOSTCFLAGS is not affected by any CONFIG option.
>
> The reason is because HOSTCC is needed for building Kconfig.
> If the flags for HOSTCC is changed by a CONFIG option,
> it would be a chicken-egg problem.
> Also, some host programs might be compiled even
> without .config at all. (e.g. scripts/unifdef)
>
> I know Rust will not become a part of the core infrastructure
> of the build system, but IMHO, host programs should not be
> affected by any CONFIG option.
>
> I do not like this patch.
Thanks Masahiro -- yeah, I can see how it makes sense for C host
programs, and consistency with those may be best, even if it is not
used for core infrastructure in the case of Rust.
Do you think it would be OK if we do it only for everything else, i.e.
no host programs? That covers most of the Rust things so far and would
have helped with the warning I linked above (which is why I would like
to change this -- one can pass the flag manually, of course, but
having more targets affected by `CONFIG_WERROR` means more developers
will have it enabled and thus notice earlier).
(I am also thinking whether it could make sense to eventually
explicitly mark the C host programs that would be exempt from
`CONFIG_WERROR` so that we could apply it to everything else -- I
guess it could be easy to get wrong and/or forget when new ones are
added.)
Cheers,
Miguel
On Tue, May 21, 2024 at 5:05 PM Miguel Ojeda
<[email protected]> wrote:
>
> On Tue, May 21, 2024 at 6:15 AM Masahiro Yamada <masahiroy@kernelorg> wrote:
> >
> > Rust started to do something different from C.
> >
> > KBUILD_HOSTCFLAGS is not affected by any CONFIG option.
> >
> > The reason is because HOSTCC is needed for building Kconfig.
> > If the flags for HOSTCC is changed by a CONFIG option,
> > it would be a chicken-egg problem.
> > Also, some host programs might be compiled even
> > without .config at all. (e.g. scripts/unifdef)
> >
> > I know Rust will not become a part of the core infrastructure
> > of the build system, but IMHO, host programs should not be
> > affected by any CONFIG option.
> >
> > I do not like this patch.
>
> Thanks Masahiro -- yeah, I can see how it makes sense for C host
> programs, and consistency with those may be best, even if it is not
> used for core infrastructure in the case of Rust.
>
> Do you think it would be OK if we do it only for everything else, i.e.
> no host programs?
What does "everything else" mean exactly?
I just noticed that $(rust_common_flags) is passed
not only to RUSTC but also to RUSTDOC.
It seems to be intentional because it explicitly says
"requires kernel .config".
@echo ' rustdoc - Generate Rust documentation'
@echo ' (requires kernel .config)'
This is different from how other "make htmldocs" etc. work.
Why is the .config required for generating documentation?
> That covers most of the Rust things so far and would
> have helped with the warning I linked above (which is why I would like
> to change this -- one can pass the flag manually, of course, but
> having more targets affected by `CONFIG_WERROR` means more developers
> will have it enabled and thus notice earlier).
>
> (I am also thinking whether it could make sense to eventually
> explicitly mark the C host programs that would be exempt from
> `CONFIG_WERROR` so that we could apply it to everything else -- I
> guess it could be easy to get wrong and/or forget when new ones are
> added.)
>
> Cheers,
> Miguel
--
Best Regards
Masahiro Yamada
On Wed, May 22, 2024 at 12:14 PM Masahiro Yamada <[email protected]> wrote:
>
> What does "everything else" mean exactly?
Everything but the host programs. Or as many targets as possible, if
you think there are other cases that we should avoid.
> Why is the .config required for generating documentation?
`rustdoc` sees the code in a similar way as the compiler (it uses
parts of the compiler); in particular, it processes conditional
compilation like the compiler. So we need a given configuration to
generate it.
Cheers,
Miguel
On Wed, May 22, 2024 at 7:52 PM Miguel Ojeda
<[email protected]> wrote:
>
> On Wed, May 22, 2024 at 12:14 PM Masahiro Yamada <[email protected]> wrote:
> >
> > What does "everything else" mean exactly?
>
> Everything but the host programs. Or as many targets as possible, if
> you think there are other cases that we should avoid.
You can do this if rebuilding makes sense
when any CONFIG option is changed.
> > Why is the .config required for generating documentation?
>
> `rustdoc` sees the code in a similar way as the compiler (it uses
> parts of the compiler); in particular, it processes conditional
> compilation like the compiler. So we need a given configuration to
> generate it.
Surprising.
It potentially generates different documentations
depending on the .config file.
--
Best Regards
Masahiro Yamada