[rfc] removing/conditionalising WERROR= in Makefiles

Alexander Best arundel at freebsd.org
Tue Dec 27 11:27:43 UTC 2011


On Tue Dec 27 11, Philip Paeps wrote:
> On 2011-12-26 10:10:40 (+0000), Alexander Best <arundel at freebsd.org> wrote:
> > 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=.
> 
> Wouldn't it be better to set WARNS=x rather than WERROR=?  WERROR= says "this
> code has bugs, it breaks tinderbox" whereas WARNS=x says "this code has the
> following kind of bugs which break tinderbox".
> 
> Possibly wrapped in an architecture-test where appropriate.

well there are a few issues here:

1) Jan Beich informed me via a private email that enclosing WERROR in arch
   specific conditions is a bad idea. if the code gets ported to a new
   architecture WERROR doesn't get set and so for every new architecture one
   has to evaluate again, whether WERROR needs to be set or not.
   so i'm inclined to agree with dim@ that we should not add architecture
   specific conditions -- however i think adding compiler specific conditions
   is a good idea.

2) the problem with settings WARNS= or specific -Wno-X or -Wno-error=X is that
   expecially GCC doesn't have specific -WX flags for certain warnings. some
   warnings are implied by -Wall and cannot be turned off seperately. so in
   order to get rid of these warnings (which are being handled as errors), we
   would need to disable -Wall. and i think setting WERROR= in order to handle
   all warnings for specific code as warnings rather than as errors is the
   better solution.

i've reworked the patch to only remove WERROR=, where it is not needed anymore
for any supported arch, or where it can be wrapped in a compiler condition.

cheers.
alex

> 
>  - Philip
> 
> -- 
> Philip Paeps
> Senior Reality Engineer
> Ministry of Information
-------------- next part --------------
Index: sys/modules/xfs/Makefile
===================================================================
--- sys/modules/xfs/Makefile	(revision 228911)
+++ sys/modules/xfs/Makefile	(working copy)
@@ -6,8 +6,6 @@
 
 KMOD=	 xfs
 
-WERROR=
-
 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 228911)
+++ 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
-WERROR=
 
 .include <bsd.kmod.mk>
Index: sys/modules/aic7xxx/ahd/Makefile
===================================================================
--- sys/modules/aic7xxx/ahd/Makefile	(revision 228911)
+++ sys/modules/aic7xxx/ahd/Makefile	(working copy)
@@ -4,7 +4,6 @@
 .PATH:	${.CURDIR}/../../../dev/aic7xxx
 KMOD=	ahd
 
-WERROR=
 GENSRCS= aic79xx_seq.h aic79xx_reg.h
 REG_PRINT_OPT=
 AHD_REG_PRETTY_PRINT=1
Index: sys/modules/agp/Makefile
===================================================================
--- sys/modules/agp/Makefile	(revision 228911)
+++ 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
-WERROR=
 
 EXPORT_SYMS=	agp_find_device		\
 		agp_state		\
Index: sys/modules/bios/smapi/Makefile
===================================================================
--- sys/modules/bios/smapi/Makefile	(revision 228911)
+++ 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
-WERROR=
 .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 228911)
+++ sys/modules/nve/Makefile	(working copy)
@@ -7,7 +7,9 @@
 	device_if.h bus_if.h pci_if.h miibus_if.h \
 	os+%DIKED-nve.h
 OBJS+=	nvenetlib.o
+.if ${CC:T:Mclang} == "clang"
 WERROR=
+.endif
 
 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