From ba8ac4c94cc6529c013dd3fa15aacfc099b304a8 Mon Sep 17 00:00:00 2001 From: Joe Marshall Date: Fri, 13 Oct 2023 12:19:17 +0100 Subject: [PATCH] review fixes --- pyodide_lock/cli.py | 102 ++++++++++++++++++++---------------------- pyodide_lock/spec.py | 5 +-- pyodide_lock/utils.py | 82 +++++++++++++++++---------------- tests/conftest.py | 5 ++- tests/test_wheel.py | 41 +++++++++++++++++ 5 files changed, 138 insertions(+), 97 deletions(-) diff --git a/pyodide_lock/cli.py b/pyodide_lock/cli.py index 9fb3fc6..af3c28e 100644 --- a/pyodide_lock/cli.py +++ b/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) diff --git a/pyodide_lock/spec.py b/pyodide_lock/spec.py index b287174..b91a4ec 100644 --- a/pyodide_lock/spec.py +++ b/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" diff --git a/pyodide_lock/utils.py b/pyodide_lock/utils.py index d4a7db0..ba03e2c 100644 --- a/pyodide_lock/utils.py +++ b/pyodide_lock/utils.py @@ -44,18 +44,12 @@ 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"): @@ -63,7 +57,6 @@ def parse_top_level_import_name(whlfile: Path) -> list[str] | None: 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: diff --git a/tests/conftest.py b/tests/conftest.py index 6c04f7e..602fe0a 100644 --- a/tests/conftest.py +++ b/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" diff --git a/tests/test_wheel.py b/tests/test_wheel.py index 0aa0558..adc718b 100644 --- a/tests/test_wheel.py +++ b/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, + )