[rfc] removing/conditionalising WERROR= in Makefiles

Alexander Best arundel at freebsd.org
Mon Dec 26 10:10:40 UTC 2011

hi there,

i grep'ed through src/sys and found several places where WERROR= was set in
order to get rid of the default -Werror setting. i tried to remove those
WERROR= overrides from any Makefile, where doing so did not break tinderbox.

in those cases, where it couldn't be completely removed, i added conditions to
only set WERROR= for the particular achitecture or compiler, where tinderbox
did not suceed without the WERROR=.

i talked to dim@ on #freebsd-clang at OFTC and he was against enclosing WERROR=
in a architecture or compiler condition. his statement was:

dim : I'm not going to riddle all those makefiles with endless comments, people
      have to learn to look in the commit history for a file.

however the problem is that most of the time the commit message by the person
who added WERROR= to a Makefile doesn't state *why* the person did it and most
importantly it doesn't state in *which* case tinderbox failed without the
WERROR= (see *).

so my suggestion would be that instead of setting WERROR= unconditionally in
Makefiles, it should be enclosed in a condition to only set it in a particular
case, where tinderbox *will* fail.

an example is r228861. here dim@ set WERROR= in sys/modules/nve/Makefile
unconditionally. however tinderbox will only fail when

A) clang is used as compiler
B) when building for i386

these facts aren't clear from the commit message. so i'd suggest (please see
the attached patch) to conditionalise such settings.

another point i'd like to make is that WERROR= might have been set in a
Makefile in the past to unbreak tinderbox for a certain arch that is no longer
supported. since most of the time the commit logs simply state something like:

"...remove -Werror for now to unbreak tinderbox..."

nobody will notice that after support for a certain arch was removed, the
WERROR= line can be removed from that particular Makefile.

that's why i'd like to propose the following patch. i ran a full tinderbox run
against r228878 and it suceeded.

-------------- next part --------------
Index: sys/modules/xfs/Makefile
--- sys/modules/xfs/Makefile	(revision 228878)
+++ sys/modules/xfs/Makefile	(working copy)
@@ -6,8 +6,6 @@
 KMOD=	 xfs
 SRCS =  vnode_if.h \
 	xfs_alloc.c \
 	xfs_alloc_btree.c \
Index: sys/modules/sound/driver/maestro/Makefile
--- sys/modules/sound/driver/maestro/Makefile	(revision 228878)
+++ sys/modules/sound/driver/maestro/Makefile	(working copy)
@@ -5,6 +5,5 @@
 KMOD=	snd_maestro
 SRCS=	device_if.h bus_if.h pci_if.h
 SRCS+=	maestro.c
 .include <bsd.kmod.mk>
Index: sys/modules/aic7xxx/ahd/Makefile
--- sys/modules/aic7xxx/ahd/Makefile	(revision 228878)
+++ sys/modules/aic7xxx/ahd/Makefile	(working copy)
@@ -4,7 +4,6 @@
 .PATH:	${.CURDIR}/../../../dev/aic7xxx
 KMOD=	ahd
 GENSRCS= aic79xx_seq.h aic79xx_reg.h
Index: sys/modules/ie/Makefile
--- sys/modules/ie/Makefile	(revision 228878)
+++ sys/modules/ie/Makefile	(working copy)
@@ -6,6 +6,8 @@
 KMOD=	if_ie
 SRCS=	if_ie.c if_ie_isa.c \
 	isa_if.h bus_if.h device_if.h
+.if ${MACHINE_CPUARCH} == "i386"
 .include <bsd.kmod.mk>
Index: sys/modules/agp/Makefile
--- sys/modules/agp/Makefile	(revision 228878)
+++ sys/modules/agp/Makefile	(working copy)
@@ -20,7 +20,6 @@
 SRCS+=	device_if.h bus_if.h agp_if.h pci_if.h
 SRCS+=	opt_agp.h opt_bus.h
 MFILES=	kern/device_if.m kern/bus_if.m dev/agp/agp_if.m dev/pci/pci_if.m
 EXPORT_SYMS=	agp_find_device		\
 		agp_state		\
Index: sys/modules/bios/smapi/Makefile
--- sys/modules/bios/smapi/Makefile	(revision 228878)
+++ sys/modules/bios/smapi/Makefile	(working copy)
@@ -6,7 +6,6 @@
 KMOD=	smapi
 SRCS=	smapi.c smapi_bios.S \
 	bus_if.h device_if.h
 .if ${CC:T:Mclang} == "clang"
 # XXX: clang integrated-as doesn't grok 16-bit assembly yet
 CFLAGS+=	${.IMPSRC:T:Msmapi_bios.S:C/^.+$/-no-integrated-as/}
Index: sys/modules/nve/Makefile
--- sys/modules/nve/Makefile	(revision 228878)
+++ sys/modules/nve/Makefile	(working copy)
@@ -7,7 +7,9 @@
 	device_if.h bus_if.h pci_if.h miibus_if.h \
 OBJS+=	nvenetlib.o
+.if ${MACHINE_CPUARCH} == "i386" && ${CC:T:Mclang} == "clang"
 CLEANFILES+=	nvenetlib.o os+%DIKED-nve.h
 nvenetlib.o: ${.CURDIR}/../../contrib/dev/nve/${MACHINE}/${.TARGET}.bz2.uu

More information about the freebsd-current mailing list