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

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

[Yann Droneaud]

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 <ydroneaud@opteya.com>
---
 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

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

[Yann Droneaud]

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 <ydroneaud@opteya.com>
---
 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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/

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

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

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> ---
>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/


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

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

[Nick Desaulniers]

On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> > ---
> >  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
>
>
> 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

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

[Randy Dunlap]

On 4/8/22 13:29, Nick Desaulniers wrote:
> On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
>>> ---
>>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
>>
>>
>> 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

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

[Masahiro Yamada]

On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
>
>
> On 4/8/22 13:29, Nick Desaulniers wrote:
> > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >>
> >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> >>> ---
> >>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
> >>
> >>
> >> 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

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

[Randy Dunlap]

On 4/8/22 18:47, Masahiro Yamada wrote:
> On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>>
>>
>> On 4/8/22 13:29, Nick Desaulniers wrote:
>>> On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>>>
>>>> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
>>>>> ---
>>>>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
>>>>
>>>>
>>>> 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

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

[Nicolas Schier]

På lø. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> Wrote:
> >
> >
> >
> > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >>
> > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> > >>> ---
> > >>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
> > >>
> > >>
> > >> 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

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

[Masahiro Yamada]

On Thu, Apr 14, 2022 at 10:19 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> På lø. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> Wrote:
> > >
> > >
> > >
> > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >>
> > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> > > >>> ---
> > > >>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
> > > >>
> > > >>
> > > >> 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

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

[Nicolas Schier]

On Fri, Apr 15, 2022 at 12:15:40AM +0900 Masahiro Yamada wrote:
> On Thu, Apr 14, 2022 at 10:19 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > På lø. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> Wrote:
> > > >
> > > >
> > > >
> > > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > > >>
> > > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> > > > >>> ---
> > > > >>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
> > > > >>
> > > > >>
> > > > >> 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

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

[Nicolas Schier]

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 <nicolas@fjasle.eu> wrote:
> > >
> > > På lø. 09. april 2022 kl. 10.47 +0000 skrev Masahiro Yamada:
> > > > On Sat, Apr 9, 2022 at 5:36 AM Randy Dunlap <rdunlap@infradead.org> Wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 4/8/22 13:29, Nick Desaulniers wrote:
> > > > > > On Fri, Apr 8, 2022 at 4:06 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > > > >>
> > > > > >> On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> > > > > >>> ---
> > > > > >>>  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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
> > > > > >>
> > > > > >>
> > > > > >> 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

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

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 5:46 PM Yann Droneaud <ydroneaud@opteya.com> 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 <ydroneaud@opteya.com>
> ---


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/1422803720-14723-1-git-send-email-ydroneaud@opteya.com/
>
> 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