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

Feature/grpc cross compile #5607

Conversation

cguentherTUChemnitz
Copy link
Contributor

@cguentherTUChemnitz cguentherTUChemnitz commented May 23, 2021

fixes #5622

grpc/1.37.1

This PR enables cross-compilation of grpc for arm targets. The build-calls in the motivation are running correctly in local tests.

Motivation:
grpc does not compile for arm cross-compilation. Example call:

docker run -v $(pwd)/conan-docker-cache/:/home/conan/.conan --rm conanio/gcc9-armv7hf bash -c "conan install -pr=/home/conan/.conan/profiles/build grpc/1.37.1@ --build missing"

host-profile

[settings]
os=Linux
arch=armv7
compiler=gcc
compiler.version=9
compiler.libcxx=libstdc++11
build_type=Release
[options]
[build_requires]
[env]

build-profile

[settings]
os=Linux
arch=x86_64
compiler=gcc
compiler.version=9
compiler.libcxx=libstdc++11
build_type=Release
[options]
[build_requires]
[env]
  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes. --> upstream grpc conanfile.py file was not pep8 formatted. So doing this now would change the whole file and disturb the discussion.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

problems:
To enable cross-compile functionality we have to provide do the following:

  • provide protoc binary for build target during grpc compilation
  • provide protoc plugins (like cpp plugin) for build target during grpc compliation

solutions:

  • grpc invokes protoc call on build-system during compilation. Protoc then invokes itself generator plugins like grpc cpp plugin. Protoc is a binary build artifact of the protobuf dependency. The generator plugins are build artifacts of the grpc build.
  • protobuf is necessary as requirement as well as build_requirement. Thus protoc binaries are added for two different architecture (host and build) to PATH. Since the build_requirement path is appended after the requirements path, the build-process chooses the wrong (host) protoc binary during grpc cross-compilation. This is solved by adding protobuf as build_requriement for cross-compilation and using its build_requriement binary path to find correct protoc. Setting the CMAKE var _gRPC_PROTOBUF_PROTOC_EXECUTABLE enforces then to use the build-version of protoc during grpc compilation. If there is a possibility to control the (build)_requirement paths to be appended or prepended, this could obsolete the cmake variable manipulation.
  • protoc plugins are provided by grpc itself. They are requiered during grpc compilation, because of protoc invocation. We now add grpc itself as build_requirement to provide the protoc plugins correctly for build architecture during grpc compliation.
  • test_package is patched to also use grpc as build_requirement for cross-compliation to be able to run protoc and protoc-plugins on the build-architecture

@CLAassistant
Copy link

CLAassistant commented May 23, 2021

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@cguentherTUChemnitz
Copy link
Contributor Author

I think we need here some discussion to highlight the desired way to go. I tried to explain the problem and the solution in the merge request above. The key-point of the discussion should be: How should we handle packages which delivers binaries for their own build-process in case of cross-compilation? This is the case for grpc, since it provides protoc plugins, which are used during grpc build. But when i add the package as build_requirement to its own recipe, then the ci-check for recursive inclusion fails. Nevertheless the local builds runs fine. I understand why this check is beneficial, but i think we should discuss:

  1. how those pacakges should be handled (Does there exist some kind of multi-stage package building for different hardware types? How should we handle those regarding to single-architecture conan compiler docker images?)
  2. can we maybe disable the requirement recursion check for build_requirements? The build itself runs fine with the mentioned cross-compilation call in the PR, but ci is failing.

Since i do not know who would be the best discussion partner for those conan mechanic fundamentals i will ping two conan and two package developers here. Maybe some of you can in turn link the best suitable discussion partner here. @memsharded @lasote @SSE4 @SpaceIm

@prince-chrismc
Copy link
Contributor

Just a naive community reviewer... this is a very complicated topic and there's a few experts (not this guy).
protobuf, it's PRs, are the best place to learn ... they always have lively discussion.

To use protoc there this code here which might give you clues

if(CMAKE_CROSSCOMPILING)

if tools.cross_building(self.settings):

Is how it's used in the test package

How should we handle packages which delivers binaries for their own build-process in case of cross-compilation?

The best answer I could point you to is this #5258

@prince-chrismc
Copy link
Contributor

Can you please "fixes #5622" to the top level comment of the PR

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@cguentherTUChemnitz
Copy link
Contributor Author

cguentherTUChemnitz commented May 27, 2021

Just a naive community reviewer... this is a very complicated topic and there's a few experts (not this guy).
protobuf, it's PRs, are the best place to learn ... they always have lively discussion.

To use protoc there this code here which might give you clues

if(CMAKE_CROSSCOMPILING)

if tools.cross_building(self.settings):

Is how it's used in the test package

How should we handle packages which delivers binaries for their own build-process in case of cross-compilation?

The best answer I could point you to is this #5258

I think i did rebuild the most behavior like you linked. The difference between protobuf and grpc ist in my eyes, that protobuf has the self-dependency as cross-compile build_requirement only for test_package. On the other hand grpc needs the cross-compile build_requirement self-dependency already in the package reciepe. This is necessary since grpc does provide plugins for protoc, which where necessary during its own lib build. But this build_requirement self-dependency in the recipe makes jenkins ci red, because it finds a dependency recursion.

To tackle this, i think we need here input from some internal developers.

@prince-chrismc
Copy link
Contributor

👋 @uilianries @SpaceIm @anton-danielsson this could use your expertise.

some fun cross-compiling self dependent two profile similar to protobuf.

@anton-danielsson
Copy link
Contributor

Nice!
This looks quite close to what I tried to get into the "old" conan grpc repo:
inexorgame-obsolete/conan-grpc#51

@anton-danielsson
Copy link
Contributor

I know this approach works in practice but I'm not 100% sure it's actually "allowed".

Our organization have cross compiled grpc for Android and iOS for quite some time now and our "internal" recipe still uses two separate recipies for grpc. One for grpc and one for the plugins (grpc-plugins).
It would be a bit sad to go with that solution though so let's try to get this one merged :)

@ghost
Copy link

ghost commented Jun 1, 2021

I detected other pull requests that are modifying grpc/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Failure in build 7 (2d66dc490f9ceb5c1046a80e0b50bd6ea511d66c):

  • grpc/1.38.0@:
    Error running command conan info grpc/1.38.0@ --json {jsonName} --dry-build -pr {profileName}:

    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=Visual Studio
    compiler.runtime=MT
    compiler.version=14
    os=Windows
    os_build=Windows
    
    ...
    ERROR: Conflict in grpc/1.38.0:
        'grpc/1.38.0' requires 'protobuf/3.15.5' while 'grpc/1.38.0' requires 'protobuf/3.17.1'.
        To fix this conflict you need to override the package 'protobuf' in your root package.
    
    
  • grpc/1.37.1@:
    Error running command conan info grpc/1.37.1@ --json {jsonName} --dry-build -pr {profileName}:

    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=Visual Studio
    compiler.runtime=MT
    compiler.version=14
    os=Windows
    os_build=Windows
    
    ...
    ERROR: Conflict in grpc/1.37.1:
        'grpc/1.37.1' requires 'protobuf/3.15.5' while 'grpc/1.37.1' requires 'protobuf/3.17.1'.
        To fix this conflict you need to override the package 'protobuf' in your root package.
    
    
  • grpc/1.37.0@:
    Error running command conan info grpc/1.37.0@ --json {jsonName} --dry-build -pr {profileName}:

    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=Visual Studio
    compiler.runtime=MT
    compiler.version=14
    os=Windows
    os_build=Windows
    
    ...
    ERROR: Conflict in grpc/1.37.0:
        'grpc/1.37.0' requires 'protobuf/3.15.5' while 'grpc/1.37.0' requires 'protobuf/3.17.1'.
        To fix this conflict you need to override the package 'protobuf' in your root package.
    
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@stale
Copy link

stale bot commented Jul 30, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 30, 2021
@@ -60,6 +60,13 @@ def requirements(self):
self.requires('abseil/20210324.0')
self.requires('re2/20210202')

def build_requirements(self):
if tools.cross_building(self.settings):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if tools.cross_building(self.settings):
if tools.cross_building(self.settings) and hasattr(self, "settings_build"):

@stale stale bot removed the stale label Jul 30, 2021
@ghost ghost mentioned this pull request Aug 7, 2021
4 tasks
@ghost ghost mentioned this pull request Aug 18, 2021
4 tasks
@ghost ghost mentioned this pull request Aug 26, 2021
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.

[package] grpc/1.37.1: add x86_64 to arm cross crompilation capabilities
7 participants