fix(tags): handle empty mac_ver and probe failures in mac_platforms - #1408
Open
CAOShurong wants to merge 2 commits into
Open
CAOShurong wants to merge 2 commits into
CAOShurong wants to merge 2 commits into
Conversation
When platform.mac_ver()[0] returns an empty string (e.g. in containerized or non-standard macOS environments), parsing the version via int() raises ValueError: invalid literal for int() with base 10: ''. Additionally, when probing for macOS 10.16 SDK compatibility, subprocess.run replaced os.environ entirely instead of copying it, and unhandled probe failures or empty output could cause further errors. This change: - Gracefully handles empty or unparseable mac_ver output by returning an empty platforms iterator. - Preserves os.environ when invoking the SYSTEM_VERSION_COMPAT subprocess probe. - Strips probe stdout and guards against SubprocessError and ValueError. - Adds unit tests covering empty mac_ver and subprocess probe failures.
- Remove unreachable dead code 'if version is None: return' in mac_platforms. - Add test cases covering SubprocessError and non-integer ValueError during macOS 10.16 version probing. - Ensure 100% line and branch coverage across src/packaging/tags.py.
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
On a failed 10.16 compatibility probe this now continues with version == (10, 16), but 10.16 is the compatibility value that caused the probe to run, not the host's real release. An actual macOS 11+ interpreter can therefore advertise only macosx_10_16_*/older tags and miss compatible macosx_11_0_* (or newer) wheels. The new test currently locks in that misclassification. Please avoid emitting tags from the unresolved compatibility version, or recover the real OS version another way, and add a regression showing a failed probe cannot downgrade a Big Sur-or-newer host to 10.16.
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
2 participants
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.
When
platform.mac_ver()[0]returns an empty string (such as in non-standard macOS environments, minimal containers, or chroots),tags.mac_platforms()attemptsint("")intuple(map(int, version_str.split(".")[:2])), raisingValueError: invalid literal for int() with base 10: ''.Additionally:
subprocess.runwas passedenv={"SYSTEM_VERSION_COMPAT": "0"}without copyingos.environ, stripping all existing environment variables (such asPATHand dynamic linker paths) for the child process.This PR:
mac_veroutput by returning early without platforms.os.environwhen probingSYSTEM_VERSION_COMPAT=0.SubprocessErrorandValueError.version is None.tests/test_tags.pyand updatesCHANGELOG.rst.