-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-99948: Support ctypes.util.find_library in emscripten environment #138519
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
Conversation
|
!buildbot emscripten |
|
You don't have write permissions to trigger a build |
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @hoodmane for commit 857b0fa 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138519%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
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.
A couple of comments/suggestions about the implementation and testing strategy.
I'd also like @hoodmane to confirm the emscripten details on this; this all makes sense assuming LD_LIBRARY_PATH exits in the environment - but my understanding was the there were very limited opportunities to actually manipulate environment variables in a runtime environment.
It's entirely possible I'm missing something here - and the fact this patch comes from Pyodide suggests I might be - I just want to make sure I understand the full context here, and this isn't encoding as Pyodide-ism that won't necessarily apply elsewhere.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
The runtime environment might not have LD_LIBRARY_PATH. If it doesn't, it will simply fail to locate the library, but if it does, it can be used to locate the library from a specific location. Emscripten utilizes I think this is a general approach that can be used in other non-Pyodide environments. We've noticed that a few packages use |
|
|
|
!buildbot Emscripten |
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit ff6a669 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138519%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
|
!buildbot emscripten |
|
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 8157604 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138519%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
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.
One very cosmetic suggestion, but otherwise this looks good to me - thanks for the patch!
|
Thanks @ryanking13 for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…nment (pythonGH-138519) Adds support for ctypes loading .so files directly. --------- (cherry picked from commit 218b8db) Co-authored-by: Gyeongjae Choi <[email protected]> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <[email protected]>
|
GH-139022 is a backport of this pull request to the 3.14 branch. |
|
Thanks! |
…onment (GH-138519) (#139022) Co-authored-by: Gyeongjae Choi <[email protected]> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <[email protected]>
This PR adds
ctypes.util.find_librarysupport in Emscripten environment. I am resubmitting the patch that was closed around 3 years ago (#99950).The previous pull request was closed at that time, as CPython did not have an official Emscripten build. But now it is back to tier-3 with buildbot support, so I think it makes sense to support this in CPython Emscripten builds.
Pyodide is applying this patch to support packages that rely on ctypes.util.find_library to locate packages.