diff --git a/docs/concepts/package-settings.rst b/docs/concepts/package-settings.rst index 2f0a0d7f..c516ff93 100644 --- a/docs/concepts/package-settings.rst +++ b/docs/concepts/package-settings.rst @@ -35,6 +35,17 @@ Settings Loading Flow ``PackageBuildInfo`` objects are created lazily the first time a package is queried and cached for the rest of the run. +``PackageSettings`` validates source settings when it loads each package. +A package-level ``source`` cannot be combined with an explicitly set +variant ``pre_built`` or ``wheel_server_url``. A variant-level ``source`` +cannot be combined with package-level ``download_source`` or +``resolver_dist``. An explicit ``pre_built: false`` also conflicts; +an omitted ``pre_built`` does not. + +.. versionchanged:: 0.102.0 + Source and legacy settings mixed across package and variant levels + now raise a validation error when package settings load. + Directory Structure ------------------- diff --git a/src/fromager/packagesettings/_models.py b/src/fromager/packagesettings/_models.py index 783cd8d4..a79f250d 100644 --- a/src/fromager/packagesettings/_models.py +++ b/src/fromager/packagesettings/_models.py @@ -32,6 +32,9 @@ logger = logging.getLogger(__name__) +_PACKAGE_LEGACY_SOURCE_FIELDS = frozenset({"download_source", "resolver_dist"}) +_VARIANT_LEGACY_SOURCE_FIELDS = frozenset({"wheel_server_url", "pre_built"}) + class SbomSettings(pydantic.BaseModel): """Global SBOM generation settings @@ -488,8 +491,7 @@ 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 + conflicts = _VARIANT_LEGACY_SOURCE_FIELDS & self.model_fields_set if conflicts: raise ValueError( f"'source' is mutually exclusive with legacy settings: " @@ -641,17 +643,36 @@ def before_none_dict( @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: + """Validate source and legacy fields across package and variant levels.""" + package_legacy_fields = _PACKAGE_LEGACY_SOURCE_FIELDS & self.model_fields_set + if self.source is not None and package_legacy_fields: raise ValueError( f"'source' is mutually exclusive with legacy settings: " - f"{sorted(conflicts)}. Use 'source' alone or remove it " + f"{sorted(package_legacy_fields)}. Use 'source' alone or remove it " f"and use the legacy settings." ) + + cross_level_conflicts: list[str] = [] + for variant_name, variant in self.variants.items(): + if self.source is not None: + variant_legacy_fields = ( + _VARIANT_LEGACY_SOURCE_FIELDS & variant.model_fields_set + ) + cross_level_conflicts.extend( + f"source and variants.{variant_name}.{field}" + for field in sorted(variant_legacy_fields) + ) + if variant.source is not None: + cross_level_conflicts.extend( + f"variants.{variant_name}.source and {field}" + for field in sorted(package_legacy_fields) + ) + + if cross_level_conflicts: + raise ValueError( + "source is mutually exclusive with legacy settings across " + "package and variant levels: " + "; ".join(cross_level_conflicts) + ) return self @classmethod diff --git a/tests/test_packagesettings.py b/tests/test_packagesettings.py index 5ecc745c..c799edb1 100644 --- a/tests/test_packagesettings.py +++ b/tests/test_packagesettings.py @@ -1309,6 +1309,67 @@ def test_variant_source_exclusivity_invalid(yaml_str: str) -> None: PackageSettings.from_string("conflict-pkg", yaml_str) +@pytest.mark.parametrize( + ("yaml_str", "source_path", "legacy_path"), + [ + pytest.param( + "source:\n provider: pypi-sdist\nvariants:\n cpu:\n pre_built: true\n", + "source", + "variants.cpu.pre_built", + id="package-source-variant-pre-built", + ), + pytest.param( + "source:\n provider: pypi-sdist\n" + "variants:\n cpu:\n pre_built: false\n", + "source", + "variants.cpu.pre_built", + id="package-source-variant-explicit-false", + ), + pytest.param( + "source:\n provider: pypi-sdist\n" + "variants:\n cpu:\n wheel_server_url: https://wheel.test/simple\n", + "source", + "variants.cpu.wheel_server_url", + id="package-source-variant-wheel-server", + ), + pytest.param( + "download_source:\n url: https://egg.test/pkg.tar.gz\n" + "variants:\n cpu:\n source:\n provider: pypi-sdist\n", + "variants.cpu.source", + "download_source", + id="variant-source-package-download-source", + ), + pytest.param( + "resolver_dist:\n sdist_server_url: https://sdist.test/simple\n" + "variants:\n cpu:\n source:\n provider: pypi-sdist\n", + "variants.cpu.source", + "resolver_dist", + id="variant-source-package-resolver-dist", + ), + ], +) +def test_cross_level_source_exclusivity_invalid( + yaml_str: str, source_path: str, legacy_path: str +) -> None: + """Reject source and legacy fields mixed across package and variant levels.""" + with pytest.raises(RuntimeError, match="mutually exclusive") as exc_info: + PackageSettings.from_string("conflict-pkg", yaml_str) + error_str = str(exc_info.value) + assert source_path in error_str + assert legacy_path in error_str + + +def test_package_source_with_unrelated_variant_setting_valid() -> None: + """Omitted variant legacy fields do not conflict with package source.""" + yaml_str = ( + "source:\n provider: pypi-sdist\n" + "variants:\n cpu:\n env:\n BUILD_MODE: fast\n" + ) + ps = PackageSettings.from_string("source-pkg", yaml_str) + assert ps.source is not None + assert ps.variants[Variant("cpu")].pre_built is False + + def test_legacy_only_config_valid() -> None: """Legacy-only config loads without error.""" yaml_str = (