Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Don't use -Werror by default #128

Closed
rhertzog opened this Issue · 5 comments

2 participants

@rhertzog

cpputest now fails to build with gcc 4.8 due a new warning. You should not use -Werror by default but only enable it for your own build (in some "maintainer mode").

See http://bugs.debian.org/713636 for the report I got but I believe it's already reported as #110 .

@rhertzog

Here's the patch that I used on the Debian package:

diff --git a/configure.ac b/configure.ac
index c55d789..80be739 100644
--- a/configure.ac
+++ b/configure.ac
@@ -67,11 +67,13 @@ saved_cflags="$CFLAGS"
 saved_cxxflags="$CXXFLAGS"
 saved_ldflags="$LDFLAGS"

+if test "x$USE_MAINTAINER_MODE" = "xyes"; then
 # FLag -Werror. 
 CFLAGS=-Werror
 AC_MSG_CHECKING([whether CC and CXX supports -Werror])
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([])], [AC_MSG_RESULT([yes]); CPPUTEST_CWARNINGFLAGS+=" -Werror"; CPPUTEST_CXXWARNINGFLAGS+=" -Werror" ], [AC_MSG_RESULT([no])])
 CFLAGS="$saved_cflags"
+fi

 # FLag -Weverything. 
 CFLAGS="-Werror -Weverything -Wno-unused-macros"
@basvodde
Owner

@rhertzog ... I understand the problem that the -Werror is causing in this case. I'm still very hesitant to move it into maintainer mode only. I don't think everyone who is contributing to CppUTest knows about AM_MAINTAINER_MODE. Also, the amount of reports we've gotten about obscure warnings from people who tried (and got an error) has certainly made CppUTest a better piece of software.

I'd propose the opposite. If you think we need an option to turn of -Werror in the configure script, then we could do that and that could be used by people who prefer to package it without -Werror. Would that work?

@rhertzog

You have AM_MAINTAINER_MODE([enable]) so it's enabled by default AFAIK. Thus I don't see any harm done and it's exactly the result that you expect, the Debian build uses --disable-maintainer-mode by default.

@basvodde
Owner

Ok, you are right. I've made the patch. There is no need for an urgent release for this for the Debian package, right?

@rhertzog

Nope, it's ok.

@basvodde basvodde closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.