-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
GH-45041: [Packaging][Python] Use ORC from vcpkg instead of bundled on Linux and macOS #45046
Conversation
…dled on Linux and macOS wheels
@github-actions crossbow submit wheel-manylinux-2-28-cp312-cp312-amd64 |
Revision: 6eb5042 Submitted crossbow builds: ursacomputing/crossbow @ actions-b309acbcb8
|
We can see on the new logs, and is not downloaded/built anymore:
the orc tests on pyarrow being successful:
Previously it was built and we could see both the build process and the following log:
|
@github-actions crossbow submit wheel-manylinux-* |
Revision: 6eb5042 Submitted crossbow builds: ursacomputing/crossbow @ actions-a95168b3c6 |
@github-actions crossbow submit wheel-macos-* |
Revision: 6eb5042 Submitted crossbow builds: ursacomputing/crossbow @ actions-ad62b7a323 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 6c9e7e7. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
We are currently downloading and building ORC every time for our wheels. We should not require to bundle ORC ourselves and we could just use vcpkg.
What changes are included in this PR?
Stop using
ORC_SOURCE=BUNDLED
and enable the vcpkg orc feature.Are these changes tested?
Yes, on the wheels.
Are there any user-facing changes?
No