Merge pull request #307 from shubhbapna/config-build-dir

feat: add new `build_dir` field in settings

docs/customization.md

@@ -430,7 +430,9 @@ To use predefined urls to download sources from, instead of overriding the entir
 - `version` - it is replaced by the version returned by the resolver  
 - `canonicalized_name` - it is replaced by the canonicalized name of the package specified in the requirement, specifically it applies `canonicalize_name(req.nam)`  
 
-Additionally, the source distribution index server used by the package resolver can be overriden for a particular package. The resolver can also be told to whether include wheels or sdist sources while trying to resolve the package. Templating is not supported here.
+The source distribution index server used by the package resolver can be overriden for a particular package. The resolver can also be told to whether include wheels or sdist sources while trying to resolve the package. Templating is not supported here.  
+
+A `build_dir` field can also be defined to indicate to fromager where the package should be build relative to the sdist root directory
 
 ```yaml
 packages:
@@ -442,4 +444,5 @@ packages:
             sdist_server_url: "https://pypi.org/simple"
             include_wheels: true
             include_sdists: false
+        build_dir: directory name relative to sdist directory, defaults to an empty string, which means to use the sdist directory
 ```

pyproject.toml

@@ -105,10 +105,11 @@ mypy_path = ["src"]
 # TODO: remove excludes and silent follow
 follow_imports = "silent"
 exclude = [
-    "^src/fromager/resolver\\.py$",
-    "^src/fromager/sources\\.py$",
-    "^src/fromager/sdist\\.py$",
-    "^src/fromager/commands/build\\.py$",
+  "^src/fromager/resolver\\.py$",
+  "^src/fromager/sources\\.py$",
+  "^src/fromager/sdist\\.py$",
+  "^src/fromager/commands/build\\.py$",
+  "^src/fromager/settings\\.py$",
 ]
 
 [[tool.mypy.overrides]]

src/fromager/dependencies.py

@@ -37,6 +37,7 @@ def get_build_system_dependencies(
         ctx=ctx,
         req=req,
         sdist_root_dir=sdist_root_dir,
+        build_dir=ctx.settings.build_dir(req.name, sdist_root_dir),
     )
     deps = _filter_requirements(req, orig_deps)
 
@@ -66,8 +67,9 @@ def default_get_build_system_dependencies(
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
+    build_dir: pathlib.Path,
 ) -> typing.Iterable[str]:
-    pyproject_toml = get_pyproject_contents(sdist_root_dir)
+    pyproject_toml = get_pyproject_contents(build_dir)
     return typing.cast(list[str], get_build_backend(pyproject_toml)["requires"])
 
 
@@ -94,6 +96,7 @@ def get_build_backend_dependencies(
         ctx=ctx,
         req=req,
         sdist_root_dir=sdist_root_dir,
+        build_dir=ctx.settings.build_dir(req.name, sdist_root_dir),
     )
     deps = _filter_requirements(req, orig_deps)
 
@@ -108,11 +111,12 @@ def default_get_build_backend_dependencies(
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
+    build_dir: pathlib.Path,
 ) -> typing.Iterable[str]:
-    pyproject_toml = get_pyproject_contents(sdist_root_dir)
+    pyproject_toml = get_pyproject_contents(build_dir)
     extra_environ = overrides.extra_environ_for_pkg(ctx.envs_dir, req.name, ctx.variant)
     hook_caller = get_build_backend_hook_caller(
-        sdist_root_dir, pyproject_toml, override_environ=extra_environ
+        build_dir, pyproject_toml, override_environ=extra_environ
     )
     return hook_caller.get_requires_for_build_wheel()
 
@@ -140,6 +144,7 @@ def get_build_sdist_dependencies(
         ctx=ctx,
         req=req,
         sdist_root_dir=sdist_root_dir,
+        build_dir=ctx.settings.build_dir(req.name, sdist_root_dir),
     )
     deps = _filter_requirements(req, orig_deps)
 
@@ -154,11 +159,12 @@ def default_get_build_sdist_dependencies(
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
+    build_dir: pathlib.Path,
 ) -> typing.Iterable[str]:
-    pyproject_toml = get_pyproject_contents(sdist_root_dir)
+    pyproject_toml = get_pyproject_contents(build_dir)
     extra_environ = overrides.extra_environ_for_pkg(ctx.envs_dir, req.name, ctx.variant)
     hook_caller = get_build_backend_hook_caller(
-        sdist_root_dir, pyproject_toml, override_environ=extra_environ
+        build_dir, pyproject_toml, override_environ=extra_environ
     )
     return hook_caller.get_requires_for_build_wheel()
 

src/fromager/settings.py

@@ -82,6 +82,20 @@ def resolver_include_sdists(
             resolve_dist.get("include_sdists"), default
         )
 
+    def build_dir(self, pkg: str, sdist_root_dir: pathlib.Path) -> pathlib.Path:
+        p = self.get_package_settings(pkg)
+        if p.get("build_dir"):
+            input_build_dir = pathlib.Path(p.get("build_dir"))
+            # hack: make path absolute to ensure that any directory escaping is contained
+            if ".." in str(input_build_dir):
+                raise ValueError(
+                    f"{pkg}: build dir {input_build_dir} defined in settings is not relative to sdist root dir"
+                )
+            # ensure that absolute build_dir path from settings is converted to a relative path
+            relative_build_dir = input_build_dir.relative_to(input_build_dir.anchor)
+            return sdist_root_dir / relative_build_dir
+        return sdist_root_dir
+
     def get_package_settings(self, pkg: str) -> dict[str, dict[str, str]]:
         p = self.packages()
         return self._return_value_or_default(

src/fromager/sources.py

@@ -416,7 +416,9 @@ def build_sdist(
     sdist_root_dir: pathlib.Path,
     build_env: wheels.BuildEnvironment,
 ) -> pathlib.Path:
-    logger.info(f"{req.name}: building source distribution in {sdist_root_dir}")
+    logger.info(
+        f"{req.name}: building source distribution in {ctx.settings.build_dir(req.name, sdist_root_dir)}"
+    )
     extra_environ = overrides.extra_environ_for_pkg(ctx.envs_dir, req.name, ctx.variant)
     sdist_filename = overrides.find_and_invoke(
         req.name,
@@ -427,6 +429,7 @@ def build_sdist(
         req=req,
         version=version,
         sdist_root_dir=sdist_root_dir,
+        build_dir=ctx.settings.build_dir(req.name, sdist_root_dir),
         build_env=build_env,
     )
     logger.info(f"{req.name}: built source distribution {sdist_filename}")
@@ -440,6 +443,7 @@ def default_build_sdist(
     version: Version,
     sdist_root_dir: pathlib.Path,
     build_env: wheels.BuildEnvironment,
+    build_dir: pathlib.Path,
 ) -> pathlib.Path:
     # It seems like the "correct" way to do this would be to run the
     # PEP 517 API in the source tree we have modified. However, quite
@@ -451,16 +455,16 @@ def default_build_sdist(
     #
     # For cases where the PEP 517 approach works, use
     # pep517_build_sdist().
-    sdist_filename = ctx.sdists_builds / (sdist_root_dir.name + ".tar.gz")
+    sdist_filename = ctx.sdists_builds / (build_dir.name + ".tar.gz")
     if sdist_filename.exists():
         sdist_filename.unlink()
     # The format argument is specified based on
     # https://peps.python.org/pep-0517/#build-sdist.
     with tarfile.open(sdist_filename, "x:gz", format=tarfile.PAX_FORMAT) as sdist:
         tarballs.tar_reproducible(
             tar=sdist,
-            basedir=sdist_root_dir,
-            prefix=sdist_root_dir.parent,
+            basedir=build_dir,
+            prefix=build_dir.parent,
         )
     return sdist_filename
 

src/fromager/wheels.py

@@ -162,6 +162,7 @@ def build_wheel(
         extra_environ=extra_environ,
         req=req,
         sdist_root_dir=sdist_root_dir,
+        build_dir=ctx.settings.build_dir(req.name, sdist_root_dir),
         version=version,
     )
     # End the timer
@@ -183,8 +184,9 @@ def default_build_wheel(
     req: Requirement,
     sdist_root_dir: pathlib.Path,
     version: Version,
+    build_dir: pathlib.Path,
 ) -> None:
-    logger.debug(f"{req.name}: building wheel in {sdist_root_dir} with {extra_environ}")
+    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
@@ -219,8 +221,8 @@ def default_build_wheel(
             "--index-url",
             ctx.wheel_server_url,  # probably redundant, but just in case
             "--log",
-            os.fspath(sdist_root_dir.parent / "build.log"),
-            os.fspath(sdist_root_dir),
+            os.fspath(build_dir.parent / "build.log"),
+            os.fspath(build_dir),
         ]
         external_commands.run(
             cmd,

tests/test_settings.py

@@ -1,3 +1,4 @@
+import pathlib
 import textwrap
 
 import pytest
@@ -91,6 +92,55 @@ def test_no_resolver_dist():
     assert s.resolver_sdist_server_url("foo") is None
 
 
+def test_relative_path_build_dir():
+    s = settings._parse(
+        textwrap.dedent("""
+    packages:
+      foo:
+        build_dir: "./build"
+    """)
+    )
+    sdist_root_dir = pathlib.Path("/foo/bar")
+    assert s.build_dir("foo", sdist_root_dir) == sdist_root_dir / "build"
+
+
+def test_only_name_build_dir():
+    s = settings._parse(
+        textwrap.dedent("""
+    packages:
+      foo:
+        build_dir: "build"
+    """)
+    )
+    sdist_root_dir = pathlib.Path("/foo/bar")
+    assert s.build_dir("foo", sdist_root_dir) == sdist_root_dir / "build"
+
+
+def test_absolute_path_build_dir():
+    s = settings._parse(
+        textwrap.dedent("""
+    packages:
+      foo:
+        build_dir: "/tmp/build"
+    """)
+    )
+    sdist_root_dir = pathlib.Path("/foo/bar")
+    assert s.build_dir("foo", sdist_root_dir) == sdist_root_dir / "tmp" / "build"
+
+
+def test_escape_sdist_root_build_dir():
+    s = settings._parse(
+        textwrap.dedent("""
+    packages:
+      foo:
+        build_dir: "../tmp/build"
+    """)
+    )
+    sdist_root_dir = pathlib.Path("/foo/bar")
+    with pytest.raises(ValueError):
+        str(s.build_dir("foo", sdist_root_dir)).startswith("/foo/bar")
+
+
 def test_resolve_template_with_no_template():
     req = Requirement("foo==1.0")
     assert settings._resolve_template(None, req, "1.0") is None