GH-50531: [Python][Packaging] Set macOS deployment target before building wheel platform tag - #50377
Conversation
|
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or See also: |
There was a problem hiding this comment.
Pull request overview
Fixes a macOS wheel packaging issue where _PYTHON_HOST_PLATFORM could be derived from an unset MACOSX_DEPLOYMENT_TARGET, producing a malformed wheel platform tag during manual/local builds.
Changes:
- Set
MACOSX_DEPLOYMENT_TARGET(with its default) before exporting_PYTHON_HOST_PLATFORMso the platform tag is always derived from the resolved value.
|
Could you open an issue for this. |
|
|
fe42499 to
a3b3ef1
Compare
a3b3ef1 to
c5084cf
Compare
|
@github-actions crossbow submit -g wheel |
|
Revision: c5084cf Submitted crossbow builds: ursacomputing/crossbow @ actions-31644df33f |
raulcd
left a comment
There was a problem hiding this comment.
@anxkhn it seems that part of the responses are LLM generated, like the description on the PR. In the description at the Are these changes tested? section the response is quite useless, what does it mean for git diff --check to pass? how is that useful or relevant info for the changes on this PR? Please, could you review.
bash -n and git diff --check pass. Targeted shell checks produce macosx-12.0-x86_64 when unset and preserve an explicit 13.0 target. A full wheel build was not run.
|
@raulcd rewrote the testing section to report only the host-platform checks and the crossbow wheel run. the git-diff-check note is gone. |
raulcd
left a comment
There was a problem hiding this comment.
The current CI failures are gone on main, could you rebase?
…e wheel tag Set the default MACOSX_DEPLOYMENT_TARGET before deriving _PYTHON_HOST_PLATFORM so local builds without an explicit target produce a valid platform tag. Preserve explicit deployment targets unchanged. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
@raulcd rebased onto current main. the macos deployment-target default still lands before the platform tag. |
c5084cf to
f028434
Compare
|
@github-actions crossbow submit wheel-macos-* |
raulcd
left a comment
There was a problem hiding this comment.
Thanks for the PR, I will merge once wheels are green.
|
Revision: f028434 Submitted crossbow builds: ursacomputing/crossbow @ actions-1f1f8ae178 |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 8e7a1f9. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 8e7a1f9. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Rationale for this change
ci/scripts/python_wheel_macos_build.shbuilds_PYTHON_HOST_PLATFORMfromMACOSX_DEPLOYMENT_TARGETbefore the script applies its default of12.0. When the variable is unset, the tag becomesmacosx--<arch>even though the rest of the build still targets macOS 12.0.What changes are included in this PR?
Move the deployment-target default above the platform-tag export so the tag always uses the resolved value. An explicit
MACOSX_DEPLOYMENT_TARGETis still honored.Are these changes tested?
I checked the two host-platform outcomes from the same assignment the script uses:
MACOSX_DEPLOYMENT_TARGETyieldsmacosx-12.0-x86_64MACOSX_DEPLOYMENT_TARGET=13.0yieldsmacosx-13.0-x86_64Crossbow macOS wheel jobs were also submitted on this branch after the rebase.
Are there any user-facing changes?
No public API change. Local or manual macOS wheels that rely on the default now get a valid platform tag instead of
macosx--<arch>.