Skip to content
This repository has been archived by the owner on May 2, 2021. It is now read-only.

Enable cross building for Android and iOS #51

Conversation

anton-danielsson
Copy link

@anton-danielsson anton-danielsson commented Feb 14, 2021

Allows grpc to be cross built using dual profiles (profile host/build).

Disables plugins when cross building and the recipe instead depends on itself to provide the plugins.

The patch is needed for the iOS build. Relates to OpenSSL stuff.
It's already fixed in master but not in 1.34.1. See grpc/grpc#24959

Cross building for Android and iOS depends on this PR to protobuf: conan-io/conan-center-index#4556
Cross building for iOS build also depends on this PR to c-ares: conan-io/conan-center-index#4492

Tested on both Android and iOS:

Android profile:

Configuration (profile_host):
[settings]
arch=armv8
build_type=Release
compiler=clang
compiler.libcxx=c++_static
compiler.version=11
os=Android
os.api_level=21
[options]
[build_requires]
*: android-ndk/r22
[env]

Configuration (profile_build):
[settings]
arch=x86_64
build_type=Release
compiler=clang
compiler.libcxx=libstdc++
compiler.version=6.0
os=Linux
[options]
[build_requires]
[env]
CC=/usr/bin/clang
CXX=/usr/bin/clang++

iOS profile:

Configuration (profile_host):
[settings]
arch=armv8
build_type=Release
compiler=apple-clang
compiler.libcxx=libc++
compiler.version=11.0
os=iOS
os.version=10.2
[options]
ios-cmake:enable_bitcode=False
[build_requires]
*: ios-cmake/3.1.2
[env]

Configuration (profile_build):
[settings]
arch=x86_64
build_type=Release
compiler=apple-clang
compiler.libcxx=libc++
compiler.version=11.0
os=Macos
[options]
[build_requires]
[env]

@Croydon
Copy link
Collaborator

Croydon commented Feb 20, 2021

The upstream patch is already in the 1.35.0 release (grpc/grpc@8a546ca). I would rather like to see it updated to that (a change in required dependencies needs to be checked for that too)

@anton-danielsson
Copy link
Author

The upstream patch is already in the 1.35.0 release (grpc/grpc@8a546ca). I would rather like to see it updated to that (a change in required dependencies needs to be checked for that too)

Sure, the patch can be dropped when grpc is updated to 135.0.
In a CCI version of this recipe I guess the patch would still be there to support the 1.34.1 version?

Anyway, I don't think the interesting part is that patch. It's the other small, but important, changes that
allows this recipe to be used when cross compiling for Android and iOS using dual profiles.
We have used this approach for quite some time but I would much prefer to have it upstream :)

conanfile.py Outdated
# When cross building the recipe depends on itself to provide the plugins
if tools.cross_building(self.settings):
self.build_requires(
f"{self.name}/{self.version}@{self.user}/{self.channel}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

While in principle Conan is still supporting Python 2, supporting Python 2 in recipes would be ideal. But at the very minimum, it has to support Python 3.5+

Please don't use f-strings

Copy link
Author

Choose a reason for hiding this comment

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

Sure! I'm just so used to being able to use them :)

find_program(_PROTOBUF_PROTOC protoc ${CONAN_BIN_DIRS_PROTOBUF} NO_DEFAULT_PATH)

# Find protoc
find_program(_PROTOBUF_PROTOC protoc REQUIRED)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change means that a pre-installed protoc can be found which is not desired

Copy link
Author

Choose a reason for hiding this comment

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

For cross complication to work find_program needs to rely on finding protoc through the PATH variable, not CONAN_BIN_DIRS_PROTOBUF.
You shouldn't find the system protoc because the protobuf package sets up the path correctly:
https://github.com/conan-io/conan-center-index/blob/4116c939117a47bf4c4285aeec2a857e6b9b7165/recipes/protobuf/all/conanfile.py#L203

I have not been able to find any other way to do this with Conan...

The protobuf PR in CCI (conan-io/conan-center-index#4556) makes sure
the path is setup correctly when using dual profiles (to make sure you find the build protoc).

@anton-danielsson
Copy link
Author

Removed the f-strings.
So what do you think @Croydon?
The change is pretty small and cross building to iOS and Android with dual profiles works with this patch.
If you think it's too "hacky" I understand. The dual profile support in Conan is still a bit "shaky"...

@anton-danielsson
Copy link
Author

Also, just out of curiosity. What's stopping this recipe from being added to CCI?
Please let me know if there is anything I can assist with :)

@TheStormN
Copy link
Contributor

TheStormN commented Mar 8, 2021

Hey @Croydon

Could you take a look again at this PR and if it's fine merge it?

@anton-danielsson
Copy link
Author

After these (conan-io/conan-center-index#4556) changes to protobuf was merged in CCI
it was possible to remove some "hacks" from this PR.
Now it's up to the protobuf recipe to find the correct protoc. The GRPC recipe just have to use PROTOC_PROGRAM.
See latest commit: 12f3dff

@anton-danielsson
Copy link
Author

Updated to use the more standard Protobuf_PROTOC_EXECUTABLE now that conan-io/conan-center-index#4776 is in CCI.

@Croydon
Copy link
Collaborator

Croydon commented May 1, 2021

This recipe is now in the Conan Center Index.

Please migrate to the recipe version there.
If this is still an issue, please create a new issue in the Conan Center Index issue tracker.

Thanks!

https://conan.io/center/grpc

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants