Stop excluding unisolated packages from build dependencies

pyodide_build/build_env.py

@@ -129,7 +129,6 @@ def get_build_environment_vars(pyodide_root: Path) -> dict[str, str]:
             "PYODIDE": "1",
             # This is the legacy environment variable used for the aforementioned purpose
             "PYODIDE_PACKAGE_ABI": "1",
-            "PYTHONPATH": env["HOSTSITEPACKAGES"],
         }
     )
 
@@ -169,24 +168,66 @@ def get_hostsitepackages() -> str:
 
 
 @functools.cache
-def get_unisolated_packages() -> list[str]:
+def get_unisolated_packages() -> dict[str, str]:
+    """
+    Get a map of unisolated packages.
+
+    Unisolated packages are packages that are used during the build process
+    and have some platform-specific files. When these packages are used
+    during the build process, we switch need to switch platform-specific files,
+    in order to build the package correctly.
+
+    Returns
+    -------
+    A dictionary of package names and versions.
+    """
+
     PYODIDE_ROOT = get_pyodide_root()
 
-    unisolated_file = PYODIDE_ROOT / "unisolated.txt"
-    if unisolated_file.exists():
-        # in xbuild env, read from file
-        unisolated_packages = unisolated_file.read_text().splitlines()
+    unisolated_packages = {}
+    if in_xbuildenv():
+        unisolated_packages_file = PYODIDE_ROOT / ".." / "requirements.txt"
+
+        for line in unisolated_packages_file.read_text().splitlines():
+            name, version = line.split("==")
+            unisolated_packages[name] = version
     else:
-        unisolated_packages = []
         recipe_dir = PYODIDE_ROOT / "packages"
         recipes = load_all_recipes(recipe_dir)
         for name, config in recipes.items():
             if config.build.cross_build_env:
-                unisolated_packages.append(name)
+                unisolated_packages[name] = config.package.version
 
     return unisolated_packages
 
 
+def get_unisolated_files(package_name: str) -> tuple[Path, list[str]]:
+    """
+    Get a list of unisolated files for a package.
+
+    Parameters
+    ----------
+    package_name
+        The name of the package
+
+    Returns
+    -------
+    A tuple of the package directory and a list of file paths relative to the package directory.
+    """
+    PYODIDE_ROOT = get_pyodide_root()
+
+    # TODO: unify libdir for in-tree and out-of-tree builds
+    if in_xbuildenv():
+        libdir = PYODIDE_ROOT / ".." / "site-packages-extras"
+    else:
+        libdir = Path(get_hostsitepackages())
+
+    package_dir = libdir / package_name
+    return libdir, [
+        str(f.relative_to(libdir)) for f in package_dir.rglob("*") if f.is_file()
+    ]
+
+
 def platform() -> str:
     emscripten_version = get_build_flag("PYODIDE_EMSCRIPTEN_VERSION")
     version = emscripten_version.replace(".", "_")

pyodide_build/buildpkg.py

@@ -460,17 +460,14 @@ def _package_wheel(
                 Path(self.build_args.host_install_dir)
                 / f"lib/{python_dir}/site-packages"
             )
-            if self.build_metadata.cross_build_env:
-                subprocess.run(
-                    ["pip", "install", "-t", str(host_site_packages), f"{name}=={ver}"],
-                    check=True,
-                )
 
+            # Copy cross build files to host site packages
             for cross_build_file in self.build_metadata.cross_build_files:
-                shutil.copy(
-                    (wheel_dir / cross_build_file),
-                    host_site_packages / cross_build_file,
-                )
+                src_file = wheel_dir / cross_build_file
+                dest_file = host_site_packages / cross_build_file
+                dest_file.parent.mkdir(parents=True, exist_ok=True)
+
+                shutil.copy(src_file, dest_file)
 
             try:
                 test_dir = self.src_dist_dir / "tests"

pyodide_build/pypabuild.py

@@ -6,7 +6,6 @@
 import traceback
 from collections.abc import Callable, Iterator, Mapping, Sequence
 from contextlib import contextmanager
-from itertools import chain
 from pathlib import Path
 from tempfile import TemporaryDirectory
 from typing import Literal, cast
@@ -18,8 +17,8 @@
 from pyodide_build import _f2c_fixes, common, pywasmcross
 from pyodide_build.build_env import (
     get_build_flag,
-    get_hostsitepackages,
     get_pyversion,
+    get_unisolated_files,
     get_unisolated_packages,
     platform,
 )
@@ -28,6 +27,7 @@
     _STYLES,
     _DefaultIsolatedEnv,
     _error,
+    _get_venv_paths,
     _handle_build_error,
     _ProjectBuilder,
 )
@@ -103,33 +103,107 @@ def symlink_unisolated_packages(env: DefaultIsolatedEnv) -> None:
 
     env_site_packages.mkdir(parents=True, exist_ok=True)
     shutil.copy(sysconfigdata_path, env_site_packages)
-    host_site_packages = Path(get_hostsitepackages())
-    for name in get_unisolated_packages():
-        for path in chain(
-            host_site_packages.glob(f"{name}*"), host_site_packages.glob(f"_{name}*")
-        ):
-            (env_site_packages / path.name).unlink(missing_ok=True)
-            (env_site_packages / path.name).symlink_to(path)
 
 
-def remove_avoided_requirements(
-    requires: set[str], avoided_requirements: set[str] | list[str]
+def _remove_avoided_requirements(
+    requires: set[str],
+    avoided_requirements: set[str] | list[str],
 ) -> set[str]:
+    """
+    Remove requirements that are in the list of avoided requirements.
+
+    Parameters
+    ----------
+    requires
+        The set of requirements to filter.
+    avoided_requirements
+        The set of requirements to avoid.
+
+    Returns
+    -------
+    The filtered set of requirements.
+    """
+    avoided_requirements = set(avoided_requirements)
     for reqstr in list(requires):
         req = Requirement(reqstr)
-        for avoid_name in set(avoided_requirements):
+        for avoid_name in avoided_requirements:
             if avoid_name in req.name.lower():
                 requires.remove(reqstr)
+                break
+
     return requires
 
 
+def _replace_unisolated_packages(
+    requires: set[str],
+    unisolated_packages: dict[str, str],
+) -> tuple[set[str], set[str]]:
+    """
+    Replace unisolated packages with the correct version.
+
+    Parameters
+    ----------
+    requires
+        The set of requirements to filter.
+    unisolated_packages
+        The dictionary of unisolated packages.
+
+    Returns
+    -------
+    tuple of (The filtered set of requirements, The set of unisolated requirements)
+    """
+    requires_new = requires.copy()
+    unisolated = set()
+    for reqstr in list(requires):
+        req = Requirement(reqstr)
+        for name, version in unisolated_packages.items():
+            if req.name == name and req.specifier.contains(version):
+                requires_new.remove(reqstr)
+                requires_new.add(f"{name}=={version}")
+                unisolated.add(name)
+                break
+        else:
+            # oldest-supported-numpy is a meta package for numpy
+            # TODO: use dependency resolution instead of hardcoding this
+            if req.name == "oldest-supported-numpy" and "numpy" in unisolated_packages:
+                requires_new.remove(reqstr)
+                requires_new.add(f"numpy=={unisolated_packages['numpy']}")
+                unisolated.add("numpy")
+                break
+
+    return requires_new, unisolated
+
+
+def _install_cross_build_files(path: str, unisolated: set[str]) -> None:
+    """
+    Install the cross build files to the isolated environment.
+
+    Parameters
+    ----------
+    path
+        The path to the isolated environment.
+
+    unisolated
+        The set of unisolated packages.
+    """
+
+    sitepackagesdir = Path(_get_venv_paths(path)["purelib"])
+    for name in unisolated:
+        base, files = get_unisolated_files(name)
+        for cross_build_file in files:
+            shutil.copy(
+                base / cross_build_file,
+                sitepackagesdir / cross_build_file,
+            )
+
+
 def install_reqs(env: DefaultIsolatedEnv, reqs: set[str]) -> None:
-    env.install(
-        remove_avoided_requirements(
-            reqs,
-            get_unisolated_packages() + AVOIDED_REQUIREMENTS,
-        )
-    )
+    reqs = _remove_avoided_requirements(reqs, AVOIDED_REQUIREMENTS)
+    reqs, unisolated = _replace_unisolated_packages(reqs, get_unisolated_packages())
+
+    env.install(reqs)
+
+    _install_cross_build_files(env.path, unisolated)
 
 
 def _build_in_isolated_env(

pyodide_build/tests/conftest.py

@@ -97,7 +97,8 @@ def dummy_xbuildenv(dummy_xbuildenv_url, tmp_path, reset_env_vars, reset_cache):
 
     manager = CrossBuildEnvManager(tmp_path / xbuildenv_dirname())
     manager.install(
-        version=None, url=dummy_xbuildenv_url, skip_install_cross_build_packages=True
+        version=None,
+        url=dummy_xbuildenv_url,
     )
 
     cur_dir = os.getcwd()

pyodide_build/tests/test_build_env.py

@@ -66,7 +66,7 @@ def test_get_build_environment_vars(
         build_vars = build_env.get_build_environment_vars(manager.pyodide_root)
 
         # extra variables that does not come from config files.
-        extra_vars = set(["PYODIDE", "PYODIDE_PACKAGE_ABI", "PYTHONPATH"])
+        extra_vars = set(["PYODIDE", "PYODIDE_PACKAGE_ABI"])
 
         all_keys = set(BUILD_KEY_TO_VAR.values()) | extra_vars
         for var in build_vars:
@@ -124,6 +124,21 @@ def test_get_build_environment_vars_host_env(
         assert "HOME" not in e
         assert "RANDOM_ENV" not in e
 
+    def test_get_unisolated_packages(
+        self, dummy_xbuildenv, reset_env_vars, reset_cache
+    ):
+        expected = {"numpy", "scipy"}  # this relies on the dummy xbuildenv file
+        pkgs = build_env.get_unisolated_packages()
+        for pkg in expected:
+            assert pkg in pkgs
+
+    def test_get_unisolated_files(self, dummy_xbuildenv, reset_env_vars, reset_cache):
+        pkgs = build_env.get_unisolated_packages()
+
+        for pkg in pkgs:
+            files = build_env.get_unisolated_files(pkg)
+            assert files
+
 
 def test_check_emscripten_version(dummy_xbuildenv, monkeypatch):
     s = None

pyodide_build/tests/test_pypabuild.py

@@ -12,7 +12,7 @@ def install(self, reqs):
 
 
 def test_remove_avoided_requirements():
-    assert pypabuild.remove_avoided_requirements(
+    assert pypabuild._remove_avoided_requirements(
         {"foo", "bar", "baz"},
         {"foo", "bar", "qux"},
     ) == {"baz"}

pyodide_build/tests/test_xbuildenv.py

@@ -205,33 +205,6 @@ def test_install_force(
         assert (tmp_path / version / ".installed").exists()
         assert manager.current_version == version
 
-    def test_install_cross_build_packages(
-        self, tmp_path, dummy_xbuildenv_url, monkeypatch_subprocess_run_pip
-    ):
-        pip_called_with = monkeypatch_subprocess_run_pip
-        manager = CrossBuildEnvManager(tmp_path)
-
-        download_path = tmp_path / "test"
-        manager._download(dummy_xbuildenv_url, download_path)
-
-        xbuildenv_root = download_path / "xbuildenv"
-        xbuildenv_pyodide_root = xbuildenv_root / "pyodide-root"
-        manager._install_cross_build_packages(xbuildenv_root, xbuildenv_pyodide_root)
-
-        assert len(pip_called_with) == 7
-        assert pip_called_with[0:4] == ["pip", "install", "--no-user", "-t"]
-        assert pip_called_with[4].startswith(
-            str(xbuildenv_pyodide_root)
-        )  # hostsitepackages
-        assert pip_called_with[5:7] == ["-r", str(xbuildenv_root / "requirements.txt")]
-
-        hostsitepackages = manager._host_site_packages_dir(xbuildenv_pyodide_root)
-        assert hostsitepackages.exists()
-
-        cross_build_files = xbuildenv_root / "site-packages-extras"
-        for file in cross_build_files.iterdir():
-            assert (hostsitepackages / file.name).exists()
-
     def test_create_package_index(self, tmp_path, dummy_xbuildenv_url):
         manager = CrossBuildEnvManager(tmp_path)
 

pyodide_build/vendor/_pypabuild.py

@@ -24,6 +24,7 @@
 import os
 import subprocess
 import sys
+import sysconfig
 import traceback
 import warnings
 from collections.abc import Iterator
@@ -124,3 +125,47 @@ def _handle_build_error() -> Iterator[None]:
             tb = traceback.format_exc(-1)  # type: ignore[unreachable]
         _cprint("\n{dim}{}{reset}\n", tb.strip("\n"))
         _error(str(e))
+
+
+def _get_venv_paths(path: str) -> dict[str, str]:
+    """
+    Find the sysconfig paths for a virtual environment.
+
+    Copied from pypabuild (https://github.com/pypa/build/blob/562907e605c3becb135ac52b6eb2aa939e84bdda/src/build/env.py#L326)
+
+    Parameters
+    ----------
+    path
+        The root path of the virtual environment
+    """
+    config_vars = (
+        sysconfig.get_config_vars().copy()
+    )  # globally cached, copy before altering it
+    config_vars["base"] = path
+    scheme_names = sysconfig.get_scheme_names()
+    if "venv" in scheme_names:
+        # Python distributors with custom default installation scheme can set a
+        # scheme that can't be used to expand the paths in a venv.
+        # This can happen if build itself is not installed in a venv.
+        # The distributors are encouraged to set a "venv" scheme to be used for this.
+        # See https://bugs.python.org/issue45413
+        # and https://github.com/pypa/virtualenv/issues/2208
+        paths = sysconfig.get_paths(scheme="venv", vars=config_vars)
+    elif "posix_local" in scheme_names:
+        # The Python that ships on Debian/Ubuntu varies the default scheme to
+        # install to /usr/local
+        # But it does not (yet) set the "venv" scheme.
+        # If we're the Debian "posix_local" scheme is available, but "venv"
+        # is not, we use "posix_prefix" instead which is venv-compatible there.
+        paths = sysconfig.get_paths(scheme="posix_prefix", vars=config_vars)
+    elif "osx_framework_library" in scheme_names:
+        # The Python that ships with the macOS developer tools varies the
+        # default scheme depending on whether the ``sys.prefix`` is part of a framework.
+        # But it does not (yet) set the "venv" scheme.
+        # If the Apple-custom "osx_framework_library" scheme is available but "venv"
+        # is not, we use "posix_prefix" instead which is venv-compatible there.
+        paths = sysconfig.get_paths(scheme="posix_prefix", vars=config_vars)
+    else:
+        paths = sysconfig.get_paths(vars=config_vars)
+
+    return paths