Merge initialization functions for in-tree and out-of-tree builds (#3883)

Makefile.envs

@@ -28,7 +28,7 @@ export TARGETINSTALLDIR=$(PYODIDE_ROOT)/cpython/installs/python-$(PYVERSION)
 export HOSTINSTALLDIR=$(PYODIDE_ROOT)/packages/.artifacts
 export HOSTSITEPACKAGES=$(PYODIDE_ROOT)/packages/.artifacts/lib/python$(PYMAJOR).$(PYMINOR)/site-packages
 export WASM_LIBRARY_DIR=$(PYODIDE_ROOT)/packages/.libs
-export WASM_PKG_CONFIG_PATH=$(PYODIDE_ROOT)/packages/.libs/lib/pkgconfig
+export PKG_CONFIG_PATH=$(PYODIDE_ROOT)/packages/.libs/lib/pkgconfig
 
 export PYTHONINCLUDE=$(PYODIDE_ROOT)/cpython/installs/python-$(PYVERSION)/include/python$(PYMAJOR).$(PYMINOR)
 

pyodide-build/pyodide_build/cli/build.py

@@ -10,13 +10,13 @@ import requests
 import typer
 
 from .. import common
+from ..common import init_environment
 from ..out_of_tree import build
 from ..out_of_tree.pypi import (
     build_dependencies_for_wheel,
     build_wheels_from_pypi_requirements,
     fetch_pypi_package,
 )
-from ..out_of_tree.utils import initialize_pyodide_root
 
 
 def pypi(
@@ -29,8 +29,6 @@ def pypi(
     ctx: typer.Context = typer.Context,
 ) -> Path:
     """Fetch a wheel from pypi, or build from source if none available."""
-    initialize_pyodide_root()
-    common.check_emscripten_version()
     backend_flags = ctx.args
     with tempfile.TemporaryDirectory() as tmpdir:
         srcdir = Path(tmpdir)
@@ -70,8 +68,6 @@ def url(
     ctx: typer.Context = typer.Context,
 ) -> Path:
     """Fetch a wheel or build sdist from url."""
-    initialize_pyodide_root()
-    common.check_emscripten_version()
     backend_flags = ctx.args
     with tempfile.TemporaryDirectory() as tmpdir:
         tmppath = Path(tmpdir)
@@ -100,8 +96,6 @@ def source(
     ctx: typer.Context = typer.Context,
 ) -> Path:
     """Use pypa/build to build a Python package from source"""
-    initialize_pyodide_root()
-    common.check_emscripten_version()
     backend_flags = ctx.args
     built_wheel = build.run(source_location, output_directory, exports, backend_flags)
     return built_wheel
@@ -147,6 +141,10 @@ def main(
     ctx: typer.Context = typer.Context,
 ) -> None:
     """Use pypa/build to build a Python package from source, pypi or url."""
+
+    init_environment()
+    common.check_emscripten_version()
+
     if output_directory_compat:
         print(
             "--output-directory is deprecated, use --outdir or -o instead",

pyodide-build/pyodide_build/cli/build_recipes.py

@@ -5,7 +5,6 @@ import typer
 from .. import buildall, buildpkg, common, pywasmcross
 from ..common import get_num_cores, init_environment
 from ..logger import logger
-from ..out_of_tree.utils import initialize_pyodide_root
 
 
 def recipe(
@@ -67,7 +66,7 @@ def recipe(
     ),
 ) -> None:
     """Build packages using yaml recipes and create repodata.json"""
-    initialize_pyodide_root()
+    init_environment()
 
     if common.in_xbuildenv():
         common.check_emscripten_version()
@@ -81,8 +80,6 @@ def recipe(
     if not recipe_dir_.is_dir():
         raise FileNotFoundError(f"Recipe directory {recipe_dir_} not found")
 
-    init_environment()
-
     build_args = pywasmcross.BuildArgs(
         cflags=cflags,
         cxxflags=cxxflags,

pyodide-build/pyodide_build/cli/config.py

@@ -1,7 +1,6 @@
 import typer
 
-from ..common import get_build_environment_vars
-from ..out_of_tree.utils import initialize_pyodide_root
+from ..common import get_build_environment_vars, init_environment
 
 app = typer.Typer(help="Manage config variables used in pyodide")
 
@@ -19,7 +18,7 @@ def callback() -> None:
 
 
 def _get_configs() -> dict[str, str]:
-    initialize_pyodide_root(quiet=True)
+    init_environment(quiet=True)
 
     configs: dict[str, str] = get_build_environment_vars()
 

pyodide-build/pyodide_build/cli/venv.py

@@ -2,8 +2,8 @@ from pathlib import Path
 
 import typer
 
+from ..common import init_environment
 from ..out_of_tree import venv
-from ..out_of_tree.utils import initialize_pyodide_root
 
 
 def main(
@@ -13,5 +13,5 @@ def main(
     ),
 ) -> None:
     """Create a Pyodide virtual environment"""
-    initialize_pyodide_root()
+    init_environment()
     venv.create_pyodide_venv(dest)

pyodide-build/pyodide_build/common.py

@@ -10,7 +10,8 @@ import textwrap
 import zipfile
 from collections import deque
 from collections.abc import Generator, Iterable, Iterator, Mapping
-from contextlib import contextmanager
+from contextlib import contextmanager, nullcontext, redirect_stdout
+from io import StringIO
 from pathlib import Path
 from tempfile import TemporaryDirectory
 from typing import Any, NoReturn
@@ -49,9 +50,8 @@ BUILD_VARS: set[str] = {
     "SIDE_MODULE_CXXFLAGS",
     "SIDE_MODULE_LDFLAGS",
     "STDLIB_MODULE_CFLAGS",
-    "UNISOLATED_PACKAGES",
     "WASM_LIBRARY_DIR",
-    "WASM_PKG_CONFIG_PATH",
+    "PKG_CONFIG_PATH",
     "CARGO_BUILD_TARGET",
     "CARGO_TARGET_WASM32_UNKNOWN_EMSCRIPTEN_LINKER",
     "RUSTFLAGS",
@@ -232,16 +232,8 @@ def get_build_environment_vars() -> dict[str, str]:
     # TODO: Add modifiable configuration file instead.
     # (https://github.com/pyodide/pyodide/pull/3737/files#r1161247201)
     env.update({key: os.environ[key] for key in BUILD_VARS if key in os.environ})
-
     env["PYODIDE"] = "1"
 
-    if "PYODIDE_JOBS" in os.environ:
-        env["PYODIDE_JOBS"] = os.environ["PYODIDE_JOBS"]
-
-    env["PKG_CONFIG_PATH"] = env["WASM_PKG_CONFIG_PATH"]
-    if "PKG_CONFIG_PATH" in os.environ:
-        env["PKG_CONFIG_PATH"] += f":{os.environ['PKG_CONFIG_PATH']}"
-
     tools_dir = Path(__file__).parent / "tools"
 
     env["CMAKE_TOOLCHAIN_FILE"] = str(
@@ -348,18 +340,67 @@ def search_pyodide_root(curdir: str | Path, *, max_depth: int = 5) -> Path:
     )
 
 
-def init_environment() -> None:
+def init_environment(*, quiet: bool = False) -> None:
+    """
+    Initialize Pyodide build environment.
+    This function needs to be called before any other Pyodide build functions.
+    """
     if os.environ.get("__LOADED_PYODIDE_ENV"):
         return
+
     os.environ["__LOADED_PYODIDE_ENV"] = "1"
+
+    _set_pyodide_root(quiet=quiet)
+
+
+def _set_pyodide_root(*, quiet: bool = False) -> None:
+    """
+    Set PYODIDE_ROOT environment variable.
+
+    This function works both in-tree and out-of-tree builds:
+    - In-tree builds: Searches for the root of the Pyodide repository in parent directories
+    - Out-of-tree builds: Downloads and installs the Pyodide build environment into the current directory
+
+    Note: this function is supposed to be called only in init_environment(), and should not be called directly.
+
+    Parameters
+    ----------
+    quiet
+        If True, do not print any messages
+    """
+
+    from . import install_xbuildenv  # avoid circular import
+
     # If we are building docs, we don't need to know the PYODIDE_ROOT
     if "sphinx" in sys.modules:
         os.environ["PYODIDE_ROOT"] = ""
+        return
 
+    # 1) If PYODIDE_ROOT is already set, do nothing
     if "PYODIDE_ROOT" in os.environ:
-        os.environ["PYODIDE_ROOT"] = str(Path(os.environ["PYODIDE_ROOT"]).resolve())
-    else:
-        os.environ["PYODIDE_ROOT"] = str(search_pyodide_root(os.getcwd()))
+        return
+
+    # 2) If we are doing an in-tree build,
+    #    set PYODIDE_ROOT to the root of the Pyodide repository
+    try:
+        os.environ["PYODIDE_ROOT"] = str(search_pyodide_root(Path.cwd()))
+        return
+    except FileNotFoundError:
+        pass
+
+    # 3) If we are doing an out-of-tree build,
+    #    download and install the Pyodide build environment
+    xbuildenv_path = Path(".pyodide-xbuildenv").resolve()
+
+    if xbuildenv_path.exists():
+        os.environ["PYODIDE_ROOT"] = str(xbuildenv_path / "xbuildenv" / "pyodide-root")
+        return
+
+    context = redirect_stdout(StringIO()) if quiet else nullcontext()
+    with context:
+        # install_xbuildenv will set PYODIDE_ROOT env variable, so we don't need to do it here
+        # TODO: return the path to the xbuildenv instead of setting the env variable inside install_xbuildenv
+        install_xbuildenv.install(xbuildenv_path, download=True)
 
 
 @functools.cache
@@ -428,29 +469,6 @@ def chdir(new_dir: Path) -> Generator[None, None, None]:
         os.chdir(orig_dir)
 
 
-def set_build_environment(env: dict[str, str]) -> None:
-    """Assign build environment variables to env.
-
-    Sets common environment between in tree and out of tree package builds.
-    """
-    env.update({key: os.environ[key] for key in BUILD_VARS if key in os.environ})
-    env["PYODIDE"] = "1"
-
-    pkg_config_parts = []
-    if "WASM_PKG_CONFIG_PATH" in os.environ:
-        pkg_config_parts.append(env["WASM_PKG_CONFIG_PATH"])
-    if "PKG_CONFIG_PATH" in os.environ:
-        pkg_config_parts.append(os.environ["PKG_CONFIG_PATH"])
-    env["PKG_CONFIG_PATH"] = ":".join(pkg_config_parts)
-
-    tools_dir = Path(__file__).parent / "tools"
-
-    env["CMAKE_TOOLCHAIN_FILE"] = str(
-        tools_dir / "cmake/Modules/Platform/Emscripten.cmake"
-    )
-    env["PYO3_CONFIG_FILE"] = str(tools_dir / "pyo3_config.ini")
-
-
 def get_num_cores() -> int:
     """
     Return the number of CPUs the current process can use.

pyodide-build/pyodide_build/install_xbuildenv.py

@@ -10,7 +10,7 @@ from .create_pypa_index import create_pypa_index
 from .logger import logger
 
 
-def download_xbuildenv(
+def _download_xbuildenv(
     version: str, xbuildenv_path: Path, *, url: str | None = None
 ) -> None:
     from shutil import rmtree, unpack_archive
@@ -73,7 +73,7 @@ def install_xbuildenv(version: str, xbuildenv_path: Path) -> None:
     create_pypa_index(repodata["packages"], xbuildenv_root, cdn_base)
 
 
-def install(path: Path, *, download: bool = False, url: str | None = None) -> None:
+def install(path: Path, *, download: bool = True, url: str | None = None) -> None:
     """
     Install cross-build environment.
 
@@ -96,6 +96,15 @@ def install(path: Path, *, download: bool = False, url: str | None = None) -> No
     from . import __version__
 
     version = __version__
-    if download:
-        download_xbuildenv(version, path, url=url)
+
+    if not download and not path.exists():
+        logger.error("xbuild environment not exists")
+        raise FileNotFoundError(path)
+
+    if download and path.exists():
+        logger.warning("xbuild environment already exists, skipping download")
+
+    elif download:
+        _download_xbuildenv(version, path, url=url)
+
     install_xbuildenv(version, path)

pyodide-build/pyodide_build/out_of_tree/utils.py

@@ -1,39 +0,0 @@
-import os
-from contextlib import ExitStack, redirect_stdout
-from io import StringIO
-from pathlib import Path
-
-from ..common import search_pyodide_root
-
-
-def ensure_env_installed(env: Path, *, quiet: bool = False) -> None:
-    if env.exists():
-        return
-    from .. import __version__
-    from ..install_xbuildenv import download_xbuildenv, install_xbuildenv
-
-    if "dev" in __version__:
-        raise RuntimeError(
-            "To use out of tree builds with development Pyodide, you must explicitly set PYODIDE_ROOT"
-        )
-
-    with ExitStack() as stack:
-        if quiet:
-            # Prevent writes to stdout
-            stack.enter_context(redirect_stdout(StringIO()))
-
-        download_xbuildenv(__version__, env)
-        install_xbuildenv(__version__, env)
-
-
-def initialize_pyodide_root(*, quiet: bool = False) -> None:
-    if "PYODIDE_ROOT" in os.environ:
-        return
-    try:
-        os.environ["PYODIDE_ROOT"] = str(search_pyodide_root(Path.cwd()))
-        return
-    except FileNotFoundError:
-        pass
-    env = Path(".pyodide-xbuildenv").resolve()
-    os.environ["PYODIDE_ROOT"] = str(env / "xbuildenv/pyodide-root")
-    ensure_env_installed(env, quiet=quiet)

pyodide-build/pyodide_build/tests/test_build_env.py

@@ -0,0 +1,165 @@
+# This file contains tests that ensure build environment is properly initialized in
+# both in-tree and out-of-tree builds.
+
+# TODO: move functions that are tested here to a separate module
+
+import os
+from pathlib import Path
+
+import pytest
+
+from conftest import ROOT_PATH
+from pyodide_build import common
+
+
+@pytest.fixture(scope="function")
+def reset_env_vars():
+    # Will reset the environment variables to their original values after each test.
+
+    os.environ.pop("PYODIDE_ROOT", None)
+    os.environ.pop("__LOADED_PYODIDE_ENV", None)
+    old_environ = dict(os.environ)
+
+    try:
+        yield
+    finally:
+        os.environ.clear()
+        os.environ.update(old_environ)
+
+
+@pytest.fixture(scope="function")
+def reset_cache():
+    # Will remove all caches before each test.
+
+    common.get_pyodide_root.cache_clear()
+    common.get_build_environment_vars.cache_clear()
+    common.get_unisolated_packages.cache_clear()
+
+    yield
+
+
+class TestInTree:
+    def test_init_environment(self, reset_env_vars, reset_cache):
+        assert "PYODIDE_ROOT" not in os.environ
+
+        common.init_environment()
+
+        assert "PYODIDE_ROOT" in os.environ
+        assert os.environ["PYODIDE_ROOT"] == str(ROOT_PATH)
+
+    def test_init_environment_pyodide_root_already_set(
+        self, reset_env_vars, reset_cache
+    ):
+        assert "PYODIDE_ROOT" not in os.environ
+        os.environ["PYODIDE_ROOT"] = "/set_by_user"
+
+        common.init_environment()
+
+        assert os.environ["PYODIDE_ROOT"] == "/set_by_user"
+
+    def test_get_pyodide_root(self, reset_env_vars, reset_cache):
+        assert "PYODIDE_ROOT" not in os.environ
+
+        assert common.get_pyodide_root() == ROOT_PATH
+
+    def test_get_pyodide_root_pyodide_root_already_set(
+        self, reset_env_vars, reset_cache
+    ):
+        assert "PYODIDE_ROOT" not in os.environ
+        os.environ["PYODIDE_ROOT"] = "/set_by_user"
+
+        assert str(common.get_pyodide_root()) == "/set_by_user"
+
+    def test_search_pyodide_root(self, tmp_path, reset_env_vars, reset_cache):
+        pyproject_file = tmp_path / "pyproject.toml"
+        pyproject_file.write_text("[tool.pyodide]")
+        assert common.search_pyodide_root(tmp_path) == tmp_path
+        assert common.search_pyodide_root(tmp_path / "subdir") == tmp_path
+        assert common.search_pyodide_root(tmp_path / "subdir" / "subdir") == tmp_path
+
+        pyproject_file.unlink()
+        with pytest.raises(FileNotFoundError):
+            common.search_pyodide_root(tmp_path)
+
+    def test_in_xbuildenv(self, reset_env_vars, reset_cache):
+        assert not common.in_xbuildenv()
+
+    def test_get_build_environment_vars(self, reset_env_vars, reset_cache):
+        build_vars = common.get_build_environment_vars()
+        extra_vars = set(
+            ["PYODIDE", "PKG_CONFIG_PATH", "CMAKE_TOOLCHAIN_FILE", "PYO3_CONFIG_FILE"]
+        )
+
+        for var in build_vars:
+            assert var in common.BUILD_VARS | extra_vars, f"Unknown {var}"
+
+        # Additionally we set these variables
+        for var in extra_vars:
+            assert var in build_vars, f"Missing {var}"
+
+    def test_get_build_flag(self, reset_env_vars, reset_cache):
+        for key, val in common.get_build_environment_vars().items():
+            assert common.get_build_flag(key) == val
+
+        with pytest.raises(ValueError):
+            common.get_build_flag("UNKNOWN_VAR")
+
+
+class TestOutOfTree(TestInTree):
+    # TODO: selenium fixture is a hack to make these tests run only after building Pyodide.
+    @pytest.fixture(scope="function", autouse=True)
+    def xbuildenv(self, selenium, tmp_path, reset_env_vars, reset_cache):
+        import subprocess as sp
+
+        assert "PYODIDE_ROOT" not in os.environ
+
+        envpath = Path(tmp_path) / ".pyodide-xbuildenv"
+        result = sp.run(
+            [
+                "pyodide",
+                "xbuildenv",
+                "create",
+                str(envpath),
+                "--root",
+                ROOT_PATH,
+                "--skip-missing-files",
+            ]
+        )
+
+        assert result.returncode == 0
+
+        yield tmp_path
+
+    @pytest.fixture(scope="function", autouse=True)
+    def chdir_xbuildenv(self, xbuildenv):
+        cur_dir = os.getcwd()
+
+        os.chdir(xbuildenv)
+
+        try:
+            yield
+        finally:
+            os.chdir(cur_dir)
+
+    # Note: other tests are inherited from TestInTree
+
+    def test_init_environment(self, xbuildenv, reset_env_vars, reset_cache):
+        assert "PYODIDE_ROOT" not in os.environ
+
+        common.init_environment()
+
+        assert "PYODIDE_ROOT" in os.environ
+        assert os.environ["PYODIDE_ROOT"] == str(
+            xbuildenv / ".pyodide-xbuildenv/xbuildenv/pyodide-root"
+        )
+
+    def test_get_pyodide_root(self, xbuildenv, reset_env_vars, reset_cache):
+        assert "PYODIDE_ROOT" not in os.environ
+
+        assert (
+            common.get_pyodide_root()
+            == xbuildenv / ".pyodide-xbuildenv/xbuildenv/pyodide-root"
+        )
+
+    def test_in_xbuildenv(self, reset_env_vars, reset_cache):
+        assert common.in_xbuildenv()

pyodide-build/pyodide_build/tests/test_common.py

@@ -14,7 +14,6 @@ from pyodide_build.common import (
     platform,
     repack_zip_archive,
     search_pyodide_root,
-    set_build_environment,
 )
 
 
@@ -249,29 +248,37 @@ def test_repack_zip_archive(
     assert input_path.stat().st_size == expected_size
 
 
-def test_set_build_environment(monkeypatch):
+def test_get_build_environment_vars_host_env(monkeypatch):
+    # host environment variables should have precedence over
+    # variables defined in Makefile.envs
+
     import os
 
-    monkeypatch.delenv("PKG_CONFIG_PATH", raising=False)
-    monkeypatch.delenv("WASM_PKG_CONFIG_PATH", raising=False)
-    monkeypatch.setenv("RANDOM_ENV", 1234)
-    e: dict[str, str] = {}
-    set_build_environment(e)
-    assert e.get("HOME") == os.environ.get("HOME")
-    assert e.get("PATH") == os.environ.get("PATH")
+    get_build_environment_vars.cache_clear()
+    e = get_build_environment_vars()
     assert e["PYODIDE"] == "1"
-    assert "RANDOM_ENV" not in e
-    assert e["PKG_CONFIG_PATH"] == ""
 
-    e = {}
+    monkeypatch.setenv("HOME", "/home/user")
+    monkeypatch.setenv("PATH", "/usr/bin:/bin")
     monkeypatch.setenv("PKG_CONFIG_PATH", "/x/y/z:/c/d/e")
-    set_build_environment(e)
-    assert e["PKG_CONFIG_PATH"] == "/x/y/z:/c/d/e"
+
+    get_build_environment_vars.cache_clear()
+
+    e_host = get_build_environment_vars()
+    assert e_host.get("HOME") == os.environ.get("HOME")
+    assert e_host.get("PATH") == os.environ.get("PATH")
+    assert e_host["PKG_CONFIG_PATH"].endswith("/x/y/z:/c/d/e")
+
+    assert e_host.get("HOME") != e.get("HOME")
+    assert e_host.get("PATH") != e.get("PATH")
+    assert e_host.get("PKG_CONFIG_PATH") != e.get("PKG_CONFIG_PATH")
+
+    get_build_environment_vars.cache_clear()
 
     monkeypatch.delenv("HOME")
-    monkeypatch.setenv("WASM_PKG_CONFIG_PATH", "/a/b/c")
-    monkeypatch.setenv("PKG_CONFIG_PATH", "/x/y/z:/c/d/e")
-    e = {}
-    set_build_environment(e)
+    monkeypatch.setenv("RANDOM_ENV", "1234")
+
+    get_build_environment_vars.cache_clear()
+    e = get_build_environment_vars()
     assert "HOME" not in e
-    assert e["PKG_CONFIG_PATH"] == "/a/b/c:/x/y/z:/c/d/e"
+    assert "RANDOM_ENV" not in e

src/tests/test_cmdline_runner.py

@@ -11,7 +11,7 @@ import pytest
 
 import pyodide
 from pyodide_build.common import emscripten_version, get_pyodide_root
-from pyodide_build.install_xbuildenv import download_xbuildenv, install_xbuildenv
+from pyodide_build.install_xbuildenv import _download_xbuildenv, install_xbuildenv
 
 only_node = pytest.mark.xfail_browsers(
     chrome="node only", firefox="node only", safari="node only"
@@ -441,7 +441,7 @@ def test_pypa_index(tmp_path):
     expected."""
     path = Path(tmp_path)
     version = "0.21.0"  # just need some version that already exists
-    download_xbuildenv(version, path)
+    _download_xbuildenv(version, path)
 
     # We don't need host dependencies for this test so zero them out
     (path / "xbuildenv/requirements.txt").write_text("")