fix(sources): honor resolver sdist build strategy

docs/concepts/resolver-architecture.rst

@@ -118,6 +118,14 @@ the artifact kind available for source preparation; consuming it in preparation
 and integrating prebuilt detection into the bootstrapper are separate steps.
 See :ref:`download_source_hook` for the return contract and compatibility rules.
 
+.. versionchanged:: 0.102.0
+   Configured source resolvers also select the sdist build method. Git checkout
+   resolvers use PEP 517 by default; hook sdist resolvers require an explicit
+   ``build_sdist: pep517`` or ``build_sdist: tarball``. PyPI sdist resolvers
+   repack a tarball. Both configured strategies take precedence over an installed
+   ``build_sdist`` override and log a warning when it is ignored. Legacy
+   settings continue to use the override.
+
 Version Filtering Window
 -------------------------
 

docs/reference/hooks.rst

@@ -264,8 +264,8 @@ Source hooks
        downloads require a download hook. The ``get_resolver_provider`` hook
        remains required for both profiles.
 
-    To migrate a raw-tarball hook to ``source: {provider: hook-sdist}``, return
-    its artifact kind explicitly:
+    To migrate a raw-tarball hook to ``hook-sdist``, specify ``build_sdist``
+    in YAML and return its artifact kind explicitly:
 
     .. code-block:: python
 

src/fromager/packagesettings/_resolver.py

@@ -69,6 +69,9 @@ class AbstractResolver(pydantic.BaseModel):
 
     provider: str
 
+    build_sdist: BuildSDist | None
+    """Sdist build strategy; None for resolvers that do not build from source."""
+
     supports_override_hooks: typing.ClassVar[bool] = False
     """Does resolver support override hooks?"""
 
@@ -208,7 +211,7 @@ class PyPISDistResolver(AbstractPyPIResolver):
 
     # It is not safe to use PEP 517 to re-generate a source distribution.
     # Some PEP 517 backends require VCS to generate correct sdist.
-    build_sdist: typing.ClassVar[BuildSDist | None] = BuildSDist.tarball
+    build_sdist: typing.Literal[BuildSDist.tarball] = BuildSDist.tarball
     download_kinds: typing.ClassVar[DownloadKindSet] = frozenset({DownloadKind.sdist})
 
     def resolver_provider(
@@ -258,7 +261,7 @@ class PyPIPrebuiltResolver(AbstractPyPIResolver):
 
     provider: typing.Literal["pypi-prebuilt"]
 
-    build_sdist: typing.ClassVar[BuildSDist | None] = None
+    build_sdist: None = None
     download_kinds: typing.ClassVar[DownloadKindSet] = frozenset(
         {DownloadKind.prebuilt_wheel}
     )
@@ -322,7 +325,7 @@ class PyPIDownloadResolver(AbstractPyPIResolver):
     URL must contain '{version}' template string.
     """
 
-    build_sdist: typing.ClassVar[BuildSDist | None] = BuildSDist.tarball
+    build_sdist: typing.Literal[BuildSDist.tarball] = BuildSDist.tarball
     download_kinds: typing.ClassVar[DownloadKindSet] = frozenset(
         {DownloadKind.sdist, DownloadKind.tarball}
     )
@@ -747,6 +750,7 @@ class NotAvailableResolver(AbstractResolver):
     """Prevent resolve and download"""
 
     provider: typing.Literal["not-available"]
+    build_sdist: None = None
 
     def resolver_provider(
         self,
@@ -901,13 +905,18 @@ class HookSDistResolver(AbstractHookResolver):
     a tarball, or a git checkout. Download hooks can return ``DownloadedSource``
     with a concrete kind, or a legacy archive ``Path`` treated as an sdist.
     Without a download hook, the candidate is downloaded as an sdist.
+    Specify ``build_sdist`` explicitly: ``tarball`` to repack an existing
+    sdist, or ``pep517`` to build from a git checkout.
 
     Example::
 
         provider: hook-sdist
+        build_sdist: tarball
     """
 
     provider: typing.Literal["hook-sdist"]
+    build_sdist: BuildSDist
+    """Source distribution build method."""
 
     download_kinds: typing.ClassVar[DownloadKindSet] = frozenset(
         {DownloadKind.sdist, DownloadKind.tarball, DownloadKind.git_checkout}
@@ -957,6 +966,7 @@ class HookPrebuiltResolver(AbstractHookResolver):
     """
 
     provider: typing.Literal["hook-prebuilt"]
+    build_sdist: None = None
 
     download_kinds: typing.ClassVar[DownloadKindSet] = frozenset(
         {DownloadKind.prebuilt_wheel}

src/fromager/sources.py

@@ -512,8 +512,9 @@ def build_sdist(
     sdist_root_dir: pathlib.Path,
     build_env: build_environment.BuildEnvironment,
 ) -> pathlib.Path:
-    """Build source distribution"""
+    """Build a source distribution using the configured strategy or legacy hook."""
     pbi = ctx.package_build_info(req)
+    source_resolver = pbi.source_resolver
     build_dir = pbi.build_dir(sdist_root_dir)
 
     logger.info(f"building {ctx.variant} source distribution for {req} in {build_dir}")
@@ -524,18 +525,56 @@ def build_sdist(
         sdist_root_dir=sdist_root_dir,
         build_env=build_env,
     )
-    sdist_file: pathlib.Path = overrides.find_and_invoke(
-        req.name,
-        "build_sdist",
-        default_build_sdist,
-        ctx=ctx,
-        extra_environ=extra_environ,
-        req=req,
-        version=version,
-        sdist_root_dir=sdist_root_dir,
-        build_dir=build_dir,
-        build_env=build_env,
-    )
+    if source_resolver is not None:
+        if overrides.find_override_method(req.name, "build_sdist") is not None:
+            logger.warning(
+                "%s: ignoring build_sdist override because source.build_sdist is %s",
+                req.name,
+                source_resolver.build_sdist,
+            )
+    if (
+        source_resolver is not None
+        and source_resolver.build_sdist is packagesettings.BuildSDist.pep517
+    ):
+        sdist_file = pep517_build_sdist(
+            ctx=ctx,
+            extra_environ=extra_environ,
+            req=req,
+            version=version,
+            sdist_root_dir=sdist_root_dir,
+            build_env=build_env,
+        )
+    elif (
+        source_resolver is not None
+        and source_resolver.build_sdist is packagesettings.BuildSDist.tarball
+    ):
+        sdist_file = default_build_sdist(
+            ctx=ctx,
+            extra_environ=extra_environ,
+            req=req,
+            version=version,
+            sdist_root_dir=sdist_root_dir,
+            build_dir=build_dir,
+            build_env=build_env,
+        )
+    elif source_resolver is None:
+        sdist_file = overrides.find_and_invoke(
+            req.name,
+            "build_sdist",
+            default_build_sdist,
+            ctx=ctx,
+            extra_environ=extra_environ,
+            req=req,
+            version=version,
+            sdist_root_dir=sdist_root_dir,
+            build_dir=build_dir,
+            build_env=build_env,
+        )
+    else:
+        raise ValueError(
+            f"{req.name}: unsupported source.build_sdist strategy "
+            f"{source_resolver.build_sdist!r}"
+        )
     logger.info(f"built source distribution {sdist_file}")
 
     # validate location and file name

tests/test_packagesettings_resolver.py

@@ -19,6 +19,7 @@
 from fromager.candidate import Candidate, Cooldown
 from fromager.context import WorkContext
 from fromager.packagesettings._resolver import (
+    AbstractResolver,
     BuildSDist,
     DownloadedSource,
     DownloadKind,
@@ -128,6 +129,22 @@ def _parse(raw_yaml: str) -> SourceResolver:
     return wrapper.source
 
 
+def test_resolver_requires_build_sdist_strategy() -> None:
+    """Require every resolver to declare an sdist build strategy."""
+    with pytest.raises(pydantic.ValidationError, match="build_sdist"):
+        AbstractResolver.model_validate({"provider": "test"})
+
+
+@pytest.mark.parametrize(
+    "provider",
+    ["pypi-sdist", "pypi-prebuilt", "hook-prebuilt", "not-available"],
+)
+def test_resolver_rejects_unsupported_build_sdist_strategy(provider: str) -> None:
+    """Keep fixed resolver strategies from accepting a different build method."""
+    with pytest.raises(pydantic.ValidationError, match="build_sdist"):
+        _parse(f"source:\n  provider: {provider}\n  build_sdist: pep517\n")
+
+
 # -- PyPI resolvers -----------------------------------------------------------
 
 
@@ -679,18 +696,34 @@ class TestHookSDistResolver:
     YAML = """\
         source:
           provider: hook-sdist
+          build_sdist: tarball
     """
 
     def test_parse(self) -> None:
         r = _parse(self.YAML)
         assert isinstance(r, HookSDistResolver)
         assert r.provider == "hook-sdist"
+        assert r.build_sdist is BuildSDist.tarball
         assert r.supports_override_hooks is True
         assert r.resolves_prebuilt_wheel is False
         assert r.download_kinds == frozenset(
             {DownloadKind.sdist, DownloadKind.tarball, DownloadKind.git_checkout}
         )
 
+    @pytest.mark.parametrize("build_method", list(BuildSDist))
+    def test_build_sdist_method(self, build_method: BuildSDist) -> None:
+        """Load the hook sdist build method from YAML."""
+        r = _parse(
+            f"source:\n  provider: hook-sdist\n  build_sdist: {build_method.value}\n"
+        )
+
+        assert r.build_sdist is build_method
+
+    def test_build_sdist_method_required(self) -> None:
+        """Require a build method because hook artifacts need different strategies."""
+        with pytest.raises(pydantic.ValidationError, match="build_sdist"):
+            _parse("source:\n  provider: hook-sdist\n")
+
     @mock.patch("fromager.packagesettings._resolver.overrides.find_override_method")
     def test_resolver_provider(
         self,
@@ -805,7 +838,11 @@ def test_hook_download_preserves_typed_result_and_arguments(
     tmp_context: WorkContext, provider: str, kind: DownloadKind
 ) -> None:
     """Preserve the hook's explicit kind and forward the legacy arguments."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
     candidate = _CANDIDATE_WHEEL if r.resolves_prebuilt_wheel else _CANDIDATE_SDIST
     expected = DownloadedSource(path=tmp_context.work_dir / "artifact", kind=kind)
     captured: dict[str, object] = {}
@@ -862,7 +899,11 @@ def test_hook_download_adapts_legacy_path_and_narrow_signature(
     tmp_context: WorkContext, provider: str, filename: str, kind: DownloadKind
 ) -> None:
     """Adapt bare paths without requiring hooks to accept every argument."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
     expected = tmp_context.work_dir / filename
 
     def hook(req: Requirement, version: Version) -> pathlib.Path:
@@ -924,7 +965,11 @@ def test_hook_download_rejects_invalid_results_without_fallback(
     message: str,
 ) -> None:
     """Reject malformed hook results with package and provider context."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
 
     def hook() -> object:
         return hook_result
@@ -953,7 +998,11 @@ def test_hook_download_rejects_kind_incompatible_with_profile(
     tmp_context: WorkContext, provider: str, kind: DownloadKind
 ) -> None:
     """Do not relabel an explicit kind to fit the selected profile."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
 
     def hook() -> DownloadedSource:
         return DownloadedSource(path=tmp_context.work_dir, kind=kind)
@@ -983,7 +1032,11 @@ def test_hook_download_requires_kind_for_unsupported_legacy_paths(
     tmp_context: WorkContext, provider: str, filename: str, is_directory: bool
 ) -> None:
     """Require an explicit kind when a path cannot use the legacy default."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
     path = tmp_context.work_dir / filename
     if is_directory:
         path.mkdir()
@@ -1005,7 +1058,11 @@ def test_hook_download_uses_profile_default_when_hook_is_missing(
     tmp_context: WorkContext, provider: str
 ) -> None:
     """Select the correct download helper and destination without a hook."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
     prebuilt = r.resolves_prebuilt_wheel
     candidate = _CANDIDATE_WHEEL if prebuilt else _CANDIDATE_SDIST
     destination = (
@@ -1032,7 +1089,11 @@ def test_hook_download_chains_failure_without_fallback(
     tmp_context: WorkContext, provider: str
 ) -> None:
     """Preserve the hook failure and do not attempt a default download."""
-    r = _parse(f"source:\n  provider: {provider}")
+    r = _parse(
+        TestHookSDistResolver.YAML
+        if provider == "hook-sdist"
+        else TestHookPrebuiltResolver.YAML
+    )
     cause = OSError("download failed")
 
     def hook() -> pathlib.Path:
@@ -1087,7 +1148,9 @@ def legacy_hook(
         return provider
 
     find_override_method.return_value = legacy_hook
-    resolver_model = HookSDistResolver(provider="hook-sdist")
+    resolver_model = HookSDistResolver(
+        provider="hook-sdist", build_sdist=BuildSDist.tarball
+    )
 
     result = resolver_model.resolver_provider(tmp_context, _REQ, _REQ_TYPE)
 
@@ -1105,7 +1168,9 @@ def test_hook_resolver_does_not_fall_back_when_hook_is_missing(
     tmp_context: WorkContext,
 ) -> None:
     """A missing hook fails instead of selecting the default PyPI provider."""
-    resolver_model = HookSDistResolver(provider="hook-sdist")
+    resolver_model = HookSDistResolver(
+        provider="hook-sdist", build_sdist=BuildSDist.tarball
+    )
 
     with pytest.raises(ValueError, match="requires a get_resolver_provider"):
         resolver_model.resolver_provider(tmp_context, _REQ, _REQ_TYPE)
@@ -1164,7 +1229,9 @@ def hook(
         raise ValueError("hook failure")
 
     find_override_method.return_value = hook
-    resolver_model = HookSDistResolver(provider="hook-sdist")
+    resolver_model = HookSDistResolver(
+        provider="hook-sdist", build_sdist=BuildSDist.tarball
+    )
 
     with pytest.raises(RuntimeError, match="hook-sdist") as exc_info:
         resolver_model.resolver_provider(tmp_context, _REQ, _REQ_TYPE)