From f45c52161d6d364495e589313ac6d5e63bbb251f Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Wed, 27 Dec 2017 11:13:20 -0800 Subject: [PATCH] Remove Werror from CXXFLAGS As newer & stricter C++ compiler versions are released, users run into issues when building and installing bmv2. While we should keep using Werror when developing and running CI tests, the flag should probably not be used by default. A new configure flags (`--enable-Werror`) can be used to add `Werror` to `CXXFLAGS`. Users should use the configure flags instead of adding `Werror` to `CXXFLAGS` themselves, as the project includes some auto-generated sources. --- CONTRIBUTING.md | 26 +++++++++++++++++++++++++ Dockerfile | 4 ++-- README.md | 15 +------------- configure.ac | 11 +++++++++-- targets/simple_switch_grpc/Makefile.am | 2 ++ targets/simple_switch_grpc/configure.ac | 11 ++++++++++- 6 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..96929abc8 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,26 @@ +You can fork the repo and submit a pull request in Github. For more information +send us an email (p4-dev@lists.p4.org). + +### Apache CLA + +All developers must sign the [P4.org](http://p4.org) CLA and return it to +(membership@p4.org) before making contributions. The CLA is available +[here](http://p4.org/wp-content/uploads/2015/07/P4_Language_Consortium_Membership_Agreement.pdf). + +### Coding guidelines + +Any contribution to the C++ core code (in particular the [bm_sim](src/bm_sim) +module) must respect the coding guidelines. We rely heavily on the [Google C++ +Style Guide](https://google.github.io/styleguide/cppguide.html), with some +differences listed in this repository's +[wiki](https://github.com/p4lang/behavioral-model/wiki/Coding-guidelines). Every +submitted pull request will go through our Travis tests, which include running +`cpplint.py` to ensure correct style and formatting. + +### Building locally + +We recommend that you build with a recent C++ compiler and with `-Werror` since +our CI tests use this flag. In order to build with the same flags as the CI +tester, please run `configure` with the following flags: + + ./configure --with-pdfixed --with-pi --with-stress-tests --enable-debugger --enable-Werror diff --git a/Dockerfile b/Dockerfile index 959a423b7..6324355dc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -45,8 +45,8 @@ RUN apt-get update && \ apt-get update && \ apt-get install -y --no-install-recommends $BM_DEPS $BM_RUNTIME_DEPS && \ ./autogen.sh && \ - if [ "$GCOV" != "" ]; then ./configure --with-pdfixed --with-stress-tests --enable-debugger --enable-coverage; fi && \ - if [ "$GCOV" = "" ]; then ./configure --with-pdfixed --with-stress-tests --enable-debugger; fi && \ + if [ "$GCOV" != "" ]; then ./configure --with-pdfixed --with-stress-tests --enable-debugger --enable-coverage --enable-Werror; fi && \ + if [ "$GCOV" = "" ]; then ./configure --with-pdfixed --with-stress-tests --enable-debugger --enable-Werror; fi && \ make && \ make install-strip && \ ldconfig && \ diff --git a/README.md b/README.md index b428833e8..7d1b8ef85 100644 --- a/README.md +++ b/README.md @@ -244,17 +244,4 @@ Please submit an issue with the appropriate label on ### How can I contribute ? -You can fork the repo and submit a pull request in Github. For more information -send us an email (p4-dev@p4.org). - -All developers must sign the [P4.org](http://p4.org) CLA and return it to -(membership@p4.org) before making contributions. The CLA is available -[here](http://p4.org/wp-content/uploads/2015/07/P4_Language_Consortium_Membership_Agreement.pdf). - -Any contribution to the C++ core code (in particular the [bm_sim](src/bm_sim) -module) must respect the coding guidelines. We rely heavily on the [Google C++ -Style Guide](https://google.github.io/styleguide/cppguide.html), with some -differences listed in this repository's -[wiki](https://github.com/p4lang/behavioral-model/wiki/Coding-guidelines). Every -submitted pull request will go through our Travis tests, which include running -`cpplint.py` to ensure correct style and formatting. +See [CONTRIBUTING.md](CONTRIBUTING.md). diff --git a/configure.ac b/configure.ac index 962740740..e5ce177a6 100644 --- a/configure.ac +++ b/configure.ac @@ -108,6 +108,10 @@ AC_ARG_WITH([pi], AM_CONDITIONAL([COND_PI], [test "$want_pi" = yes]) +AC_ARG_ENABLE([Werror], + AS_HELP_STRING([--enable-Werror], [Make all compiler warnings fatal]), + [enable_Werror="$enableval"], [enable_Werror=no]) + # Checks for programs. AC_PROG_CXX AC_PROG_CC @@ -215,9 +219,12 @@ AC_SUBST([AM_CPPFLAGS], ["$MY_CPPFLAGS \ -I\$(top_srcdir)/include \ -isystem\$(top_srcdir)/third_party/jsoncpp/include \ -isystem\$(top_srcdir)/third_party/spdlog"]) - -AC_SUBST([AM_CXXFLAGS], ["$PTHREAD_CFLAGS -Wall -Werror -Wextra"]) AC_SUBST([AM_CFLAGS], ["$PTHREAD_CFLAGS"]) +# Using ax_append_compile_flags requires copying 4 macro definitions from the +# autoconf archive to m4/ +MY_CXXFLAGS="-Wall -Wextra" +AS_IF([test "$enable_Werror" = "yes"], [MY_CXXFLAGS="$MY_CXXFLAGS -Werror"]) +AC_SUBST([AM_CXXFLAGS], ["$MY_CXXFLAGS $PTHREAD_CFLAGS"]) # Checks for typedefs, structures, and compiler characteristics. # not supported by autoconf 2.68, add to m4/ ? diff --git a/targets/simple_switch_grpc/Makefile.am b/targets/simple_switch_grpc/Makefile.am index e5adf6b54..f3550b4b4 100644 --- a/targets/simple_switch_grpc/Makefile.am +++ b/targets/simple_switch_grpc/Makefile.am @@ -110,5 +110,7 @@ AM_CPPFLAGS += -Icpp_out -Igrpc_out \ -I$(builddir)/cpp_out -I$(builddir)/grpc_out nodist_libbm_grpc_dataplane_la_SOURCES = $(proto_cpp_files) $(proto_grpc_files) +# no warnings or Werror for auto-generated sources +nodist_libbm_grpc_dataplane_la_CXXFLAGS = CLEANFILES = $(BUILT_SOURCES) proto_files.ts diff --git a/targets/simple_switch_grpc/configure.ac b/targets/simple_switch_grpc/configure.ac index 7808cd5c0..df17910f1 100644 --- a/targets/simple_switch_grpc/configure.ac +++ b/targets/simple_switch_grpc/configure.ac @@ -30,8 +30,17 @@ AC_TYPE_UINT32_T AC_TYPE_UINT64_T AC_TYPE_SIZE_T +AC_ARG_ENABLE([Werror], + AS_HELP_STRING([--enable-Werror], [Make all compiler warnings fatal]), + [enable_Werror="$enableval"], [enable_Werror=no]) + AC_SUBST([AM_CPPFLAGS], ["$MY_CPPFLAGS"]) -AC_SUBST([AM_CFLAGS], ["$PTHREAD_CFLAGS -Wall -Werror -Wextra"]) +AC_SUBST([AM_CFLAGS], ["$PTHREAD_CFLAGS"]) +# Using ax_append_compile_flags requires copying 4 macro definitions from the +# autoconf archive to m4/ +MY_CXXFLAGS="-Wall -Wextra" +AS_IF([test "$enable_Werror" = "yes"], [MY_CXXFLAGS="$MY_CXXFLAGS -Werror"]) +AC_SUBST([AM_CXXFLAGS], ["$MY_CXXFLAGS $PTHREAD_CFLAGS"]) PKG_CHECK_MODULES([PROTOBUF], [protobuf >= 3.0.0]) AC_SUBST([PROTOBUF_CFLAGS])