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 wincrypt symbols conflict #1151

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

hudayou
Copy link

@hudayou hudayou commented Jan 4, 2023

With openssl variants such as boring ssl, the conflict symbols are not undefined such as openssl.
Explicit undefine those so it works for both normal openssl and boring ssl

@michael-grunder
Copy link
Collaborator

I've never used boringSSL or wincrypt (I don't have a windows machine), so can you provide a bit more context?

I'm not sure what downstream consequences undefining the symbols might have if we do it globally. Maybe we could limit the lines to only execute if there is some kind of IS_BORINGSSL macro.

Similar issue:
https://bugs.chromium.org/p/boringssl/issues/detail?id=371

@hudayou hudayou force-pushed the wincrypt-boring-ssl branch from b22cca7 to 26273b3 Compare January 4, 2023 23:48
@hudayou
Copy link
Author

hudayou commented Jan 4, 2023

I was trying to make a PR with envoy and their windows CI detected the issue

I fixed the problem of BoringSSL with the patch here

envoyproxy/envoy#24742

Looks like there is one
https://boringssl.googlesource.com/boringssl/+/HEAD/PORTING.md

So I updated the change with that

#ifdef OPENSSL_IS_BORINGSSL

@michael-grunder michael-grunder merged commit d13c091 into redis:master Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants