-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
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
bedevere-bot
commented
Sep 5, 2025
You don't have write permissions to trigger a build
!buildbot emscripten
bedevere-bot
commented
Sep 5, 2025
🤖 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: emscripten
The builders matched are:
WASM Emscripten PR
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.
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.
This seems overly complicated; suggest:
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.
There's a few cases missing from this test suite, including:
- Ignoring non-WASM files
- Finding a
libdummy.wasmfile - Finding nothing if
LD_LIBRARY_PATHisn't defined - Finding nothing if
LD_LIBRARY_PATHis defined, but doesn't include a WASM file.
It might be worth setting up a directory that contains libdummy.so and libother.wasm as a class-based startup fixture, and then adding tests that manipulate LD_LIBRARY_PATH to find (or not find) those two files.
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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
Uh oh!
There was an error while loading. Please reload this page.
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.