Fix dynamic library loading in cli runner#4871
Conversation
hoodmane
changed the title
All dependent libs should be loaded as globals. We should try to use this to simplify some of the logic in dynload.ts. Most likely what needs to happen is we need to inspect the wheel at build time to determine which libraries are "leaves" that are not dependencies of anything else and add this list to the package metadata.
hoodmane
force-pushed
the
scipy-cli-runner
branch
from
90defb6 to
d5d3924
Compare
|
Thanks @hoodmane. I am okay with this approach as a temporary fix, but I'll try to investigate why the upstream fix (#3865 (comment)) is not working for us... |
|
Thanks, @hoodmane! I tried this with |
|
Perhaps there are several bugs =( could you give a minimal reproduction of the stats models problem? A single |
|
Yes, I shall open a new issue for this – I'll have to investigate how to provide a minimal enough reproducer, please hold on :) |
|
It doesn't have to be that small, if you can give me a single file that does it that would be good enough for me. |
|
Also @agriyakhetarpal can you check it against #4876? Since that changes this same code again. |
|
|
All dependent libs should be loaded as globals. We should try to use this to simplify some of the logic in dynload.ts. Most likely what needs to happen is we need to inspect the wheel at build time to determine which libraries are "leaves" that are not dependencies of anything else and add this list to the package metadata. This should be removed eventually, because it is a bug somewhere else in the dynamic loader that dependents need to be global.
|
Sorry for the late response over here, @ryanking13 and @hoodmane, but I did spend some time on this a while ago – and the problem has now subsided with either #5011 or #5031 having been merged. I have no issues when running the Side note: we should really have an option to use any Pyodide version (>0.22, perhaps) in |
|
And |
…sting (#7525) This PR follows up on changes made in gh-7350. Here's a small, point-wise summary noting the changes here: - It updates the Emscripten/Pyodide CI job added in gh-7350 and removes the workaround JavaScript-based test suite file, since we fixed the `s_cmp` OpenBLAS unresolved symbol coming from SciPy upstream, having updated SciPy in-tree in Pyodide (see pyodide/pyodide#4719, pyodide/pyodide#5012, pyodide/pyodide#5031, and more). This should make the testing slightly cleaner than before, and the test suite now runs till the end without any Pyodide fatal errors being reported. The [Pyodide xbuildenv](https://github.com/pyodide/pyodide/releases/tag/0.27.0a2) can now be used to set up a virtual environment in which the tests can be run in the same way they were before. The [`pyodide-build` tool](https://github.com/pyodide/pyodide-build) has been unvendored from the Pyodide repository and now infers the Pyodide version from the releases, downloading the relevant files from the GitHub release. Hence, the `PYODIDE_VERSION` environment variable is no longer required. - There were some tests that have been failing on 32-bit platforms, which were last discussed quite a while ago: #3091. They surprisingly passed in a WASM 32-bit environment here without issues, which is great. I have updated the skipping conditions to accommodate this. - There's a known problem with the pytest bytecode generation when running the tests with the xbuildenv interpreter – I disabled the pytest internal cache plugin to fix that, but ignoring the warning (pypa/cibuildwheel#1966 (comment)) works equally as well. Please let me know if there is a preference. :) Additional context - Pyodide CI support was first discussed in gh-7265. - At the time of writing, Pyodide 0.27alpha2 is used in the CI job; once 0.27 stable comes out (should be soon) and `cibuildwheel` updates to it, that (plus this PR) will help unblock another PR that I had opened a while back: gh-7440. I can rebase that PR when ready. - pyodide/pyodide#4871 (comment) --- Co-authored-by: Lars Grüter <[email protected]>
3 participants
All dependent libs should be loaded as globals. We should try to use this to simplify some of the logic in dynload.ts. Most likely what needs to happen is we need to inspect the wheel at build time to determine which libraries are "leaves" that are not dependencies of anything else and add this list to the package metadata.
Resolves #3865.