Windows: enable tests and envoy-static.exe pdb file #13688

sunjayBhatia · 2020-10-21T20:42:01Z

Commit Message: Windows: enable tests and envoy-static.exe pdb file
Additional Description:

Introduce debugging output for msvc-cl and clang-cl opt binaries in
order to be able to unwind stack traces from windows core files.
Mark additional tests fails on windows based on clang-cl build
observed test failures
Work around problematic googletests with clang-cl compilation
Solve problematic missing override declarations of mocks by temporarilly
adding -Wno-inconsistent-missing-override for clang. (It appears
googletest on clang-cl isn't handling something that our gcc, clang and
msvc-cl builds all ignore without the warning override.
Workaround THROW tests which googletest with clang-cl are framing as
as -Wdiscard errors.
Workaround nodiscard next() method error
Follow up about discarded results of singleton construction and other
discarded odd or apparently irrelevant function call results.
Holding on adding clang-cl CI until LLVM 11.0.0 can be used on Windows
with Bazel (requires Bazel 3.7.1 and 4.0.0 releases)

Risk Level: Low (only affects Windows build)
Testing: Verified locally
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
Related to #11974 (partially addresses)

sunjayBhatia · 2020-10-21T20:42:11Z

cc @envoyproxy/windows-dev

Solve problematic missing override declarations of mocks by temporarilly adding -Wno-inconsistent-missing-override for clang. (It appears googletest on clang-cl isn't handling something that our gcc, clang and msvc-cl builds all ignore without the warning override. Workaround THROW tests which googletest with clang-cl are framing as as -Wdiscard errors. Workaround nodiscard next() method error Follow up about discarded results of singleton construction and other discarded odd or apparently irrelevant function call results. Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com>

- Adds a second build clang-cl using the LLVM toolchain - Introduce debugging output for msvc-cl and clang-cl opt binaries in order to be able to unwind stack traces from windows core files. (required increasing the available heap on the RBE GCP workers) - Fix lld-link.exe errors caused by unexported destructors - Mark additional tests flaky/fails on windows based on clang-cl build observed test failures Potential additional changes for more efficient fastbuild linkage - Stop using alwayslink/whole_archive logic to link all //source/... libraries used by //test/... binaries, because this increases the .exe and .pdb output file sizes and computational time. This requires we identify the libraries which must be resolved at run-time such as singletons, filter configs, etc, vs. those libraries which do not need to be preserved in full, and then override any default alwayslink. - Simplify binary of the objects used. (-OPT:REF optimization) due to tremendous size of resulting .exe and .pdb files, one alwayslink is resolved. - Enable dynamic envoy libs and clib/stdlib for test fastbuild objects (this presumes the objects would always be invoked on the machine with the same compiler and dynamic runtime already installed.) Requires that we fix the luajit/moonjit patches to toggle the /MD or /MT flag based on bazel config (much like we vary their msvcbuild.bat file for debug vs. non-debug.) Potential changes to bazel rules_cc for better builds - Deduplicate `-ignore:4221` flag and system library names from link params. - Allow /DEBUG:FASTLINK to be replaced with /DEBUG:FULL or /DEBUG:NONE, and validate that FASTLINK results in faster linkage+invocation times for tests than /DEBUG:FULL. Also determine whether /DEBUG:NONE is in the spirit of "minimal debugging information" as documented by Bazel. Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com>

Add the broken_clang_cl_dtor tag to tests which cannot compile due to some windows-specific clang scope or optimization issue. Issue is observed when the class used is based on a : public virtual foo, base class, that ~foo() = default; destructor declaration is ignored, and the linker reports that foo's dtor symbol cannot be resolved at link time. It appears that when the base is redefined as : public foo the defect goes away. (The virtual base class may be nested in a non-virtual base class). It isn't possible to solve this by dropping every virtual class inherited, due to diamond inheritance patterns. This may be a compiler bug, or an optimization toggled by /O2 in clang-cl which they do not enable in -O2 on linux. Tableing this for the time being. Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com>

This restores commit 01c6aa7. This commit needs a finer grain of precision to avoid building on clang-cl but not avoid building on msvc-cl. Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com>

These fixes reflect that the timing on windows is often less synchronous than linux observations, due to the design of the underlying socket stack. - listener_impl_test TcpListenerImplTest.SetListenerRejectFractionIntermediate - Enable no-longer-failing tests (no failure observed in RBE or locally) //test/extensions/grpc_credentials/file_based_metadata:integration_test //test/extensions/transport_sockets/alts:tsi_handshaker_test //test/integration:http2_flood_integration_test //test/integration:http2_upstream_integration_test //test/integration:overload_integration_test //test/server:guarddog_impl_test Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com>

- These skipped tests are all missing exported destructors of pure virtual classes. Enable the compilation to fail the clang-cl build and demonstrate what needs to be fixed (this may be a clang-cl compiler bug, or a gmocks bug, or buried elsewhere, but they do not appear to be envoy source code bugs.) Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Signed-off-by: William A Rowe Jr <wrowe@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>

wrowe

Reassigning to @htuch since #11974 was instigated by himself.

Note this patch includes one commit of miscellaneous failure resolutions, which actually land in PR #13837 ... As I'm converting this to ready for review, we are done force-pushing changes, but we will merge master once that other straightforward PR is done and these will be simplified here.

Also, some 16 tests fail to compile. These are set up to all deliberately fail to build in the clang-cl pipeline after-the-fact, but the failures can be examined in that build pipeline's log. It's possible to merge this PR and simply skip those tests in the short-term, but if we can identify a fix in the coming week, we may as well hold off introducing a new pipeline till that's solved.

wrowe · 2020-10-30T19:23:25Z

bazel/envoy_binary.bzl

@@ -60,7 +60,7 @@ def _envoy_linkopts():
            "-pagezero_size 10000",
            "-image_base 100000000",
        ],
-        "@envoy//bazel:clang_cl_opt_build": [
+        "@envoy//bazel:windows_opt_build": [


This expands creation of a .pdb debuginfo symbol file to the opt build of msvc + clang, now that our rbe gcp pool has a larger working set of memory to collect that during the link phase.

wrowe · 2020-10-30T19:26:11Z

bazel/envoy_internal.bzl

@@ -54,7 +54,7 @@ def envoy_copts(repository, test = False):
               repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
               repository + "//bazel:fastbuild_build": [],
               repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
-               repository + "//bazel:windows_opt_build": [],
+               repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],


This collects debuginfo symbol info to the .obj files during the opt build of msvc + clang, only for non-test sources. For test sources, do not collect debuginfo for the hundreds of test executables. (We would build fastbuild or dbg individually to troubleshoot a crashing test.)

Can you add this to the .bzl comments so this intuition is kept for posterity?

I thought that this comment...

Simplify the amount of symbolic debug info for test binaries, targets listed in order from generic to increasing specificity.

did exactly that?

You might be technically correct here, the best kind of correct. I think this is not really as helpful as something like "do not collect debuginfo for the hundreds of test executables. (We would build fastbuild or dbg individually to troubleshoot a crashing test.)"

@htuch have a look at the new iteration and let me know if that's helpful.

wrowe · 2020-10-30T19:26:50Z

ci/windows_ci_steps.sh

+  skip_test_tags="${skip_test_tags},-skip_on_windows_clang,-fails_on_windows_clang"
+  fail_test_tags="${fail_test_tags},fails_on_windows_clang"
+fi
+


This extra logic will be discarded after all compile and runtime failures unique to clang-cl have been resolved.

wrowe · 2020-10-30T19:59:15Z

test/common/config/datasource_test.cc

@@ -480,7 +480,7 @@ TEST_F(AsyncDataSourceTest, BaseIntervalGreaterThanMaxInterval) {
  TestUtility::loadFromYamlAndValidate(yaml, config);
  EXPECT_TRUE(config.has_remote());

-  EXPECT_THROW_WITH_MESSAGE(std::make_unique<Config::DataSource::RemoteAsyncDataProvider>(
+  EXPECT_THROW_WITH_MESSAGE((void)std::make_unique<Config::DataSource::RemoteAsyncDataProvider>(
                                cm_, init_manager_, config.remote(), dispatcher_, random_, true,
                                [&](const std::string&) {}),
                            EnvoyException,


This is a wide open question, @htuch - is there someone of the google / googletest participants who could identify why this emits an error under clang-cl, but on no other architectures (such as gcc, linux clang, and not even msvc.)

We could add the explicit flag -Wno-discard-results for clang-cl, but we hadn't done this for other architectures, and I'm wondering if that would be wise. I've fixed a couple of discarded non-expectation results, but in this EXPECT_[NO]THROW googletest pattern, it does get a little wordy.

Thoughts?

Definitely would rather not have this void cast. I'll follow up on this offline, but one option is to build the cast explicitly into the EXPECT_THROW_WITH_MESSAGE macro, which is something we define.

LMK if this is still an issue on Clang 11.

This is still an issue with -Wall -Werror; however, it makes sense to keep this potential defect detection in source/... --- but it doesn't seem interesting to track discarded results in test/... sources. Moved the warning suppression to the appropriate tests-only copt logic for clang-cl builds, and left a handful of corrections for source/... --- including documentation of why the results are discarded.

wrowe · 2020-10-30T20:19:47Z

test/common/http/http2/BUILD

@@ -52,6 +54,8 @@ envoy_cc_test(
        "--runtime-feature-disable-for-tests=envoy.reloadable_features.new_codec_behavior",
    ],
    shard_count = 5,
+    # TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
+    tags = ["skip_on_windows_clang"],


This is what I'm looking for some possible assistance from google folks, @htuch - we are having serious problems deciphering why clang-cl is failing in a manner unique to building opt (/O2) on Windows using clang-cl, and in no other environments or compilers. This particular link failure reads as follows;

ERROR: C:/users/wrowe/dev/envoy/test/common/http/http2/BUILD:40:14: Linking of rule '//test/common/http/http2:codec_impl_test' failed (Exit 1) lld-link: error: undefined symbol: public: virtual __cdecl Envoy::Http::ConnectionCallbacks::~ConnectionCallbacks(void) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>, class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>)'::`1'::dtor$69) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>, class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>)'::`1'::dtor$76) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: void __cdecl Envoy::Http::MockServerConnectionCallbacks::`vbase dtor'(void)) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: void __cdecl Envoy::Http::MockConnectionCallbacks::`vbase dtor'(void))>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: virtual __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::~Http2CodecImplTestFixture(void)) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: virtual __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::~Http2CodecImplTestFixture(void)) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(void)'::`1'::dtor$18) >>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(void)'::`1'::dtor$25) Target //test/common/http/http2:codec_impl_test failed to build

Any chance you can find some support for us researching what might be happening here?

Quick update for @htuch and @lizan - we attempted to verify this error with the llvm 11.0.0 release of clang-cl, however that update breaks bazel. Holding for this 1-line PR to be accepted to allow us to pick up the new clang-cl, and confirm whether the bug remains or has been resolved.

bazelbuild/rules_cc#88

/wait

This does appear to be resolved in clang 11.0.0 release.

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe · 2020-11-03T22:20:52Z

Currently stalled to pick up clang-cl 11 and reproduce the linkage error. See bazelbuild/rules_cc#88

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

- per htuch feedback keep these expectation simple - macros can be enhanced, but discard vs non-discard flavors would be required - work around the error messages for clang-cl only, and clean up c[xx]opt formatting Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

- This activates a few tests observed to fail locally, to confirm behavior in the CI pipeline when the clang-cl pipeline is added next week. Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com> Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com> Co-authored-by: William A Rowe Jr <wrowe@vmware.com> Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe · 2020-11-20T23:11:38Z

Apologies, had to force-push the DCO I'd missed in the first push.

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe · 2020-11-21T05:05:39Z

This is, finally, ready for final review and adoption ahead of integrating LLVM clang-cl in 11.0.0 (#14135).

We will need to watch in the coming days and mark any newfound exceptions promptly as "flaky_on_windows", but there are future patches in the pipeline to automate this. I'd only ask that we note which specific test(s) fail (or timeout) to make quick progress on diagnosing the last few flakes.

htuch · 2020-11-23T16:41:15Z

.bazelrc

@@ -326,6 +326,12 @@ build:clang-cl --define clang_cl=1
 # Override determinism flags (DATE etc) is valid on clang-cl compiler
 build:clang-cl --copt="-Wno-macro-redefined"
 build:clang-cl --copt="-Wno-builtin-macro-redefined"
+# Workaround problematic missing override declarations of mocks
+# TODO: resolve this class of problematic mocks, e.g.


Is the plan to do this in this PR?

Unclear whether 11.0.0 is going to fix this for us, whether we can fix this in a broad stroke, or whether every mock declaration affected must be updated and was missed in the last googletest update. This is why it was left unassigned.

Can you file a tracking issue?

htuch · 2020-11-23T16:42:15Z

bazel/envoy_internal.bzl

@@ -54,7 +54,7 @@ def envoy_copts(repository, test = False):
               repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
               repository + "//bazel:fastbuild_build": [],
               repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
-               repository + "//bazel:windows_opt_build": [],
+               repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],


Can you add this to the .bzl comments so this intuition is kept for posterity?

test/common/network/socket_option_test.h

htuch · 2020-11-23T16:44:18Z

test/extensions/transport_sockets/tls/ssl_socket_test.cc

-                                                             {"+", "%2B"},
-                                                             {"/", "%2F"},
-                                                             {"=", "%3D"}});
+        std::string urlencoded = absl::StrReplaceAll(


I think it would be better to split the test and build changes where possible, upstream these tests changes in a dedicated PR.

There are a ridiculously small number of source changes here, which are all easily understood from the primary commit context. If you insist, I'll split it.

I disagree with the definition of ridiculously small ;-) I'm not going to insist, but as a reviewer I think it makes life easier to split these concerns usually.

I've commented above on the one //source/... case we should consider peeling away from this patch, everything else is test or build.

There are no build changes that don't depend on the test changes, and vis-a-versa. So there would be no means to pass CI, so I'm really unclear on your ask.

htuch · 2020-11-23T16:45:21Z

test/test_common/environment.cc

@@ -342,8 +342,8 @@ std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,
 }

 std::string TestEnvironment::readFileToStringForTest(const std::string& filename,
-                                                     bool require_existence) {
-  std::ifstream file(filename, std::ios::binary);
+                                                     bool require_existence, bool read_binary) {


I'm a little worried about this proliferating in options; should we have a struct for options here? We can keep the existing signature for backwards compatibility as a wrapper to avoid changing the entire world.

I can't envision additional flags, so a struct seems like overkill.

Arguably it makes the tests that have been modified easier to read (should the default be true or false?), but I don't feel that strongly.

I'm personally a fan of bitwise flags, and was annoyed as we wrote this that named args aren't a c++ thing, so I'll rewrite it with enums if you insist. But I'd like a use case that suggests a third toggle before we bother.

wrowe

This appears complete to me. @lizan has identified issues with our coverage tests which suggest this will not be workable as bazel 3.7.1 lands, so there is less urgency and time for further discussion, @htuch.

wrowe · 2020-11-23T21:42:52Z

.bazelrc

@@ -326,6 +326,12 @@ build:clang-cl --define clang_cl=1
 # Override determinism flags (DATE etc) is valid on clang-cl compiler
 build:clang-cl --copt="-Wno-macro-redefined"
 build:clang-cl --copt="-Wno-builtin-macro-redefined"
+# Workaround problematic missing override declarations of mocks
+# TODO: resolve this class of problematic mocks, e.g.


Unclear whether 11.0.0 is going to fix this for us, whether we can fix this in a broad stroke, or whether every mock declaration affected must be updated and was missed in the last googletest update. This is why it was left unassigned.

wrowe · 2020-11-23T21:51:01Z

bazel/envoy_internal.bzl

@@ -54,7 +54,7 @@ def envoy_copts(repository, test = False):
               repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
               repository + "//bazel:fastbuild_build": [],
               repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
-               repository + "//bazel:windows_opt_build": [],
+               repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],


I thought that this comment...

Simplify the amount of symbolic debug info for test binaries, targets listed in order from generic to increasing specificity.

did exactly that?

test/common/network/socket_option_test.h

wrowe · 2020-11-23T22:00:30Z

test/extensions/transport_sockets/tls/ssl_socket_test.cc

-                                                             {"+", "%2B"},
-                                                             {"/", "%2F"},
-                                                             {"=", "%3D"}});
+        std::string urlencoded = absl::StrReplaceAll(


There are a ridiculously small number of source changes here, which are all easily understood from the primary commit context. If you insist, I'll split it.

htuch · 2020-11-23T23:23:53Z

.bazelrc

@@ -326,6 +326,12 @@ build:clang-cl --define clang_cl=1
 # Override determinism flags (DATE etc) is valid on clang-cl compiler
 build:clang-cl --copt="-Wno-macro-redefined"
 build:clang-cl --copt="-Wno-builtin-macro-redefined"
+# Workaround problematic missing override declarations of mocks
+# TODO: resolve this class of problematic mocks, e.g.


Can you file a tracking issue?

htuch · 2020-11-23T23:27:04Z

bazel/envoy_internal.bzl

@@ -54,7 +54,7 @@ def envoy_copts(repository, test = False):
               repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
               repository + "//bazel:fastbuild_build": [],
               repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
-               repository + "//bazel:windows_opt_build": [],
+               repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],


You might be technically correct here, the best kind of correct. I think this is not really as helpful as something like "do not collect debuginfo for the hundreds of test executables. (We would build fastbuild or dbg individually to troubleshoot a crashing test.)"

htuch · 2020-11-23T23:28:03Z

test/extensions/transport_sockets/tls/ssl_socket_test.cc

-                                                             {"+", "%2B"},
-                                                             {"/", "%2F"},
-                                                             {"=", "%3D"}});
+        std::string urlencoded = absl::StrReplaceAll(


I disagree with the definition of ridiculously small ;-) I'm not going to insist, but as a reviewer I think it makes life easier to split these concerns usually.

htuch · 2020-11-23T23:32:14Z

test/test_common/environment.cc

@@ -342,8 +342,8 @@ std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,
 }

 std::string TestEnvironment::readFileToStringForTest(const std::string& filename,
-                                                     bool require_existence) {
-  std::ifstream file(filename, std::ios::binary);
+                                                     bool require_existence, bool read_binary) {


Arguably it makes the tests that have been modified easier to read (should the default be true or false?), but I don't feel that strongly.

test/common/network/cidr_range_test.cc

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe · 2020-11-24T22:56:21Z

source/extensions/clusters/redis/redis_cluster.cc

-    std::next(it, rand_idx);
-    host = Upstream::HostSharedPtr{new RedisHost(parent_.info(), "", *it, parent_, true)};
+    host = Upstream::HostSharedPtr{
+        new RedisHost(parent_.info(), "", *std::next(it, rand_idx), parent_, true)};


@htuch - this is the only non-build, non-test change in this entire patchset. Would you like me to break it out for you as an independently reviewed PR, or are we ok to merge and iterate at this point? It's the remaining example of a discarded no-op result in the //source/... since I re-merged.

No, I was not asking for this. I was pointing out that the test changes alone could have been upstreamed independently. Anyway, NBD.

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>

htuch · 2020-12-01T02:01:00Z

/lgtm deps

* master: (70 commits) upstream: avoid reset after end_stream in TCP HTTP upstream (envoyproxy#14106) bazelci: add fuzz coverage (envoyproxy#14179) dependencies: allowlist CVE-2020-8277 to prevent false positives. (envoyproxy#14228) cleanup: replace ad-hoc [0, 1] value types with UnitFloat (envoyproxy#14081) Update docs for skywalking tracer (envoyproxy#14210) Fix some errors in the switch statement when decode dubbo response (envoyproxy#14207) Windows: enable tests and envoy-static.exe pdb file (envoyproxy#13688) http: add Kill Request HTTP filter (envoyproxy#14170) dependencies: fix release_dates error behavior. (envoyproxy#14216) thrift filter: support skip decoding data after metadata in the thrift message (envoyproxy#13592) update cares (envoyproxy#14213) docs: clarify behavior of hedge_on_per_try_timeout (envoyproxy#12983) repokitteh: add support for randomized auto-assign. (envoyproxy#14185) [grpc] validate grpc config for illegal characters (envoyproxy#14129) server: Return nullopt when process_context is nullptr (envoyproxy#14181) [Windows] Fix thrift proxy tests (envoyproxy#13220) kafka: add missing unit tests (envoyproxy#14195) doc: mention gperftools explicitly in PPROF.md (envoyproxy#14199) Removed `--use-fake-symbol-table` option. (envoyproxy#14178) filter contract: clarification around local replies (envoyproxy#14193) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>

sunjayBhatia mentioned this pull request Oct 22, 2020

Windows builds with clang-cl.exe #11974

Closed

mattklein123 assigned wrowe Oct 28, 2020

wrowe and others added 6 commits October 29, 2020 16:39

wrowe reviewed Oct 30, 2020

View reviewed changes

wrowe assigned htuch and unassigned wrowe Oct 30, 2020

sunjayBhatia changed the title ~~Draft: Windows: Add clang cl build~~ Windows: Add clang-cl pipeline Oct 30, 2020

sunjayBhatia marked this pull request as ready for review October 30, 2020 20:33

sunjayBhatia requested review from alyssawilk, asraa, dio, htuch, lizan, mattklein123, PiotrSikora and zuercher as code owners October 30, 2020 20:33

Fix bazel syntax

869b718

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe added area/build area/windows labels Oct 31, 2020

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

5451544

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

repokitteh-read-only bot added the waiting label Nov 9, 2020

wrowe added 2 commits November 10, 2020 13:31

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

747e250

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe added 2 commits November 20, 2020 20:25

Reintroduce flaky tag with detail based on CI observation

d575f73

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

446f5f5

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe mentioned this pull request Nov 21, 2020

Step 5: Add clang cl pipeline for Windows #14135

Closed

Re-flake a few tests observed in CI

f26c01a

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

htuch reviewed Nov 23, 2020

View reviewed changes

wrowe previously approved these changes Nov 23, 2020

View reviewed changes

htuch reviewed Nov 23, 2020

View reviewed changes

wrowe added 2 commits November 24, 2020 12:11

Quick doc fixes prompted by htuch

294dd22

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

dc68e63

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

wrowe dismissed their stale review via dc68e63 November 24, 2020 22:53

wrowe reviewed Nov 24, 2020

View reviewed changes

htuch previously approved these changes Nov 24, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

5a058de

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>

mattklein123 added the waiting label Nov 28, 2020

Merge remote-tracking branch 'origin/master' into add-clang-cl-build

779a753

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>

sunjayBhatia dismissed htuch’s stale review via 779a753 November 30, 2020 18:13

repokitteh-read-only bot removed the waiting label Nov 30, 2020

sunjayBhatia requested a review from htuch November 30, 2020 19:03

htuch approved these changes Dec 1, 2020

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Dec 1, 2020

htuch merged commit 2745d66 into envoyproxy:master Dec 1, 2020

sunjayBhatia deleted the add-clang-cl-build branch December 1, 2020 15:46

htuch mentioned this pull request Dec 4, 2020

Deflake xds_integration_test for Windows release builds #14286

Closed

Windows: enable tests and envoy-static.exe pdb file #13688

Windows: enable tests and envoy-static.exe pdb file #13688

Conversation

sunjayBhatia commented Oct 21, 2020 • edited Loading

sunjayBhatia commented Oct 21, 2020

wrowe left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrowe commented Nov 3, 2020

wrowe commented Nov 20, 2020

wrowe commented Nov 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wrowe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htuch commented Dec 1, 2020

sunjayBhatia commented Oct 21, 2020 •

edited

Loading

wrowe left a comment •

edited

Loading