Add CI setup that builds and tests Nighthawk using gcc #382

qqustc · 2020-06-23T14:02:23Z

error msg:

/external/envoy/test/mocks/config/BUILD:11:1: C++ compilation of rule '@envoy//test/mocks/config:config_mocks' failed (Exit 1) gcc failed: error executing command /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer '-std=c++0x' -MD -MF ...

Use --sandbox_debug to see verbose messages from the sandbox
In file included from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/upstream/_virtual_includes/cluster_manager_interface/envoy/upstream/cluster_manager.h:15,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/factory_context_interface/envoy/server/factory_context.h:26,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/filter_config_interface/envoy/server/filter_config.h:10,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/config_provider_manager_interface/envoy/config/config_provider_manager.h:6,
                 from external/envoy/test/mocks/config/mocks.h:3,
                 from external/envoy/test/mocks/config/mocks.cc:1:
bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/grpc_mux_interface/envoy/config/grpc_mux.h:75:16: error: 'virtual void Envoy::Config::GrpcMux::pause(std::vector<std::__cxx11::basic_string<char> >)' was hidden [-Werror=overloaded-virtual]
   75 |   virtual void pause(const std::vector<std::string> type_urls) PURE;
      |                ^~~~~
In file included from external/com_google_googletest/googlemock/include/gmock/gmock.h:61,
                 from external/envoy/test/test_common/utility.h:35,
                 from external/envoy/test/mocks/config/mocks.h:15,
                 from external/envoy/test/mocks/config/mocks.cc:1:
external/envoy/test/mocks/config/mocks.h:74:21: error:   by 'virtual testing::internal::Function<void(const std::__cxx11::basic_string<char>&)>::Result Envoy::Config::MockGrpcMux::pause(testing::internal::ElemFromListImpl<const std::__cxx11::basic_string<char>&, 0, 0>::type)' [-Werror=overloaded-virtual]
   74 |   MOCK_METHOD(void, pause, (const std::string& type_url));
      |                     ^~~~~
In file included from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/upstream/_virtual_includes/cluster_manager_interface/envoy/upstream/cluster_manager.h:15,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/factory_context_interface/envoy/server/factory_context.h:26,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/filter_config_interface/envoy/server/filter_config.h:10,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/config_provider_manager_interface/envoy/config/config_provider_manager.h:6,
                 from external/envoy/test/mocks/config/mocks.h:3,
                 from external/envoy/test/mocks/config/mocks.cc:1:
bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/grpc_mux_interface/envoy/config/grpc_mux.h:106:16: error: 'virtual bool Envoy::Config::GrpcMux::paused(std::vector<std::__cxx11::basic_string<char> >) const' was hidden [-Werror=overloaded-virtual]
  106 |   virtual bool paused(const std::vector<std::string> type_urls) const PURE;
      |                ^~~~~~
In file included from external/com_google_googletest/googlemock/include/gmock/gmock.h:61,
                 from external/envoy/test/test_common/utility.h:35,
                 from external/envoy/test/mocks/config/mocks.h:15,
                 from external/envoy/test/mocks/config/mocks.cc:1:
external/envoy/test/mocks/config/mocks.h:76:21: error:   by 'virtual testing::internal::Function<bool(const std::__cxx11::basic_string<char>&)>::Result Envoy::Config::MockGrpcMux::paused(testing::internal::ElemFromListImpl<const std::__cxx11::basic_string<char>&, 0, 0>::type) const' [-Werror=overloaded-virtual]
   76 |   MOCK_METHOD(bool, paused, (const std::string& type_url), (const));
      |                     ^~~~~~
In file included from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/upstream/_virtual_includes/cluster_manager_interface/envoy/upstream/cluster_manager.h:15,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/factory_context_interface/envoy/server/factory_context.h:26,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/server/_virtual_includes/filter_config_interface/envoy/server/filter_config.h:10,
                 from bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/config_provider_manager_interface/envoy/config/config_provider_manager.h:6,
                 from external/envoy/test/mocks/config/mocks.h:3,
                 from external/envoy/test/mocks/config/mocks.cc:1:
bazel-out/k8-fastbuild/bin/external/envoy/include/envoy/config/_virtual_includes/grpc_mux_interface/envoy/config/grpc_mux.h:90:16: error: 'virtual void Envoy::Config::GrpcMux::resume(std::vector<std::__cxx11::basic_string<char> >)' was hidden [-Werror=overloaded-virtual]
   90 |   virtual void resume(const std::vector<std::string> type_urls) PURE;
      |                ^~~~~~
In file included from external/com_google_googletest/googlemock/include/gmock/gmock.h:61,
                 from external/envoy/test/test_common/utility.h:35,
                 from external/envoy/test/mocks/config/mocks.h:15,
                 from external/envoy/test/mocks/config/mocks.cc:1:
external/envoy/test/mocks/config/mocks.h:75:21: error:   by 'virtual testing::internal::Function<void(const std::__cxx11::basic_string<char>&)>::Result Envoy::Config::MockGrpcMux::resume(testing::internal::ElemFromListImpl<const std::__cxx11::basic_string<char>&, 0, 0>::type)' [-Werror=overloaded-virtual]
   75 |   MOCK_METHOD(void, resume, (const std::string& type_url));
      |                     ^~~~~~

The text was updated successfully, but these errors were encountered:

oschaaf · 2020-06-23T14:08:10Z

That looks like the Envoy dependency is failing to build. I wonder if this is specific to gcc or a specific gcc version?
(CI seems to be doing fine on the current head of master here, but looking at do_ci.sh we no longer seem to have any test in there that uses gcc as far as I can tell, it's all clang these days).

qqustc · 2020-06-23T14:42:29Z

Thanks Otto.
If that is the case, should I close this issue then?

oschaaf · 2020-06-23T15:05:32Z

I'm not sure; maybe we should ask around over at Envoy's Slack channel. If gcc support was intentionally dropped then there's not much we can do about it. If it's not, this may resolve when we update the Envoy dependency at some point (or we go in and fix it if we want to)

oschaaf · 2020-06-23T16:24:18Z

@htuch ^^ do you have any thoughts on this?

htuch · 2020-06-24T00:39:53Z

gcc support is best effort, @dmitri-d do you know if there are known regressions here?

dmitri-d · 2020-06-24T00:51:50Z

It's a regression, noticed it last week. I wonder if I should just add --cxxopt -Wno-error=overloaded-virtual to .bazelrc. It's an annoying failure -- it keeps coming up for perfectly legitimate c++.

mum4k · 2020-06-24T13:48:41Z

Thanks for the updates here. If Envoy no longer supports gcc, neither should we. @qqustc please attempt to switch to clang and once you figure out the correct procedure (settings), we should document that in the Nighthawk repository.

Let's keep this issue to track the missing documentation.

oschaaf · 2020-06-24T14:22:21Z

We link to Envoy's documentation from our own docs for the first steps in the build procedure, including supported compiler versions. That has:

We now require Clang >= 5.0 due to known issues with std::string thread safety and C++14 support. 
GCC >= 7 is also known to work. Currently the CI is running with Clang 10.

So on the tin it says it is known to support GCC7++. Not sure how to take that :-)

This Envoy doc fragment leaves less wiggle room however:

Building Envoy has the following requirements:

GCC 7+ or Clang/LLVM 7+ (for C++14 support).

I'm not sure what the future looks like, but maybe if we do turn out to want to support GCC, maybe we should add
an explicit CI task for that to guard against it breaking without us knowing about it first.

dmitri-d · 2020-06-24T14:49:39Z

I'm not sure what the future looks like, but maybe if we do turn out to want to support GCC, maybe we should add
an explicit CI task for that to guard against it breaking without us knowing about it first.

It's a WIP: envoyproxy/envoy#10236

oschaaf · 2020-06-24T14:52:14Z

@dmitri-d awesome. @mum4k should we double down and add a CI check ourselves too when after envoyproxy/envoy#10236 lands to make sure we don't accidentally break GCC builds in our own code base?

mum4k · 2020-06-25T15:03:10Z

@oschaaf I think that is a very good idea.

On top of that we should verify if we can build with stock settings or do need any modification of .bazelrc, etc. If we do we should track that separately and add documentation to ease the entry for new developers. Unless we have that somewhere already?

@qqustc can you verify if you do need any custom configuration to build Nighthawk after envoyproxy/envoy#10236 lands?

qqustc · 2020-06-25T15:07:04Z

Yes, I will verify that after envoyproxy/envoy#10236 lands and update in this issue.

twghu · 2020-06-26T13:28:44Z

Some background in the causes of the above error:

The use of MOCK_METHOD is causing the hiding of the base class methods pause, paused and start.

Essentially the redeclaration of method using virtual in a base class (and here we have overloading with two different signature)

 virtual void pause(const std::string& type_url) PURE;
 virtual void pause(const std::vector<std::string> type_urls) PURE;

will:

"Adding virtual here creates a new virtual function that could be overridden in the derived classes of Derived itself. It doesn’t check that f in Derived overrides f in Base. The code compiles without a problem and lets the bug slip in." https://www.fluentcpp.com/2020/02/21/virtual-final-and-override-in-cpp/

The safest option is to use the following:

  // resolve hiding of base class method
  using GrpcMux::pause;
  using GrpcMux::paused;
  using GrpcMux::resume;

This indicates to the compiler to reference the base class methods rather than redefine.

This pattern is explained

The patch needed, that is tested and works with both gcc and clang is:

diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h
index 99a6a08bc..c9bfeae56 100644
--- a/test/mocks/config/mocks.h
+++ b/test/mocks/config/mocks.h
@@ -70,6 +70,11 @@ public:
   MockGrpcMux();
   ~MockGrpcMux() override;

+  // resolve hiding of base class method
+  using GrpcMux::pause;
+  using GrpcMux::paused;
+  using GrpcMux::resume;
+
   MOCK_METHOD(void, start, ());
   MOCK_METHOD(void, pause, (const std::string& type_url));
   MOCK_METHOD(void, resume, (const std::string& type_url));
(END)

- First iteration on testing gcc support in CI. This restricts the task to just building Nighthawk, and not testing it yet, as that needs a change in Envoy to go in first. - Small cleanup, and a bugfix for those who use the script to run locally, which would be detected as running in CircleCI. Related issue: #382 (needs a follow up to close, left in as a code-level todo). Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Also, run gcc-built tests in CI because now we can. Fixes envoyproxy#382 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Also, run gcc-built tests in CI because now we can. Fixes envoyproxy#382 ----- Prerequisite: envoyproxy/envoy#12057 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

- First iteration on testing gcc support in CI. This restricts the task to just building Nighthawk, and not testing it yet, as that needs a change in Envoy to go in first. - Small cleanup, and a bugfix for those who use the script to run locally, which would be detected as running in CircleCI. Related issue: envoyproxy#382 (needs a follow up to close, left in as a code-level todo). Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Update Envoy + run gcc-built tests in CI because now we can. Fixes #382 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

mum4k assigned qqustc Jun 24, 2020

mum4k added the enhancement New feature or request label Jun 24, 2020

mum4k changed the title ~~bazel test test:all failed to build at master branch~~ Add documentation on how to buildNighthawk (clang instead of unsupported gcc) Jun 24, 2020

mum4k changed the title ~~Add documentation on how to buildNighthawk (clang instead of unsupported gcc)~~ Add CI setup that builds and tests Nighthawk using gcc Jun 25, 2020

mum4k unassigned qqustc Jun 25, 2020

twghu mentioned this issue Jun 26, 2020

Tests are now being executed during gcc builds envoyproxy/envoy#10236

Merged

oschaaf mentioned this issue Jul 3, 2020

CI: Test that we can produce artifacts with gcc #397

Merged

1 task

oschaaf added a commit to oschaaf/nighthawk that referenced this issue Jul 13, 2020

Update Envoy to 84affaad1173a1e48bbee5b3049c305a555eb3b2

2deb31e

Also, run gcc-built tests in CI because now we can. Fixes envoyproxy#382 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

oschaaf mentioned this issue Jul 13, 2020

Update Envoy to 7abb0e0bbed4f6b6304403b93762614ad385f80d #401

Merged

1 task

mum4k closed this as completed in #401 Jul 15, 2020

mum4k pushed a commit that referenced this issue Jul 15, 2020

Update Envoy to 7abb0e0bbed4f6b6304403b93762614ad385f80d (#401)

0b23bbf

Update Envoy + run gcc-built tests in CI because now we can. Fixes #382 Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CI setup that builds and tests Nighthawk using gcc #382

Add CI setup that builds and tests Nighthawk using gcc #382

qqustc commented Jun 23, 2020

oschaaf commented Jun 23, 2020

qqustc commented Jun 23, 2020

oschaaf commented Jun 23, 2020

oschaaf commented Jun 23, 2020

htuch commented Jun 24, 2020

dmitri-d commented Jun 24, 2020 •

edited

Loading

mum4k commented Jun 24, 2020

oschaaf commented Jun 24, 2020

dmitri-d commented Jun 24, 2020

oschaaf commented Jun 24, 2020

mum4k commented Jun 25, 2020

qqustc commented Jun 25, 2020

twghu commented Jun 26, 2020

Add CI setup that builds and tests Nighthawk using gcc #382

Add CI setup that builds and tests Nighthawk using gcc #382

Comments

qqustc commented Jun 23, 2020

oschaaf commented Jun 23, 2020

qqustc commented Jun 23, 2020

oschaaf commented Jun 23, 2020

oschaaf commented Jun 23, 2020

htuch commented Jun 24, 2020

dmitri-d commented Jun 24, 2020 • edited Loading

mum4k commented Jun 24, 2020

oschaaf commented Jun 24, 2020

dmitri-d commented Jun 24, 2020

oschaaf commented Jun 24, 2020

mum4k commented Jun 25, 2020

qqustc commented Jun 25, 2020

twghu commented Jun 26, 2020

dmitri-d commented Jun 24, 2020 •

edited

Loading