On Wed, 27 Mar 2024, Maxime Ripard <mripard@xxxxxxxxxx> wrote: > 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@xxxxxxx> # v1 >> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> > --- >> > 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@xxxxxxxxxxxxxx/ >> https://lore.kernel.org/all/20230522105049.1467313-1-schnelle@xxxxxxxxxxxxx/ >> >> 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? But then what would be the point of having DRM_WERROR at all? For me the point is, "werror in drm, ignore the rest, they're someone else's problem". An alternative would be to "depends on !COMPILE_TEST" that we have in i915, but then some folks want to have COMPILE_TEST in drm, because some drivers are otherwise hard for people to build. Nathan, we do want to fix any issues switfly. Are you hitting specific build problems? BR, Jani. > > Maxime -- Jani Nikula, Intel
