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

Build manylinux_2_24 compliant wheels #844

Merged
merged 2 commits into from
Apr 6, 2021

Conversation

diegoferigo
Copy link
Member

@diegoferigo diegoferigo commented Apr 6, 2021

This is take 2, after #774, #776 and #777.

Finally, all the tooling necessary to build manylinux_2_24 is ready 🎉 It requires a modern pip>=21.0 version.

In theory, this PR could also enable the macOS and windows support, but let's do things progressively.

strategy:
fail-fast: false
matrix:
python-version:
- 3.8
# - 3.9
os:
- ubuntu-latest
Copy link
Member

Choose a reason for hiding this comment

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

Can we stick to ubuntu-20.04? Mostly for reproducibility, even if cibuildwheel provide some abstraction better to be conservative.

Copy link
Member Author

Choose a reason for hiding this comment

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

@traversaro
Copy link
Member

This is take 2, after #774, #776 and #777.

Finally, all the tooling necessary to build manylinux_2_24 is ready 🎉 It requires a modern pip>=21.0 version.

In theory, this PR could also enable the macOS and windows support, but let's do things progressively.

Just to understand, this still uses the assimp/ipopt from apt, so it is only compatible with Ubuntu 20.04 (or any distro that by chance have assimp/ipopt with the same ABI), right?

@diegoferigo
Copy link
Member Author

diegoferigo commented Apr 6, 2021

This is take 2, after #774, #776 and #777.
Finally, all the tooling necessary to build manylinux_2_24 is ready tada It requires a modern pip>=21.0 version.
In theory, this PR could also enable the macOS and windows support, but let's do things progressively.

Just to understand, this still uses the assimp/ipopt from apt, so it is only compatible with Ubuntu 20.04 (or any distro that by chance have assimp/ipopt with the same ABI), right?

No, cibuildwheel calls auditwheel that repairs the wheel archive. It means that it includes in the wheel all the external shared libraries from the CI system (particularly, those shipped in the quay image used by cibuildwheel). The resulting wheels are self-contained, and work on all the platforms supported by manylinux_2_24.

The new standard basically allows distributing wheels for most of the modern Linux distribution, allowing to compile the C++ project with a modern enough compiler that does not mess up C++ >= 11. This means that the CMake project included in the wheel could get imported by downstream libraries.

@traversaro
Copy link
Member

Ah cool, so everything will work until some other wheel uses assimp, ipopt or libxml2 . Good to know.

@diegoferigo
Copy link
Member Author

diegoferigo commented Apr 6, 2021

so everything will work until some other wheel uses assimp, ipopt or libxml2

Can you elaborate on this? If I understood, it's about the case where an application imports the new idyntree and also other packages that require assimp, ipopt or libxml2. In this case, the first one that is imported will load its own version, which could not be compatible with the packages imported later. Unfortunately, there's no way to prevent this to happen using packages from PyPI.

A good thing is that these external shared libraries that are shipped in the wheel remain shared libraries. The result is similar to a static compilation. I think, though, that a static compilation would create even more problems, so this seems a good trade-off.

@traversaro
Copy link
Member

so everything will work until some other wheel uses assimp, ipopt or libxml2

Can you elaborate on this? If I understood, it's about the case where an application imports the new idyntree and also other packages that require assimp, ipopt or libxml2. In this case, the first one that is imported will load its own version, which could not be compatible with the packages imported later. Unfortunately, there's no way to prevent this to happen using packages from PyPI.

A good thing is that these external shared libraries that are shipped in the wheel remain shared libraries. The result is similar to a static compilation. I think, though, that a static compilation would create even more problems, so this seems a good trade-off.

Yes, I meant exactly what you are referring to, I think we are aligned. Static compilation could be better by proper dealing with symbols (i.e. making sure that ipopt/libxml2/assimp symbol visibility is local, or even better renaming the symbols after linking), but I don't think that tools to do that in a clean way exist, so this PR is already a huge improvement and probably the best option at the moment.

@diegoferigo
Copy link
Member Author

Yes, I meant exactly what you are referring to, I think we are aligned. Static compilation could be better by proper dealing with symbols (i.e. making sure that ipopt/libxml2/assimp symbol visibility is local, or even better renaming the symbols after linking), but I don't think that tools to do that in a clean way exist, so this PR is already a huge improvement and probably the best option at the moment.

Good point on the symbols visibility. It could be possible obtaining such setup if also the static version of all the dependencies is found in the system. However, this is not the case.

@traversaro
Copy link
Member

Yes, I meant exactly what you are referring to, I think we are aligned. Static compilation could be better by proper dealing with symbols (i.e. making sure that ipopt/libxml2/assimp symbol visibility is local, or even better renaming the symbols after linking), but I don't think that tools to do that in a clean way exist, so this PR is already a huge improvement and probably the best option at the moment.

Good point on the symbols visibility. It could be possible obtaining such setup if also the static version of all the dependencies is found in the system. However, this is not the case.

Yes, if you need static version of the library vcpkg may be more indicated.

@traversaro traversaro merged commit a07522e into robotology:devel Apr 6, 2021
@diegoferigo diegoferigo deleted the feature/manylinux_2_24 branch April 6, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants