Re: [RESEND v3 2/2] drm: Add CONFIG_DRM_WERROR

Maxime Ripard
2024-03-27T07:35:20Z

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

Hi,

On Tue, Mar 26, 2024 at 03:56:50PM -0700, Nathan Chancellor wrote:
> On Tue, Mar 05, 2024 at 11:07:36AM +0200, Jani Nikula wrote:
> > Add kconfig to enable -Werror subsystem wide. This is useful for
> > development and CI to keep the subsystem warning free, while avoiding
> > issues outside of the subsystem that kernel wide CONFIG_WERROR=y might
> > hit.
> >
> > v2: Don't depend on COMPILE_TEST
> >
> > Reviewed-by: Hamza Mahfooz <hamza.mahfooz@amd.com> # v1
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> > drivers/gpu/drm/Kconfig | 13 +++++++++++++
> > drivers/gpu/drm/Makefile | 3 +++
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index 6e853acf15da..c08e18108c2a 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -416,3 +416,16 @@ config DRM_LIB_RANDOM
> > config DRM_PRIVACY_SCREEN
> > bool
> > default n
> > +
> > +config DRM_WERROR
> > + bool "Compile the drm subsystem with warnings as errors"
> > + depends on EXPERT
> > + default n
> > + help
> > + A kernel build should not cause any compiler warnings, and this
> > + enables the '-Werror' flag to enforce that rule in the drm subsystem.
> > +
> > + The drm subsystem enables more warnings than the kernel default, so
> > + this config option is disabled by default.
> > +
> > + If in doubt, say N.
>
> While I understand the desire for an easy switch that maintainers and
> developers can use to ensure that their changes are warning free for the
> drm subsystem specifically, I think subsystem specific configuration
> options like this are actively detrimental to developers and continuous
> integration systems that build test the entire kernel. For example, we
> turned off CONFIG_WERROR for our Hexagon builds because of warnings that
> appear with -Wextra that are legitimate but require treewide changes to
> resolve in a manner sufficient for Linus:
>
> https://github.com/ClangBuiltLinux/linux/issues/1285
> https://lore.kernel.org/all/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
> https://lore.kernel.org/all/20230522105049.1467313-1-schnelle@linux.ibm.com/
>
> But now, due to CONFIG_DRM_WERROR getting enabled by all{mod,yes}config
> and -Wextra being unconditionally enabled for DRM, those warnings hard
> break the build despite CONFIG_WERROR=n...

Would making DRM_WERROR depends on WERROR address your concerns?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]