GNOME Bugzilla – Bug 729407
Deprecate GNOME_COMPILE_WARNINGS
Last modified: 2015-10-28 11:54:04 UTC
After discussion with David at the developer hackfest, we think GNOME_COMPILE_WARNINGS could be deprecated and moved into autoconf-archive[1], since it has wider application than just within GNOME. The upstreamed macro could then be copied back into gnome-common, or copied into the m4 directory in each project which uses it. I suggest the latter, since it interacts quite closely with the module’s build system, so should be next to it rather than in an external module. [1]: http://www.gnu.org/software/autoconf-archive/
I’ve done some work on autoconf-archive-ing this here: https://github.com/peti/autoconf-archive/pull/4
Created attachment 294671 [details] [review] macros2: Deprecate GNOME_COMPILER_FLAGS Use AX_COMPILER_FLAGS instead, which comes from the autoconf-archive. It supports more compilers than just GCC, has a wider selection of flags, supports LDFLAGS, and is maintained for more projects than just GNOME. GNOME_COMPILER_FLAGS has not been ported to AX_COMPILER_FLAGS as: • the default argument is a little difficult to map to AX_COMPILER_FLAGS’ IS-RELEASE argument; and • AX_COMPILER_FLAGS depends on some other macros in autoconf-archive licenced as GPLv3, which would potentially be tricky to copy into gnome-common. No further changes should be made to GNOME_COMPILER_FLAGS: please make them to AX_COMPILER_FLAGS instead.
Created attachment 294672 [details] [review] macros2: Deprecate GNOME_COMPILE_WARNINGS Use AX_COMPILER_FLAGS instead, which comes from the autoconf-archive. It supports more compilers than just GCC, has a wider selection of flags, supports LDFLAGS, and is maintained for more projects than just GNOME. GNOME_COMPILE_WARNINGS has not been ported to AX_COMPILER_FLAGS as: • the default argument is a little difficult to map to AX_COMPILER_FLAGS’ IS-RELEASE argument; and • AX_COMPILER_FLAGS depends on some other macros in autoconf-archive licenced as GPLv3, which would potentially be tricky to copy into gnome-common. The --enable-iso-c argument is deprecated as well. Projects should choose which C standard they want to use, and hard-code that in their compiler flags. It is not something which should be set by the developer. GNOME_CXX_WARNINGS is not deprecated, but may be in future. No further changes should be made to GNOME_COMPILE_WARNINGS: please make them to AX_COMPILER_FLAGS instead.
Review of attachment 294672 [details] [review]: I do not think that it is a good idea to move the list of warning flags outside of gnome-common, as it becomes more difficult to ensure consistency among GNOME projects. It is considerably easier to make modifications to gnome-common than the autoconf archive, so deprecating GNOME_COMPILE_WARNINGS reduces the flexibility that we have of changing the flags.
(In reply to comment #4) > Review of attachment 294672 [details] [review]: > > I do not think that it is a good idea to move the list of warning flags outside > of gnome-common, as it becomes more difficult to ensure consistency among GNOME > projects. It is considerably easier to make modifications to gnome-common than > the autoconf archive, so deprecating GNOME_COMPILE_WARNINGS reduces the > flexibility that we have of changing the flags. I understand where you’re coming from, but: • The list of compiler warnings is essentially append-only, and changes infrequently. • The whole point of AX_COMPILER_FLAGS is to enable a set of warnings which it would be silly to compile _any_ code without — I see no reason why GNOME has to have special coding standards compared to other projects. What kind of modifications do you have in mind which would be difficult to make if the macro was in autoconf-archive? Additionally, my pull requests to autoconf-archive have been reviewed and accepted within a few days, every time. That’s no guarantee of future responsiveness, but at the moment I’m very impressed with the amount of upstream activity on the project.
(In reply to comment #5) > • The whole point of AX_COMPILER_FLAGS is to enable a set of warnings which it > would be silly to compile _any_ code without — I see no reason why GNOME has to > have special coding standards compared to other projects. gcc has warning categories for that (-Wall, -Wextra and the default set), and the GNOME_COMPILE_WARNINGS warnings go well beyond them. It is not silly to do so, but in my opinion it is quite particular to a project, especially marking certain warnings as errors, and not as generic as you make out. > What kind of modifications do you have in mind which would be difficult to make > if the macro was in autoconf-archive? Ones like you suggested in bug 743029 would be a good example.
(In reply to comment #6) > (In reply to comment #5) > > • The whole point of AX_COMPILER_FLAGS is to enable a set of warnings which it > > would be silly to compile _any_ code without — I see no reason why GNOME has to > > have special coding standards compared to other projects. > > gcc has warning categories for that (-Wall, -Wextra and the default set), and > the GNOME_COMPILE_WARNINGS warnings go well beyond them. It is not silly to do > so, but in my opinion it is quite particular to a project, especially marking > certain warnings as errors, and not as generic as you make out. A lot of these flags are useful, but are not in a gcc warning category. The gcc categories cannot be modified, as that could cause some code (e.g. code configured with -Wall -Werror) to stop compiling. So we need to add the flags to another layer above gcc. The question is whether that layer is specific to GNOME, or useful more generally. Are there any flags in the list which you particularly think are not generally applicable? > > What kind of modifications do you have in mind which would be difficult to make > > if the macro was in autoconf-archive? > > Ones like you suggested in bug 743029 would be a good example. I think a patch to support GIR in AX_COMPILER_FLAGS would be accepted upstream (though I can’t be sure without trying). I’m not exaggerating when I say they’re extremely responsive.
(In reply to comment #7) > A lot of these flags are useful, but are not in a gcc warning category. The gcc > categories cannot be modified, as that could cause some code (e.g. code > configured with -Wall -Werror) to stop compiling. I do not think that it is appropriate to attempt to support code that compiles with -Wall -Werror, as the base set of warnings included in -Wall can and does change with new releases of gcc. It is not a technique that should be encouraged, in my opinion. > So we need to add the flags > to another layer above gcc. The question is whether that layer is specific to > GNOME, or useful more generally. I think that the list of flags should be specific to a project, and is not generally applicable. A mechanism to take lists of flags and place them at different levels seems generally useful. > Are there any flags in the list which you particularly think are not generally > applicable? I think the format errors are rather questionable, given that gcc handled them poorly in older versions. This leads to an interesting point, in that it might make sense to restrict certain warning flags to certain compiler versions (although I would rather not have to think about it). I find that the strict-prototypes warning is also quite annoying, and for little gain. > I think a patch to support GIR in AX_COMPILER_FLAGS would be accepted upstream > (though I can’t be sure without trying). I do not think that adding GObject-Instrospection support to a generic compiler flags macro is appropriate. At least if there was a GNOME theme to the macro (GNOME_WARNING_FLAGS?), I might expect it, but I would be surprised if AX_COMPILER_FLAGS started changing GObject-Introspection warning options for me.
So, we discussed this in person at the DX hackfest, and decided: • GNOME_COMPILE_WARNINGS goes. • We drop the C++ support, as it has very little value and few users. • We drop the ISO C flag, as it should not be an option. Maintainers should decide what version of C they want to use and hard-code it in their CFLAGS. • AX_COMPILER_FLAGS grows support for GIR, and has CFLAGS and LDFLAGS support split out. • A baseline set of compiler flags is useful for all modules. If modules have a particular problem (e.g. the -Wformat false positives from a few GCC versions ago), they should use #pragmas to disable that specific warning for the few lines of code it breaks on. An updated version of AX_COMPILER_FLAGS is here: • https://github.com/peti/autoconf-archive/pull/13 I think the patch here is still ready to commit, and fits what we want.
Review of attachment 294672 [details] [review]: Yep, I am now convinced. Good job!
\o/ Attachment 294672 [details] pushed as b57bae0 - macros2: Deprecate GNOME_COMPILE_WARNINGS
AX_COMPILER_FLAGS() seems to default to -Werror, requiring a specific configure option to avoid that, with no way to change that default in your configure.ac. Surely -Werror by default is not widely wanted by package maintainers because it just makes life harder for packagers.
(In reply to Murray Cumming from comment #12) > AX_COMPILER_FLAGS() seems to default to -Werror, requiring a specific > configure option to avoid that, with no way to change that default in your > configure.ac. Surely -Werror by default is not widely wanted by package > maintainers because it just makes life harder for packagers. See the first paragraph of the documentation for AX_COMPILER_FLAGS: http://www.gnu.org/software/autoconf-archive/ax_compiler_flags.html tl;dr: -Werror is enabled by default *for development builds*. For release builds, it is disabled. You can also disable it by passing --disable-Werror to configure. Use AX_IS_RELEASE (or whatever other logic you want) to determine what counts as a release build.
Thanks. That's helpful.