From owner-freebsd-arch@FreeBSD.ORG Tue Dec 27 11:27:43 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1233) id 2183D1065678; Tue, 27 Dec 2011 11:27:43 +0000 (UTC) Date: Tue, 27 Dec 2011 11:27:43 +0000 From: Alexander Best To: freebsd-current@freebsd.org, freebsd-arch@freebsd.org Message-ID: <20111227112743.GA154@freebsd.org> References: <20111226101040.GA6361@freebsd.org> <20111227010449.GA6244@twoflower.paeps.cx> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Dxnq1zWXvFF0Q93v" Content-Disposition: inline In-Reply-To: <20111227010449.GA6244@twoflower.paeps.cx> Cc: Subject: Re: [rfc] removing/conditionalising WERROR= in Makefiles X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Dec 2011 11:27:43 -0000 --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue Dec 27 11, Philip Paeps wrote: > On 2011-12-26 10:10:40 (+0000), Alexander Best 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 --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="remove-werror.diff2" 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 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 --Dxnq1zWXvFF0Q93v--