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

Bump dependencies in Windows build container #113

Merged
merged 5 commits into from
Nov 19, 2020
Merged

Bump dependencies in Windows build container #113

merged 5 commits into from
Nov 19, 2020

Conversation

wrowe
Copy link
Contributor

@wrowe wrowe commented Nov 17, 2020

Bump Bazel versions to 3.6.0 and 3.7.0 and bump various Windows dependencies

Exclude bumping to LLVM 11 for now:

sunjayBhatia and others added 2 commits November 17, 2020 11:47
LLVM 11 bump may require Bazel 4.0.0 to avoid erronious LIB and INCLUDE
path construction, testing whether 3.7.0 can build this toolchain
itself, adjusting rules_cc specific to envoy in
  envoyproxy/envoy#13688

If this fails, we will need to wait till 2nd week December for Bazel 4.0.0
release.

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>
The anticipated flavors for envoy to build for clang-cl are 3.7.1/4.0.0,
these obviously do not exist, yet.

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Co-authored-by: William A Rowe Jr <wrowe@vmware.com>
@wrowe wrowe added the enhancement New feature or request label Nov 17, 2020
@wrowe wrowe requested a review from a team as a code owner November 17, 2020 18:51
@wrowe
Copy link
Contributor Author

wrowe commented Nov 17, 2020

FYI @envoyproxy/windows-dev folks, several significant bumps coming up, including MSVC toolchain to current

https://github.com/llvm/llvm-project/releases/download/llvmorg-10.0.0/LLVM-10.0.0-win64.exe `
893f8a12506f8ad29ca464d868fb432fdadd782786a10655b86575fc7fc1a562
https://github.com/llvm/llvm-project/releases/download/llvmorg-11.0.0/LLVM-11.0.0-win64.exe `
a773ee3519ecc8d68d91f0ec72ee939cbed8ded483ba8e10899dc19bccba1e22
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break clang-cl out of the box for developers. We haven't made any announcement of this, and llvm 10.0.0 had it's own list of optimization defects that didn't let us build a number of tests on Windows.

My suggestion is to ignore this breakage, it will be self-healing when the envoy WORKSPACE is updated to either bazel 3.7.1 or 4.0.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to pull the LLVM and Bazel version changes out into a separate draft PR? We can bump the other deps standalone in this PR and get them working in the build quicker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this "works", I would rather have llvm 11.0.0 live. Bumping the WORKSPACE to bazel 4.0.0 will cause this to "just work" - I expected we would give this specific docker image a full spin, using the test build of bazel with the rules_cc bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, regenerating the toolchains now would be nice for testing bazel 3.7.0 ahead of the 4.0.0 build (across the board.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we saw that even the MSVC toolchain doesn't work given a Bazel version without our fix so this is unmergable as-is (the CI check to regenerate toolchains passes in this PR b/c of this line), the CI check for post-merge and generate toolchains will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to correct myself. We should leave the bazel change to 3.6.0+3.7.0 in place. We need to back off the llvm change until bazel 3.7.1/4.0.0. I'll adjust this now.

Even invoking bazel to build msvc-cl, building against llvm 11.0.0 still fails (this was @davinci26's earlier observation), see
envoyproxy/envoy#13688 (comment)

Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
Signed-off-by: William A Rowe Jr <wrowe@vmware.com>
# python installer needs to be run as an installer with Start-Process
RunAndCheckError "$env:TEMP\python3-installer.exe" @("/quiet", "InstallAllUsers=1", "Include_launcher=0", "InstallLauncherAllUsers=0") $true
AddToPath $env:ProgramFiles\Python38
AddToPath $env:ProgramFiles\Python38\Scripts
AddToPath $env:ProgramFiles\Python39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to automate this at some point down the road.

sunjayBhatia
sunjayBhatia previously approved these changes Nov 17, 2020
@sunjayBhatia
Copy link
Member

@wrowe I think this is good to go, maybe just update the PR description?

@sunjayBhatia
Copy link
Member

cc @lizan before we merge since this includes a Bazel version bump in the generated toolchains

@sunjayBhatia
Copy link
Member

/wait

we might get a bazel 3.7.1 very soon and can bump to 3.7.1 instead of 3.7.0, see bazelbuild/bazel#12188 (comment)

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia
Copy link
Member

@lizan before you merge we've got a couple changes coming, we want to wait until Bazel 3.7.1 is out and some dockerfile changes

@lizan
Copy link
Member

lizan commented Nov 19, 2020

@sunjayBhatia sure, or we can just merge this and do them as follow up. Then we can do some test in envoy, up to you.

@sunjayBhatia
Copy link
Member

@lizan actually, cool to merge as is

@lizan lizan merged commit b19d749 into envoyproxy:master Nov 19, 2020
@sunjayBhatia sunjayBhatia deleted the windows-bump-dependencies branch November 19, 2020 21:54
htuch pushed a commit that referenced this pull request Nov 19, 2020
  [skip ci]
  Bump dependencies in Windows build container (#113)

LLVM 11 bump may require Bazel 4.0.0 to avoid erronious LIB and INCLUDE
path construction, testing whether 3.7.0 can build this toolchain
itself, adjusting rules_cc specific to envoy in
  envoyproxy/envoy#13688

If this fails, we will need to wait till 2nd week December for Bazel 4.0.0
release.

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Co-authored-by: William A Rowe Jr <wrowe@vmware.com>
htuch pushed a commit that referenced this pull request Nov 19, 2020
  [skip ci]
  Regenerate linux toolchains from b19d749

  [skip ci]
  Bump dependencies in Windows build container (#113)

LLVM 11 bump may require Bazel 4.0.0 to avoid erronious LIB and INCLUDE
path construction, testing whether 3.7.0 can build this toolchain
itself, adjusting rules_cc specific to envoy in
  envoyproxy/envoy#13688

If this fails, we will need to wait till 2nd week December for Bazel 4.0.0
release.

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Co-authored-by: William A Rowe Jr <wrowe@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants