Enable Windows CI //source/exe:envoy-static build by wrowe · Pull Request #10293 · envoyproxy/envoy

wrowe · 2020-03-06T22:19:00Z

Description:

Build and CI tooling to get windows compiling
(Reintroduces an experimental ci/windows_build_steps.ps1)
Source changes required for Windows compilation on msvc
Pick up a flavor of nameser.h for Windows only from c-ares dependency
Excludes all source fixes unrelated to compilation passing on Windows

Risk Level: Low
Testing: local on linux/gcc windows/msvc
Docs Changes: n/a
Release Notes: n/a

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

wrowe · 2020-03-06T22:40:12Z

Will review CI PATH issues on Monday, hopefully in a cleaner way to inject devenv variables and avoid msys2 link.exe
/wait

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

sunjayBhatia · 2020-03-10T19:47:22Z

@@ -0,0 +1,16 @@
+#powershell.exe -Command


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

If powershell is desired, we can refactor with an "invoke bazel" function etc. so we dont have to keep rewriting the error code checking

I'm OK either way as long as we have one canonical solution.

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>

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

dio · 2020-03-11T21:39:37Z

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


sunjayBhatia · 2020-03-12T13:54:53Z

@@ -18,4 +18,4 @@ echo "ENVOY_BAZEL_ROOT: $env:ENVOY_BAZEL_ROOT"
 echo "ENVOY_SRCDIR: $env:ENVOY_SRCDIR"


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

we're deferring changing around how CI is done on Windows for now

sunjayBhatia · 2020-03-12T13:55:08Z

@@ -14,41 +14,53 @@ function bazel_binary_build($type) {
 }


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

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>

sunjayBhatia · 2020-03-12T14:12:03Z

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>

sunjayBhatia · 2020-03-12T16:50:51Z

MacOS CI should only be failing due to: #10357

lizan

Fantastic, thanks! Just a few comment.

lizan · 2020-03-12T22:09:30Z

@@ -0,0 +1,16 @@
+#powershell.exe -Command


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>

danzh2010 · 2020-03-13T19:20:55Z

+
+# Required to work around quiche build defect
+# Unguarded gcc pragmas are not recognized by MSVC
+build:msvc-cl --copt="/wd4068"


Is this the walkaround for #10239?

Well I don't think it is from the comment... How did you walk around that failure?

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

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

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)

sunjayBhatia · 2020-03-13T20:20:31Z

/retest

seeing if the coverage tests are flakes, they passed previously so actual failures would be introduced by more recent commits to master

repokitteh-read-only · 2020-03-13T20:20:35Z

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

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

see: more, trace.

wrowe · 2020-03-14T00:11:14Z

Bitten again by flakes; please

/retest

repokitteh-read-only · 2020-03-14T00:11:18Z

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

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

see: more, trace.

wrowe and others added 2 commits March 6, 2020 17:15

wrowe requested review from alyssawilk and mattklein123 as code owners March 6, 2020 22:19

repokitteh-read-only Bot added the waiting label Mar 6, 2020

repokitteh-read-only Bot removed the waiting label Mar 9, 2020

sunjayBhatia reviewed Mar 10, 2020

View reviewed changes

wrowe and others added 20 commits March 10, 2020 16:29

Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>

65791f1

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

Review mkdir output, try alternate quoting

eae1d35

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>

Review mkdir output, try alternate quoting

83fafa2

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>

Inquiring minds want to know why cmd.exe misbehaves

0e2fca7

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>

Inquiring minds want to know why cmd.exe misbehaves

ef7c9b2

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>

Inquiring minds want to know why cmd.exe misbehaves

14709e4

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>

Inquiring minds want to know why cmd.exe misbehaves

e1b89ad

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>

This might fix it

2c31401

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>

Some adjustments to get some info

ac48407

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>

Some adjustments to get some info

b9f387c

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>

try a more forceful delete

59cd381

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>

Cleanup debugging

3071695

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>

add back debugging

cd5de9a

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>

add more debugging

9d48e56

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>

Resume experiments, validate cwd of cmd.exe process

665a38a

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>

use full path to cmd, arguments that make it less ambiguous

de11e79

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>

use workingDirectory option to change into git bash filesystem

da253d9

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>

check on directory structure in actual build step

d53f474

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>

dio reviewed Mar 11, 2020

View reviewed changes

Comment thread bazel/envoy_internal.bzl Outdated

dio reviewed Mar 11, 2020

View reviewed changes

sunjayBhatia reviewed Mar 12, 2020

View reviewed changes

wrowe mentioned this pull request Mar 12, 2020

Review support of AF_UNIX sockets on the Windows port #9571

Closed

Add comment referencing rules_foreign_cc issue

bd55a8c

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>

lizan reviewed Mar 12, 2020

View reviewed changes

lizan self-assigned this Mar 12, 2020

lizan added the waiting label Mar 12, 2020

Clarify comments on Windows build flags suggested by maintainers

a8844d4

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>

repokitteh-read-only Bot removed the waiting label Mar 13, 2020

sunjayBhatia and others added 2 commits March 13, 2020 11:11

Remove duplicated ci script

ce0227c

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>

wrowe requested review from dio and lizan March 13, 2020 15:57

sunjayBhatia and others added 3 commits March 13, 2020 14:34

Merge remote-tracking branch 'origin/master' into test-windows-source…

b9a5b54

…-build Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>

Re-disable luajit compilation on Windows

d4b040d

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>

danzh2010 reviewed Mar 13, 2020

View reviewed changes

lizan approved these changes Mar 13, 2020

View reviewed changes

lizan merged commit e7fc5fc into envoyproxy:master Mar 14, 2020

wrowe deleted the test-windows-source-build branch March 14, 2020 16:06

		@@ -18,4 +18,4 @@ echo "ENVOY_BAZEL_ROOT: $env:ENVOY_BAZEL_ROOT"
		echo "ENVOY_SRCDIR: $env:ENVOY_SRCDIR"

Conversation

wrowe commented Mar 6, 2020

Uh oh!

wrowe commented Mar 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia commented Mar 12, 2020

Uh oh!

sunjayBhatia commented Mar 12, 2020

Uh oh!

lizan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunjayBhatia commented Mar 13, 2020

Uh oh!

repokitteh-read-only Bot commented Mar 13, 2020

Uh oh!

wrowe commented Mar 14, 2020

Uh oh!

repokitteh-read-only Bot commented Mar 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sunjayBhatia Mar 13, 2020 •

edited

Loading