build-sys: Add --enable-werror, rework compiler flags

So...at some point we somehow lost `-Wall` in our
default compiler flags which means we were missing some
potentially important warnings.  And
we used to have `-Werror` on in CI which combined
with the above was strongly opinionated about not
landing warnings in git master.

Our default stance here remains the same; we have
an opinionated set of `-Werror=` that applies in
*all* configurations.   However that set moves
into Automake - I don't think we need to do
compiler version detection anymore, we can assume
a modern compiler.

We also add back in `-Wall` by default now.

Further in CI, add `-Werror`.  The implementation
here is in our buildsystem rather than
`export CXXFLAGS=-Werror` because unfortunately
we have to fix things in libdnf too, and I don't
want to block entirely on that.

Makefile.am

@@ -36,20 +36,28 @@ AM_CPPFLAGS += -DDATADIR='"$(datadir)"' \
 	-DRPM_OSTREE_FEATURES='"$(RPM_OSTREE_FEATURES)"' \
 	-DRPM_OSTREE_GITREV='"$(RPM_OSTREE_GITREV)"' \
 	-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_56 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_56
+# Compiler warnings
+warnings_skipped = sign-compare
+warnings_error = empty-body format=2 format-security format-nonliteral \
+  missing-declarations return-type switch overflow undef \
+  misleading-indentation missing-include-dirs unused-result \
+  $(NULL)
+warning_flags = -Wall \
+  $(patsubst %,-Wno-%,$(warnings_skipped)) \
+  $(patsubst %,-Werror=%,$(warnings_error)) \
+  $(NULL)
+if BUILDOPT_WERROR
+warning_flags += -Werror
+endif
 # We make some C warnings always a hard error; these should never happen in our code.
 # These flags are not valid for C++.
-warn_only_c_flags = -Werror=strict-prototypes \
+warnings_error_only_c = strict-prototypes missing-prototypes \
-  -Werror=missing-prototypes \
+  implicit-function-declaration int-conversion incompatible-pointer-types \
-  -Werror=implicit-function-declaration \
-  -Werror=int-conversion \
-  -Werror=incompatible-pointer-types \
-  -Werror=int-conversion \
   $(NULL)
-# Keep this in sync with the AM_CFLAGS in libostree; see
+# See the AM_CFLAGS in libostree for more information about -fno-strict-aliasing
-# that project for more information about e.g. -fno-strict-aliasing
+AM_CFLAGS += -std=gnu11 -fno-strict-aliasing $(warning_flags) $(patsubst %,-Werror=%,$(warnings_error_only_c))
-AM_CFLAGS += -std=gnu11 -fno-strict-aliasing $(WARN_CFLAGS) $(warn_only_c_flags)
 # Our default CXX flags
-AM_CXXFLAGS += -std=c++17 -fno-strict-aliasing $(WARN_CFLAGS)
+AM_CXXFLAGS += -std=c++17 -fno-strict-aliasing $(warning_flags)
 
 EXTRA_DIST += autogen.sh COPYING
 

ci/build-check.sh

@@ -7,5 +7,8 @@ set -xeuo pipefail
 
 dn=$(dirname $0)
 . ${dn}/libbuild.sh
-${dn}/build.sh
+# Hard fail on compiler warnings in CI.  We control our compiler
+# version as part of the coreos-assembler buildroot and expect
+# that to be clean.
+CONFIGOPTS="--enable-werror" ${dn}/build.sh
 make check

ci/clang-build-check.sh

@@ -8,5 +8,4 @@ set -xeuo pipefail
 dn=$(dirname $0)
 . ${dn}/libbuild.sh
 export CC=clang CXX=clang++
-${dn}/build.sh
+${dn}/build-check.sh
-make check

configure.ac

@@ -27,26 +27,6 @@ dnl if not set, which we definitely want; cmake doesn't do that.
 AC_PROG_CXX
 AM_PROG_CC_C_O
 
-dnl Keep this in sync with the version in ostree
-AS_IF([echo "$CFLAGS" | grep -q -E -e '-Werror($| )'], [], [
-CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\
-  -pipe \
-  -Werror=empty-body \
-  "-Werror=format=2 -Werror=format-security -Werror=format-nonliteral" \
-  -Werror=pointer-arith -Werror=init-self \
-  -Werror=missing-declarations \
-  -Werror=return-type \
-  -Werror=switch \
-  -Werror=overflow \
-  -Werror=parenthesis \
-  -Werror=undef \
-  -Werror=misleading-indentation \
-  -Werror=missing-include-dirs \
-  -Wstrict-aliasing=2 \
-  -Werror=unused-result \
-])])
-AC_SUBST(WARN_CFLAGS)
-
 AC_MSG_CHECKING([for -fsanitize=address in CFLAGS])
 if echo $CFLAGS | grep -q -e -fsanitize=address; then
 AC_MSG_RESULT([yes])
@@ -98,6 +78,12 @@ GTK_DOC_CHECK([1.15], [--flavour no-tmpl])
 AM_CONDITIONAL([ENABLE_GTK_DOC],[false])
 ])
 
+AC_ARG_ENABLE(werror,
+              AS_HELP_STRING([--enable-werror],
+                             [Enable -Werror for C/C++ (default: no)]),,
+              [enable_werror=no])
+AM_CONDITIONAL(BUILDOPT_WERROR, [test x$enable_werror != xno])
+
 AC_ARG_ENABLE(sqlite_rpmdb_default,
               AS_HELP_STRING([--enable-sqlite-rpmdb-default],
                              [Default to sqlite rpmdb backend (default: no)]),,