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

Fix assorted breakages and Windows compilation changes #9966

Merged
merged 9 commits into from
Feb 7, 2020
Merged

Fix assorted breakages and Windows compilation changes #9966

merged 9 commits into from
Feb 7, 2020

Conversation

sunjayBhatia
Copy link
Member

Description:

  • Fixes assorted master breakages and changes required for Windows compilation.
  • Bumps rapidjson to support Windows and clang-cl
  • Moves existing os_sys_call_impl for future parallel posix and win32 implementations
  • patch boringssl build to use static library

Risk Level: Low
Testing: Local on gcc Linux, msvc Windows
Docs Changes: N/A
Release Notes: N/A
Fixes #8351

wrowe and others added 7 commits February 7, 2020 11:32
…uild

This fix to foreign_cc BoringSSL build addresses the fact that its own bazel
build doesn't respect Envoy's choice to build a static library. The patch
deserve further additions and adjustment, but resolves #8754 which could
not be reopened due to my force push. It was intended for PR9915 but
missed the boat.

Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: Yechiel Kalmenson <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
Co-authored-by: William A Rowe Jr <[email protected]>
Co-authored-by: Yechiel Kalmenson <[email protected]>
Co-authored-by: Sunjay Bhatia <[email protected]>
This picks up a bugfix from Ylavic which avoids using Win32 MSVC
warning pragmas in the case of clang-cl compilation. Unfortunately
clang-cl presents itself with an _MSC_VER preprocessor built-in
macro, which is very misleading. Note that compiling with MSVC's
own distribution of clang-cl is not corrected, but it seems very
unlikely that will ever happen in the future (32 not 64 bit compiler.)

Co-authored-by: William A Rowe Jr <[email protected]>
Co-authored-by: Sunjay Bhatia <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
Support platform process scope requirements, such as WSAStartup behavior for Windows.

Signed-off-by: William A Rowe Jr <[email protected]>
This creates a space for source/common/api/win32 and
seperates changes to posix from the commit which here
relocates the files

Signed-off-by: Sunjay Bhatia <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]>
Otherwise compiling the mocks w/ gcc on Linux fails due to
`-Werror=overloaded-virtual` being treated as an error

Signed-off-by: William A Rowe Jr <[email protected]>
Co-authored-by: Sunjay Bhatia <[email protected]>
APC is a common acronym we will likely use more for Windows

Signed-off-by: William A Rowe Jr <[email protected]>
Co-authored-by: Sunjay Bhatia <[email protected]>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM with small question, thank you!

/wait-any

Comment on lines 45 to 46
] + select({
"//conditions:default": [],
Copy link
Member

Choose a reason for hiding this comment

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

This can be deleted, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'd prefer to leave it (fill-in-the-gap adds windows specifics there in a upcoming patch, if that's alright.) Trying to keep as many changes in the upcoming commits to be line-oriented diffs.

Copy link
Member

Choose a reason for hiding this comment

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

OK sounds good.

Copy link
Member Author

Choose a reason for hiding this comment

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

we found that we didn't need the subsequent changes in the later commit for Windows, so we will remove this bit

mattklein123
mattklein123 previously approved these changes Feb 7, 2020
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

Signed-off-by: Sunjay Bhatia <[email protected]>
Co-authored-by: William A Rowe Jr <[email protected]>
Co-authored-by: Sunjay Bhatia <[email protected]>
@mattklein123 mattklein123 merged commit 7801d05 into envoyproxy:master Feb 7, 2020
@wrowe wrowe deleted the fix-windows-feb-5 branch February 10, 2020 21:39
@sunjayBhatia sunjayBhatia mentioned this pull request Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Externals boringssl target fails to build on Windows
3 participants