review fixes

pyodide_lock/cli.py

@@ -1,61 +1,57 @@
 from pathlib import Path
 
-try:
-    import typer
+import typer
 
-    from .spec import PyodideLockSpec
-    from .utils import add_wheels_to_spec
+from .spec import PyodideLockSpec
+from .utils import add_wheels_to_spec
 
-    main = typer.Typer()
+main = typer.Typer(help="manipulate pyodide-lock.json lockfiles.")
 
-    @main.command()
-    def add_wheels(
-        wheels: list[Path],
-        ignore_missing_dependencies: bool = typer.Option(
-            help="If this is true, dependencies "
-            "which are not in the original lockfile or "
-            "the added wheels will be added to the lockfile. "
-            "Warning: This will allow a broken lockfile to "
-            "be created.",
-            default=False,
-        ),
-        in_lockfile: Path = typer.Option(
-            help="Source lockfile (input)", default=Path("pyodide-lock.json")
-        ),
-        out_lockfile: Path = typer.Option(
-            help="Updated lockfile (output)", default=Path("pyodide-lock-new.json")
-        ),
-        base_path: Path = typer.Option(
-            help="Base path for wheels - wheel file "
-            "names will be created relative to this path.",
-            default=None,
-        ),
-        wheel_url: str = typer.Option(
-            help="Base url which will be appended to the wheel location."
-            "Use this if you are hosting these wheels on a different "
-            "server to core pyodide packages",
-            default="",
-        ),
-    ):
-        """Add a set of package wheels to an existing pyodide-lock.json and
-        write it out to pyodide-lock-new.json
 
-        Each package in the wheel will be added to the output lockfile,
-        including resolution of dependencies in the lock file. By default
-        this will fail if a dependency isn't available in either the
-        existing lock file, or in the set of new wheels.
+@main.command()
+def add_wheels(
+    wheels: list[Path],
+    ignore_missing_dependencies: bool = typer.Option(
+        help="If this is true, dependencies "
+        "which are not in the original lockfile or "
+        "the added wheels will be added to the lockfile. "
+        "Warning: This will allow a broken lockfile to "
+        "be created.",
+        default=False,
+    ),
+    input: Path = typer.Option(
+        help="Source lockfile", default=Path("pyodide-lock.json")
+    ),
+    output: Path = typer.Option(
+        help="Updated lockfile", default=Path("pyodide-lock-new.json")
+    ),
+    base_path: Path = typer.Option(
+        help="Base path for wheels - wheel file "
+        "names will be created relative to this path.",
+        default=None,
+    ),
+    wheel_url: str = typer.Option(
+        help="Base url which will be appended to the wheel location."
+        "Use this if you are hosting these wheels on a different "
+        "server to core pyodide packages",
+        default="",
+    ),
+):
+    """Add a set of package wheels to an existing pyodide-lock.json and
+    write it out to pyodide-lock-new.json
 
-        """
-        sp = PyodideLockSpec.from_json(in_lockfile)
-        add_wheels_to_spec(
-            sp,
-            wheels,
-            base_path=base_path,
-            base_url=wheel_url,
-            ignore_missing_dependencies=ignore_missing_dependencies,
-        )
-        sp.to_json(out_lockfile)
+    Each package in the wheel will be added to the output lockfile,
+    including resolution of dependencies in the lock file. By default
+    this will fail if a dependency isn't available in either the
+    existing lock file, or in the set of new wheels.
 
-except ImportError:
-    pass
-    # no typer = no cli
+    """
+    sp = PyodideLockSpec.from_json(input)
+    add_wheels_to_spec(
+        sp,
+        wheels,
+        base_path=base_path,
+        base_url=wheel_url,
+        ignore_missing_dependencies=ignore_missing_dependencies,
+    )
+    sp.to_json(output)

pyodide_lock/spec.py

@@ -1,12 +1,9 @@
 import json
 from pathlib import Path
-from typing import TYPE_CHECKING, Literal
+from typing import Literal
 
 from pydantic import BaseModel, Extra, Field
 
-if TYPE_CHECKING:
-    pass
-
 
 class InfoSpec(BaseModel):
     arch: Literal["wasm32", "wasm64"] = "wasm32"

pyodide_lock/utils.py

@@ -44,26 +44,19 @@ def parse_top_level_import_name(whlfile: Path) -> list[str] | None:
 
     whlzip = zipfile.Path(whlfile)
 
-    # if there is a directory with .dist_info at the end with a top_level.txt file
-    # then just use that
-    for subdir in whlzip.iterdir():
-        if subdir.name.endswith(".dist-info"):
-            top_level_path = subdir / "top_level.txt"
-            if top_level_path.exists():
-                return top_level_path.read_text().splitlines()
-
-    # If there is no top_level.txt file, we will find top level imports by
+    # We will find top level imports by
     # 1) a python file on a top-level directory
     # 2) a sub directory with __init__.py
     # following: https://github.com/pypa/setuptools/blob/d680efc8b4cd9aa388d07d3e298b870d26e9e04b/setuptools/discovery.py#L122
+    # - n.b. this is more reliable than using top-level.txt which is
+    # sometimes broken
     top_level_imports = []
     for subdir in whlzip.iterdir():
         if subdir.is_file() and subdir.name.endswith(".py"):
             top_level_imports.append(subdir.name[:-3])
         elif subdir.is_dir() and _valid_package_name(subdir.name):
             if _has_python_file(subdir):
                 top_level_imports.append(subdir.name)
-
     if not top_level_imports:
         logger.warning(
             f"WARNING: failed to parse top level import name from {whlfile}."
@@ -166,20 +159,25 @@ def add_wheels_to_spec(
 ) -> None:
     """Add a list of wheel files to this pyodide-lock.json
 
-    Args:
-        wheel_files (list[Path]): A list of wheel files to import.
-        base_path (Path | None, optional):
-            Filenames are stored relative to this base path. By default the
-            filename is stored relative to the path of the first wheel file
-            in the list.
-
-        base_url (str, optional):
-            The base URL stored in the pyodide-lock.json. By default this
-            is empty which means that wheels must be stored in the same folder
-            as the core pyodide packages you are using. If you want to store
-            your custom wheels somewhere else, set this base_url to point to it.
+    Parameters:
+    wheel_files : list[Path]
+         A list of wheel files to import.
+    base_path : Path | None, optional
+        Filenames are stored relative to this base path. By default the
+        filename is stored relative to the path of the first wheel file
+        in the list.
+    base_url : str, optional
+        The base URL stored in the pyodide-lock.json. By default this
+        is empty which means that wheels must be stored in the same folder
+        as the core pyodide packages you are using. If you want to store
+        your custom wheels somewhere else, set this base_url to point to it.
+    ignore_missing_dependencies: bool, optional
+        If this is set to True, any dependencies not found in the lock file
+        or the set of wheels being added will be added to the spec. This is
+        not 100% reliable, because it ignores any extras and does not do any
+        sub-dependency or version resolution.
     """
-    if len(wheel_files) <= 0:
+    if not wheel_files:
         return
     wheel_files = [f.resolve() for f in wheel_files]
     if base_path is None:
@@ -254,7 +252,7 @@ def _fix_extra_dep(
     extra_req: "Requirement",
     new_packages: dict[str, PackageSpec],
     ignore_missing_dependencies: bool,
-):
+) -> list["Requirement"]:
     from packaging.utils import canonicalize_name
 
     requirements_with_extras = []
@@ -303,26 +301,14 @@ def _set_package_paths(
         p.file_name = base_url + str(relative_path)
 
 
-def package_spec_from_wheel(path: Path, info: InfoSpec) -> "PackageSpec":
-    """Build a package spec from an on-disk wheel.
-
-    Warning - to reliably handle dependencies, we need:
-        1) To have access to all the wheels being added at once (to handle extras)
-        2) To know whether dependencies are available in the combined lockfile.
-        3) To fix up wheel urls and paths consistently
-
-        This is called by add_wheels_to_spec
-    """
+def _check_wheel_compatible(path: Path, info: InfoSpec) -> None:
     from packaging.utils import (
         InvalidWheelFilename,
-        canonicalize_name,
         parse_wheel_filename,
     )
     from packaging.version import InvalidVersion
     from packaging.version import parse as version_parse
 
-    path = path.absolute()
-    # throw an error if this is an incompatible wheel
     target_python = version_parse(info.python)
     target_platform = info.platform + "_" + info.arch
     try:
@@ -349,10 +335,30 @@ def package_spec_from_wheel(path: Path, info: InfoSpec) -> "PackageSpec":
                     tag_match = True
     if not tag_match:
         raise RuntimeError(
-            f"Package tags {tags} don't match Python version in lockfile:"
+            f"Package tags for {path} don't match Python version in lockfile:"
             f"Lockfile python {target_python.major}.{target_python.minor}"
             f"on platform {target_platform} ({python_binary_abi})"
         )
+
+
+def package_spec_from_wheel(path: Path, info: InfoSpec) -> "PackageSpec":
+    """Build a package spec from an on-disk wheel.
+
+    Warning - to reliably handle dependencies, we need:
+        1) To have access to all the wheels being added at once (to handle extras)
+        2) To know whether dependencies are available in the combined lockfile.
+        3) To fix up wheel urls and paths consistently
+
+        This is called by add_wheels_to_spec
+    """
+    from packaging.utils import (
+        canonicalize_name,
+    )
+
+    path = path.absolute()
+    # throw an error if this is an incompatible wheel
+
+    _check_wheel_compatible(path, info)
     metadata = _wheel_metadata(path)
 
     if not metadata:

tests/conftest.py

@@ -6,6 +6,7 @@
 
 import build
 import pytest
+from packaging.utils import canonicalize_name
 
 from pyodide_lock import PyodideLockSpec
 from pyodide_lock.utils import _get_marker_environment
@@ -87,8 +88,8 @@ def make_test_wheel(
 ):
     package_dir = dir / package_name
     package_dir.mkdir()
-    if modules is None:
-        modules = [package_name]
+    if not modules:
+        modules = [canonicalize_name(package_name).replace("-", "_")]
     for m in modules:
         (package_dir / f"{m}.py").write_text("")
     toml = package_dir / "pyproject.toml"

tests/test_wheel.py

@@ -2,9 +2,11 @@
 from pathlib import Path
 
 import pytest
+from packaging.version import parse as version_parse
 
 from pyodide_lock import PackageSpec
 from pyodide_lock.utils import (
+    _check_wheel_compatible,
     _generate_package_hash,
     add_wheels_to_spec,
 )
@@ -140,3 +142,42 @@ def test_bad_names(tmp_path, bad_name, example_lock_spec):
         whlzip.writestr("README.md", data="Not a wheel")
     with pytest.raises(RuntimeError, match="Wheel filename"):
         add_wheels_to_spec(example_lock_spec, [wheel])
+
+
+def test_wheel_compatibility_checking(example_lock_spec):
+    target_python = version_parse(example_lock_spec.info.python)
+    python_tag = f"py{target_python.major}{target_python.minor}"
+    cpython_tag = f"cp{target_python.major}{target_python.minor}"
+    emscripten_tag = example_lock_spec.info.platform + "_" + example_lock_spec.info.arch
+
+    # pure python 3 wheel
+    _check_wheel_compatible(
+        Path("test_wheel-1.0.0-py3-none-any.whl"), example_lock_spec.info
+    )
+    # pure python 3.X wheel
+    _check_wheel_compatible(
+        Path(f"test_wheel-1.0.0-{python_tag}-none-any.whl"), example_lock_spec.info
+    )
+    # pure python 2 or 3 wheel
+    _check_wheel_compatible(
+        Path("test_wheel-1.0.0-py2.py3-none-any.whl"), example_lock_spec.info
+    )
+    # cpython emscripten correct version
+    _check_wheel_compatible(
+        Path(f"test_wheel-1.0.0-{cpython_tag}-{cpython_tag}-{emscripten_tag}.whl"),
+        example_lock_spec.info,
+    )
+    with pytest.raises(RuntimeError):
+        # cpython emscripten incorrect version
+        _check_wheel_compatible(
+            Path(
+                f"test_wheel-1.0.0-{cpython_tag}-{cpython_tag}-emscripten_3_1_2_wasm32.whl"
+            ),
+            example_lock_spec.info,
+        )
+    with pytest.raises(RuntimeError):
+        # a linux wheel
+        _check_wheel_compatible(
+            Path(f"test_wheel-1.0.0-{cpython_tag}-{cpython_tag}-linux_x86_64.whl"),
+            example_lock_spec.info,
+        )