Date: Mon, 26 Dec 2011 10:10:40 +0000 From: Alexander Best <arundel@freebsd.org> To: current@freebsd.org Cc: freebsd-toolchain@freebsd.org, freebsd-arch@freebsd.org Subject: [rfc] removing/conditionalising WERROR= in Makefiles Message-ID: <20111226101040.GA6361@freebsd.org>
next in thread | raw e-mail | index | archive | help
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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@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.
cheers.
alex
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="remove-werror.diff"
Index: sys/modules/xfs/Makefile
===================================================================
--- sys/modules/xfs/Makefile (revision 228878)
+++ 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 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
-WERROR=
.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
-WERROR=
GENSRCS= aic79xx_seq.h aic79xx_reg.h
REG_PRINT_OPT=
AHD_REG_PRETTY_PRINT=1
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"
WERROR=
+.endif
.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
-WERROR=
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
-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 228878)
+++ 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 ${MACHINE_CPUARCH} == "i386" && ${CC:T:Mclang} == "clang"
WERROR=
+.endif
CLEANFILES+= nvenetlib.o os+%DIKED-nve.h
nvenetlib.o: ${.CURDIR}/../../contrib/dev/nve/${MACHINE}/${.TARGET}.bz2.uu
--W/nzBZO5zC0uMSeA--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111226101040.GA6361>