Skip to content

Commit

Permalink
Remove Werror from CXXFLAGS
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
antoninbas committed Dec 28, 2017
1 parent 3fe2ecf commit 4470738
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 20 deletions.
26 changes: 26 additions & 0 deletions 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
4 changes: 2 additions & 2 deletions Dockerfile
Expand Up @@ -48,8 +48,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-pi --with-stress-tests --enable-debugger --enable-coverage; fi && \
if [ "$GCOV" = "" ]; then ./configure --with-pdfixed --with-pi --with-stress-tests --enable-debugger; fi && \
if [ "$GCOV" != "" ]; then ./configure --with-pdfixed --with-pi --with-stress-tests --enable-debugger --enable-coverage --enable-Werror; fi && \
if [ "$GCOV" = "" ]; then ./configure --with-pdfixed --with-pi --with-stress-tests --enable-debugger --enable-Werror; fi && \
make && \
make install-strip && \
(test "$sswitch_grpc" = "yes" && \
Expand Down
15 changes: 1 addition & 14 deletions README.md
Expand Up @@ -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).
11 changes: 9 additions & 2 deletions configure.ac
Expand Up @@ -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
Expand Down Expand Up @@ -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/ ?
Expand Down
2 changes: 2 additions & 0 deletions targets/simple_switch_grpc/Makefile.am
Expand Up @@ -118,5 +118,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
11 changes: 10 additions & 1 deletion targets/simple_switch_grpc/configure.ac
Expand Up @@ -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])
Expand Down
2 changes: 1 addition & 1 deletion targets/simple_switch_grpc/switch_runner.cpp
Expand Up @@ -299,8 +299,8 @@ void
SimpleSwitchGrpcRunner::port_status_cb(
bm::DevMgrIface::port_t port, const bm::DevMgrIface::PortStatus status) {
_BM_UNUSED(port);
using PortStatus = bm::DevMgrIface::PortStatus;
#ifdef WITH_SYSREPO
using PortStatus = bm::DevMgrIface::PortStatus;
if (status == PortStatus::PORT_ADDED || status == PortStatus::PORT_REMOVED)
sysrepo_test->refresh_ports();
#else
Expand Down

0 comments on commit 4470738

Please sign in to comment.