-
Notifications
You must be signed in to change notification settings - Fork 917
Modify the handing of Frameworks on Apple platforms #5606
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
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
cf315b3
Modify handling of Python Framework linking.
freakboy3742 9f0aac5
Add news fragment.
freakboy3742 5fc325b
Correct rustfmt issues.
freakboy3742 3820fa3
Correct some problems identified by clippy.
freakboy3742 ca120b7
Interpret an empty PYTHONFRAMEWORK as None not an empty string.
freakboy3742 55a1d95
Corrected handling of empty PYTHONFRAMEWORK definitions.
freakboy3742 4ed808d
Only add a framework linking path if there's a framework.
freakboy3742 4a77d15
Cleanup rustfmt problems.
freakboy3742 4bc523d
Merge branch 'main' into framework-linking
freakboy3742 20d4cb3
Document framework linking behavior.
freakboy3742 f2260c0
Convert empty framework prefixes to None.
freakboy3742 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| If the `sysconfigdata` defines `PYTHONFRAMEWORK`, and the build is configured to link the against Python library, the link will be performed against that framework, using `PYTHONFRAMEWORKPREFIX` as a framework search path. |
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
I think the 3.11 CI failure has found a problem with this approach.
Seems like
framework=Pythondoes not specify the version, and nor does the search prefix which is just/Library/Frameworks(for the official distributions).Not sure what implications this has for the iOS build either? Seems like it's fundamentally impossible to use frameworks and link the right version.
I wonder, maybe we don't need this PR? If cross-compile env has the correct
lib_dir, potentially PyO3mainis now already doing the correct thing to link against iOS via the versioned.dylibdirectly with the other patches placed into PyO3 and maturin?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.
Which failure is this? I'm seeing 11 skips, 32 successful tests in the most recent CI pass.
So - macOS frameworks and iOS frameworks (and every other non-macOS framework) are structured slightly differently.
macOS frameworks are versioned - there's a single top-level framework, with a "Versions" subfolder; each version of the framework is then contained in the top level framework. A
Currentsymlink exists to point at one specific version, and the top level framework links the content of the current version so that/Library/Frameworks/Python.frameworkcan be used for linking. However, that doesn't give you control over the specific version. If you want a specific version, you need to use-F /Library/Frameworks/Python.framework/Versions/3.13as your framework search path.So... that doesn't currently work, because iOS doesn't ship a copy of
libPython*.dylibin thelibfolder. There's a range of reasons why that is the case... but with some other accomodations, that might indeed be an easier way to solve the problem. I'll experiment with this and report back.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.
In the merge queue https://github.com/PyO3/pyo3/actions/runs/19310395011/job/55228919449
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.
I think I attempted this yesterday and had no luck. Possibly because
Python.Frameworkonly exists in/Library/Frameworksand not in any of the versioned subdirectories?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.
I added CI-build-full label to this PR so any further pushes will run the full suite of versions we run in the merge queue.