Make needs_rebuild aware of wheel tags (#134)

This switches to using the wheel itself instead of the ".packaged"
marker to decide if a wheel needs to be rebuilt. Static and dynamic
libraries still use the ".packaged" marker.

This solves the problem that whenever I switch branches between a Python
update branch and any other branch, absolutely everything has to be
rebuilt. Instead, this saves the wheels from each branch next to each
other.

This should also resolve the annoying `Found multiple matching wheels`
errors when switching abis or Python versions.

---------

Co-authored-by: Gyeongjae Choi <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

pyodide_build/build_env.py

@@ -6,7 +6,7 @@
 import re
 import subprocess
 import sys
-from collections.abc import Iterator
+from collections.abc import Iterator, Sequence
 from contextlib import nullcontext, redirect_stdout
 from io import StringIO
 from pathlib import Path
@@ -219,7 +219,7 @@ def wheel_platform() -> str:
     return f"pyodide_{abi_version}_wasm32"
 
 
-def pyodide_tags() -> Iterator[Tag]:
+def pyodide_tags_() -> Iterator[Tag]:
     """
     Returns the sequence of tag triples for the Pyodide interpreter.
 
@@ -235,6 +235,11 @@ def pyodide_tags() -> Iterator[Tag]:
     yield Tag(interpreter=f"cp{PYMAJOR}{PYMINOR}", abi="none", platform="any")
 
 
+@functools.cache
+def pyodide_tags() -> Sequence[Tag]:
+    return list(pyodide_tags_())
+
+
 def replace_so_abi_tags(wheel_dir: Path) -> None:
     """Replace native abi tag with emscripten abi tag in .so file names"""
     import sysconfig

pyodide_build/common.py

@@ -14,7 +14,7 @@
 import warnings
 import zipfile
 from collections import deque
-from collections.abc import Generator, Iterable, Iterator, Mapping
+from collections.abc import Generator, Iterable, Iterator, Mapping, Sequence
 from contextlib import contextmanager
 from pathlib import Path
 from tempfile import NamedTemporaryFile, TemporaryDirectory
@@ -35,12 +35,16 @@ def xbuildenv_dirname() -> str:
     return f".pyodide-xbuildenv-{__version__}"
 
 
-def find_matching_wheels(
-    wheel_paths: Iterable[Path], supported_tags: Iterator[Tag]
+def _find_matching_wheels(
+    wheel_paths: Iterable[Path],
+    supported_tags: Sequence[Tag],
+    version: str | None = None,
 ) -> Iterator[Path]:
     """
     Returns the sequence wheels whose tags match the Pyodide interpreter.
 
+    We don't bother ordering them carefully because we are only hoping to find one.
+
     Parameters
     ----------
     wheel_paths
@@ -52,17 +56,41 @@ def find_matching_wheels(
     -------
     The subset of wheel_paths that have tags that match the Pyodide interpreter.
     """
-    wheel_paths = list(wheel_paths)
-    wheel_tags_list: list[frozenset[Tag]] = []
-
-    for wheel in wheel_paths:
-        _, _, _, tags = parse_wheel_filename(wheel.name)
-        wheel_tags_list.append(tags)
-
-    for supported_tag in supported_tags:
-        for wheel_path, wheel_tags in zip(wheel_paths, wheel_tags_list, strict=True):
+    for wheel_path in wheel_paths:
+        _, wheel_version, _, wheel_tags = parse_wheel_filename(wheel_path.name)
+        if version and version != str(wheel_version):
+            continue
+        for supported_tag in supported_tags:
             if supported_tag in wheel_tags:
                 yield wheel_path
+                continue
+
+
+def find_matching_wheel(
+    wheel_paths: Iterable[Path], supported_tags: Sequence[Tag], version: str = None
+) -> Path | None:
+    """
+    Find a matching wheel or raise an error if none is present.
+
+    Parameters
+    ----------
+    wheel_paths
+        A list of paths to wheels
+    supported_tags
+        A list of tags that the environment supports
+
+    Returns
+    -------
+    The subset of wheel_paths that have tags that match the Pyodide interpreter.
+    """
+    result = list(_find_matching_wheels(wheel_paths, supported_tags, version))
+    if not result:
+        return None
+    if len(result) > 1:
+        raise RuntimeError(
+            "Found multiple matching wheels:\n" + "\n".join(w.name for w in result)
+        )
+    return result[0]
 
 
 def parse_top_level_import_name(whlfile: Path) -> list[str] | None:

pyodide_build/recipe/builder.py

@@ -33,7 +33,7 @@
     _get_sha256_checksum,
     chdir,
     exit_with_stdio,
-    find_matching_wheels,
+    find_matching_wheel,
     make_zip_archive,
     modify_wheel,
     retag_wheel,
@@ -119,6 +119,7 @@ def __init__(
         self.source_metadata = self.recipe.source
         self.build_metadata = self.recipe.build
         self.package_type = self.build_metadata.package_type
+        self.is_wheel = self.package_type in ["package", "cpython_module"]
 
         self.build_dir = (
             Path(build_dir).resolve() if build_dir else self.pkg_root / "build"
@@ -176,7 +177,8 @@ def build(self) -> None:
         try:
             self._build()
 
-            (self.build_dir / ".packaged").touch()
+            if not self.is_wheel:
+                (self.build_dir / ".packaged").touch()
         except (Exception, KeyboardInterrupt):
             success = False
             raise
@@ -199,7 +201,11 @@ def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None:
 
     def _build(self) -> None:
         if not self.force_rebuild and not needs_rebuild(
-            self.pkg_root, self.build_dir, self.source_metadata
+            self.pkg_root,
+            self.build_dir,
+            self.source_metadata,
+            self.is_wheel,
+            self.version,
         ):
             return
 
@@ -523,8 +529,7 @@ def _build_package(self, bash_runner: BashRunnerWithSharedEnvironment) -> None:
             self._compile(bash_runner)
 
         self._package_wheel(bash_runner)
-        shutil.rmtree(self.dist_dir, ignore_errors=True)
-        shutil.copytree(self.src_dist_dir, self.dist_dir)
+        shutil.copytree(self.src_dist_dir, self.dist_dir, dirs_exist_ok=True)
 
     def _package_wheel(
         self,
@@ -541,12 +546,13 @@ def _package_wheel(
             The runner we will use to execute our bash commands. Preserves
             environment variables from one invocation to the next.
         """
-        wheel, *rest = find_matching_wheels(
-            self.src_dist_dir.glob("*.whl"), pyodide_tags()
+        wheel = find_matching_wheel(
+            self.src_dist_dir.glob("*.whl"), pyodide_tags(), version=self.version
         )
-        if rest:
-            raise Exception(
-                f"Unexpected number of wheels {len(rest) + 1} when building {self.name}"
+        if not wheel:
+            raise RuntimeError(
+                f"Found no wheel while building {self.name}. Candidates:\n"
+                + "\n".join(f.name for f in self.src_dist_dir.glob("*.whl"))
             )
 
         if "emscripten" in wheel.name:
@@ -781,7 +787,11 @@ def unvendor_tests(
 
 # TODO: move this to common.py or somewhere else
 def needs_rebuild(
-    pkg_root: Path, buildpath: Path, source_metadata: _SourceSpec
+    pkg_root: Path,
+    buildpath: Path,
+    source_metadata: _SourceSpec,
+    is_wheel: bool = True,
+    version: str | None = None,
 ) -> bool:
     """
     Determines if a package needs a rebuild because its meta.yaml, patches, or
@@ -798,14 +808,22 @@ def needs_rebuild(
     src_metadata
         The source section from meta.yaml.
     """
-    packaged_token = buildpath / ".packaged"
-    if not packaged_token.is_file():
-        logger.debug(
-            "%s needs rebuild because %s does not exist", pkg_root, packaged_token
+    dist_dir = pkg_root / "dist"
+    if is_wheel:
+        previous_wheel = find_matching_wheel(
+            dist_dir.glob("*.whl"), pyodide_tags(), version=version
         )
-        return True
-
-    package_time = packaged_token.stat().st_mtime
+        if not previous_wheel:
+            return True
+        package_time = previous_wheel.stat().st_mtime
+    else:
+        packaged_token = buildpath / ".packaged"
+        if not packaged_token.is_file():
+            logger.debug(
+                "%s needs rebuild because %s does not exist", pkg_root, packaged_token
+            )
+            return True
+        package_time = packaged_token.stat().st_mtime
 
     def source_files() -> Iterator[Path]:
         yield pkg_root / "meta.yaml"

pyodide_build/recipe/graph_builder.py

@@ -33,7 +33,7 @@
     download_and_unpack_archive,
     exit_with_stdio,
     extract_wheel_metadata_file,
-    find_matching_wheels,
+    find_matching_wheel,
     find_missing_executables,
     repack_zip_archive,
 )
@@ -78,6 +78,7 @@ def __init__(self, pkgdir: Path, config: MetaConfig):
         self.version = self.meta.package.version
         self.disabled = self.meta.package.disabled
         self.package_type = self.meta.build.package_type
+        self.is_wheel = self.package_type in ["package", "cpython_module"]
 
         assert self.name == pkgdir.name, f"{self.name} != {pkgdir.name}"
 
@@ -119,7 +120,14 @@ def build_path(self, build_dir: Path) -> Path:
         return build_dir / self.name / "build"
 
     def needs_rebuild(self, build_dir: Path) -> bool:
-        return needs_rebuild(self.pkgdir, self.build_path(build_dir), self.meta.source)
+        res = needs_rebuild(
+            self.pkgdir,
+            self.build_path(build_dir),
+            self.meta.source,
+            self.is_wheel,
+            self.version,
+        )
+        return res
 
     def build(self, build_args: BuildArgs, build_dir: Path) -> None:
         p = subprocess.run(
@@ -179,16 +187,12 @@ def __init__(self, pkgdir: Path, config: MetaConfig) -> None:
 
     def dist_artifact_path(self) -> Path | None:
         dist_dir = self.pkgdir / "dist"
-        candidates = list(
-            find_matching_wheels(dist_dir.glob("*.whl"), build_env.pyodide_tags())
+        wheel = find_matching_wheel(
+            dist_dir.glob("*.whl"), build_env.pyodide_tags(), version=self.version
         )
-
-        if len(candidates) != 1:
-            raise RuntimeError(
-                f"Unexpected number of wheels/archives {len(candidates)} when building {self.name}"
-            )
-
-        return candidates[0]
+        if not wheel:
+            raise RuntimeError(f"Found no wheel while building {self.name}")
+        return wheel
 
     def tests_path(self) -> Path | None:
         tests = list((self.pkgdir / "dist").glob("*-tests.tar"))

pyodide_build/tests/recipe/test_builder.py

@@ -248,11 +248,23 @@ def _check_patches_extra(self) -> Self:
         return self
 
 
-def test_needs_rebuild(tmpdir):
+@pytest.mark.parametrize("is_wheel", [False, True])
+def test_needs_rebuild(tmpdir, is_wheel):
     pkg_root = Path(tmpdir)
     buildpath = pkg_root / "build"
     meta_yaml = pkg_root / "meta.yaml"
-    packaged = buildpath / ".packaged"
+    version = "12"
+    if is_wheel:
+        dist_dir = pkg_root / "dist"
+        dist_dir.mkdir()
+        # Build of current version with wrong abi
+        (dist_dir / "regex-12-cp311-cp311-pyodide_2024_0_wasm32.whl").touch()
+        # Build of old version with current abi
+        (dist_dir / "regex-11-cp312-cp312-pyodide_2024_0_wasm32.whl").touch()
+        # the version we're trying to build
+        packaged = dist_dir / "regex-12-cp312-cp312-pyodide_2024_0_wasm32.whl"
+    else:
+        packaged = buildpath / ".packaged"
 
     patch_file = pkg_root / "patch"
     extra_file = pkg_root / "extra"
@@ -277,39 +289,53 @@ def test_needs_rebuild(tmpdir):
     src_path_file.touch()
 
     # No .packaged file, rebuild
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is True
+    assert _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # .packaged file exists, no rebuild
     packaged.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is False
+    assert not _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # newer meta.yaml file, rebuild
     packaged.touch()
     time.sleep(0.01)
     meta_yaml.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is True
+    assert _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # newer patch file, rebuild
     packaged.touch()
     time.sleep(0.01)
     patch_file.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is True
+    assert _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # newer extra file, rebuild
     packaged.touch()
     time.sleep(0.01)
     extra_file.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is True
+    assert _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # newer source path, rebuild
     packaged.touch()
     time.sleep(0.01)
     src_path_file.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is True
+    assert _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
     # newer .packaged file, no rebuild
     packaged.touch()
-    assert _builder.needs_rebuild(pkg_root, buildpath, source_metadata) is False
+    assert not _builder.needs_rebuild(
+        pkg_root, buildpath, source_metadata, is_wheel, version
+    )
 
 
 def test_copy_sharedlib(tmp_path):

pyodide_build/tests/test_build_env.py

@@ -219,17 +219,21 @@ def test_wheel_paths(dummy_xbuildenv):
                 strings.append(f"wrapt-1.13.3-{interp}-{abi}-{arch}.whl")
 
     paths = [Path(x) for x in strings]
-    assert [
-        x.stem.split("-", 2)[-1]
-        for x in common.find_matching_wheels(paths, build_env.pyodide_tags())
-    ] == [
-        f"{current_version}-{current_version}-{PLATFORM}",
-        f"{current_version}-abi3-{PLATFORM}",
-        f"{current_version}-none-{PLATFORM}",
-        f"{old_version}-abi3-{PLATFORM}",
-        f"py3-none-{PLATFORM}",
-        f"py2.py3-none-{PLATFORM}",
-        "py3-none-any",
-        "py2.py3-none-any",
-        f"{current_version}-none-any",
-    ]
+    assert sorted(
+        [
+            x.stem.split("-", 2)[-1]
+            for x in common._find_matching_wheels(paths, build_env.pyodide_tags())
+        ]
+    ) == sorted(
+        [
+            f"{current_version}-{current_version}-{PLATFORM}",
+            f"{current_version}-abi3-{PLATFORM}",
+            f"{current_version}-none-{PLATFORM}",
+            f"{old_version}-abi3-{PLATFORM}",
+            f"py3-none-{PLATFORM}",
+            f"py2.py3-none-{PLATFORM}",
+            "py3-none-any",
+            "py2.py3-none-any",
+            f"{current_version}-none-any",
+        ]
+    )