Skip to content
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

Fix platform tag version regression for macOS 11 and later #219

Merged
merged 7 commits into from
Aug 29, 2024

Conversation

HexDecimal
Copy link
Collaborator

@HexDecimal HexDecimal commented Jul 6, 2024

Zero out minor version of platform tags for macOS 11 and later which fixes the invalid tags for those releases. Closes #214

Delocate has a bias for testing older versions of macOS. I've added a test to ensure this works as intended.

Also discussed about in #214. This updates the behavior of MACOSX_DEPLOYMENT_TARGET and --require-target-macos-version. These will now raise the resulting platform tag versions if they are lower than the requested version. I'm certain this is desirable for MACOSX_DEPLOYMENT_TARGET but I'm not sure if it should be the same for --require-target-macos-version.

Copy link

codecov bot commented Jul 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.99%. Comparing base (c93f910) to head (19b5121).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
+ Coverage   96.94%   96.99%   +0.05%     
==========================================
  Files          16       16              
  Lines        1308     1333      +25     
==========================================
+ Hits         1268     1293      +25     
  Misses         40       40              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HexDecimal
Copy link
Collaborator Author

Multiple sections of code had mislabeled intel as universal. I didn't see any issues on this, possibly because those platforms are old. These have been fixed.

Only universal2 and intel are supported as combined architectures right now. In theory more can be supported but it's hard to find documentation on which architectures map to each other. The most I found for this would be in the packaging source code. For now universal2 seems to be the only one being used publicly.

I've decided to deprecate --require-target-macos-version since it causes confusion with MACOSX_DEPLOYMENT_TARGET. From now on all instructions should only mention MACOSX_DEPLOYMENT_TARGET.

@HexDecimal HexDecimal marked this pull request as ready for review July 8, 2024 14:55
@HexDecimal HexDecimal changed the title Zero out minor version of platform tags for macOS 11 and later Fix platform tag version regression for macOS 11 and later Jul 8, 2024
Add tests for target release versions

Tests using env need to copy environ to ensure that subprocess works correctly
`--require-target-macos-version` could cause confusion in its current state.
It was probably a mistake to add it.
Simplify code involving architectures by working with their unpacked forms

Fixed code and tests which mislabeled `intel` as `universal`
This flag is redundant and does not mesh well with the MACOSX_DEPLOYMENT_TARGET environment variable.
Add relevant architecture to the deceptive tag warning.
@HexDecimal HexDecimal merged commit 95fb09c into matthew-brett:master Aug 29, 2024
16 checks passed
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.

Tool delocate-wheel alters tag in wheel name to seemingly invalid tag/number
1 participant