-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Enable Windows CI //source/exe:envoy-static build #10293
Enable Windows CI //source/exe:envoy-static build #10293
Conversation
- Pick up nameser.h for Windows from c-ares dependency Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>
Will review CI PATH issues on Monday, hopefully in a cleaner way to inject devenv variables and avoid msys2 link.exe |
- This PATH is too explicit, and we likely want to use devenv in the VC toolkit to resolve the correct build envvars in conjunction with the specific build instance. Perhaps a build machine local .bazelrc is a possible workaround? Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
See bazel-contrib/rules_foreign_cc#334 Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
ci/windows_ci_steps.ps1
Outdated
@@ -0,0 +1,16 @@ | |||
#powershell.exe -Command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the maintainers to consider: we added this version of the window_ci_steps
script in case we wanted to converge on using powershell only instead of a mix of bash and powershell, up to y'all, we can remove it if we want to stick with bash scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If powershell is desired, we can refactor with an "invoke bazel" function etc. so we dont have to keep rewriting the error code checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK either way as long as we have one canonical solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, removing this one
Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
- we have to assume the path of the bash mounted filesystem Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
- working directory of actual bazel tasks is in D: drive, so /tmp resolves to D:\tmp - having both covers our bases Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
@@ -208,7 +208,8 @@ CompressorFilter::chooseEncoding(const Http::ResponseHeaderMap& headers) const { | |||
|
|||
// Find all encodings accepted by the user agent and adjust the list of allowed compressors. | |||
for (const auto token : StringUtil::splitToken(*accept_encoding_, ",", false /* keep_empty */)) { | |||
EncPair pair = std::make_pair(StringUtil::trim(StringUtil::cropRight(token, ";")), 1); | |||
EncPair pair = | |||
std::make_pair(StringUtil::trim(StringUtil::cropRight(token, ";")), static_cast<float>(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -18,4 +18,4 @@ echo "ENVOY_BAZEL_ROOT: $env:ENVOY_BAZEL_ROOT" | |||
echo "ENVOY_SRCDIR: $env:ENVOY_SRCDIR" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the maintainers to consider: this file is not actually used in CI and only is referenced in do_ci.ps1
, should we change over our CI pipelines to use the do_ci
pattern like on Linux? (or should we just simply keep it around until we can do the full set of builds/tests?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're deferring changing around how CI is done on Windows for now
@@ -14,41 +14,53 @@ function bazel_binary_build($type) { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the maintainers to consider: should we change over our CI pipelines to use the do_ci
pattern like on Linux? (or should we just simply keep it around until we can do the full set of builds/tests?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we defer this discussion until this is merged? Good conversation, and probably doesn't have to hold up adoption of this specific PR.
…-build Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
merging master to get recent fix that should get coverage tests passing |
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
MacOS CI should only be failing due to: #10357 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic, thanks! Just a few comment.
ci/windows_ci_steps.ps1
Outdated
@@ -0,0 +1,16 @@ | |||
#powershell.exe -Command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK either way as long as we have one canonical solution.
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
- measure time - fastbuild and debug are producing exes that fail an absl assertion Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
…-build Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>
|
||
# Required to work around quiche build defect | ||
# Unguarded gcc pragmas are not recognized by MSVC | ||
build:msvc-cl --copt="/wd4068" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the walkaround for #10239?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I don't think it is from the comment... How did you walk around that failure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, nothing to do with 10239, that fix is not offered in this PR. That fix will become required when we begin compiling //test/... (which this PR doesn't attempt to do). This PR simply gets us compiling //source/exe:envoy-static
You can see the workaround to 10239 at https://github.com/greenhouse-org/envoy/commit/66eff7c1e12544a37792140039867481b9bc91f8 which we will offer in the next PR that addresses compiling //test/...
This fix above resolves gcc pragmas which cause fatal warnings-as-errors failure when we trip over the gcc pragmas in poorly constructed quiche headers. See;
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4068?view=vs-2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, no that was a comment that snuck in from when we were trying to fix the quiche thing before, will remove the comment that warning is around unrecognized pragmas: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4068?view=vs-2019, not the same issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note it is extra confusing because we transform the source tree from clang pragmas into gcc pragmas, both of which cannot be understood by msvc-cl. (clang-cl, when we get that working, would probably recognize the clang flavor of the pragmas)
/retest seeing if the coverage tests are flakes, they passed previously so actual failures would be introduced by more recent commits to master |
🔨 rebuilding |
Bitten again by flakes; please /retest |
🔨 rebuilding |
Description:
Risk Level: Low
Testing: local on linux/gcc windows/msvc
Docs Changes: n/a
Release Notes: n/a