Windows compilation: enable compiling expanded list of extensions in envoy-static #10542

sunjayBhatia · 2020-03-26T17:32:06Z

Description:

replace WINDOWS_EXTENSIONS with very short WINDOWS_SKIP_TARGETS list
- extensions _SKIP_TARGET lists converted from dict to list
- See [WIP] Restructure EXTENSIONS/WINDOWS_EXTENSIONS #10365 for attempted
  workaround development, we were unsuccessful at a short-term
  fix to integrate WINDOWS_SKIP_TARGETS / PPC_SKIP_TARGETS
fixes kafka compilation (MSVC parses the literal of INT32_MIN incorrectly)
fixes luajit and moonjit compilation (only one jit is elected)
merge windows .bazelrc into workspace .bazelrc
install MSYS2 into the CI worker to ensure the expected shell environment for bazel and build tools are
correctly on the PATH
Building/running tests in CI is not yet enabled, we are investigating options related to Windows RBE Tracking Issue #10619

Risk Level: Low (Windows compilation only, small source changes)
Testing: N/A, just enabling Windows compilation
Docs Changes: N/A
Release Notes: N/A

Test macro missing the second argument (for test name), causing compilation on Windows to fail. It appears to have no purpose, copy-paste error? 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 has indicated a desire to refactor the envoy_extension_* API's to better handle target platform builds - See #10365 for attempted workaround development, we were unsuccessful at a short-term fix to integrate WINDOWS_SKIP_TARGETS / PPC_SKIP_TARGETS - Individually exclude affected tests for the time being Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io>

- having paths greater than 240 characters breaks compilation on Windows with errors indicating header files do not exist in the includes directory - the adaptive_concurrency/concurrency_controller name is repetitive, this change shortens it to adaptive_concurrency/controller to get past Windows file path limits - namespace changed to AdaptiveConcurrency::Controller to mimic same shortening, AdaptiveConcurrency::Controller::ConcurrencyController class is kept as that seems an appropriately descriptive name for the class 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>

- replace WINDOWS_EXTENSIONS with very short WINDOWS_SKIP_TARGETS list - fixes kafka compilation (MSVC parses the literal of INT32_MIN incorrectly) - fixes luajit and moonjit compilation (only one jit is elected) - extensions _SKIP_TARGET lists converted from dict to list - compilation failures of the test packages are tagged skip_on_windows - timeouts in failing packages are tagged fails_on_windows - additional failing tests will be tagged in a subsequent commit 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: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>

sunjayBhatia · 2020-03-26T18:01:50Z

Running into Windows Azure CI environment issues, seems like the link.exe being found on the PATH is not the correct link.exe from MSVC (it is the GNU link utility we presume from mingw), we are seeing what we can do about this scenario

Also make creation of TMPDIR symlink idempotent 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>

Also adjust path find/replace to be more exact Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>

sunjayBhatia · 2020-03-26T21:09:17Z

It looks like git bash is the source of our problems, we're going to install msys2 in our CI environment in the setup phase and rely on that instead of fighting git bash (it prepends its own bin directories to the PATH, so any invocation by bazel undoes what we attempted to fix by manipulating the PATH earlier)

Hopefully, according to actions/runner-images#632 msys2 will be installed in the base image and we can rely on that installation in the future

Will stop using git's somewhat ill-behaved bash.exe. First in the path is the flavor that the Azure environment will elect for each bash step. Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

jmarantz · 2020-03-27T17:04:52Z

hi -- can you ping when this is ready for review?

jmarantz · 2020-03-27T17:04:57Z

/wait

Add comment explaining why we update the PATH temporarily Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

when tar sees 'C:' it assumes a uri Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

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

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

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

- so the powershell task does not fail on warnings - also replace unzip, zip with compression for a more full suite of tools Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

So we don't have to deal with powershell redirection Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io><Paste>

the previous syntax was working with git bash, we're using msys2 bash so potentially there are differences, use the more "correct" looking invocation Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

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

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

sunjayBhatia · 2020-04-17T14:08:40Z

Updated the title and description with the more limited scope for this PR, let us know if we should pare this set of changes down more for a more manageable review.

mattklein123 · 2020-04-18T21:26:53Z

Please merge master. Coverage should be fixed now (needs master merge anyway).

/wait

…mmon-tests Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

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

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

wrowe · 2020-04-20T16:17:33Z

/retest

for macOS

repokitteh-read-only · 2020-04-20T16:17:37Z

🤷‍♀️ nothing to rebuild.

🐱

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

see: more, trace.

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

wrowe · 2020-04-20T20:16:07Z

It doesn't seem possible to get through the macOS build step due to the current CI situation. Otherwise this appears to be looking good for acceptance.

mattklein123 · 2020-04-20T22:14:17Z

Thanks this LGTM. Can you please merge master? OSX is reliably passing now on master. Thank you!

/wait

…mmon-tests Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

as requested for PR 10822 by mattklein123 Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

wrowe · 2020-04-21T17:17:58Z

/retest
... to show coverage publish completion

repokitteh-read-only · 2020-04-21T17:18:01Z

🐴 hold your horses - no failures detected, yet.

🐱

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

see: more, trace.

mattklein123

Thanks!

…envoy-static (envoyproxy#10542) 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> Signed-off-by: pengg <pengg@google.com>

sunjayBhatia and others added 4 commits March 26, 2020 11:10

sunjayBhatia requested review from alyssawilk, htuch, jmarantz, lizan, mattklein123 and PiotrSikora as code owners March 26, 2020 17:32

Fix formatting and clean up test main invocation of WSAStartup

519f712

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

mattklein123 assigned lizan Mar 26, 2020

Prepend VS tools to the path

b24bef2

Also adjust path find/replace to be more exact Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Co-authored-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>

repokitteh-read-only bot added the waiting label Mar 27, 2020

Fix download path for msys2.tar.xz

210ee5d

Add comment explaining why we update the PATH temporarily Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

repokitteh-read-only bot removed the waiting label Mar 27, 2020

sunjayBhatia and others added 8 commits March 27, 2020 13:27

Fix tar path so it doesnt try to connect to a remote

9e0ec46

when tar sees 'C:' it assumes a uri Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

Adjust tar change directory path

6b87110

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

Use unix style paths with tar only

5c0ce5f

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

msys2 has a subdir named msys64, put that properly on the PATH

12f4d36

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

Redirect pacman stderr to stout

466c7a0

- so the powershell task does not fail on warnings - also replace unzip, zip with compression for a more full suite of tools Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

Use bash for pacman.exe invocation

9b01e89

So we don't have to deal with powershell redirection Signed-off-by: William A Rowe Jr <wrowe@pivotal.io> Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io><Paste>

change cmd.exe invocation

4e1141a

the previous syntax was working with git bash, we're using msys2 bash so potentially there are differences, use the more "correct" looking invocation Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io> Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

Use bash to check existence, cmd.exe is not working

3bbbdd5

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

Kick CI

9eda28a

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

sunjayBhatia changed the title ~~Windows compilation: test sources and extensions~~ Windows compilation: enable compiling expanded list of extensions in envoy-static Apr 17, 2020

mattklein123 self-assigned this Apr 18, 2020

repokitteh-read-only bot added the waiting label Apr 18, 2020

Merge remote-tracking branch 'origin/master' into enabling-windows-co…

50dc9b3

…mmon-tests Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

repokitteh-read-only bot removed the waiting label Apr 18, 2020

wrowe and others added 2 commits April 18, 2020 19:59

Restore some variables tweaked for successful builds

1b49268

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

Fix BAZEL_SH in Windows pipeline job

aef5e03

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

Retest for macOS

44ab8f5

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

repokitteh-read-only bot added the waiting label Apr 20, 2020

Merge remote-tracking branch 'origin/master' into enabling-windows-co…

bfba02e

…mmon-tests Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

repokitteh-read-only bot removed the waiting label Apr 21, 2020

wrowe and others added 2 commits April 21, 2020 04:38

snakeCase not hungarian

bdc19bb

as requested for PR 10822 by mattklein123 Signed-off-by: William A Rowe Jr <wrowe@pivotal.io>

snake_case is with underscores

cc4d1c8

Signed-off-by: Sunjay Bhatia <sbhatia@pivotal.io>

mattklein123 approved these changes Apr 21, 2020

View reviewed changes

mattklein123 merged commit 3bcfc2b into envoyproxy:master Apr 21, 2020

sunjayBhatia deleted the enabling-windows-common-tests branch April 21, 2020 18:05

rgs1 mentioned this pull request Apr 22, 2020

Crash regression from #10362 #10901

Closed

sunjayBhatia mentioned this pull request Apr 24, 2020

NT support #129

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows compilation: enable compiling expanded list of extensions in envoy-static #10542

Windows compilation: enable compiling expanded list of extensions in envoy-static #10542

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

jmarantz commented Mar 27, 2020

jmarantz commented Mar 27, 2020

sunjayBhatia commented Apr 17, 2020 •

edited

Loading

mattklein123 commented Apr 18, 2020

wrowe commented Apr 20, 2020

repokitteh-read-only bot commented Apr 20, 2020

wrowe commented Apr 20, 2020

mattklein123 commented Apr 20, 2020

wrowe commented Apr 21, 2020

repokitteh-read-only bot commented Apr 21, 2020

mattklein123 left a comment

Windows compilation: enable compiling expanded list of extensions in envoy-static #10542

Windows compilation: enable compiling expanded list of extensions in envoy-static #10542

Conversation

sunjayBhatia commented Mar 26, 2020 • edited Loading

sunjayBhatia commented Mar 26, 2020 • edited Loading

sunjayBhatia commented Mar 26, 2020 • edited Loading

jmarantz commented Mar 27, 2020

jmarantz commented Mar 27, 2020

sunjayBhatia commented Apr 17, 2020 • edited Loading

mattklein123 commented Apr 18, 2020

wrowe commented Apr 20, 2020

repokitteh-read-only bot commented Apr 20, 2020

wrowe commented Apr 20, 2020

mattklein123 commented Apr 20, 2020

wrowe commented Apr 21, 2020

repokitteh-read-only bot commented Apr 21, 2020

mattklein123 left a comment

Choose a reason for hiding this comment

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

sunjayBhatia commented Mar 26, 2020 •

edited

Loading

sunjayBhatia commented Apr 17, 2020 •

edited

Loading