Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Windows builds with clang-cl.exe #11974

Closed
htuch opened this issue Jul 9, 2020 · 11 comments · Fixed by #14668
Closed

Windows builds with clang-cl.exe #11974

htuch opened this issue Jul 9, 2020 · 11 comments · Fixed by #14668
Assignees
@wrowe @sunjayBhatia
Labels
area/build area/windows no stalebot Disables stalebot from closing an issue

Comments

@htuch
Copy link
Member

htuch commented Jul 9, 2020

In order to reduce the need to engineer workarounds when interoperating with projects such as Chromium that build with clang-cl, and to provide greater consistency with the rest of our Clang-based builds, we should use clang-cl.exe instead of cl.exe. Both are compatible with MSVC.

The main blocker here appears to be google/googletest#2490.
@htuch htuch added area/windows no stalebot Disables stalebot from closing an issue labels Jul 9, 2020
@htuch htuch mentioned this issue Jul 9, 2020
Merged
@htuch
Copy link
Member Author

htuch commented Jul 22, 2020

@wrowe I've been told that the gmock issue may no longer apply (see https://chromium-review.googlesource.com/c/chromium/src/+/2310189). Unclear if this works in Clang 10 or requires Clang 11.

@sunjayBhatia
Copy link
Member

See envoyproxy/envoy-build-tools#86 for installing clang in the Windows build container

@sunjayBhatia
Copy link
Member

Current issues we faced building locally:

  • curl does not build as expected, extra arguments are passed to the llvm link tool that are not relevant for libraries, this was happening with fastbuild, have not yet tried with opt, we've seen major differences in foreign cc components with different build modes (CMake projects treatment of debug builds is often faulty) so that could be something to explore
  • clang is requiring all mocks to have the override specifier, which may ultimately be correct, but clang on non-Windows does not warn/error on this
    • as a general point, we need to figure out the correct set of flags to pass to clang-cl to be consistent with non-Windows clang
  • We still need to figure out the flags/options to set in the clang-cl rbe toolchain

@sunjayBhatia
Copy link
Member

See envoyproxy/envoy-build-tools#88 for additions and issues with the clang-cl toolchain config

@sunjayBhatia
Copy link
Member

See also bazelbuild/bazel-toolchains#901

@sunjayBhatia
Copy link
Member

Still blocked on envoyproxy/envoy-build-tools#88 getting merged, cc @lizan when you get a chance

@sunjayBhatia sunjayBhatia mentioned this issue Aug 21, 2020
Merged
@sunjayBhatia
Copy link
Member

See #12776 for clang-cl RBE toolchain

@wrowe
Copy link
Contributor

wrowe commented Aug 24, 2020

We are currently working on a fork locally to determine the major roadblocks, while using the newly created toolchain and docker image. Expecting to create a PR shortly to demonstrate remaining breakage and move this forward.

@sunjayBhatia
Copy link
Member

See #12946 for some progress, draft PR to add clang-cl to CI

@sunjayBhatia sunjayBhatia mentioned this issue Sep 2, 2020
Closed
@sunjayBhatia sunjayBhatia mentioned this issue Sep 17, 2020
Merged
@sunjayBhatia
Copy link
Member

See https://github.com/envoyproxy/envoy/pull/13133/files for some modified clang-cl flags in the updated .bazelrc that are required

@wrowe wrowe mentioned this issue Sep 22, 2020
Merged
@wrowe wrowe added this to the windows-beta milestone Oct 16, 2020
@sunjayBhatia
Copy link
Member

See #13688 as a tracking draft PR

some outstanding issues it seems with clang-cl have us blocked

@sunjayBhatia sunjayBhatia mentioned this issue Oct 30, 2020
Merged
@wrowe wrowe mentioned this issue Nov 21, 2020
Closed
htuch pushed a commit that referenced this issue Dec 1, 2020
@sunjayBhatia @wrowe
Windows: enable tests and envoy-static.exe pdb file (#13688)
2745d66 
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)

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 wrowe modified the milestones: windows-beta, 1.17.0 Dec 22, 2020
@wrowe wrowe removed this from the 1.17.0 milestone Jan 6, 2021
@wrowe wrowe modified the milestones: windows-beta, 1.17.0 Jan 6, 2021
@wrowe wrowe mentioned this issue Jan 12, 2021
Merged
@wrowe wrowe removed this from the windows-ga milestone Jan 15, 2021
htuch pushed a commit that referenced this issue Jan 29, 2021
@wrowe @sunjayBhatia
Step 6: Add clang cl pipeline for Windows (#14668)
bad818c 
Now that we have regenerated envoy build tools to update bazel and llvm, and picked up the llvm 11 tooling for windows on envoyproxy/master, finally introduce a clang-cl pipeline based on clang-cl

Risk Level: low (new facility)
Testing: local
Docs Changes: TBD
Release Notes: TBD
Platform Specific Features: Windows clang-cl compilation
Replaces #14135
Fixes #11974

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
Co-authored-by: Sunjay Bhatia <sunjayb@vmware.com>
@davinci26 davinci26 mentioned this issue Mar 5, 2021
Closed
@keith keith mentioned this issue Apr 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wrowe wrowe

@sunjayBhatia sunjayBhatia

Labels
area/build area/windows no stalebot Disables stalebot from closing an issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Step 6: Add clang cl pipeline for Windows wrowe/envoy
Step 5: Add clang cl pipeline for Windows
4 participants
@wrowe @sunjayBhatia @htuch and others