Prefix BoringSSL symbols for grpc_csharp_ext iOS builds.#26109
Prefix BoringSSL symbols for grpc_csharp_ext iOS builds.#26109jtattermusch merged 1 commit intogrpc:masterfrom
Conversation
Prefix BoringSSL APIs with _GRPC_ by default on iOS, by actually isolating it against other libraries with might include openssl themselves and therefore allowing to embed gRPC on iOS completely mind free. This patch essentially does what has already been done for cocoapods ( see grpc#21194 ).
|
Thanks for the patch! @jtattermusch please take a look, I think you might be most familiar with this area. Is there anywhere else this compiler flag would need to be added? It doesn't seem to have affected the decision last time, but looking into this feature, I found in their API docs
It seems to have been stable enough for the past 2 years, but I'm curious if we had that conversation. |
drfloob
added
area/build
platform/iOS
release notes: no
jtattermusch
changed the title
| PATH_CC="$(xcrun --sdk $SDK --find clang)" | ||
| PATH_CXX="$(xcrun --sdk $SDK --find clang++)" | ||
|
|
||
| CPPFLAGS="-O2 -Wframe-larger-than=16384 -arch $ARCH -isysroot $(xcrun --sdk $SDK --show-sdk-path) -mios-version-min=9.0 -DPB_NO_PACKED_STRUCTS=1" |
There was a problem hiding this comment.
I'm not convinced that this is actually enough to put the boringssl symbol prefixes in place.
Have you tested this or was this just an experiment?
There was a problem hiding this comment.
I'm using it in our own projects and works like a charm. I checked the symbols using nm and I double confirmed it once included in the project where grpc and openssl were clashing, and they are no more. So yes, that's all needed to get it up and running :) Let me know if I can help you further.
There was a problem hiding this comment.
To explain it a little bit:
-DBORINGSSL_PREFIX=GRPCwill kick in this logic https://github.com/google/boringssl/blob/1cf78cd290542e8575c5139907d61dbad5b6f49d/include/openssl/base.h#L79- Luckily, in the repo this file is already provided ( and maintained through your support script ) in this location: https://github.com/grpc/grpc/tree/master/src/boringssl
- Although as it's a global header, all we need to do is make sure the compiler can resolve it, so we include that path in the global search paths by using
-Isrc/boringssl
All the rest will be auto-wired up by boringssl and when grpc will statically link to it, will find symbols already renamed.
|
The artifact build actually seems to be failing: https://source.cloud.google.com/results/invocations/9a9188d3-2452-4abe-9bcf-d47ff132baa4/targets/github%2Fgrpc/tests |
|
To be honest I'm not sure if that's related to my own commit because the only error that pops out is this one: What is expecting to be executing I have no idea. I know for sure that on macOS Big Sur, latest update and latest XCode it worked fine, as that was my use case. If you can point me where this execution happens in the script I'll be happy to help. |
|
Is it possible to re-run a CI check to see if it's good to be merged now? |
jtattermusch
left a comment
jtattermusch
left a comment
There was a problem hiding this comment.
LGTM, as this simply makes sure that the same symbol prefixing mechanism will be used by grpc_csharp_ext build as in the ObjC build.
|
I'm rerunning the tests now and will merge if everything ends up green. |
|
the C# windows failure is unrelated: https://source.cloud.google.com/results/invocations/db6997a6-4650-43de-bc64-8d290edb4123. Filed #27062 |
|
Thank you! |
Prefix BoringSSL APIs with _GRPC_ by default on iOS, by actually isolating it against other libraries with might include openssl themselves and therefore allowing to embed gRPC on iOS completely mind free. This patch essentially does what has already been done for cocoapods ( see grpc#21194 ).
Prefix BoringSSL APIs with _GRPC_ by default on iOS, by actually isolating it against other libraries with might include openssl themselves and therefore allowing to embed gRPC on iOS completely mind free. This patch essentially does what has already been done for cocoapods ( see grpc#21194 ).
4 participants
Prefix BoringSSL APIs with
_GRPC_by default on iOS, by actually isolating it against other libraries with might include openssl themselves and therefore allowing to embed gRPC on iOS completely mind free.This patch essentially does what has already been done for cocoapods ( see #21194 ).
@drfloob