2015-02-01 20:00:33

by Yann Droneaud

[permalink] [raw]
Subject: [PATCH] kbuild: support W=e to make build abort in case of warning

In order to piss off^W^Whelp people trying to fix warnings,
this patch introduces a new 'e' modifier to W= as a short
hand for KCFLAGS+=-Werror" so that -Werror got added to
the kernel (built-in) and modules' CFLAGS and makes the
build abort when any warning is reported by the compiler.

In the end, people not sane enough can do not so useful
thing such as 'make W=123e'.

Signed-off-by: Yann Droneaud <[email protected]>
---
Makefile | 1 +
scripts/Makefile.extrawarn | 4 ++++
2 files changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 6b69223a5267..42fd50d939e8 100644
--- a/Makefile
+++ b/Makefile
@@ -1338,6 +1338,7 @@ help:
@echo ' 1: warnings which may be relevant and do not occur too often'
@echo ' 2: warnings which occur quite often but may still be relevant'
@echo ' 3: more obscure warnings, can most likely be ignored'
+ @echo ' e: warnings are being treated as errors'
@echo ' Multiple levels can be combined with W=12 or W=123'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index f734033af219..798796e078d9 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -5,6 +5,7 @@
# W=1 - warnings that may be relevant and does not occur too often
# W=2 - warnings that occur quite often but may still be relevant
# W=3 - the more obscure warnings, can most likely be ignored
+# W=e - warnings are being treated as errors
#
# $(call cc-option, -W...) handles gcc -W.. options which
# are not supported by all versions of the compiler
@@ -45,9 +46,12 @@ warning-3 += -Wswitch-default
warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
warning-3 += $(call cc-option, -Wvla)

+warning-e := -Werror
+
warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
+warning += $(warning-$(findstring e, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))

ifeq ("$(strip $(warning))","")
$(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
--
2.1.0


2022-04-08 10:29:45

by Yann Droneaud

[permalink] [raw]
Subject: [PATCHv1] kbuild: support W=e to make build abort in case of warning

When developing new code/feature, CONFIG_WERROR is most
often turned off, especially for people using make W=12 to
get more warnings.

In such case, turning on -Werror temporarily would require
switching on CONFIG_WERROR in the configuration, building,
then switching off CONFIG_WERROR.

For this use case, this patch introduces a new 'e' modifier
to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
got added to the kernel (built-in) and modules' CFLAGS.

Signed-off-by: Yann Droneaud <[email protected]>
---
Makefile | 1 +
scripts/Makefile.extrawarn | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)

Changes since v0[0]:

- rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
- document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")

[0] https://lore.kernel.org/all/[email protected]/

diff --git a/Makefile b/Makefile
index 8c7de9a72ea2..6dc621af18d1 100644
--- a/Makefile
+++ b/Makefile
@@ -1649,6 +1649,7 @@ help:
@echo ' 1: warnings which may be relevant and do not occur too often'
@echo ' 2: warnings which occur quite often but may still be relevant'
@echo ' 3: more obscure warnings, can most likely be ignored'
+ @echo ' e: warnings are being treated as errors'
@echo ' Multiple levels can be combined with W=12 or W=123'
@echo ''
@echo 'Execute "make" or "make all" to build all targets marked with [*] '
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 650d0b8ceec3..f5f0d6f09053 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -2,8 +2,8 @@
# ==========================================================================
# make W=... settings
#
-# There are three warning groups enabled by W=1, W=2, W=3.
-# They are independent, and can be combined like W=12 or W=123.
+# There are four warning groups enabled by W=1, W=2, W=3, and W=e
+# They are independent, and can be combined like W=12 or W=123e.
# ==========================================================================

KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
@@ -94,3 +94,12 @@ KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3

endif
+
+#
+# W=e - error out on warnings
+#
+ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),)
+
+KBUILD_CFLAGS += -Werror
+
+endif
--
2.32.0

2022-04-08 12:28:24

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
>
> When developing new code/feature, CONFIG_WERROR is most
> often turned off, especially for people using make W=12 to
> get more warnings.
>
> In such case, turning on -Werror temporarily would require
> switching on CONFIG_WERROR in the configuration, building,
> then switching off CONFIG_WERROR.
>
> For this use case, this patch introduces a new 'e' modifier
> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> got added to the kernel (built-in) and modules' CFLAGS.
>
> Signed-off-by: Yann Droneaud <[email protected]>
> ---
> Makefile | 1 +
> scripts/Makefile.extrawarn | 13 +++++++++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> Changes since v0[0]:
>
> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
>
> [0] https://lore.kernel.org/all/[email protected]/


I remembered the previous submission, I liked it, but I had lost it.

It seems already 7 years ago, (before I became the Kbuild maintainer).
Thanks for coming back to this.


I like this, but I will wait some time for review comments.









> diff --git a/Makefile b/Makefile
> index 8c7de9a72ea2..6dc621af18d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1649,6 +1649,7 @@ help:
> @echo ' 1: warnings which may be relevant and do not occur too often'
> @echo ' 2: warnings which occur quite often but may still be relevant'
> @echo ' 3: more obscure warnings, can most likely be ignored'
> + @echo ' e: warnings are being treated as errors'
> @echo ' Multiple levels can be combined with W=12 or W=123'
> @echo ''
> @echo 'Execute "make" or "make all" to build all targets marked with [*] '
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 650d0b8ceec3..f5f0d6f09053 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -2,8 +2,8 @@
> # ==========================================================================
> # make W=... settings
> #
> -# There are three warning groups enabled by W=1, W=2, W=3.
> -# They are independent, and can be combined like W=12 or W=123.
> +# There are four warning groups enabled by W=1, W=2, W=3, and W=e
> +# They are independent, and can be combined like W=12 or W=123e.
> # ==========================================================================
>
> KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
> @@ -94,3 +94,12 @@ KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
> KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3
>
> endif
> +
> +#
> +# W=e - error out on warnings
> +#
> +ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),)
> +
> +KBUILD_CFLAGS += -Werror
> +
> +endif
> --
> 2.32.0
>


--
Best Regards
Masahiro Yamada

2022-04-09 06:13:33

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
>
> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> >
> > When developing new code/feature, CONFIG_WERROR is most
> > often turned off, especially for people using make W=12 to
> > get more warnings.
> >
> > In such case, turning on -Werror temporarily would require
> > switching on CONFIG_WERROR in the configuration, building,
> > then switching off CONFIG_WERROR.
> >
> > For this use case, this patch introduces a new 'e' modifier
> > to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> > got added to the kernel (built-in) and modules' CFLAGS.
> >
> > Signed-off-by: Yann Droneaud <[email protected]>
> > ---
> > Makefile | 1 +
> > scripts/Makefile.extrawarn | 13 +++++++++++--
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > Changes since v0[0]:
> >
> > - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> > - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> >
> > [0] https://lore.kernel.org/all/[email protected]/
>
>
> I remembered the previous submission, I liked it, but I had lost it.
>
> It seems already 7 years ago, (before I became the Kbuild maintainer).
> Thanks for coming back to this.
>
>
> I like this, but I will wait some time for review comments.

Dunno, this seems pretty simple:

$ ./scripts/config -e WERROR
$ make ... W=12
--
Thanks,
~Nick Desaulniers

2022-04-10 07:42:37

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> wrote:
>
>
>
> On 4/8/22 13:29, Nick Desaulniers wrote:
> > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
> >>
> >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> >>>
> >>> When developing new code/feature, CONFIG_WERROR is most
> >>> often turned off, especially for people using make W=12 to
> >>> get more warnings.
> >>>
> >>> In such case, turning on -Werror temporarily would require
> >>> switching on CONFIG_WERROR in the configuration, building,
> >>> then switching off CONFIG_WERROR.
> >>>
> >>> For this use case, this patch introduces a new 'e' modifier
> >>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> >>> got added to the kernel (built-in) and modules' CFLAGS.
> >>>
> >>> Signed-off-by: Yann Droneaud <[email protected]>
> >>> ---
> >>> Makefile | 1 +
> >>> scripts/Makefile.extrawarn | 13 +++++++++++--
> >>> 2 files changed, 12 insertions(+), 2 deletions(-)
> >>>
> >>> Changes since v0[0]:
> >>>
> >>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> >>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> >>>
> >>> [0] https://lore.kernel.org/all/[email protected]/
> >>
> >>
> >> I remembered the previous submission, I liked it, but I had lost it.
> >>
> >> It seems already 7 years ago, (before I became the Kbuild maintainer).
> >> Thanks for coming back to this.
> >>
> >>
> >> I like this, but I will wait some time for review comments.
> >
> > Dunno, this seems pretty simple:
> >
> > $ ./scripts/config -e WERROR
> > $ make ... W=12
>
> Yeah, that's about what I was thinking too..



But, you cannot change the .config
when you build external modules.

"make W=e" might be useful for people who strive to
keep their downstream modules warning-free.


W=e is the same pattern.
I do not see much downside.



(BTW, I do not like CONFIG_WERROR.)






--
Best Regards
Masahiro Yamada

2022-04-11 10:17:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning



On 4/8/22 18:47, Masahiro Yamada wrote:
> On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> wrote:
>>
>>
>>
>> On 4/8/22 13:29, Nick Desaulniers wrote:
>>> On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
>>>>
>>>> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
>>>>>
>>>>> When developing new code/feature, CONFIG_WERROR is most
>>>>> often turned off, especially for people using make W=12 to
>>>>> get more warnings.
>>>>>
>>>>> In such case, turning on -Werror temporarily would require
>>>>> switching on CONFIG_WERROR in the configuration, building,
>>>>> then switching off CONFIG_WERROR.
>>>>>
>>>>> For this use case, this patch introduces a new 'e' modifier
>>>>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
>>>>> got added to the kernel (built-in) and modules' CFLAGS.
>>>>>
>>>>> Signed-off-by: Yann Droneaud <[email protected]>
>>>>> ---
>>>>> Makefile | 1 +
>>>>> scripts/Makefile.extrawarn | 13 +++++++++++--
>>>>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> Changes since v0[0]:
>>>>>
>>>>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
>>>>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
>>>>>
>>>>> [0] https://lore.kernel.org/all/[email protected]/
>>>>
>>>>
>>>> I remembered the previous submission, I liked it, but I had lost it.
>>>>
>>>> It seems already 7 years ago, (before I became the Kbuild maintainer).
>>>> Thanks for coming back to this.
>>>>
>>>>
>>>> I like this, but I will wait some time for review comments.
>>>
>>> Dunno, this seems pretty simple:
>>>
>>> $ ./scripts/config -e WERROR
>>> $ make ... W=12
>>
>> Yeah, that's about what I was thinking too..
>
>
>
> But, you cannot change the .config
> when you build external modules.
>
> "make W=e" might be useful for people who strive to
> keep their downstream modules warning-free.
>
>
> W=e is the same pattern.
> I do not see much downside.
>

Well, I don't see much downside either.

>
>
> (BTW, I do not like CONFIG_WERROR.)

Yeah, I disable it most of the time, but I am pretty good
at searching for errors & warnings. Apparently some people
are not.

--
~Randy

2022-04-11 17:41:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning



On 4/8/22 13:29, Nick Desaulniers wrote:
> On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
>>
>> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
>>>
>>> When developing new code/feature, CONFIG_WERROR is most
>>> often turned off, especially for people using make W=12 to
>>> get more warnings.
>>>
>>> In such case, turning on -Werror temporarily would require
>>> switching on CONFIG_WERROR in the configuration, building,
>>> then switching off CONFIG_WERROR.
>>>
>>> For this use case, this patch introduces a new 'e' modifier
>>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
>>> got added to the kernel (built-in) and modules' CFLAGS.
>>>
>>> Signed-off-by: Yann Droneaud <[email protected]>
>>> ---
>>> Makefile | 1 +
>>> scripts/Makefile.extrawarn | 13 +++++++++++--
>>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> Changes since v0[0]:
>>>
>>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
>>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
>>>
>>> [0] https://lore.kernel.org/all/[email protected]/
>>
>>
>> I remembered the previous submission, I liked it, but I had lost it.
>>
>> It seems already 7 years ago, (before I became the Kbuild maintainer).
>> Thanks for coming back to this.
>>
>>
>> I like this, but I will wait some time for review comments.
>
> Dunno, this seems pretty simple:
>
> $ ./scripts/config -e WERROR
> $ make ... W=12

Yeah, that's about what I was thinking too..

--
~Randy

2022-04-16 00:29:16

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Thu, Apr 14, 2022 at 10:19 PM Nicolas Schier <[email protected]> wrote:
>
> På lø. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> Wrote:
> > >
> > >
> > >
> > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
> > > >>
> > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> > > >>>
> > > >>> When developing new code/feature, CONFIG_WERROR is most
> > > >>> often turned off, especially for people using make W=12 to
> > > >>> get more warnings.
> > > >>>
> > > >>> In such case, turning on -Werror temporarily would require
> > > >>> switching on CONFIG_WERROR in the configuration, building,
> > > >>> then switching off CONFIG_WERROR.
> > > >>>
> > > >>> For this use case, this patch introduces a new 'e' modifier
> > > >>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> > > >>> got added to the kernel (built-in) and modules' CFLAGS.
> > > >>>
> > > >>> Signed-off-by: Yann Droneaud <[email protected]>
> > > >>> ---
> > > >>> Makefile | 1 +
> > > >>> scripts/Makefile.extrawarn | 13 +++++++++++--
> > > >>> 2 files changed, 12 insertions(+), 2 deletions(-)
> > > >>>
> > > >>> Changes since v0[0]:
> > > >>>
> > > >>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> > > >>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> > > >>>
> > > >>> [0] https://lore.kernel.org/all/[email protected]/
> > > >>
> > > >>
> > > >> I remembered the previous submission, I liked it, but I had lost it.
> > > >>
> > > >> It seems already 7 years ago, (before I became the Kbuild maintainer).
> > > >> Thanks for coming back to this.
> > > >>
> > > >>
> > > >> I like this, but I will wait some time for review comments.
> > > >
> > > > Dunno, this seems pretty simple:
> > > >
> > > > $ ./scripts/config -e WERROR
> > > > $ make ... W=12
> > >
> > > Yeah, that's about what I was thinking too..
> >
> >
> >
> > But, you cannot change the .config
> > when you build external modules.
> >
> > "make W=e" might be useful for people who strive to
> > keep their downstream modules warning-free.
> >
> >
> > W=e is the same pattern.
> > I do not see much downside.
>
> If I set CONFIG_WERROR=y on the make command line, I could have the
> same result, don't I?
>
> make CONFIG_WERROR=1 ...
>
> no matter if in-tree or for external kernel modules.

Yes.

If you can change the kernel configuration,
you can enable CONFIG_WERROR.

To build external modules against the read-only
/lib/modules/$(uname -r)/build/,
it is not so feasible to change the .config file, though.


>
> Kind regards,
> Nicolas



--
Best Regards
Masahiro Yamada

2022-04-16 01:55:56

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

P? l?. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> Wrote:
> >
> >
> >
> > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
> > >>
> > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> > >>>
> > >>> When developing new code/feature, CONFIG_WERROR is most
> > >>> often turned off, especially for people using make W=12 to
> > >>> get more warnings.
> > >>>
> > >>> In such case, turning on -Werror temporarily would require
> > >>> switching on CONFIG_WERROR in the configuration, building,
> > >>> then switching off CONFIG_WERROR.
> > >>>
> > >>> For this use case, this patch introduces a new 'e' modifier
> > >>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> > >>> got added to the kernel (built-in) and modules' CFLAGS.
> > >>>
> > >>> Signed-off-by: Yann Droneaud <[email protected]>
> > >>> ---
> > >>> Makefile | 1 +
> > >>> scripts/Makefile.extrawarn | 13 +++++++++++--
> > >>> 2 files changed, 12 insertions(+), 2 deletions(-)
> > >>>
> > >>> Changes since v0[0]:
> > >>>
> > >>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> > >>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> > >>>
> > >>> [0] https://lore.kernel.org/all/[email protected]/
> > >>
> > >>
> > >> I remembered the previous submission, I liked it, but I had lost it.
> > >>
> > >> It seems already 7 years ago, (before I became the Kbuild maintainer).
> > >> Thanks for coming back to this.
> > >>
> > >>
> > >> I like this, but I will wait some time for review comments.
> > >
> > > Dunno, this seems pretty simple:
> > >
> > > $ ./scripts/config -e WERROR
> > > $ make ... W=12
> >
> > Yeah, that's about what I was thinking too..
>
>
>
> But, you cannot change the .config
> when you build external modules.
>
> "make W=e" might be useful for people who strive to
> keep their downstream modules warning-free.
>
>
> W=e is the same pattern.
> I do not see much downside.

If I set CONFIG_WERROR=y on the make command line, I could have the
same result, don't I?

make CONFIG_WERROR=1 ...

no matter if in-tree or for external kernel modules.

Kind regards,
Nicolas

2022-04-16 02:07:44

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Fri, Apr 15, 2022 at 12:15:40AM +0900 Masahiro Yamada wrote:
> On Thu, Apr 14, 2022 at 10:19 PM Nicolas Schier <[email protected]> wrote:
> >
> > P? l?. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> Wrote:
> > > >
> > > >
> > > >
> > > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
> > > > >>
> > > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> > > > >>>
> > > > >>> When developing new code/feature, CONFIG_WERROR is most
> > > > >>> often turned off, especially for people using make W=12 to
> > > > >>> get more warnings.
> > > > >>>
> > > > >>> In such case, turning on -Werror temporarily would require
> > > > >>> switching on CONFIG_WERROR in the configuration, building,
> > > > >>> then switching off CONFIG_WERROR.
> > > > >>>
> > > > >>> For this use case, this patch introduces a new 'e' modifier
> > > > >>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> > > > >>> got added to the kernel (built-in) and modules' CFLAGS.
> > > > >>>
> > > > >>> Signed-off-by: Yann Droneaud <[email protected]>
> > > > >>> ---
> > > > >>> Makefile | 1 +
> > > > >>> scripts/Makefile.extrawarn | 13 +++++++++++--
> > > > >>> 2 files changed, 12 insertions(+), 2 deletions(-)
> > > > >>>
> > > > >>> Changes since v0[0]:
> > > > >>>
> > > > >>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> > > > >>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> > > > >>>
> > > > >>> [0] https://lore.kernel.org/all/[email protected]/
> > > > >>
> > > > >>
> > > > >> I remembered the previous submission, I liked it, but I had lost it.
> > > > >>
> > > > >> It seems already 7 years ago, (before I became the Kbuild maintainer).
> > > > >> Thanks for coming back to this.
> > > > >>
> > > > >>
> > > > >> I like this, but I will wait some time for review comments.
> > > > >
> > > > > Dunno, this seems pretty simple:
> > > > >
> > > > > $ ./scripts/config -e WERROR
> > > > > $ make ... W=12
> > > >
> > > > Yeah, that's about what I was thinking too..
> > >
> > >
> > >
> > > But, you cannot change the .config
> > > when you build external modules.
> > >
> > > "make W=e" might be useful for people who strive to
> > > keep their downstream modules warning-free.
> > >
> > >
> > > W=e is the same pattern.
> > > I do not see much downside.
> >
> > If I set CONFIG_WERROR=y on the make command line, I could have the
> > same result, don't I?
> >
> > make CONFIG_WERROR=1 ...
> >
> > no matter if in-tree or for external kernel modules.
>
> Yes.
>
> If you can change the kernel configuration,
> you can enable CONFIG_WERROR.
>
> To build external modules against the read-only
> /lib/modules/$(uname -r)/build/,
> it is not so feasible to change the .config file, though.

hm, I wanted to point out something different. When I build an external module
against a read-only kbuild-tree, I _can_ change kconfig values by specifying
them on the make command line, just as I can add 'W=e':

make -C /lib/modules/$(uname -r)/build M=~+ CONFIG_SAMPLE_KOBJECT=m CONFIG_WERROR=y

Thus, I suspect, that this is the very same as if I'd run

make -C /lib/modules/$(uname -r)/build M=~+ CONFIG_SAMPLE_KOBJECT=m W=e

So, "W=e" is actually just a shortcut for "CONFIG_WERROR=1". Or have I missed
something?

Kind regards,
Nicolas

2022-04-16 02:17:22

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Thu, Apr 14, 2022 at 07:51:46PM +0200 Nicolas Schier wrote:
> On Fri, Apr 15, 2022 at 12:15:40AM +0900 Masahiro Yamada wrote:
> > On Thu, Apr 14, 2022 at 10:19 PM Nicolas Schier <[email protected]> wrote:
> > >
> > > P? l?. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > > > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <[email protected]> Wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <[email protected]> wrote:
> > > > > >>
> > > > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
> > > > > >>>
> > > > > >>> When developing new code/feature, CONFIG_WERROR is most
> > > > > >>> often turned off, especially for people using make W=12 to
> > > > > >>> get more warnings.
> > > > > >>>
> > > > > >>> In such case, turning on -Werror temporarily would require
> > > > > >>> switching on CONFIG_WERROR in the configuration, building,
> > > > > >>> then switching off CONFIG_WERROR.
> > > > > >>>
> > > > > >>> For this use case, this patch introduces a new 'e' modifier
> > > > > >>> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> > > > > >>> got added to the kernel (built-in) and modules' CFLAGS.
> > > > > >>>
> > > > > >>> Signed-off-by: Yann Droneaud <[email protected]>
> > > > > >>> ---
> > > > > >>> Makefile | 1 +
> > > > > >>> scripts/Makefile.extrawarn | 13 +++++++++++--
> > > > > >>> 2 files changed, 12 insertions(+), 2 deletions(-)
> > > > > >>>
> > > > > >>> Changes since v0[0]:
> > > > > >>>
> > > > > >>> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> > > > > >>> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
> > > > > >>>
> > > > > >>> [0] https://lore.kernel.org/all/[email protected]/
> > > > > >>
> > > > > >>
> > > > > >> I remembered the previous submission, I liked it, but I had lost it.
> > > > > >>
> > > > > >> It seems already 7 years ago, (before I became the Kbuild maintainer).
> > > > > >> Thanks for coming back to this.
> > > > > >>
> > > > > >>
> > > > > >> I like this, but I will wait some time for review comments.
> > > > > >
> > > > > > Dunno, this seems pretty simple:
> > > > > >
> > > > > > $ ./scripts/config -e WERROR
> > > > > > $ make ... W=12
> > > > >
> > > > > Yeah, that's about what I was thinking too..
> > > >
> > > >
> > > >
> > > > But, you cannot change the .config
> > > > when you build external modules.
> > > >
> > > > "make W=e" might be useful for people who strive to
> > > > keep their downstream modules warning-free.
> > > >
> > > >
> > > > W=e is the same pattern.
> > > > I do not see much downside.
> > >
> > > If I set CONFIG_WERROR=y on the make command line, I could have the
> > > same result, don't I?
> > >
> > > make CONFIG_WERROR=1 ...
> > >
> > > no matter if in-tree or for external kernel modules.
> >
> > Yes.
> >
> > If you can change the kernel configuration,
> > you can enable CONFIG_WERROR.
> >
> > To build external modules against the read-only
> > /lib/modules/$(uname -r)/build/,
> > it is not so feasible to change the .config file, though.
>
> hm, I wanted to point out something different. When I build an external module
> against a read-only kbuild-tree, I _can_ change kconfig values by specifying
> them on the make command line, just as I can add 'W=e':
>
> make -C /lib/modules/$(uname -r)/build M=~+ CONFIG_SAMPLE_KOBJECT=m CONFIG_WERROR=y
>
> Thus, I suspect, that this is the very same as if I'd run
>
> make -C /lib/modules/$(uname -r)/build M=~+ CONFIG_SAMPLE_KOBJECT=m W=e
>
> So, "W=e" is actually just a shortcut for "CONFIG_WERROR=1". Or have I missed
> something?
>
> Kind regards,
> Nicolas

oh, sorry. Yann wrote it already in his commit message, please excuse the noise.

Kind regards,
Nicolas

2022-04-22 19:56:57

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCHv1] kbuild: support W=e to make build abort in case of warning

On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <[email protected]> wrote:
>
> When developing new code/feature, CONFIG_WERROR is most
> often turned off, especially for people using make W=12 to
> get more warnings.
>
> In such case, turning on -Werror temporarily would require
> switching on CONFIG_WERROR in the configuration, building,
> then switching off CONFIG_WERROR.
>
> For this use case, this patch introduces a new 'e' modifier
> to W= as a short hand for KCFLAGS+=-Werror" so that -Werror
> got added to the kernel (built-in) and modules' CFLAGS.
>
> Signed-off-by: Yann Droneaud <[email protected]>
> ---


Applied to linux-kbuild.
Thanks.

> Makefile | 1 +
> scripts/Makefile.extrawarn | 13 +++++++++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> Changes since v0[0]:
>
> - rebase on top of commit 64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn")
> - document use case after commit 3fe617ccafd6 ("Enable '-Werror' by default for all kernel builds")
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> diff --git a/Makefile b/Makefile
> index 8c7de9a72ea2..6dc621af18d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1649,6 +1649,7 @@ help:
> @echo ' 1: warnings which may be relevant and do not occur too often'
> @echo ' 2: warnings which occur quite often but may still be relevant'
> @echo ' 3: more obscure warnings, can most likely be ignored'
> + @echo ' e: warnings are being treated as errors'
> @echo ' Multiple levels can be combined with W=12 or W=123'
> @echo ''
> @echo 'Execute "make" or "make all" to build all targets marked with [*] '
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 650d0b8ceec3..f5f0d6f09053 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -2,8 +2,8 @@
> # ==========================================================================
> # make W=... settings
> #
> -# There are three warning groups enabled by W=1, W=2, W=3.
> -# They are independent, and can be combined like W=12 or W=123.
> +# There are four warning groups enabled by W=1, W=2, W=3, and W=e
> +# They are independent, and can be combined like W=12 or W=123e.
> # ==========================================================================
>
> KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
> @@ -94,3 +94,12 @@ KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
> KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3
>
> endif
> +
> +#
> +# W=e - error out on warnings
> +#
> +ifneq ($(findstring e, $(KBUILD_EXTRA_WARN)),)
> +
> +KBUILD_CFLAGS += -Werror
> +
> +endif
> --
> 2.32.0
>


--
Best Regards
Masahiro Yamada