Repository: pyodide/pyodide
Pull request: Fix micropip package name resolution
### Description

Hopefully resolve #2408

### Checklists

- [x] Add a [CHANGELOG](https://github.com/pyodide/pyodide/blob/main/docs/project/changelog.md) entry
- [x] Add / update tests


Problem statement:
micropip does not resolve locally available versions 
I have three wheels:

* travertino is v0.1.3
* toga-core is v0.3.0.dev33, and declares travertino>=0.1.3 as a requirement
* toga-web is v0.3.0.dev33, and declares toga-core==0.3.0.dev33 as a requirement

All three exist as `.whl` files in my static files folder. Travertino 0.1.3 has been published to PyPI; toga-core and toga-web dev32 have been published to PyPI, but dev33 are local development builds. 

On my Pyodide page, I run:
```
import micropip
micropip.install([
    './static/travertino-0.1.3-py3-none-any.whl',
    './static/toga_core-0.3.0.dev33-py3-none-any.whl',
    './static/toga_web-0.3.0.dev33-py3-none-any.whl',
])
```
This raises the following exception:
```
Traceback (most recent call last):
  File "/lib/python3.10/asyncio/futures.py", line 201, in result
    raise self._exception
  File "/lib/python3.10/asyncio/tasks.py", line 234, in __step
    result = coro.throw(exc)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 183, in install
    transaction = await self.gather_requirements(requirements, ctx, keep_going)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 173, in gather_requirements
    await gather(*requirement_promises)
  File "/lib/python3.10/asyncio/futures.py", line 284, in __await__
    yield self  # This tells Task to wait for completion.
  File "/lib/python3.10/asyncio/tasks.py", line 304, in __wakeup
    future.result()
  File "/lib/python3.10/asyncio/futures.py", line 201, in result
    raise self._exception
  File "/lib/python3.10/asyncio/tasks.py", line 232, in __step
    result = coro.send(None)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 245, in add_requirement
    await self.add_wheel(name, wheel, version, (), ctx, transaction)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 316, in add_wheel
    await self.add_requirement(recurs_req, ctx, transaction)
  File "/lib/python3.10/site-packages/micropip/_micropip.py", line 286, in add_requirement
    raise ValueError(
ValueError: Couldn't find a pure Python 3 wheel for 'toga-core==0.3.0.dev33'. You can use `micropip.install(..., keep_going=True)` to get a list of all packages with missing wheels.
```

If I relax the `install_requires` on toga-web to remove the declared dependency on toga-core, the import succeeds, and the code runs successfully.

I've tried splitting the install into two passes:
```
await micropip.install([
    './static/travertino-0.1.3-py3-none-any.whl',
    './static/toga_core-0.3.0.dev33-py3-none-any.whl',
])
await micropip.install([
    './static/toga_web-0.3.0.dev33-py3-none-any.whl',
])
```
thinking that if toga-core's installation was completed, toga-web would be able to find it; that makes no difference. 

I've also tried producing the packages without the .dev33 version suffix, thinking that it might be "prerelease" versions that are the issue, but that made no difference either.

However, if I revert the version number on toga-core and toga-web to 0.3.0.dev32 (the last versions published to PyPI), the import *succeeds* - but the old version (published to PyPI) version of the toga-core code is used. 

Looking at the network requests that the page has made indicates that the wheels were all loaded - and then micropip requested the toga-core PyPI metadata, and toga_core-0.3.0.dev32 (the most recent published version) was downloaded from PyPI. Interestingly, *no* request was made to load Travertino 0.1.3 from PyPI as a result of the dependency declared in toga-core.

Hints from issue discussion:
Thanks for the report. Yes I can confirm that this a bug and splitting the install should work.

The reason of this bug is that micropip is handling the package name differently when it is installed from PyPI or from `.whl` url.

```python
>>> import micropip
>>> await micropip.install("toga-web==0.3.0.dev32")
>>> micropip.list()
Name       | Version     | Source
---------- | ----------- | -------
travertino | 0.1.3       | pypi
toga-core  | 0.3.0.dev32 | pypi
toga-web   | 0.3.0.dev32 | pypi
...
```

```python
>>> import micropip
>>> await micropip.install("https://files.pythonhosted.org/packages/56/93/7bf199546e0de51404d7c714c618f9e67d215c2723351e1af0392e5e0a59/toga_web-0.3.0.dev32-py3-none-any.whl")
>>> micropip.list()
Name       | Version     | Source
---------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------
travertino | 0.1.3       | pypi
toga-core  | 0.3.0.dev32 | pypi
toga_web   | 0.3.0.dev32 | https://files.pythonhosted.org/packages/56/93/7bf199546e0de51404d7c714c618f9e67d215c2723351e1af0392e5e0a59/toga_web-0.3.0.dev32-py3-none-any.whl
...
```

When installed from PyPI, micropip think its name is `toga-web`, while when installed from whl url, micropip parses the name from whl url: `toga_web`.
Thanks for the report. I think in any case  it would be good to be able to disable dependency resolution to force install a set of packages https://github.com/pyodide/pyodide/issues/2412 

> The reason of this bug is that micropip is handling the package name differently when it is installed from PyPI or from .whl URL.

Indeed, as far as I remember this was done because for PyPI packages we have the package index with all the metadata, while for custom URL everything is inside the wheel. So far we are parsing the wheel name, to determine if it's a pure python wheel and also how to reference the package in question by name. The issue is that all  `-` in the package name get transformed into `_` in the wheel filename. 

I think it should work if we take the package name from inside the wheel metadata [here](https://github.com/pyodide/pyodide/blob/main/packages/micropip/src/micropip/_micropip.py#L314) and correct it if it doesn't match. Though then we would need to make sure the names  in `transaction["locked"]` also use this transformation with `_` since that is set before the wheel is downloaded.
@rth Makes sense; and I agree that `no_deps` would be a good workaround - possibly even a preferable one for "production" situations where you'd be looking for an analog of lockfile handling so you know exactly what is being delivered to the client.

Commit message: Update changelog

Patch to review:
diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index 0f94b332707..dded3ab9070 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -14,6 +14,9 @@ substitutions:
 
 ## Unreleased
 
+- {{ Fix }} micropip now correctly handles dash-included package names
+  {pr}`2414`
+
 - {{ Enhancement }} We now put our built files into the `dist` directory rather
   than the `build` directory. {pr}`2387`
 
diff --git a/packages/micropip/src/micropip/_micropip.py b/packages/micropip/src/micropip/_micropip.py
index bdcfeea5498..a42120ee0ae 100644
--- a/packages/micropip/src/micropip/_micropip.py
+++ b/packages/micropip/src/micropip/_micropip.py
@@ -293,7 +293,10 @@ async def add_requirement(self, requirement: str | Requirement, ctx, transaction
             )
 
     async def add_wheel(self, name, wheel, version, extras, ctx, transaction):
-        transaction["locked"][name] = PackageMetadata(name=name, version=version)
+        transaction["locked"][name] = PackageMetadata(
+            name=name,
+            version=version,
+        )
 
         try:
             wheel_bytes = await fetch_bytes(wheel["url"])
@@ -312,10 +315,43 @@ async def add_wheel(self, name, wheel, version, extras, ctx, transaction):
 
         with ZipFile(io.BytesIO(wheel_bytes)) as zip_file:
             dist = pkg_resources_distribution_for_wheel(zip_file, name, "???")
+
+        # the package name and version in the wheel file name can be different from
+        # real package project name specified in METADATA file.
+        # The most common example of this is when the package name contains "-",
+        # it will be replaced with "_" in wheel file name.
+        real_name = dist.project_name
+        if real_name == "UNKNOWN":
+            real_name = name
+        try:
+            real_version = dist.version
+        except ValueError:
+            real_version = version
+
+        if real_name != name or real_version != version:
+            transaction["locked"].pop(name)
+            transaction["locked"][real_name] = PackageMetadata(
+                name=real_name, version=real_version
+            )
+
         for recurs_req in dist.requires(extras):
             await self.add_requirement(recurs_req, ctx, transaction)
 
-        transaction["wheels"].append((name, wheel, version))
+        # We have to recheck that duplicated packages are not added,
+        # because the package name can be changed (e.g. "foo_bar" -> "foo-bar")
+        for _name, _, _version in transaction["wheels"]:
+            if real_name != _name:
+                continue
+
+            if real_version == _version:
+                return
+
+            raise ValueError(
+                f"Dependency resolution failed:"
+                f"Trying to install multiple versions of {real_name} ({real_version} <==> {_version})."
+            )
+
+        transaction["wheels"].append((real_name, wheel, real_version))
 
     def find_wheel(
         self, metadata: dict[str, Any], req: Requirement
diff --git a/packages/micropip/test_micropip.py b/packages/micropip/test_micropip.py
index ccc6ffdf6dd..5b7b262e09d 100644
--- a/packages/micropip/test_micropip.py
+++ b/packages/micropip/test_micropip.py
@@ -401,6 +401,27 @@ def test_list_wheel_package(monkeypatch):
     assert "dummy" in pkg_list and pkg_list["dummy"].source.lower() == dummy_url
 
 
+def test_list_wheel_name_mismatch(monkeypatch):
+    pytest.importorskip("packaging")
+    from micropip import _micropip
+
+    dummy_pkg_name = "dummy-dummy"
+    dummy_url = (
+        f"https://dummy.com/{dummy_pkg_name.replace('-', '_')}-1.0.0-py3-none-any.whl"
+    )
+    _mock_fetch_bytes = mock_fetch_bytes(dummy_pkg_name, f"Name: {dummy_pkg_name}")
+
+    monkeypatch.setattr(_micropip, "fetch_bytes", _mock_fetch_bytes)
+
+    asyncio.get_event_loop().run_until_complete(_micropip.install(dummy_url))
+
+    pkg_list = _micropip._list()
+    assert (
+        "dummy-dummy" in pkg_list
+        and pkg_list["dummy-dummy"].source.lower() == dummy_url
+    )
+
+
 def test_list_pyodide_package(selenium_standalone_micropip):
     selenium = selenium_standalone_micropip
     selenium.run_js(


Code review comments:
diff_hunk: @@ -337,6 +337,20 @@ async def add_wheel(self, name, wheel, version, extras, ctx, transaction):
         for recurs_req in dist.requires(extras):
             await self.add_requirement(recurs_req, ctx, transaction)
 
+        # We have to recheck that duplicated packages are not added,
+        # because the package name can be changed (e.g. "foo_bar" -> "foo-bar")
+        for _name, _, _version in transaction["wheels"]:
+            if real_name != _name:
+                continue
+
+            if real_version == _version:
+                return
+
+            raise ValueError(
+                f"Dependency resolution failed:"
+                f"Trying to install multiple versions of {real_name} ({real_version} <==> {_version})."
+            )
path: packages/micropip/src/micropip/_micropip.py
text: @user1:
It's not clear to me that this needs to be an error -- it could be that we manually installed a slightly old version which satisfies the current constraint, but it didn't recognize the slightly old version, so then we try to install a newer version, then notice that the package was already installed, then notice the version conflict but we could just drop the newer version. We don't have any way of backtracking at this point to find out whether `real_version` is satisfactory though...

@author:
I added this to prevent from multiple versions of packages being queued. But I think we won't need this if we normalize names beforehand.

diff_hunk: @@ -312,10 +315,43 @@ async def add_wheel(self, name, wheel, version, extras, ctx, transaction):
 
         with ZipFile(io.BytesIO(wheel_bytes)) as zip_file:
             dist = pkg_resources_distribution_for_wheel(zip_file, name, "???")
+
+        # the package name and version in the wheel file name can be different from
+        # real package project name specified in METADATA file.
+        # The most common example of this is when the package name contains "-",
+        # it will be replaced with "_" in wheel file name.
+        real_name = dist.project_name
+        if real_name == "UNKNOWN":
+            real_name = name
+        try:
+            real_version = dist.version
+        except ValueError:
+            real_version = version
+
+        if real_name != name or real_version != version:
+            transaction["locked"].pop(name)
+            transaction["locked"][real_name] = PackageMetadata(
path: packages/micropip/src/micropip/_micropip.py
text: @user1:
I'm not sure that's the best approach. 

How about rather in the list of our locked and installed packages use `name.replace('-', '_')` as key? Then updating the actual name in the PackageMetadata.name.
That would be better than erroring if multiple variants are installed. Unless there are cases when a package name with a '-' vs '_' are different packages, but I think that would very ill-advised anyway.

@user2:
Yeah +1 for this. We should be normalizing the keys. Maybe packaging has a method to do that?

@user1:
For normalization it's this regexp https://peps.python.org/pep-0427/#escaping-and-unicode

@author:
Yes, I think keeping both the normalized name and actual project name will work without this update thing. I'll try with that. Thanks for the suggestion.