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.

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

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
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
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
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
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

@sunjayBhatia
Copy link
Member

See #13688 as a tracking draft PR

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

htuch pushed a commit that referenced this issue Dec 1, 2020
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 removed this from the windows-ga milestone Jan 15, 2021
htuch pushed a commit that referenced this issue Jan 29, 2021
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build area/windows no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@wrowe @sunjayBhatia @htuch and others