Skip to content
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

Fix micropip.install to search shared libraries inside the wheel correctly #97

Merged
merged 9 commits into from
Jan 31, 2024

Conversation

ryanking13
Copy link
Member

Previously, micropip was not able to locate shared libraries in the .libs directory of the wheel.

For instance, in Pyodide 0.25.0,

await pyodide.loadPackage("Shapely")

works, while

>>> import micropip
>>> await micropip.install("https://cdn.jsdelivr.net/pyodide/v0.25.0/full/Shapely-1.8.
2-cp311-cp311-emscripten_3_1_46_wasm32.whl")
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/lib/python3.11/site-packages/micropip/_commands/install.py", line 176, in ins
tall
    await asyncio.gather(*wheel_promises)
  File "/lib/python3.11/site-packages/micropip/transaction.py", line 168, in install
    await self.load_libraries(target)
  File "/lib/python3.11/site-packages/micropip/transaction.py", line 159, in load_libr
aries
    await asyncio.gather(*map(lambda dynlib: loadDynlib(dynlib, False), dynlibs))
pyodide.ffi.JsException: Error: Didn't expect to load any more file_packager files!

does not work.

This is because micropip.install does not search the path that pyodide.loadPackage looks for.

This PR fixes it by using the same API pyodide.loadPackage is using when loading shared libraries.

  • changelog

Copy link
Member

@hoodmane hoodmane left a comment

Choose a reason for hiding this comment

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

Thanks @ryanking13! Could you add a changelog entry?

name = parse_wheel_filename(path.name)[0]
url = self._register_handler(path)

if name in self._wheels and not replace:
return
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should raise here? I think silently doing nothing is likely to be confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used this method to register all the wheels included in the Pyodide distribution as test wheels, as without this, they would conflict with pytest wheel, etc that I put in for testing.

I think it will be fine as long as we know what we are doing. I added some comments about it.

.github/workflows/main.yml Show resolved Hide resolved
@ryanking13 ryanking13 merged commit 6d5f630 into pyodide:main Jan 31, 2024
7 checks passed
@ryanking13 ryanking13 deleted the load-sharedlib branch January 31, 2024 12:31
@ryanking13
Copy link
Member Author

Oh, I forgot to update the changelog...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants