-
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
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
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 <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
- 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 <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]>
- 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 <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]>
- 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 <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]>
Running into Windows Azure CI environment issues, seems like the |
Also make creation of TMPDIR symlink idempotent Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]>
Also adjust path find/replace to be more exact Signed-off-by: William A Rowe Jr <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: Sunjay Bhatia <[email protected]>
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 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 <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
hi -- can you ping when this is ready for review? |
/wait |
Add comment explaining why we update the PATH temporarily Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]>
when tar sees 'C:' it assumes a uri Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]><Paste>
Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]>
- 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 <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
So we don't have to deal with powershell redirection Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]><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 <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
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. |
Please merge master. Coverage should be fixed now (needs master merge anyway). /wait |
…mmon-tests Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
/retest for macOS |
🤷♀️ nothing to rebuild. |
Signed-off-by: Sunjay Bhatia <[email protected]> Co-authored-by: William Rowe <[email protected]>
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. |
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 <[email protected]>
as requested for PR 10822 by mattklein123 Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
/retest |
🐴 hold your horses - no failures detected, yet. |
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.
Thanks!
…envoy-static (envoyproxy#10542) Co-authored-by: Sunjay Bhatia <[email protected]> Co-authored-by: William A Rowe Jr <[email protected]> Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: William A Rowe Jr <[email protected]> Signed-off-by: pengg <[email protected]>
Description:
workaround development, we were unsuccessful at a short-term
fix to integrate WINDOWS_SKIP_TARGETS / PPC_SKIP_TARGETS
correctly on the PATH
Risk Level: Low (Windows compilation only, small source changes)
Testing: N/A, just enabling Windows compilation
Docs Changes: N/A
Release Notes: N/A