GNOME Bugzilla – Bug 616275
-Werror should not be enabled by default (or should be possible to disable)
Last modified: 2010-07-12 15:11:21 UTC
While I understand that "Warnings are there for a reason" it causes disruptions for non-developer users which tries to build software. It may happen that simple reinstall of software using new compilator requires to fix a 'bug' which, while improving the code is simply annoying (see bug #616274).
Created attachment 159156 [details] [review] [PATCH] Make -Werror overridable Simply change of ordering allows user to write CFLAGS="-Wno-error" ./configure which disables errors. While I understand that developers should clean code and that users of git version are expected to be power-users (at least) it still should be overridable at least (I'd prefer to have it disabled by default but this is quick & simple compromise solution).
-Werror can be overridden by using the --enable-compile-warnings flag in the configure script, eg `./configure --enable-compile-warnings=no`
Review of attachment 159156 [details] [review]: I'd rather see a patch that adopts the code from gnome-shell's configure.ac and provides a command line option with the GNOME standard --enable-compile-warnings=yes ("no" means "no warnings", while "yes" means "non-fatal warnings"; --enable-compile-warnings=error is the default)
Created attachment 163630 [details] [review] add --enable-compile-warnings This is pretty much a straight copy from the gnome-shell code, but it seems to work just fine. Differentiation between the no/minimum/yes/maximum/error options might be something to think about adding in the future.
Review of attachment 163630 [details] [review]: This patch simply removes -Werror and makes the *other* warnings configurable. Behavior needs to be as with gnome-shell.
(In reply to comment #5) > Review of attachment 163630 [details] [review]: > > This patch simply removes -Werror and makes the *other* warnings configurable. > Behavior needs to be as with gnome-shell. Oops. Sorry, I can't believe I did that. This patch fixes that. Is there any other way that the behavior is inconsistent with that of gnome-shell? Also, is $enable_ansi a vestige of an earlier version of mutter, or the product of a macro? I ask because I don't see any other references to it within configure.in or configure. Assuming that it is supposed to be there, should the test for $enable_ansi = yes be inside or outside the test for enable_compile_warnings != no.
Created attachment 163956 [details] [review] new patch Disables warnings if --enable-compile-warnings = no, enables -Wall and friends for minimum, yes and maximum, and enables -Werror otherwise.
Review of attachment 163956 [details] [review]: - Commit message should say probably mention inspiration for the change ("The default --enable-warnings=error gives the previous behavior. Warnings can be made non-fatal with --enable-warnings=yes) - Yes, the $enable_ansi part is a left-over, and should be removed - I'd also remove the trailing section that adds -ansi; there's no reason that Mutter needs -ansi more than the rest of GNOME, where we don't use -ansi. (If we were going to leave it, I'd prefer to replace -ansi with the more self-documenting alternative --std=c89)
Created attachment 165064 [details] [review] Add --enable-compile-warnings option to configure script Removed -ansi and changed commit message to be more informative.
Created attachment 165065 [details] [review] Add --enable-compile-warnings option to configure script Fixed "typo" in commit message.
Pushed with some minor commit message changes. (Described the removal of -ansi, mostly)