KEMBAR78
gh-99948: Add emscripten platform support to ctypes.util.find_library by ryanking13 · Pull Request #99950 · python/cpython · GitHub
Skip to content

Conversation

@ryanking13
Copy link
Contributor

@ryanking13 ryanking13 commented Dec 2, 2022

This adds ctypes.util.find_library function for emscripten platform.

Emscripten internally uses LD_LIBRARY_PATH env variable to search libraries.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Dec 2, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Contributor

@mdboom mdboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to write a test for this? (I don't actually know if we have other emscripten platform tests, but if we do it would certainly be good to assert that we can load and make calls to a wasm binary here.)

@ryanking13
Copy link
Contributor Author

I added a test. Is there a way I can run Emscripten platform tests locally or via CI? @brettcannon

@brettcannon brettcannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 7, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @brettcannon for commit 07818b6 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 7, 2022
@brettcannon
Copy link
Member

brettcannon commented Dec 7, 2022

@ryanking13 we can run PRs against the whole buildbot fleet (which I have now triggered).

@ryanking13
Copy link
Contributor Author

ryanking13 commented Dec 8, 2022

Thanks, @brettcannon. Though I think the test_ctypes test suite is skipped because _ctypes module is omitted in the current WASM platform builds.

@brettcannon
Copy link
Member

You can run them locally via https://github.com/python/cpython/tree/main/Tools/wasm , but if the ctypes tests are turned off then I'm not comfortable accepting this PR until they are turned on and passing.

@ryanking13
Copy link
Contributor Author

but if the ctypes tests are turned off then I'm not comfortable accepting this PR until they are turned on and passing.

Yes, that makes sense. I guess we (Pyodide) should just add a patch for this until CPython includes more external libraries to WASM build.

@brettcannon
Copy link
Member

I guess we (Pyodide) should just add a patch for this until CPython includes more external libraries to WASM build.

I'm going to close this then as I don't know if/when that would happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants