Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On 8/13/11 2:23 PM, Jim Meyering wrote: I'd start with -O2 -D_FORTIFY_SOURCE=2 and something like this subset of -Wall: -Warray-bounds -Wchar-subscripts -Wsequence-point gcc now has: -Werror= Make the specified warning into an error. The specifier for a warning is appended, for example -Werror=switch turns the warnings controlled by -Wswitch into errors. This switch takes a negative form, to be used to negate -Werror for specific warnings, for example -Wno-error=switch makes -Wswitch warnings not be errors, even when -Werror is in effect. There's quite a few warnings we could reasonably promote to errors like this. FESCO would be happy to listen to a proposal for such, if anyone feels like doing the research. - ajax -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
Jan Kratochvil wrote: On Tue, 09 Aug 2011 19:45:15 +0200, Matthew Garrett wrote: If a package fails to build in a mass rebuild because -Werror was enabled then that's additional work for several people to fix something that may not have ever actually been broken. 99% of warnings will not lead to user visible bugs. Finding that remaining 1% of bugs (warnings) takes more than 100x time than to fix the warnings. I base my -Werror recommendation on this assumption, YMMV. Nice argument. I agree wholeheartedly with your numbers and with the recommendation to use -Werror and as many -W___ options as you/upstream can bear. However, it does depend on the version of gcc you use, especially if you use some of the newer warning options. Not too long ago, coreutils was getting invalid warning/errors from F15's gcc 4.6.x. Luckily, the bug was fixed in gcc-4.7.x, so I opted to use the newer gcc and retain those rather aggressive warnings. Besides, they're enabled only when you configure coreutils with --enable-gcc-warnings. Whether to invest in enabling -Werror for all packages in a mass rebuild however is another question. There will be many build failures, and some will be unwarranted. Having non-upstream take time to avoid the warnings may not be productive, since there are many ways to avoid them, and the way you choose may not suit upstream. Also, do you want to invest in avoiding warnings that affect only test-related code? That said, if there are thick-skinned volunteers with the expertise and enough time/energy, enabling -Werror globally (or even in a few selected packages) and addressing failures would be great. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On 08/13/2011 10:51 AM, Jim Meyering wrote: Whether to invest in enabling -Werror for all packages in a mass rebuild however is another question. Pardon, but this is not a question, this is beyond reason and foolish. There will be many build failures, and some will be unwarranted. Exactly ... and ... depending upon the gcc version, OS and architecture being in use, many of them will be bogus. That said, if there are thick-skinned volunteers with the expertise and enough time/energy, enabling -Werror globally (or even in a few selected packages) and addressing failures would be great. Well, I don't see any thing great in this proposal - I find is silly and not even close to being discussworthy. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
Ralf Corsepius wrote: On 08/13/2011 10:51 AM, Jim Meyering wrote: Whether to invest in enabling -Werror for all packages in a mass rebuild however is another question. Pardon, but this is not a question, this is beyond reason and foolish. There will be many build failures, and some will be unwarranted. Exactly ... and ... depending upon the gcc version, OS and architecture being in use, many of them will be bogus. That said, if there are thick-skinned volunteers with the expertise and enough time/energy, enabling -Werror globally (or even in a few selected packages) and addressing failures would be great. Well, I don't see any thing great in this proposal - I find is silly and not even close to being discussworthy. Hi Ralf, Your opinion might carry more weight if you backed up your abrasive comments with data, code or anything else that might be construed as constructive. Toning down the abrasiveness (or, gasp! even trying to seem pleasant) would also have the advantage of encouraging people to participate rather than pushing them away. Whether to use -Werror as proposed really is not a black and white issue. It all depends on which warnings are enabled along with -Werror. Obviously, if you enable no other warning option, then there's no risk at all. If we're talking about just a few that are carefully chosen to minimize false positives, then a large percentage of the resulting build failures will correspond to real bugs. I'd start with -O2 -D_FORTIFY_SOURCE=2 and something like this subset of -Wall: -Warray-bounds -Wchar-subscripts -Wsequence-point That's just off the top of my head, since those are useful and, IME, seldom provoke false positives. Note also that if you choose carefully, then compiler/glibc versions need not come into play. It's only if you choose brand new (or otherwise unstable) warning options that the version of the compiler matters. Once you've resolved all of those, start adding ones like -Wformat=2 that, while still very useful, do evoke more false positives. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 2011-08-09 at 11:11 -0400, Tom Lane wrote: Looks fine to me. The only reason I have to dislike it is the temptation for people to inspect build logs as a proof of what flags a package was built with (since the only sane thing is to store that in the binary itself, which the tools team is working on). But for debugging build failures this is great. What happens in packages using a (possibly old) autoconf script that doesn't recognize --disable-silent-rules? IMO it would be better to add this option to the %configure calls in packages where it's actually an issue (which is surely a small minority, unless Colin has got evidence to the contrary). Hi, I would like you to give me an option to not use --disable-silent-rules, because it breaks waf build. I was just trying to build samba4 update for rawhide and I got this [1]: + ./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --disable-silent-rules --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-modulesdir=/usr/lib64/samba --with-lockdir=/var/lib/samba4 --with-piddir=/var/run --with-privatedir=/var/lib/samba4/private --with-sockets-dir=/var/run --sysconfdir=/etc/samba4 --datadir=/usr/share/samba --disable-gnutls --disable-rpath-install --builtin-libraries=ccan,wbclient '--bundled-libraries=heimdal,!talloc,!tdb,!tevent,!ldb,!zlib' waf [command] [options] Main commands (example: ./waf build -j4) build : build all targets clean : removes the build files configure : configures the project ctags : build 'tags' file using ctags dist: makes a tarball for distribution distcheck : test that distribution tarball builds and installs distclean : removes the build directory etags : build TAGS file using etags install : installs the build files pydoctor: build python apidocs reconfigure : reconfigure if config scripts have changed test: Run the test suite (see test options below) testonly: run tests without doing a build first uninstall : removes the installed files wafdocs : build wafsamba apidocs wildcard_cmd: called on a unknown command waf: error: no such option: --disable-silent-rules error: Bad exit status from /var/tmp/rpm-tmp.5kugM6 (%build) Bad exit status from /var/tmp/rpm-tmp.5kugM6 (%build) RPM build errors: Child returncode was: 1 EXCEPTION: Command failed. See logs for output. Thus even autoconf is fine, then waf build system is not. Could Colin fix this, please? Thanks in advance and bye, Milan [1]http://koji.fedoraproject.org/koji/getfile?taskID=3265556name=build.log -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Thu, Aug 11, 2011 at 5:56 AM, Milan Crha mc...@redhat.com wrote: I would like you to give me an option to not use --disable-silent-rules, because it breaks waf build. Ugh; pretty lame that waf chose to replicate all of the standard autoconf flags as well as some automake ones (--disable-dependency-tracking), but NOT replicate the behavior of ignoring unknown options. I'll work on a patch - I guess the RPM approach is more overrides rather than detecting things, so I'll go with adding an option. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Thu, Aug 11, 2011 at 9:43 AM, Colin Walters walt...@verbum.org wrote: I'll work on a patch - I guess the RPM approach is more overrides rather than detecting things, so I'll go with adding an option. Actually looking at this more, while waf does support the GNU autoconf options by loading gnu_dirs.py (and Samba does this), it actually doesn't support --disable-dependency-tracking. That bit lives in buildtools/wafsamba/wscript. So I think it makes sense to patch samba's wscript to also support --disable-silent-rules for now. It may make sense to also have an automake_compat.py in upstream waf which does something with the configure options shipped with Automake (which are currently dependency-tracking, maintainer-mode, multilib, and silent-rules)...and while we're in here an option to ignore unknown options =) I filed http://code.google.com/p/waf/issues/detail?id=1023 -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Thu, 2011-08-11 at 11:16 -0400, Colin Walters wrote: So I think it makes sense to patch samba's wscript to also support --disable-silent-rules for now. Hi, yup, I made it that way, for now. It may make sense to also have an automake_compat.py in upstream waf which does something with the configure options shipped with Automake (which are currently dependency-tracking, maintainer-mode, multilib, and silent-rules)...and while we're in here an option to ignore unknown options =) I filed http://code.google.com/p/waf/issues/detail?id=1023 Thanks, I hope they will take care of the issue better than me. Bye, Milan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
Hi Jan, On Tue, Aug 9, 2011 at 12:56 PM, Jan Kratochvil jan.kratoch...@redhat.com wrote: On Tue, 09 Aug 2011 16:44:31 +0200, Colin Walters wrote: the goal being that they see warnings more easily. You should make -Werror default instead, by compiling packages without -Werror various bugs creep in which would be much easier fixed before the compilation. I've gone back and forth on this over the years - what I ultimately concluded is that what one really wants in general is a system for tracking warning *regressions*. That's obviously harder, but I've been thinking about how to do it in our GNOME builds. Anyways - seems like there was rough consensus for the original patch so I've committed it, pushed and started a build for rawhide. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 2011-08-09 at 10:44 -0400, Colin Walters wrote: See attached. Looks fine to me. The only reason I have to dislike it is the temptation for people to inspect build logs as a proof of what flags a package was built with (since the only sane thing is to store that in the binary itself, which the tools team is working on). But for debugging build failures this is great. You appear to be in provenpackagers, as far as I'm concerned feel free to commit. Also, props for using the devel list as a development list. I've had an off-list request for exposing just the hardening bits of the rpm macros as their own variables (to make it easier to build part of a package hardened, ie the coreutils case), so I'll probably be spinning another update to redhat-rpm-macros soon anyway. - ajax signature.asc Description: This is a digitally signed message part -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
Adam Jackson a...@redhat.com writes: On Tue, 2011-08-09 at 10:44 -0400, Colin Walters wrote: See attached. Looks fine to me. The only reason I have to dislike it is the temptation for people to inspect build logs as a proof of what flags a package was built with (since the only sane thing is to store that in the binary itself, which the tools team is working on). But for debugging build failures this is great. What happens in packages using a (possibly old) autoconf script that doesn't recognize --disable-silent-rules? IMO it would be better to add this option to the %configure calls in packages where it's actually an issue (which is surely a small minority, unless Colin has got evidence to the contrary). regards, tom lane -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, Aug 9, 2011 at 11:11 AM, Tom Lane t...@redhat.com wrote: What happens in packages using a (possibly old) autoconf script that doesn't recognize --disable-silent-rules? Autoconf convention is to ignore unknown rules. And indeed, all that results is a warning: configure: WARNING: unrecognized options: --disable-silent-rules IMO it would be better to add this option to the %configure calls in packages where it's actually an issue (which is surely a small minority, unless Colin has got evidence to the contrary). I actually did this in the GNOME build system originally (pattern match for bits in configure), but after some discussion on the Yocto list we decided there the warning was harmless. https://lists.yoctoproject.org/pipermail/poky/2011-March/004944.html -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
Tom Lane t...@redhat.com writes: What happens in packages using a (possibly old) autoconf script that doesn't recognize --disable-silent-rules? Autoconf-generated configure scripts generally ignore unknown --enable and --with options (newer versions give a warning). Andreas. -- Andreas Schwab, sch...@redhat.com GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E And now for something completely different. -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 16:44:31 +0200, Colin Walters wrote: Various projects have been adding AM_SILENT_RULES from Automake to their Makefiles for developer convenience; the goal being that they see warnings more easily. It is inconvenient as one can no longer easily reproduce the compilation for various problems either of the toolchain itself or adjusting it when troubleshooting tools processing the output binaries. More reasons are listed in the follow Bug, package kernel already uses such mode, I got request for the normal verbose compilation WONTFIXed: https://bugzilla.redhat.com/show_bug.cgi?id=716563 Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 16:44:31 +0200, Colin Walters wrote: the goal being that they see warnings more easily. You should make -Werror default instead, by compiling packages without -Werror various bugs creep in which would be much easier fixed before the compilation. Regards, Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, Aug 09, 2011 at 06:56:21PM +0200, Jan Kratochvil wrote: On Tue, 09 Aug 2011 16:44:31 +0200, Colin Walters wrote: the goal being that they see warnings more easily. You should make -Werror default instead, by compiling packages without -Werror various bugs creep in which would be much easier fixed before the compilation. Never, ever ship software with -Werror enabled. It's a development-only option. You have no idea what gcc will decide is a warning in future, so it's effectively a Please break my build in six months toggle. -- Matthew Garrett | mj...@srcf.ucam.org -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 19:14:27 +0200, Adam Jackson wrote: If you're volunteering to fix and/or paper over all the spurious warnings gcc and glibc introduce with every phase of the moon, then sure. Yes, I do it for my component, GDB has -Werror default in development phases upstream. It cleans up the code, it finds various minor bugs etc. Otherwise -Werror would essentially mean never shipping anything ever again. So the maintainers either care about the warnings - and then they should use -Werror - or they do not care about the warnings - and then it does not matter regarding warning messages which and how many of them can be found on the Koji server in the log files. Please decide. As always I guess it should be per maintainer / per package. Something like default -Werror with easy opt-out. And we can forget about hiding the compilation commands useful for toolchain troubleshooting. Thanks, Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 19:16:54 +0200, Matthew Garrett wrote: Never, ever ship software with -Werror enabled. I agree - for source distribution. Yes, GDB releases have -Werror turned off. It's a development-only option. You have no idea what gcc will decide is a warning in future, so it's effectively a Please break my build in six months toggle. I believe -Werror is appropriate for .src.rpm as only .arch.rpm is what is being shipped to the real users. -Werror is only of concern to the package maintainer who should keep warnings under control. -Werror is probably the most easy way to keep them non-regressing. One can only argue -Werror is not appropriate for rpmbuild --rebuild by users. But they will see new warnings only in non-standardard distro/environment/configuration, such advanced users should be aware of everything how to deal with it anyway. Thanks, Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On 08/09/2011 07:50 PM, Jan Kratochvil wrote: On Tue, 09 Aug 2011 16:44:31 +0200, Colin Walters wrote: Various projects have been adding AM_SILENT_RULES from Automake to their Makefiles for developer convenience; the goal being that they see warnings more easily. It is inconvenient as one can no longer easily reproduce the compilation for various problems either of the toolchain itself or adjusting it when troubleshooting tools processing the output binaries. The quote is taken out of context. Please reread the whole message; this passage only reasons why various UPSTREAMS have chosen to use silent rules. The patch is all about globally enabling the verbose mode, exactly the same you were proposing in the kernel ticket. More reasons are listed in the follow Bug, package kernel already uses such mode, I got request for the normal verbose compilation WONTFIXed: https://bugzilla.redhat.com/show_bug.cgi?id=716563 Jan -- Kalev -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 19:39:55 +0200, Kalev Lember wrote: Please reread the whole message; this passage only reasons why various UPSTREAMS have chosen to use silent rules. The patch is all about globally enabling the verbose mode, exactly the same you were proposing in the kernel ticket. OK, sorry, I agree, I withdraw my inappropriate comment. Well, at least the topic of -Werror has been highlighted. Thanks, Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, Aug 09, 2011 at 07:34:48PM +0200, Jan Kratochvil wrote: On Tue, 09 Aug 2011 19:16:54 +0200, Matthew Garrett wrote: It's a development-only option. You have no idea what gcc will decide is a warning in future, so it's effectively a Please break my build in six months toggle. I believe -Werror is appropriate for .src.rpm as only .arch.rpm is what is being shipped to the real users. -Werror is only of concern to the package maintainer who should keep warnings under control. -Werror is probably the most easy way to keep them non-regressing. Adding an additional warning to gcc that triggers for a specific application doesn't make that application any more broken than it was before the warning was added. If a package fails to build in a mass rebuild because -Werror was enabled then that's additional work for several people to fix something that may not have ever actually been broken. Warnings are appropriate during development. -Werror may even make sense when packagers are performing local builds before upload. I just don't think there's any way that the improvement in quality it'd bring to the distribution outweighs the extra effort involved in maintaining it whenever the toolchain changes. -- Matthew Garrett | mj...@srcf.ucam.org -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On 08/09/2011 07:19 PM, Jan Kratochvil wrote: On Tue, 09 Aug 2011 19:14:27 +0200, Adam Jackson wrote: If you're volunteering to fix and/or paper over all the spurious warnings gcc and glibc introduce with every phase of the moon, then sure. Yes, I do it for my component, GDB has -Werror default in development phases upstream. Yes, and gdb's best configuration feature is --disable-werrors, without which it was non-compilable almost everywhere. It cleans up the code, it finds various minor bugs etc. Agreed, iff you are an upstream developer, otherwise not. In all other cases, it only causes minor issues (often negligible cosmetic stuff) being treated as errors, often causing Heisenbugs with each GCC/glibc release - Not worth mentioning the additional Heisenbugs you face when taking other OSes into consideration. Otherwise -Werror would essentially mean never shipping anything ever again. So the maintainers either care about the warnings - and then they should use -Werror - or they do not care about the warnings - and then it does not matter regarding warning messages which and how many of them can be found on the Koji server in the log files. Please decide. I can only second what others already said: Switch off -Werror, unless you have too much time. As always I guess it should be per maintainer / per package. Something like default -Werror with easy opt-out. It often is not, but requires heavy modifications to a package. Something which often is beyond the skills of an occasional Fedora packager. And we can forget about hiding the compilation commands useful for toolchain troubleshooting. Well, ... are you sure your package has it's include paths and it's CFLAGS right? You won't see this class of bugs with AM_SILENT_RULES. Ralf -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 2011-08-09 at 19:19 +0200, Jan Kratochvil wrote: On Tue, 09 Aug 2011 19:14:27 +0200, Adam Jackson wrote: If you're volunteering to fix and/or paper over all the spurious warnings gcc and glibc introduce with every phase of the moon, then sure. Yes, I do it for my component, GDB has -Werror default in development phases upstream. It cleans up the code, it finds various minor bugs etc. I didn't say for your component. Otherwise -Werror would essentially mean never shipping anything ever again. So the maintainers either care about the warnings - and then they should use -Werror - or they do not care about the warnings - and then it does not matter regarding warning messages which and how many of them can be found on the Koji server in the log files. Please decide. I wasn't arguing for or against finding warnings. I was commenting on the ability to see build failures. Admittedly, you were initially responding to Colin, not myself. - ajax signature.asc Description: This is a digitally signed message part -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
Re: [PATCH] macros: Globally add --disable-silent-rules to configure
On Tue, 09 Aug 2011 19:45:15 +0200, Matthew Garrett wrote: If a package fails to build in a mass rebuild because -Werror was enabled then that's additional work for several people to fix something that may not have ever actually been broken. 99% of warnings will not lead to user visible bugs. Finding that remaining 1% of bugs (warnings) takes more than 100x time than to fix the warnings. I base by -Werror recommendation on this assumption, YMMV. Warnings are appropriate during development. It also depends how much / if do you consider the packager to be also the package developer. I just don't think there's any way that the improvement in quality it'd bring to the distribution outweighs the extra effort involved in maintaining it whenever the toolchain changes. The new warnings in toolchain intend to point at more problematic/buggy points in the code. It seems you disagree with my 99-1 assumption above, I agree I do not have such statistics proven anywhere. Thanks, Jan -- devel mailing list devel@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/devel
