diff --git a/src/fromager/packagesettings/__init__.py b/src/fromager/packagesettings/__init__.py index abca2f21..484ef57f 100644 --- a/src/fromager/packagesettings/__init__.py +++ b/src/fromager/packagesettings/__init__.py @@ -28,6 +28,7 @@ PyPIGitResolver, PyPIPrebuiltResolver, PyPISDistResolver, + SourceResolver, pep440_tag_matcher, ) from ._settings import Settings, SettingsFile @@ -84,6 +85,7 @@ "SbomSettings", "Settings", "SettingsFile", + "SourceResolver", "Template", "Variant", "VariantChangelog", diff --git a/src/fromager/packagesettings/_models.py b/src/fromager/packagesettings/_models.py index e8723061..783cd8d4 100644 --- a/src/fromager/packagesettings/_models.py +++ b/src/fromager/packagesettings/_models.py @@ -16,7 +16,7 @@ from pydantic import AnyUrl, Field, PrivateAttr, StringConstraints from pydantic_core import core_schema -# from ._resolver import SourceResolver +from ._resolver import SourceResolver from ._typedefs import ( MODEL_CONFIG, BuildDirectory, @@ -480,9 +480,23 @@ class VariantInfo(pydantic.BaseModel): pre_built: bool = False """Use pre-built wheel from index server?""" - # TODO - # source: SourceResolver | None - # """Source resolver and downloader""" + source: SourceResolver | None = None + """Source resolver and downloader""" + + @pydantic.model_validator(mode="after") + def validate_source_exclusivity(self) -> typing.Self: + """Validate that ``source`` is mutually exclusive with legacy fields.""" + if self.source is None: + return self + legacy_fields = {"wheel_server_url", "pre_built"} + conflicts = legacy_fields & self.model_fields_set + if conflicts: + raise ValueError( + f"'source' is mutually exclusive with legacy settings: " + f"{sorted(conflicts)}. Use 'source' alone or remove it " + f"and use the legacy settings." + ) + return self class GitOptions(pydantic.BaseModel): @@ -608,9 +622,8 @@ class PackageSettings(pydantic.BaseModel): project_override: ProjectOverride = Field(default_factory=ProjectOverride) """Patch project settings""" - # TODO - # source: SourceResolver | None - # """Source resolver and downloader""" + source: SourceResolver | None = None + """Source resolver and downloader""" variants: Mapping[Variant, VariantInfo] = Field(default_factory=dict) """Variant configuration""" @@ -626,6 +639,21 @@ def before_none_dict( v = {} return v + @pydantic.model_validator(mode="after") + def validate_source_exclusivity(self) -> typing.Self: + """Validate that ``source`` is mutually exclusive with legacy fields.""" + if self.source is None: + return self + legacy_fields = {"download_source", "resolver_dist"} + conflicts = legacy_fields & self.model_fields_set + if conflicts: + raise ValueError( + f"'source' is mutually exclusive with legacy settings: " + f"{sorted(conflicts)}. Use 'source' alone or remove it " + f"and use the legacy settings." + ) + return self + @classmethod def from_mapping( cls, diff --git a/src/fromager/packagesettings/_pbi.py b/src/fromager/packagesettings/_pbi.py index 2e262004..ba62f9bf 100644 --- a/src/fromager/packagesettings/_pbi.py +++ b/src/fromager/packagesettings/_pbi.py @@ -26,6 +26,7 @@ if typing.TYPE_CHECKING: from .. import build_environment + from ._resolver import SourceResolver from ._settings import Settings logger = logging.getLogger(__name__) @@ -176,6 +177,19 @@ def wheel_server_url(self) -> str | None: return str(vi.wheel_server_url) return None + @property + def source_resolver(self) -> SourceResolver | None: + """Source resolver for the package variant. + + Returns the variant-level source resolver if configured, + otherwise falls back to the package-level source resolver, + or ``None`` if neither is set. + """ + vi = self._ps.variants.get(self.variant) + if vi is not None and vi.source is not None: + return vi.source + return self._ps.source + @property def override_module_name(self) -> str: """Override module name from package name""" diff --git a/tests/test_packagesettings.py b/tests/test_packagesettings.py index a5107ad9..5ecc745c 100644 --- a/tests/test_packagesettings.py +++ b/tests/test_packagesettings.py @@ -18,9 +18,12 @@ Package, PackageBuildInfo, PackageSettings, + PyPIPrebuiltResolver, + PyPISDistResolver, ResolverDist, Settings, SettingsFile, + SourceResolver, Variant, substitute_template, ) @@ -90,6 +93,7 @@ "use_pypi_org_metadata": True, "min_release_age": None, }, + "source": None, "variants": { "cpu": { "annotations": { @@ -98,6 +102,7 @@ "env": {"EGG": "spam ${EGG}", "EGG_AGAIN": "$EGG"}, "wheel_server_url": "https://wheel.test/simple", "pre_built": False, + "source": None, }, "rocm": { "annotations": { @@ -106,12 +111,14 @@ "env": {"SPAM": ""}, "wheel_server_url": None, "pre_built": True, + "source": None, }, "cuda": { "annotations": None, "env": {}, "wheel_server_url": None, "pre_built": False, + "source": None, }, }, } @@ -152,6 +159,7 @@ "use_pypi_org_metadata": None, "min_release_age": None, }, + "source": None, "variants": {}, } @@ -193,11 +201,13 @@ "use_pypi_org_metadata": None, "min_release_age": None, }, + "source": None, "variants": { "cpu": { "annotations": None, "env": {}, "pre_built": True, + "source": None, "wheel_server_url": None, }, }, @@ -1152,3 +1162,190 @@ def test_filter_env( ) -> None: ec = ExternalCommands(keep_env=keep, delete_env=delete) assert ec.filter_env(env) == expected + + +# -- source field and source_resolver PBI property ---------------------------- + + +def test_source_resolver_importable() -> None: + """``SourceResolver`` is importable from ``fromager.packagesettings``.""" + args = typing.get_args(SourceResolver) + assert len(args) == 2 + union_members = typing.get_args(args[0]) + assert PyPISDistResolver in union_members + assert PyPIPrebuiltResolver in union_members + + +def test_package_settings_with_source() -> None: + """``PackageSettings`` loads YAML with ``source:`` correctly.""" + yaml_str = "source:\n provider: pypi-sdist\n" + ps = PackageSettings.from_string("test-source-pkg", yaml_str) + assert ps.source is not None + assert isinstance(ps.source, PyPISDistResolver) + assert ps.source.provider == "pypi-sdist" + + +def test_variant_info_with_source() -> None: + """``VariantInfo`` with ``source:`` works.""" + yaml_str = "variants:\n cpu:\n source:\n provider: pypi-prebuilt\n" + ps = PackageSettings.from_string("test-variant-source-pkg", yaml_str) + vi = ps.variants[Variant("cpu")] + assert vi.source is not None + assert isinstance(vi.source, PyPIPrebuiltResolver) + + +def test_package_settings_without_source() -> None: + """``PackageSettings`` without ``source:`` has ``source=None``.""" + ps = PackageSettings.from_default("test-pkg") + assert ps.source is None + + +def test_pbi_source_resolver_none(tmp_path: pathlib.Path) -> None: + """``pbi.source_resolver`` returns ``None`` when no source configured.""" + ps = PackageSettings.from_default("test-pkg") + settings = Settings( + settings=SettingsFile(), + package_settings=[ps], + variant="cpu", + patches_dir=tmp_path, + max_jobs=1, + ) + pbi = settings.package_build_info("test-pkg") + assert pbi.source_resolver is None + + +def test_pbi_source_resolver_package_level(tmp_path: pathlib.Path) -> None: + """``pbi.source_resolver`` returns package-level source.""" + yaml_str = "source:\n provider: pypi-sdist\n" + ps = PackageSettings.from_string("test-pkg", yaml_str) + settings = Settings( + settings=SettingsFile(), + package_settings=[ps], + variant="cpu", + patches_dir=tmp_path, + max_jobs=1, + ) + pbi = settings.package_build_info("test-pkg") + assert pbi.source_resolver is not None + assert isinstance(pbi.source_resolver, PyPISDistResolver) + + +def test_pbi_source_resolver_variant_overrides_package( + tmp_path: pathlib.Path, +) -> None: + """``pbi.source_resolver`` returns variant source over package source.""" + yaml_str = ( + "source:\n" + " provider: pypi-sdist\n" + "variants:\n" + " cpu:\n" + " source:\n" + " provider: pypi-prebuilt\n" + ) + ps = PackageSettings.from_string("test-pkg", yaml_str) + settings = Settings( + settings=SettingsFile(), + package_settings=[ps], + variant="cpu", + patches_dir=tmp_path, + max_jobs=1, + ) + pbi = settings.package_build_info("test-pkg") + assert isinstance(pbi.source_resolver, PyPIPrebuiltResolver) + + +# -- source vs legacy mutual exclusivity validation -------------------------- + + +@pytest.mark.parametrize( + "yaml_str", + [ + pytest.param( + "source:\n provider: pypi-sdist\n" + "download_source:\n url: https://egg.test/pkg-${version}.tar.gz\n", + id="source-with-download-source", + ), + pytest.param( + "source:\n provider: pypi-sdist\n" + "resolver_dist:\n sdist_server_url: https://sdist.test/simple\n", + id="source-with-resolver-dist", + ), + pytest.param( + "source:\n provider: pypi-sdist\n" + "download_source:\n url: https://egg.test/pkg.tar.gz\n" + "resolver_dist:\n sdist_server_url: https://sdist.test/simple\n", + id="source-with-both-legacy-fields", + ), + pytest.param( + "source:\n provider: pypi-sdist\ndownload_source:\n", + id="source-with-empty-download-source", + ), + ], +) +def test_package_source_exclusivity_invalid(yaml_str: str) -> None: + """Setting ``source`` alongside legacy package fields raises.""" + with pytest.raises(RuntimeError, match="mutually exclusive"): + PackageSettings.from_string("conflict-pkg", yaml_str) + + +@pytest.mark.parametrize( + "yaml_str", + [ + pytest.param( + "variants:\n cpu:\n source:\n provider: pypi-prebuilt\n" + " wheel_server_url: https://wheel.test/simple\n", + id="source-with-wheel-server-url", + ), + pytest.param( + "variants:\n cpu:\n source:\n provider: pypi-prebuilt\n" + " pre_built: true\n", + id="source-with-pre-built", + ), + ], +) +def test_variant_source_exclusivity_invalid(yaml_str: str) -> None: + """Setting ``source`` alongside legacy variant fields raises.""" + with pytest.raises(RuntimeError, match="mutually exclusive"): + PackageSettings.from_string("conflict-pkg", yaml_str) + + +def test_legacy_only_config_valid() -> None: + """Legacy-only config loads without error.""" + yaml_str = ( + "download_source:\n url: https://egg.test/pkg-${version}.tar.gz\n" + "resolver_dist:\n sdist_server_url: https://sdist.test/simple\n" + ) + ps = PackageSettings.from_string("legacy-pkg", yaml_str) + assert ps.source is None + assert ps.download_source.url is not None + + +def test_source_only_config_valid() -> None: + """Source-only config loads without error.""" + yaml_str = "source:\n provider: pypi-sdist\n" + ps = PackageSettings.from_string("new-pkg", yaml_str) + assert ps.source is not None + assert ps.download_source.url is None + + +def test_variant_legacy_only_valid() -> None: + """Variant with only legacy fields loads without error.""" + yaml_str = "variants:\n cpu:\n pre_built: true\n wheel_server_url: https://wheel.test/simple\n" + ps = PackageSettings.from_string("legacy-variant-pkg", yaml_str) + vi = ps.variants[Variant("cpu")] + assert vi.source is None + assert vi.pre_built is True + + +def test_source_exclusivity_error_lists_fields() -> None: + """Error message includes the specific conflicting field names.""" + yaml_str = ( + "source:\n provider: pypi-sdist\n" + "download_source:\n url: https://egg.test/a.tar.gz\n" + "resolver_dist:\n sdist_server_url: https://sdist.test\n" + ) + with pytest.raises(RuntimeError) as exc_info: + PackageSettings.from_string("conflict-pkg", yaml_str) + error_str = str(exc_info.value) + assert "download_source" in error_str + assert "resolver_dist" in error_str