Skip to content

GH-33804: [Python] Add support for manylinux_2_28 wheel #33805

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

Closed
wants to merge 27 commits into from

Conversation

sjperkins
Copy link
Contributor

@sjperkins sjperkins commented Jan 20, 2023

Closes #33804

Rationale for this change

At some point, it would be useful to support the new C++ ABI _GLIBCXX_USE_CXX11_ABI=1 in pyarrow wheels, especially when moving to C++17:

I wanted to create a pyarrow wheel that supported the above ABI and adapted the existing CENTOS 7 manylinux2014 Dockerfile/wheel to produce a AlmaLinux 8 manylinux_2_28 Dockerfile/wheel.

Publishing wheels with a new ABI needs careful consideration so I think this is low priority, but I thought I'd provide this manylinux_2_28 implementation in case it was useful for current/future adoption.

What changes are included in this PR?

A manylinux_2_28 Dockerfile, adopted from the existing manylinux2014 Dockerfile

Are these changes tested?

Manually tested at present

Are there any user-facing changes?

Yes, there's a major ABI change, as pyarrow will be compiled with _GLIBCXX_USE_CXX11_ABI=1

@github-actions
Copy link

@github-actions
Copy link

⚠️ GitHub issue #33804 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

@sjperkins
Copy link
Contributor Author

Could you also add entries for this to https://github.com/apache/arrow/blob/master/dev/tasks/tasks.yml#L455-L465 ?

I've done so, but I'm not sure how to test correctness locally. This seems to be part of the crossbow archery component which kicks off nightly CI services which I'm fairly certain I don't have permissions for.

Crossbow is a subcomponent of Archery and can be used to manually trigger builds. The tasks which can be run on Crossbow can be found in the dev/tasks directory. This directory contains:

  • the file dev/tasks/tasks.yml containing the configuration for various tasks which can be run via Crossbow
  • subdirectories containing different task templates (specified using jinja2 syntax <https://jinja.palletsprojects.com/>_), divided roughly by language or package management system.

Most of these tasks are run as part of the nightly builds, though they can also be triggered manually by add a comment to a PR which begins with @github-actions crossbow submit followed by the name of the task to be run.

For convenience purpose, the tasks in dev/tasks/tasks.yml are defined in groups, which makes it simpler for multiple tasks to be submitted to Crossbow at once. The task definitions here contain information about which service defined in docker-compose.yml to run, the CI service to run the task on, and which template file to use as the basis for that task.

@sjperkins
Copy link
Contributor Author

sjperkins commented Jan 21, 2023

Could you also add entries for this to https://github.com/apache/arrow/blob/master/dev/tasks/tasks.yml#L455-L465 ?

I've done so, but I'm not sure how to test correctness locally. This seems to be part of the crossbow archery component which kicks off nightly CI services which I'm fairly certain I don't have permissions for.

Crossbow is a subcomponent of Archery and can be used to manually trigger builds. The tasks which can be run on Crossbow can be found in the dev/tasks directory. This directory contains:

  • the file dev/tasks/tasks.yml containing the configuration for various tasks which can be run via Crossbow
  • subdirectories containing different task templates (specified using jinja2 syntax <https://jinja.palletsprojects.com/>_), divided roughly by language or package management system.

Most of these tasks are run as part of the nightly builds, though they can also be triggered manually by add a comment to a PR which begins with @github-actions crossbow submit followed by the name of the task to be run.

For convenience purpose, the tasks in dev/tasks/tasks.yml are defined in groups, which makes it simpler for multiple tasks to be submitted to Crossbow at once. The task definitions here contain information about which service defined in docker-compose.yml to run, the CI service to run the task on, and which template file to use as the basis for that task.

I see this started CI and related tests which revealed issues, addressed in 424fa08 and bf6f3b1.

@sjperkins
Copy link
Contributor Author

I don't think I understand the Archery/Crossbow setup. May I have some guidance on how to correct it please?

@kou
Copy link
Member

kou commented Jan 21, 2023

We can run added tasks by @github-actions crossbow submit wheel-manylinux* comment after we fix "Archery & Crossbow" CI failures.
(But you can't do it yet because you're a first-time contributor for now.)

See also: https://arrow.apache.org/docs/dev/developers/continuous_integration/overview.html#action-triggered-builds

@kou
Copy link
Member

kou commented Jan 21, 2023

You don't need to setup Crossbow locally.

@kou
Copy link
Member

kou commented Jan 22, 2023

@github-actions crossbow submit wheel-manylinux*

@github-actions
Copy link

Revision: ee4d79d

Submitted crossbow builds: ursacomputing/crossbow @ actions-05f1584a4c

Task Status
wheel-manylinux2014-cp310-amd64 Github Actions
wheel-manylinux2014-cp310-arm64 Travis CI
wheel-manylinux2014-cp311-amd64 Github Actions
wheel-manylinux2014-cp311-arm64 Travis CI
wheel-manylinux2014-cp37-amd64 Github Actions
wheel-manylinux2014-cp37-arm64 Travis CI
wheel-manylinux2014-cp38-amd64 Github Actions
wheel-manylinux2014-cp38-arm64 Travis CI
wheel-manylinux2014-cp39-amd64 Github Actions
wheel-manylinux2014-cp39-arm64 Travis CI
wheel-manylinux_2_28-cp310-amd64 Github Actions
wheel-manylinux_2_28-cp310-arm64 Travis CI
wheel-manylinux_2_28-cp311-amd64 Github Actions
wheel-manylinux_2_28-cp311-arm64 Travis CI
wheel-manylinux_2_28-cp37-amd64 Github Actions
wheel-manylinux_2_28-cp37-arm64 Travis CI
wheel-manylinux_2_28-cp38-amd64 Github Actions
wheel-manylinux_2_28-cp38-arm64 Travis CI
wheel-manylinux_2_28-cp39-amd64 Github Actions
wheel-manylinux_2_28-cp39-arm64 Travis CI

@raulcd
Copy link
Member

raulcd commented Jan 23, 2023

@github-actions crossbow submit wheel-manylinux*

@kou
Copy link
Member

kou commented Jan 30, 2023

@github-actions crossbow submit wheel-manylinux*

@github-actions
Copy link

Revision: 21a7ff1

Submitted crossbow builds: ursacomputing/crossbow @ actions-a5e60b310f

Task Status
wheel-manylinux-2-28-cp310-amd64 Github Actions
wheel-manylinux-2-28-cp310-arm64 Travis CI
wheel-manylinux-2-28-cp311-amd64 Github Actions
wheel-manylinux-2-28-cp311-arm64 Travis CI
wheel-manylinux-2-28-cp37-amd64 Github Actions
wheel-manylinux-2-28-cp37-arm64 Travis CI
wheel-manylinux-2-28-cp38-amd64 Github Actions
wheel-manylinux-2-28-cp38-arm64 Travis CI
wheel-manylinux-2-28-cp39-amd64 Github Actions
wheel-manylinux-2-28-cp39-arm64 Travis CI
wheel-manylinux-2014-cp310-amd64 Github Actions
wheel-manylinux-2014-cp310-arm64 Travis CI
wheel-manylinux-2014-cp311-amd64 Github Actions
wheel-manylinux-2014-cp311-arm64 Travis CI
wheel-manylinux-2014-cp37-amd64 Github Actions
wheel-manylinux-2014-cp37-arm64 Travis CI
wheel-manylinux-2014-cp38-amd64 Github Actions
wheel-manylinux-2014-cp38-arm64 Travis CI
wheel-manylinux-2014-cp39-amd64 Github Actions
wheel-manylinux-2014-cp39-arm64 Travis CI

@sjperkins
Copy link
Contributor Author

sjperkins commented Jan 30, 2023

I still see complaints re: missing linux-headers required for openssl unfortunately so it seems dnf didn't help in this case:

https://app.travis-ci.com/github/ursacomputing/crossbow/builds/260129759#L2059

I'll investigate more locally.

@sjperkins
Copy link
Contributor Author

I still see complaints re: missing linux-headers required for openssl unfortunately so it seems dnf didn't help in this case:

https://app.travis-ci.com/github/ursacomputing/crossbow/builds/260129759#L2059

If I insert the following into the Dockerfile

RUN vcpkg install \
        --clean-after-build \
        --x-install-root=${VCPKG_ROOT}/installed \
        openssl

and run

$ PYTHON=3.8 ARCH=arm64v8 archery docker run python-wheel-manylinux-2-28 

it'll warn about missing linux-headers but the step will succeed.

I need to focus on other priorities for the next week. I'll try pick it up again after that. Note this error was reported on the vcpkg repo

where the reported solution was to install perl-IPC-Cmd (which is installed in the manylinux Dockerfile in any case).

Would it be possible to run the ci once more to get around the typo in 0cc575c?

@raulcd
Copy link
Member

raulcd commented Jan 30, 2023

@github-actions crossbow submit wheel-manylinux*

@github-actions
Copy link

Revision: 0cc575c

Submitted crossbow builds: ursacomputing/crossbow @ actions-14d1746afa

Task Status
wheel-manylinux-2-28-cp310-amd64 Github Actions
wheel-manylinux-2-28-cp310-arm64 Travis CI
wheel-manylinux-2-28-cp311-amd64 Github Actions
wheel-manylinux-2-28-cp311-arm64 Travis CI
wheel-manylinux-2-28-cp37-amd64 Github Actions
wheel-manylinux-2-28-cp37-arm64 Travis CI
wheel-manylinux-2-28-cp38-amd64 Github Actions
wheel-manylinux-2-28-cp38-arm64 Travis CI
wheel-manylinux-2-28-cp39-amd64 Github Actions
wheel-manylinux-2-28-cp39-arm64 Travis CI
wheel-manylinux-2014-cp310-amd64 Github Actions
wheel-manylinux-2014-cp310-arm64 Travis CI
wheel-manylinux-2014-cp311-amd64 Github Actions
wheel-manylinux-2014-cp311-arm64 Travis CI
wheel-manylinux-2014-cp37-amd64 Github Actions
wheel-manylinux-2014-cp37-arm64 Travis CI
wheel-manylinux-2014-cp38-amd64 Github Actions
wheel-manylinux-2014-cp38-arm64 Travis CI
wheel-manylinux-2014-cp39-amd64 Github Actions
wheel-manylinux-2014-cp39-arm64 Travis CI

@sjperkins
Copy link
Contributor Author

This PR is still on my mind, but I haven't been able to devote time to working out a way forward.

@sjperkins
Copy link
Contributor Author

Looks like the following issue is relevant:

@sjperkins
Copy link
Contributor Author

Applied the fix 95d6827 described at the end of microsoft/vcpkg#29674 (comment):

Edit: In the port Unix/CMakeLists.txt I added "--cross-compile-prefix=\"\"" to the flags for the Perl configure custom command and now it works. I think this is a platform problem because I didn't have that problem with an ubuntu based docker image. Feel free to close if you agree.

I'm still testing this locally with a slow, emulated arm64v8 build, but it seems to have passed the openssl build step:

$ ARCH=arm64v8 PYTHON=3.8 archery docker run python-wheel-manylinux-2-28
[+] Building 4964.0s (18/22)                                                                                                                   
 => [ 4/17] COPY ci/scripts/install_cmake.sh arrow/ci/scripts/                                                                           0.0s
 => [ 5/17] RUN /arrow/ci/scripts/install_cmake.sh arm64v8 linux 3.21.4 /usr/local                                                      16.8sc 
 => [ 6/17] COPY ci/scripts/install_ninja.sh arrow/ci/scripts/                                                                           0.0s
 => [ 7/17] RUN /arrow/ci/scripts/install_ninja.sh 1.10.2 /usr/local                                                                   399.5s  
 => [ 8/17] COPY ci/scripts/install_ccache.sh arrow/ci/scripts/                                                                          0.0s
 => [ 9/17] RUN /arrow/ci/scripts/install_ccache.sh 4.1 /usr/local                                                                     368.1s  
 => [10/17] COPY ci/vcpkg/*.patch      ci/vcpkg/*linux*.cmake      arrow/ci/vcpkg/                                                       0.0s
 => [11/17] COPY ci/scripts/install_vcpkg.sh      arrow/ci/scripts/                                                                      0.0s  
 => [12/17] RUN arrow/ci/scripts/install_vcpkg.sh /opt/vcpkg 2871ddd918cecb9cb642bcb9c56897f397283192                                 1270.0s
 => [13/17] COPY ci/vcpkg/vcpkg.json arrow/ci/vcpkg/                                                                                     0.0s 
 => [14/17] RUN vcpkg install         --clean-after-build         --x-install-root=/opt/vcpkg/installed         --x-manifest-root=/a  2853.9s
 => => # -- Extracting source /opt/vcpkg/downloads/google-snappy-1.1.9.tar.gz                                                                  
 => => # -- Applying patch fix_clang-cl_build.patch                                                                                          
 => => # -- Applying patch snappy-disable-bmi.patch                                                                                            
 => => # -- Using source at /opt/vcpkg/buildtrees/snappy/src/1.1.9-405ddc5021.clean                                                          
 => => # -- Configuring arm64-linux-static-release                                                                                           
 => => # -- Building arm64-linux-static-release-rel   

@kou @raulcd Could you please initiate a CI run?

@sjperkins
Copy link
Contributor Author

@kou @raulcd Could you please initiate a CI run?

Ah this failed locally, nevermind.

@amol-
Copy link
Member

amol- commented Mar 30, 2023

Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍

@amol- amol- closed this Mar 30, 2023
@sjperkins
Copy link
Contributor Author

Thanks for bumping this. Unfortunately I don't have a local arm64 system that would make it possible to quickly debug the build issues on that platform.

Perhaps if the need for this change arises in future, someone with an arm64 system could take a look.

@felipecrv
Copy link
Contributor

@sjperkins try rebasing and push -f to get a clean CI run that might actually pass.

@sjperkins
Copy link
Contributor Author

@sjperkins try rebasing and push -f to get a clean CI run that might actually pass.

I think the PR needs to re-opened for this to work? Pushes to the fork don't seem to be reflecting here anymore.

https://github.com/sjperkins/arrow/tree/manylinux-2-28

@jorisvandenbossche
Copy link
Member

Pushes to the fork don't seem to be reflecting here anymore.

Because you (force) pushed while the PR was closed, it can't be reopened anymore .. Sorry! (unless you would push again the branch as it was before, https://gist.github.com/robertpainsi/2c42c15f1ce6dab03a0675348edd4e2c)

@amol-
Copy link
Member

amol- commented Mar 31, 2023

It's probably easier to just open a new PR, that wouldn't require any change on @sjperkins side and we could link the two PRs

@sjperkins
Copy link
Contributor Author

It's probably easier to just open a new PR, that wouldn't require any change on @sjperkins side and we could link the two PRs

Done in #34818

kou added a commit that referenced this pull request May 3, 2023
Closes #33804 

### Rationale for this change

At some point, it would be useful to support the new C++ ABI `_GLIBCXX_USE_CXX11_ABI=1` in pyarrow wheels, especially when moving to C++17:

- #32415

I wanted to create a pyarrow wheel that supported the above ABI and adapted the existing CENTOS 7 manylinux2014 Dockerfile/wheel to produce a AlmaLinux 8 manylinux_2_28 Dockerfile/wheel.

Publishing wheels with a new ABI needs [careful consideration](https://pypackaging-native.github.io/key-issues/native-dependencies/cpp_deps/) so I think this is low priority, but I thought I'd provide this manylinux_2_28 implementation in case it was useful for current/future adoption. 

### What changes are included in this PR?

A manylinux_2_28 Dockerfile, adopted from the existing manylinux2014 Dockerfile

### Are these changes tested?

Manually tested at present

### Are there any user-facing changes?

Yes, there's a major ABI change, as pyarrow will be compiled with `_GLIBCXX_USE_CXX11_ABI=1`
* Closes: #33804

Supercedes:
* #33805
* Closes: #33804

Lead-authored-by: Simon Perkins <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
liujiacheng777 pushed a commit to LoongArch-Python/arrow that referenced this pull request May 11, 2023
…#34818)

Closes apache#33804 

### Rationale for this change

At some point, it would be useful to support the new C++ ABI `_GLIBCXX_USE_CXX11_ABI=1` in pyarrow wheels, especially when moving to C++17:

- apache#32415

I wanted to create a pyarrow wheel that supported the above ABI and adapted the existing CENTOS 7 manylinux2014 Dockerfile/wheel to produce a AlmaLinux 8 manylinux_2_28 Dockerfile/wheel.

Publishing wheels with a new ABI needs [careful consideration](https://pypackaging-native.github.io/key-issues/native-dependencies/cpp_deps/) so I think this is low priority, but I thought I'd provide this manylinux_2_28 implementation in case it was useful for current/future adoption. 

### What changes are included in this PR?

A manylinux_2_28 Dockerfile, adopted from the existing manylinux2014 Dockerfile

### Are these changes tested?

Manually tested at present

### Are there any user-facing changes?

Yes, there's a major ABI change, as pyarrow will be compiled with `_GLIBCXX_USE_CXX11_ABI=1`
* Closes: apache#33804

Supercedes:
* apache#33805
* Closes: apache#33804

Lead-authored-by: Simon Perkins <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
…#34818)

Closes apache#33804 

### Rationale for this change

At some point, it would be useful to support the new C++ ABI `_GLIBCXX_USE_CXX11_ABI=1` in pyarrow wheels, especially when moving to C++17:

- apache#32415

I wanted to create a pyarrow wheel that supported the above ABI and adapted the existing CENTOS 7 manylinux2014 Dockerfile/wheel to produce a AlmaLinux 8 manylinux_2_28 Dockerfile/wheel.

Publishing wheels with a new ABI needs [careful consideration](https://pypackaging-native.github.io/key-issues/native-dependencies/cpp_deps/) so I think this is low priority, but I thought I'd provide this manylinux_2_28 implementation in case it was useful for current/future adoption. 

### What changes are included in this PR?

A manylinux_2_28 Dockerfile, adopted from the existing manylinux2014 Dockerfile

### Are these changes tested?

Manually tested at present

### Are there any user-facing changes?

Yes, there's a major ABI change, as pyarrow will be compiled with `_GLIBCXX_USE_CXX11_ABI=1`
* Closes: apache#33804

Supercedes:
* apache#33805
* Closes: apache#33804

Lead-authored-by: Simon Perkins <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
rtpsw pushed a commit to rtpsw/arrow that referenced this pull request May 16, 2023
…#34818)

Closes apache#33804 

### Rationale for this change

At some point, it would be useful to support the new C++ ABI `_GLIBCXX_USE_CXX11_ABI=1` in pyarrow wheels, especially when moving to C++17:

- apache#32415

I wanted to create a pyarrow wheel that supported the above ABI and adapted the existing CENTOS 7 manylinux2014 Dockerfile/wheel to produce a AlmaLinux 8 manylinux_2_28 Dockerfile/wheel.

Publishing wheels with a new ABI needs [careful consideration](https://pypackaging-native.github.io/key-issues/native-dependencies/cpp_deps/) so I think this is low priority, but I thought I'd provide this manylinux_2_28 implementation in case it was useful for current/future adoption. 

### What changes are included in this PR?

A manylinux_2_28 Dockerfile, adopted from the existing manylinux2014 Dockerfile

### Are these changes tested?

Manually tested at present

### Are there any user-facing changes?

Yes, there's a major ABI change, as pyarrow will be compiled with `_GLIBCXX_USE_CXX11_ABI=1`
* Closes: apache#33804

Supercedes:
* apache#33805
* Closes: apache#33804

Lead-authored-by: Simon Perkins <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Packaging][Python] Add support for manylinux_2_28 wheels
6 participants