[build] Build wheels with manylinux2014 #11621
Conversation
|
Now to see if Travis builds it correctly.... 🤔 |
|
Is this ready for review? Removing tag for now |
| build --remote_upload_local_results=false | ||
| EOF | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Why do we need this new script?
ericl
added
@author-action-required
barakmich
force-pushed
the
manylinux2014
branch
4 times, most recently
from
90c718c to
52236f1
Compare
python/build-wheel-manylinux2014.sh
Outdated
| @@ -54,6 +60,8 @@ for ((i=0; i<${#PYTHONS[@]}; ++i)); do | |||
| # dashboard directory. | |||
| git clean -f -f -x -d -e .whl -e python/ray/new_dashboard/client -e dashboard/client | |||
|
|
|||
| export BAZEL_LINKLIBS="-l%:libstdc++.a" | |||
There was a problem hiding this comment.
Hmm, does this mean we are linking libstdc++ statically? That looks a little sketchy to me. Probably we should dynamically link against the C++ standard library that comes with the manylinux2014 container.
There was a problem hiding this comment.
Yep. That's exactly what's happening though -- we're linking the libstdc++ in the manylinux2014 container.
There was a problem hiding this comment.
I think this is linking it statically, but we should be linking it dynamically. I have seen problems in the past where we have had two different versions of the standard library used in one executable which produced segfaults because some global objects clashed. My understanding is that this could be happening with this if e.g. a python module imports the c++ standard lib dynamically.
There was a problem hiding this comment.
@barakmich What happens if you remove this line, does it still work?
| @@ -9,7 +9,7 @@ produce .whl files owned by root. | |||
| Inside the root directory (i.e., one level above this python directory), run | |||
|
|
|||
| ``` | |||
| docker run -e TRAVIS_COMMIT=<commit_number_to_use> --rm -w /ray -v `pwd`:/ray -ti rayproject/arrow_linux_x86_64_base:python-3.8.0 /ray/python/build-wheel-manylinux1.sh | |||
| docker run -e TRAVIS_COMMIT=<commit_number_to_use> --rm -w /ray -v `pwd`:/ray -ti quay.io/pypa/manylinux2014_x86_64 /ray/python/build-wheel-manylinux2014.sh | |||
barakmich
force-pushed
the
manylinux2014
branch
from
e592731 to
b4c35d9
Compare
ci/travis/build-docker-images.py
Outdated
| @@ -110,8 +110,8 @@ def _build_cpu_gpu_images(image_name) -> List[str]: | |||
| ) - current_iter >= datetime.timedelta(minutes=5): | |||
| current_iter = datetime.datetime.now() | |||
| elapsed = datetime.datetime.now() - start | |||
| print(f"Still building {tagged_name} after " | |||
| f"{elapsed.seconds} seconds") | |||
| # print(f"Still building {tagged_name} after " | |||
python/build-wheel-manylinux2014.sh
Outdated
| @@ -54,6 +61,8 @@ for ((i=0; i<${#PYTHONS[@]}; ++i)); do | |||
| # dashboard directory. | |||
| git clean -f -f -x -d -e .whl -e python/ray/new_dashboard/client -e dashboard/client | |||
|
|
|||
| # export BAZEL_LINKLIBS="-l%:libstdc++.a" | |||
barakmich
force-pushed
the
manylinux2014
branch
from
b4c35d9 to
2a15d41
Compare
|
This is great! |
Why are these changes needed?
We've been building our wheels from an apache-arrow manylinux1 container. In discussion with the community and the team, we feel manylinux2014 is an appropriate standard for our users going forward. This change makes that happen, and also uses the upstream, standard manylinux build containers (https://github.com/pypa/manylinux) instead of one we maintain.
Related issue number
Closes #9666
Checks
scripts/format.shto lint the changes in this PR.