Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg #13133

wrowe · 2020-09-16T18:47:22Z

Commit Message: Clean up build flags for clang + msvc opt/fastbuild/dbg
Additional Description:

Move what were '--config=msvc-cl' settings out of .bazelrc into the
default cflags for Windows compilation. This enables bazel build ...
and bazel test ... to simply work out of the box.
Cleans up .bazelrc lists of windows vs. clang-cl settings, dividing
the list into desired toggles vs. bazel or envoy bug workarounds
Fix errors in curl library creation in fastbuild on Windows
Introduce toggles and defaults for alwayslink argument to library objects.
Defaults remain the same for now.
Introduce, but do not toggle clang-cl flags (this will arrive in a
later PR when we resolve the opt build errors missing intrinsic base
class destructors in the mock classes)

Risk Level: low
Testing: local, msvc and clang-cl on windows
Docs Changes: n/a
Release Notes: n/a

wrowe · 2020-09-16T18:48:20Z

@envoyproxy/windows-dev note the resolution to Nick and Randy's observations about omitting -config msvc-cl, this should now work.

sunjayBhatia · 2020-09-17T18:10:35Z

/retest

repokitteh-read-only · 2020-09-17T18:10:40Z

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13133 (comment) was created by @sunjayBhatia.

see: more, trace.

repokitteh-read-only · 2020-09-17T21:09:22Z

CC @envoyproxy/dependency-watchers: FYI only for changes made to (bazel/repository_locations\.bzl)|(api/bazel/repository_locations\.bzl)|(.*/requirements\.txt).

🐱

Caused by: #13133 was synchronize by sunjayBhatia.

see: more, trace.

wrowe · 2020-09-20T04:35:21Z

/retest

repokitteh-read-only · 2020-09-20T04:35:26Z

Retrying Azure Pipelines, to retry CircleCI checks, use /retest-circle.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13133 (comment) was created by @wrowe.

see: more, trace.

wrowe · 2020-09-22T21:41:32Z

@envoyproxy/windows-dev for your inspection, this looks ready to run better msvc-cl and new clang-cl pipelines, and defaults should be more sensible for invoking bazel from the command line
.

ci/run_envoy_docker_windows_large_disk.sh

.bazelrc

wrowe · 2020-09-22T22:08:39Z

@htuch Just FYI that this has progressed r.e. #11974

It may be unwise to simply drop msvc-cl from the matrix, because that compiler is giving us additional insight into mistaken assumptions about C++ inherited from gcc/clang, but we'll zero in on one-or-the-other as "official binaries" coming from the project.

htuch · 2020-09-23T18:59:13Z

@wrowe great, I think this will pay off with our Chromium interop and other OSS libs, thanks, seems awesome progress.

- Move what were '--config=msvc-cl' settings out of .bazelrc into the default cflags for Windows compilation. This enables bazel build ... and bazel test ... to simply work out of the box. - Cleans up .bazelrc lists of windows vs. clang-cl settings, dividing the list into desired toggles vs. bazel or envoy bug workarounds - Fix errors in curl library creation in fastbuild on Windows - Introduce toggles and defaults for alwayslink argument to library objects. Defaults remain the same for now. - Introduce, but do not toggle clang-cl flags (this will arrive in a later PR when we resolve the opt build errors missing intrinsic base class destructors in the mock classes) Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]>

wrowe · 2020-10-02T22:10:35Z

This patch has been greatly simplified, it adds the minimal support for introducing some new features in future patches (such as exposing linkalways flags for potential overrides) and relocates a number of flags without changing the behaviors significantly.

It should continue to permit building fastbuild out-of-the-box by default without invoking ci/windows_build_steps.sh, but it will no longer introduce release 'opt' build debug .pdb's on Windows due to the memory constraints of the GCP RBE environment. We'll address that in a later patch in sync with changes to the GCP worker pool, and wait to introduce clang-cl build pipeline until those opt build bugs are sorted out.

The idea is to facilitate kicking off bazel build --config clang-cl, without addressing every defect until the issues can be sorted out.

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

Correct flag '--features' typo Signed-off-by: William A Rowe Jr <[email protected]>

- The CMP0091 rule cannot simply be disabled, it breaks any build where that rule isn't recognized by the installed cmake. - Applying a limit of rules applicable to CMake 3.16 breaks corrections made to later versions of CMake. - Using -U0 for the delta of this patch decreases the likelyhood it might not apply cleanly to the next iteration of curl. - This now corrects both windows msvc-cl fastbuild and all clang-cl builds. - All of this becomes irrelevant once this issue is addressed; #11816 Signed-off-by: William A Rowe Jr <[email protected]>

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

- With the correct CFLAGS handling, the flags are correctly passed from bazel while building curl using rules_foreign_cc. This override is no longer applicable. Signed-off-by: William A Rowe Jr <[email protected]>

bazel/foreign_cc/curl.patch

Signed-off-by: William A Rowe Jr <[email protected]>

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

wrowe · 2020-10-08T20:40:46Z

Note, this PR is a https://github.com/envoyproxy/envoy/milestone/16 dependency

wrowe · 2020-10-09T14:38:13Z

@lizan could you give this a final pass so we can move on to the clang-cl pipeline PR which depends on these updates?

lizan · 2020-10-10T08:38:46Z

.bazelrc

+# Drop the determinism feature (-DDATE etc are a no-op in msvc-cl)
+build:msvc-cl --action_env=USE_CLANG_CL=""
+build:msvc-cl --define clang_cl=0
+build:msvc-cl --features=-determinism


Why this is needed? it wasn't here before?

Three things going on.

Config msvc-cl is no longer strictly needed, those flags moved to bazel/envoy_*.bzl

When specifically requesting msvc-cl, we will disable clang-cl even if it the default in the dev's environment

We mark the build non-deterministic because the DATE and other timestamp overrides do absolutely nothing in msvc-cl, and that build cannot be deterministic. We are in a different, better situation with clang-cl, it is very noisy about overriding these intrinsic macros, but the reassignment takes place.

PR #13133 disabled various tests as they failed or flaked when compiled with clang-cl. Given we have some significant other blockers to adding clang-cl support and enabling as many tests to run in CI as possible is important to progress to beta/GA support, we can un-skip these tests. We could be "clever" and use a select to only tag tests to be skipped when built with a specific compiler and compilation mode as well as an alternative. Signed-off-by: Sunjay Bhatia <[email protected]>

wrowe added area/windows waiting labels Sep 17, 2020

wrowe marked this pull request as draft September 17, 2020 16:07

repokitteh-read-only bot removed the waiting label Sep 17, 2020

wrowe mentioned this pull request Sep 20, 2020

Draft: Windows: Add clang-cl CI job #12946

Closed

wrowe changed the title ~~Draft changes to better support -c fastbuild on Windows~~ Correct windows build flags for clang + msvc opt/fastbuild/dbg Sep 22, 2020

wrowe added the area/build label Sep 22, 2020

wrowe marked this pull request as ready for review September 22, 2020 21:45

wrowe requested a review from lizan September 22, 2020 21:46

sunjayBhatia suggested changes Sep 22, 2020

View reviewed changes

ci/run_envoy_docker_windows_large_disk.sh Outdated Show resolved Hide resolved

.bazelrc Show resolved Hide resolved

wrowe changed the title ~~Correct windows build flags for clang + msvc opt/fastbuild/dbg~~ Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg Sep 22, 2020

htuch assigned lizan Sep 23, 2020

wrowe requested review from alyssawilk, dio and mattklein123 as code owners September 26, 2020 04:41

wrowe marked this pull request as draft September 29, 2020 16:50

wrowe added 3 commits October 6, 2020 10:58

Merge remote-tracking branch 'origin/master' into fix-windows-fastbui…

b556bd5

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

Confirm redundant toggle, document issue/workaround

47634e8

Correct flag '--features' typo Signed-off-by: William A Rowe Jr <[email protected]>

wrowe added 2 commits October 6, 2020 21:52

Merge remote-tracking branch 'origin/master' into fix-windows-fastbui…

3eff4c5

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

Drop no-op flag

77c7249

- With the correct CFLAGS handling, the flags are correctly passed from bazel while building curl using rules_foreign_cc. This override is no longer applicable. Signed-off-by: William A Rowe Jr <[email protected]>

wrowe marked this pull request as ready for review October 7, 2020 05:40

lizan reviewed Oct 7, 2020

View reviewed changes

bazel/foreign_cc/curl.patch Show resolved Hide resolved

wrowe added 4 commits October 7, 2020 14:51

Note upstream patch submission

49cf93c

Signed-off-by: William A Rowe Jr <[email protected]>

Correct typo

1155f5c

Signed-off-by: William A Rowe Jr <[email protected]>

Revise patch to reflect PR discussion at curl

3520884

Signed-off-by: William A Rowe Jr <[email protected]>

Merge remote-tracking branch 'origin/master' into fix-windows-fastbui…

7ede887

…ld-default Signed-off-by: William A Rowe Jr <[email protected]>

wrowe added this to the windows-beta milestone Oct 8, 2020

lizan approved these changes Oct 10, 2020

View reviewed changes

lizan reviewed Oct 10, 2020

View reviewed changes

lizan merged commit 3c86d8d into envoyproxy:master Oct 11, 2020

wrowe deleted the fix-windows-fastbuild-default branch October 16, 2020 14:52

sunjayBhatia mentioned this pull request Oct 22, 2020

Windows: bring back tests disabled by #13133 #13711

Merged

wrowe removed this from the windows-ga milestone Jan 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg #13133

Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg #13133

wrowe commented Sep 16, 2020 •

edited

Loading

wrowe commented Sep 16, 2020

sunjayBhatia commented Sep 17, 2020

repokitteh-read-only bot commented Sep 17, 2020

repokitteh-read-only bot commented Sep 17, 2020

wrowe commented Sep 20, 2020

repokitteh-read-only bot commented Sep 20, 2020

wrowe commented Sep 22, 2020 •

edited

Loading

wrowe commented Sep 22, 2020

htuch commented Sep 23, 2020

wrowe commented Oct 2, 2020 •

edited

Loading

wrowe commented Oct 8, 2020

wrowe commented Oct 9, 2020

lizan Oct 10, 2020

wrowe Oct 10, 2020

lizan Oct 11, 2020

Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg #13133

Correct windows build flags for clang-cl/msvc-cl under opt/fastbuild/dbg #13133

Conversation

wrowe commented Sep 16, 2020 • edited Loading

wrowe commented Sep 16, 2020

sunjayBhatia commented Sep 17, 2020

repokitteh-read-only bot commented Sep 17, 2020

repokitteh-read-only bot commented Sep 17, 2020

wrowe commented Sep 20, 2020

repokitteh-read-only bot commented Sep 20, 2020

wrowe commented Sep 22, 2020 • edited Loading

wrowe commented Sep 22, 2020

htuch commented Sep 23, 2020

wrowe commented Oct 2, 2020 • edited Loading

wrowe commented Oct 8, 2020

wrowe commented Oct 9, 2020

lizan Oct 10, 2020

Choose a reason for hiding this comment

wrowe Oct 10, 2020

Choose a reason for hiding this comment

lizan Oct 11, 2020

Choose a reason for hiding this comment

wrowe commented Sep 16, 2020 •

edited

Loading

wrowe commented Sep 22, 2020 •

edited

Loading

wrowe commented Oct 2, 2020 •

edited

Loading