Add run method to BuildEnvironment

The logic to run a command in a virtual environment has been moved from
`build_wheel()` function into `BuildEnvironment.run()` method. It allows
users to run any program in the context of a virtual environment.

The `PackageBuildInfo.get_extra_environ()` method applies all
package-related env vars and optionally `BuildEnvironment` virtual env
vars `PATH` and `VIRTUAL_ENV`. Before, parallel job settings were only
applied for wheel building. Package env did not have access to modified
`PATH` and `VIRTUAL_ENV`.

Signed-off-by: Christian Heimes <[email protected]>

docs/hooks.rst

@@ -242,8 +242,16 @@ Additional types
 ----------------
 
 .. autoclass:: fromager.build_environment.BuildEnvironment
+   :members: run
+
+   .. autoproperty:: python
+
 .. autoclass:: fromager.context.WorkContext
+
 .. autoclass:: fromager.resolver.PyPIProvider
+
 .. autoclass:: fromager.resolver.GenericProvider
+
 .. autoclass:: fromager.resolver.GitHubTagProvider
+
 .. autofunction:: fromager.sources.prepare_new_source

src/fromager/build_environment.py

@@ -1,11 +1,13 @@
 import importlib.metadata
 import logging
+import os
 import pathlib
 import platform
 import re
 import subprocess
 import sys
 import typing
+from io import TextIOWrapper
 
 from packaging.requirements import Requirement
 
@@ -75,13 +77,78 @@ def __init__(
         build_requirements: typing.Iterable[Requirement] | None,
     ):
         self._ctx = ctx
-        self.path = parent_dir / f"build-{platform.python_version()}"
+        self.path = parent_dir.absolute() / f"build-{platform.python_version()}"
         self._build_requirements = build_requirements
         self._createenv()
 
     @property
     def python(self) -> pathlib.Path:
-        return (self.path / "bin/python3").absolute()
+        """Path to Python interpreter in virtual env"""
+        return self.path / "bin" / "python3"
+
+    def get_venv_environ(
+        self, template_env: dict[str, str] | None = None
+    ) -> dict[str, str]:
+        """Add virtual env to extra environ
+
+        The method activates the virtualenv
+
+        1. Put the build environment at the front of the :envvar:`PATH`
+           to ensure any build tools are picked up from there and not global
+           versions. If the caller has already set a path, start there.
+        2. Set :envvar:`VIRTUAL_ENV` so tools looking for that (for example,
+           maturin) find it.
+        """
+        venv_environ: dict[str, str] = {
+            "VIRTUAL_ENV": str(self.path),
+        }
+
+        # pre-pend virtualenv's bin
+        # 1) template_env PATH, 2) process' PATH, 3) default: "/usr/bin"
+        envpath: str | None = None
+        if template_env:
+            envpath = template_env.get("PATH")
+        if envpath is None:
+            envpath = os.environ.get("PATH", "/usr/bin")
+        envpath_list = envpath.split(os.pathsep)
+        venv_bin = str(self.path / "bin")
+        if envpath_list[0] != venv_bin:
+            envpath_list.insert(0, venv_bin)
+        venv_environ["PATH"] = os.pathsep.join(envpath_list)
+        return venv_environ
+
+    def run(
+        self,
+        cmd: typing.Sequence[str],
+        *,
+        cwd: str | None = None,
+        extra_environ: dict[str, str] | None = None,
+        network_isolation: bool | None = None,
+        log_filename: str | None = None,
+        stdin: TextIOWrapper | None = None,
+    ) -> str:
+        """Run command in a virtual environment
+
+        `network_isolation` defaults to context setting.
+        """
+        extra_environ = extra_environ.copy() if extra_environ else {}
+        extra_environ.update(self.get_venv_environ(template_env=extra_environ))
+
+        # default from context
+        if network_isolation is None:
+            network_isolation = self._ctx.network_isolation
+        if network_isolation:
+            # Build Rust dependencies without network access
+            extra_environ.setdefault("CARGO_NET_OFFLINE", "true")
+
+        return external_commands.run(
+            cmd,
+            cwd=cwd,
+            extra_environ=extra_environ,
+            network_isolation=network_isolation,
+            log_filename=log_filename,
+            stdin=stdin,
+        )
 
     def _createenv(self) -> None:
         if self.path.exists():
@@ -103,7 +170,7 @@ def _createenv(self) -> None:
                     f.write(f"{r}\n")
         if not self._build_requirements:
             return
-        external_commands.run(
+        self.run(
             [
                 str(self.python),
                 "-m",

src/fromager/packagesettings.py

@@ -17,6 +17,9 @@
 
 from . import overrides
 
+if typing.TYPE_CHECKING:
+    from . import build_environment
+
 logger = logging.getLogger(__name__)
 
 
@@ -583,23 +586,46 @@ def build_tag(self, version: Version) -> BuildTag:
         return release, suffix
 
     def get_extra_environ(
-        self, *, template_env: dict[str, str] | None = None
+        self,
+        *,
+        template_env: dict[str, str] | None = None,
+        build_env: "build_environment.BuildEnvironment | None" = None,
     ) -> dict[str, str]:
         """Get extra environment variables for a variant
 
+        1. parallel jobs: ``MAKEFLAGS``, ``MAX_JOBS``, ``CMAKE_BUILD_PARALLEL_LEVEL``
+        2. PATH and VIRTUAL_ENV from ``build_env`` (if given)
+        3. package's env settings
+        4. package variant's env settings
+
         `template_env` defaults to `os.environ`.
         """
         if template_env is None:
             template_env = os.environ.copy()
         else:
             template_env = template_env.copy()
+
+        # configure max jobs settings, settings depend on package, available
+        # CPU cores, and available virtual memory.
+        jobs = self.parallel_jobs()
+        extra_environ: dict[str, str] = {
+            "MAKEFLAGS": f"{template_env.get('MAKEFLAGS', '')} -j{jobs}",
+            "CMAKE_BUILD_PARALLEL_LEVEL": str(jobs),
+            "MAX_JOBS": str(jobs),
+        }
+
+        # add VIRTUAL_ENV and update PATH, so templates can use the values
+        if build_env is not None:
+            venv_environ = build_env.get_venv_environ(template_env=template_env)
+            template_env.update(venv_environ)
+            extra_environ.update(venv_environ)
+
         # chain entries so variant entries can reference general entries
         entries = list(self._ps.env.items())
         vi = self._ps.variants.get(self.variant)
         if vi is not None:
             entries.extend(vi.env.items())
 
-        extra_environ: dict[str, str] = {}
         for key, value in entries:
             value = string.Template(value).substitute(template_env)
             extra_environ[key] = value

src/fromager/sources.py

@@ -436,7 +436,7 @@ def build_sdist(
     build_dir = pbi.build_dir(sdist_root_dir)
 
     logger.info(f"{req.name}: building source distribution in {build_dir}")
-    extra_environ = pbi.get_extra_environ()
+    extra_environ = pbi.get_extra_environ(build_env=build_env)
     sdist_filename = overrides.find_and_invoke(
         req.name,
         "build_sdist",

src/fromager/wheels.py

@@ -243,36 +243,28 @@ def build_wheel(
         f"{req.name}: building wheel for {req} in {sdist_root_dir} writing to {ctx.wheels_build}"
     )
 
-    extra_environ: dict[str, str] = {}
-    # TODO: refactor?
-    # Build Rust without network access
-    extra_environ["CARGO_NET_OFFLINE"] = "true"
-    # configure max jobs settings, settings depend on package, available
-    # CPU cores, and available virtual memory.
-    jobs = pbi.parallel_jobs()
-    extra_environ["MAKEFLAGS"] = f"{extra_environ.get('MAKEFLAGS', '')} -j{jobs}"
-    extra_environ["CMAKE_BUILD_PARALLEL_LEVEL"] = str(jobs)
-    extra_environ["MAX_JOBS"] = str(jobs)
-
-    if pbi.build_ext_parallel:
+    # add package and variant env vars, package's parallel job vars, and
+    # build_env's virtual env vars.
+    extra_environ = pbi.get_extra_environ(build_env=build_env)
+
+    if (
+        pbi.build_ext_parallel
+        and "DIST_EXTRA_CONFIG" not in extra_environ
+        and "MAX_JOBS" in extra_environ
+    ):
         # configure setuptools to use parallel builds
         # https://setuptools.pypa.io/en/latest/deprecated/distutils/configfile.html
         dist_extra_cfg = build_env.path / "dist-extra.cfg"
         dist_extra_cfg.write_text(
             textwrap.dedent(
                 f"""
                 [build_ext]
-                parallel = {jobs}
+                parallel = {extra_environ['MAX_JOBS']}
                 """
             )
         )
         extra_environ["DIST_EXTRA_CONFIG"] = str(dist_extra_cfg)
 
-    # add package and variant env vars last
-    template_env = os.environ.copy()
-    template_env.update(extra_environ)
-    extra_environ.update(pbi.get_extra_environ(template_env=template_env))
-
     # Start the timer
     start = datetime.now().replace(microsecond=0)
     overrides.find_and_invoke(
@@ -308,30 +300,14 @@ def build_wheel(
 def default_build_wheel(
     ctx: context.WorkContext,
     build_env: build_environment.BuildEnvironment,
-    extra_environ: dict[str, typing.Any],
+    extra_environ: dict[str, str],
     req: Requirement,
     sdist_root_dir: pathlib.Path,
     version: Version,
     build_dir: pathlib.Path,
 ) -> None:
     logger.debug(f"{req.name}: building wheel in {build_dir} with {extra_environ}")
 
-    # Activate the virtualenv for the subprocess:
-    # 1. Put the build environment at the front of the PATH to ensure
-    #    any build tools are picked up from there and not global
-    #    versions. If the caller has already set a path, start there.
-    # 2. Set VIRTUAL_ENV so tools looking for that (for example,
-    #    maturin) find it.
-    existing_path = extra_environ.get("PATH") or os.environ.get("PATH") or ""
-    path_parts = [str(build_env.python.parent)]
-    if existing_path:
-        path_parts.append(existing_path)
-    updated_path = ":".join(path_parts)
-    override_env = dict(os.environ)
-    override_env.update(extra_environ)
-    override_env["PATH"] = updated_path
-    override_env["VIRTUAL_ENV"] = str(build_env.path)
-
     with tempfile.TemporaryDirectory() as dir_name:
         cmd = [
             os.fspath(build_env.python),
@@ -352,10 +328,10 @@ def default_build_wheel(
             os.fspath(build_dir.parent / "build.log"),
             os.fspath(build_dir),
         ]
-        external_commands.run(
+        build_env.run(
             cmd,
             cwd=dir_name,
-            extra_environ=override_env,
+            extra_environ=extra_environ,
             network_isolation=ctx.network_isolation,
         )
 

tests/test_packagesettings.py

@@ -7,7 +7,7 @@
 from packaging.utils import NormalizedName
 from packaging.version import Version
 
-from fromager import context
+from fromager import build_environment, context
 from fromager.packagesettings import (
     BuildDirectory,
     EnvVars,
@@ -131,32 +131,72 @@ def test_default_settings() -> None:
     assert p.model_dump() == expected
 
 
-def test_pbi_test_pkg_extra_environ(testdata_context: context.WorkContext) -> None:
-    pbi = testdata_context.settings.package_build_info(TEST_PKG)
-    assert pbi.get_extra_environ(template_env={"EXTRA": "extra"}) == {
-        "EGG": "spam spam",
-        "EGG_AGAIN": "spam spam",
-        "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
-        "SPAM": "alot extra",
+def test_pbi_test_pkg_extra_environ(
+    tmp_path: pathlib.Path, testdata_context: context.WorkContext
+) -> None:
+    testdata_context.settings.max_jobs = 1
+    parallel = {
+        "CMAKE_BUILD_PARALLEL_LEVEL": "1",
+        "MAKEFLAGS": " -j1",
+        "MAX_JOBS": "1",
     }
 
+    pbi = testdata_context.settings.package_build_info(TEST_PKG)
+    assert (
+        pbi.get_extra_environ(template_env={"EXTRA": "extra"})
+        == {
+            "EGG": "spam spam",
+            "EGG_AGAIN": "spam spam",
+            "QUOTES": "A\"BC'$EGG",  # $$EGG is transformed into $EGG
+            "SPAM": "alot extra",
+        }
+        | parallel
+    )
+
     testdata_context.settings.variant = Variant("rocm")
     pbi = testdata_context.settings.package_build_info(TEST_PKG)
-    assert pbi.get_extra_environ(template_env={"EXTRA": "extra"}) == {
-        "EGG": "spam",
-        "EGG_AGAIN": "spam",
-        "QUOTES": "A\"BC'$EGG",
-        "SPAM": "",
-    }
+    assert (
+        pbi.get_extra_environ(template_env={"EXTRA": "extra"})
+        == {
+            "EGG": "spam",
+            "EGG_AGAIN": "spam",
+            "QUOTES": "A\"BC'$EGG",
+            "SPAM": "",
+        }
+        | parallel
+    )
 
     testdata_context.settings.variant = Variant("cuda")
     pbi = testdata_context.settings.package_build_info(TEST_PKG)
-    assert pbi.get_extra_environ(template_env={"EXTRA": "spam"}) == {
-        "EGG": "spam",
-        "EGG_AGAIN": "spam",
-        "QUOTES": "A\"BC'$EGG",
-        "SPAM": "alot spam",
-    }
+    assert (
+        pbi.get_extra_environ(template_env={"EXTRA": "spam"})
+        == {
+            "EGG": "spam",
+            "EGG_AGAIN": "spam",
+            "QUOTES": "A\"BC'$EGG",
+            "SPAM": "alot spam",
+        }
+        | parallel
+    )
+
+    build_env = build_environment.BuildEnvironment(
+        testdata_context, parent_dir=tmp_path, build_requirements=None
+    )
+    result = pbi.get_extra_environ(
+        template_env={"EXTRA": "spam", "PATH": "/sbin:/bin"}, build_env=build_env
+    )
+    assert (
+        result
+        == {
+            "EGG": "spam",
+            "EGG_AGAIN": "spam",
+            "QUOTES": "A\"BC'$EGG",
+            "SPAM": "alot spam",
+            "PATH": f"{build_env.path / 'bin'}:/sbin:/bin",
+            "VIRTUAL_ENV": str(build_env.path),
+        }
+        | parallel
+    )
 
 
 def test_pbi_test_pkg(testdata_context: context.WorkContext) -> None: