[Python][Packaging] Set MACOSX_DEPLOYMENT_TARGET default before building wheel platform tag#50377
Draft
anxkhn wants to merge 1 commit into
Draft
[Python][Packaging] Set MACOSX_DEPLOYMENT_TARGET default before building wheel platform tag#50377anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
…ing wheel platform tag In ci/scripts/python_wheel_macos_build.sh the wheel platform tag _PYTHON_HOST_PLATFORM was assembled from MACOSX_DEPLOYMENT_TARGET one line before that variable's :-12.0 default was applied. When the script runs without MACOSX_DEPLOYMENT_TARGET already exported (for example a manual or local macOS build), the interpolation yields a malformed tag with an empty version component (macosx--<arch>), while the C++ libraries and the wheel are actually built for 12.0. That produces an inconsistent, invalid platform tag on the built wheel. Swap the two lines so the default is set first and the platform tag is derived from the resolved value. When MACOSX_DEPLOYMENT_TARGET is already set (as in the packaging CI, which exports it as a top-level env var), the output is unchanged, so there is no behavior change for existing jobs. Verified the two lines in isolation with the variable unset: before the change _PYTHON_HOST_PLATFORM=macosx--x86_64, after the change macosx-12.0-x86_64. The full macOS wheel build was not run (no Arrow C++ toolchain available locally).
|
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: |
Contributor
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.
Member
|
Could you open an issue for this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
In
ci/scripts/python_wheel_macos_build.shthe wheel platform tag_PYTHON_HOST_PLATFORMis built fromMACOSX_DEPLOYMENT_TARGETone line beforethat variable's
:-12.0default is applied. If the script runs withoutMACOSX_DEPLOYMENT_TARGETalready exported (for example a manual or local macOSbuild), the interpolation yields a malformed tag with an empty version
component,
macosx--<arch>, even though the C++ libraries and the wheel areactually built for 12.0. That produces an inconsistent, invalid platform tag on
the built wheel. The two
exportlines are simply in the wrong order.What changes are included in this PR?
Swap the two adjacent
exportlines so the default is set first and the platformtag is derived from the resolved value:
When
MACOSX_DEPLOYMENT_TARGETis already set (as in the packaging CI, whichexports it as a top-level
env:var), the output is unchanged, so there is nobehavior change for existing jobs. No other lines are touched.
Are these changes tested?
There is no unit-test harness for these packaging shell scripts. I verified the
two lines in isolation:
_PYTHON_HOST_PLATFORM=macosx--x86_64(malformed), after the change
_PYTHON_HOST_PLATFORM=macosx-12.0-x86_64.MACOSX_DEPLOYMENT_TARGETalready set (for example11.0): output ismacosx-11.0-x86_64both before and after, confirming no behavior change on theCI path.
shellcheckreports the same findings on this file before and after the change,and
bash -npasses. I did not run a full macOS wheel build (no Arrow C++toolchain available in my environment); the existing packaging CI jobs exercise
the script end to end.
Are there any user-facing changes?
No API changes. The only observable difference is that a wheel produced by a
manual/local run of this script without
MACOSX_DEPLOYMENT_TARGETexported nowgets a correct platform tag (
macosx-12.0-<arch>) instead of a malformed one.Wheels produced by the packaging CI are unaffected.
This PR contains a "Critical Fix". It fixes a bug that produced an invalid
platform tag (
macosx--<arch>) on a built wheel when the script is run withoutMACOSX_DEPLOYMENT_TARGETexported.