From 09fb462d88d6afd8f9d3de4ed3cdacf46362d137 Mon Sep 17 00:00:00 2001 From: Mike DePaulo Date: Thu, 22 Oct 2026 17:34:21 -0400 Subject: [PATCH] feat(settings): support explicit per-version wheel build numbers Add an optional `build_number` mapping keyed by public version. Explicit numbers replace combined variant and package changelog counts while keeping changelogs and `build_tag_hook` suffixes. Reject local-version keys and use the public version when looking up a requested local version. Require exact tags for configured versions in bootstrap and build cache checks, including numeric zero. Preserve legacy behavior for unconfigured versions and retain pre-built wheel tags. Keep exact dependency installation as a proof of concept pending the scope discussion in #1392. Any loaded non-prebuilt counter activates local-only installation, including unused versions. Log this policy, reuse filtered views per worker, reinstall dependencies, and check installed WHEEL metadata before compilation. All required dependency wheels must be available locally. Related: #1392 Co-authored-by: Codex Signed-off-by: Mike DePaulo --- docs/concepts/package-settings.rst | 34 +- docs/customization.md | 16 +- e2e/test_bootstrap_build_tags.sh | 53 +++ src/fromager/build_environment.py | 147 ++++++- src/fromager/commands/build.py | 24 +- src/fromager/context.py | 2 + src/fromager/packagesettings/_models.py | 32 +- src/fromager/packagesettings/_pbi.py | 22 +- src/fromager/packagesettings/_settings.py | 13 + tests/test_build_environment.py | 23 +- tests/test_build_number.py | 511 ++++++++++++++++++++++ tests/test_packagesettings.py | 3 + 12 files changed, 838 insertions(+), 42 deletions(-) create mode 100644 tests/test_build_number.py diff --git a/docs/concepts/package-settings.rst b/docs/concepts/package-settings.rst index 2f0a0d7f3..c33f1a48d 100644 --- a/docs/concepts/package-settings.rst +++ b/docs/concepts/package-settings.rst @@ -119,7 +119,39 @@ adds variant awareness, patch discovery, and template resolution. - Merged list of unversioned + version-specific + variant-specific patch files, sorted by filename * - **Metadata** - - Build tag (from changelog length), annotations, PURL config, + - Build tag (explicit number or changelog length), annotations, PURL config, pyproject.toml overrides See :doc:`/reference/config-reference` for the full field reference. + +Explicit Build Numbers +---------------------- + +``build_number`` maps public versions to nonnegative integers, replacing the +combined variant and package changelog count for those versions in every +variant. Variant-specific explicit numbers are not supported. Changelogs +remain available as history. Keys with local version suffixes are rejected; +requested versions with local suffixes use their public version for lookup. + +.. code-block:: yaml + + build_number: + "1.2.3": 0 + "2.0": 7 + +.. versionadded:: 0.102.0 + +Explicit numbers require exact build tags in local and remote cache lookups +across ``bootstrap``, ``build`` and ``build-sequence``. Zero produces a numeric +``0`` tag; a missing or different tag is a cache miss. ``build_tag_hook`` still +appends suffixes, including for zero. Unconfigured versions retain their +changelog-derived tags and cache behavior; pre-built wheels keep their tags. + +Any loaded non-prebuilt package with a non-empty ``build_number`` mapping +switches all build-environment dependency installation to a filtered local +wheel view, even if its configured versions are not used. Fromager logs this +mode and reinstalls all dependencies, including unconfigured versions and +pre-built wheels. Installation does not fetch from ``--wheel-server-url``; +all required dependency wheels must already be available locally. Installed +``WHEEL`` metadata is checked before building. Without such settings, +dependency installation retains its existing behavior. diff --git a/docs/customization.md b/docs/customization.md index feece1dc3..22df76ea0 100644 --- a/docs/customization.md +++ b/docs/customization.md @@ -446,19 +446,27 @@ def build_tag_hook( return [suffixes[ctx.variant]] ``` -Fromager joins segments in order with `_`. If the changelog build tag is `2`, -returning `["cuda13", "torch2.10"]` produces `2_cuda13_torch2.10`. +Fromager joins segments in order with `_`. If the numeric part of the build +tag is `2`, returning `["cuda13", "torch2.10"]` produces +`2_cuda13_torch2.10`. With an explicit `build_number` of zero, the same +segments produce `0_cuda13_torch2.10`. **Important notes:** -- The hook runs only when the package has a changelog-derived build tag. It - is skipped when there is no changelog entry or the package is prebuilt. +- The hook runs when the package version has an explicit `build_number` + (including zero) or a changelog-derived build tag. It is skipped for + pre-built wheels, or when neither an explicit number nor a non-empty + changelog supplies a build tag. - Return a sequence of strings, not a single string or bytes object. Each segment must be non-empty and contain only ASCII letters, digits, or dots. Map variant names with other characters to distinct valid segments. - The hook must be deterministic and independent of wheel contents, build environment, or ELF metadata, so that fresh builds and cache lookups produce identical tags. +- When explicit counters enable dependency-installation checks, the suffix + must depend only on the requirement name, version, variant, and `wheel_tags`. + These checks reconstruct a bare requirement from the wheel or installed + distribution name, without the original extras, markers, specifier, or URL. - Use `wheel_tags` only to distinguish pure wheels from platform wheels. The hook must return identical results across architectures for the same configured variant. diff --git a/e2e/test_bootstrap_build_tags.sh b/e2e/test_bootstrap_build_tags.sh index a92523b8e..087387547 100755 --- a/e2e/test_bootstrap_build_tags.sh +++ b/e2e/test_bootstrap_build_tags.sh @@ -114,3 +114,56 @@ if ! grep -q "added extra metadata and build tag" "$LOGFILE"; then fi $pass + +# Explicit numbers override both changelogs and rebuild in a reused repository. +mkdir -p "$OUTDIR/explicit-build-settings" +cat > "$OUTDIR/explicit-global-settings.yaml" < "$OUTDIR/explicit-build-settings/stevedore.yaml" < None: + self._temporary = tempfile.TemporaryDirectory( + prefix="install-wheels-", dir=work_dir + ) + self.path = pathlib.Path(self._temporary.name) + self._links: dict[str, pathlib.Path] = {} + self._identities: dict[ + str, tuple[NormalizedName, Version, BuildTag, frozenset[Tag]] + ] = {} + + def update(self, ctx: context.WorkContext) -> None: + """Refresh changed links without caching settings or hook results.""" + desired: dict[str, pathlib.Path] = {} + seen: set[str] = set() + for source in (ctx.wheels_downloads, ctx.wheels_prebuilt): + for path in sorted(source.glob("*.whl")): + seen.add(path.name) + identity = self._identities.get(path.name) + if identity is None: + identity = parse_wheel_filename(path.name) + self._identities[path.name] = identity + name, version, actual_tag, wheel_tags = identity + pbi = ctx.package_build_info(name) + if not pbi.pre_built and pbi.get_build_number(version) is not None: + expected = wheels.get_build_tag( + ctx=ctx, + req=Requirement(name), + version=version, + wheel_tags=wheel_tags, + ) + if actual_tag != expected: + continue + desired.setdefault(path.name, path) + for filename, target in self._links.items(): + if desired.get(filename) != target: + (self.path / filename).unlink() + for filename, target in desired.items(): + if self._links.get(filename) != target: + (self.path / filename).symlink_to(target) + self._links = desired + for filename in self._identities.keys() - seen: + del self._identities[filename] + + class MissingDependency(Exception): # noqa: N818 def __init__( self, @@ -212,6 +264,30 @@ def _createenv(self) -> None: ) logger.info("created build environment in %s", self.path) + @contextlib.contextmanager + def _installation_wheel_args(self, explicit: bool) -> typing.Iterator[list[str]]: + """Expose permitted local builds, or preserve legacy source arguments.""" + if not explicit: + yield self._ctx.pip_wheel_server_args + return + + view: _InstallationWheelView | None = getattr( + self._ctx._installation_wheel_views, "view", None + ) + if view is None: + logger.info( + "Explicit build numbers in loaded settings enable local-only " + "dependency installation for all packages, including unrelated " + "versions. --wheel-server-url is not used for installation; " + "all required wheels must be available locally." + ) + view = _InstallationWheelView(self._ctx.work_dir) + self._ctx._installation_wheel_views.view = view + view.update(self._ctx) + # uv treats build tags as preferences, not constraints. Keep other + # builds out of its candidate set, including its installed/cache state. + yield ["--no-index", "--offline", "--find-links", str(view.path), "--reinstall"] + def install(self, reqs: typing.Iterable[Requirement]) -> None: if not reqs: return @@ -227,20 +303,48 @@ def install(self, reqs: typing.Iterable[Requirement]) -> None: ":all:", ] cmd.extend(self._ctx.pip_constraint_args) - cmd.extend(self._ctx.pip_wheel_server_args) - cmd.extend(str(req) for req in reqs) - - self.run( - cmd, - cwd=str(self.path.parent), - network_isolation=False, - ) + explicit = self._ctx.settings.has_explicit_build_numbers + with self._installation_wheel_args(explicit) as source_args: + cmd.extend(source_args) + cmd.extend(str(req) for req in reqs) + + self.run( + cmd, + cwd=str(self.path.parent), + network_isolation=False, + ) + if explicit: + self._validate_installed_build_numbers() logger.info( "installed dependencies %s into build environment in %s", reqs, self.path, ) + def _validate_installed_build_numbers(self) -> None: + """Fail before compilation if direct references bypass the filtered source.""" + for name, raw_version, raw_wheel in self._installed_distributions(): + version = Version(raw_version) + pbi = self._ctx.package_build_info(name) + if pbi.pre_built or pbi.get_build_number(version) is None: + continue + metadata = Parser().parsestr(raw_wheel or "") + tags = frozenset( + tag for value in metadata.get_all("Tag", []) for tag in parse_tag(value) + ) + expected = wheels.get_build_tag( + ctx=self._ctx, req=Requirement(name), version=version, wheel_tags=tags + ) + assert expected # Explicit counters, including zero, are non-empty. + expected_text = f"{expected[0]}{expected[1]}" + match = re.fullmatch(r"([0-9]+)(.*)", metadata.get("Build") or "") + actual: BuildTag = (int(match[1]), match[2]) if match else () + if not tags or actual != expected: + raise ValueError( + f"{name}=={version}: installed wheel build {metadata.get('Build')!r} " + f"does not match explicit build {expected_text!r}" + ) + def get_sysconfig_path(self, name: str) -> str: """Get ``sysconfig.get_path(name)`` in environment""" if not name.isidentifier(): @@ -255,18 +359,25 @@ def get_sysconfig_path(self, name: str) -> str: def get_distributions(self) -> typing.Mapping[str, Version]: """Get a mapping of installed package names and their versions in the build env""" - lines = [ - "import sys,json", - "from importlib.metadata import distributions", - "json.dump({d.name: d.version for d in distributions()}, sys.stdout)", - ] - result = self.run([str(self.python), "-c", "; ".join(lines)]) + mapping = { + name: Version(version) + for name, version, _ in self._installed_distributions() + } + return dict(sorted(mapping.items())) + + def _installed_distributions(self) -> list[tuple[str, str, str | None]]: + """Read distribution identities and wheel metadata in the build env.""" + script = ( + "import json,sys; from importlib.metadata import distributions; " + "json.dump([(d.name, d.version, d.read_text('WHEEL')) " + "for d in distributions()], sys.stdout)" + ) + result = self.run([str(self.python), "-c", script]) try: - mapping = json.loads(result.strip()) - except Exception: + return typing.cast(list[tuple[str, str, str | None]], json.loads(result)) + except json.JSONDecodeError: logger.exception("failed to de-serialize JSON: %s", result) raise - return {name: Version(version) for name, version in sorted(mapping.items())} @metrics.timeit(description="prepare build environment") diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index a5ef79f06..df55b95ec 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -547,16 +547,20 @@ def _is_wheel_built( wheel_tags=wheel_tags, ) build_tag = expected_tag if expected_tag else (0, "") - existing_build_tag = ( - build_tag_from_name if build_tag_from_name else (0, "") - ) - if ( - existing_build_tag[0] > build_tag[0] - and existing_build_tag[1] == build_tag[1] - ): - raise ValueError( - f"{dist_name}: changelog for version {candidate_version} is inconsistent. Found build tag {existing_build_tag} but expected {build_tag}" - ) + explicit = pbi.get_build_number(candidate_version) is not None + # An explicit zero is not an untagged wheel. Higher counters are + # cache misses for explicit settings, just as in bootstrap. + if explicit: + existing_build_tag = build_tag_from_name + else: + existing_build_tag = build_tag_from_name or (0, "") + if ( + existing_build_tag[0] > build_tag[0] + and existing_build_tag[1] == build_tag[1] + ): + raise ValueError( + f"{dist_name}: changelog for version {candidate_version} is inconsistent. Found build tag {existing_build_tag} but expected {build_tag}" + ) if existing_build_tag != build_tag: logger.info( "candidate wheel build tag %s does not match expected build tag %s", diff --git a/src/fromager/context.py b/src/fromager/context.py index 5525de804..5ff231d9c 100644 --- a/src/fromager/context.py +++ b/src/fromager/context.py @@ -94,6 +94,8 @@ def __init__( self.time_description_store: dict[str, str] = {} self._parallel_builds = False + # Each worker reuses its own filtered dependency wheel view. + self._installation_wheel_views = threading.local() self.cooldown: candidate.Cooldown | None = cooldown self._max_release_age: datetime.timedelta | None = max_release_age diff --git a/src/fromager/packagesettings/_models.py b/src/fromager/packagesettings/_models.py index 7aa54e0c0..70935c3ec 100644 --- a/src/fromager/packagesettings/_models.py +++ b/src/fromager/packagesettings/_models.py @@ -22,6 +22,7 @@ BuildDirectory, EnvVars, Package, + PackageVersion, PurlType, RawAnnotations, Template, @@ -55,8 +56,8 @@ class WheelSettings(pydantic.BaseModel): ``version``, and ``wheel_tags`` and returns ``Sequence[str]`` of suffix segments. - Only invoked when the package already has a non-empty build tag - from its changelog entry for the given version; otherwise the hook + Only invoked when the package has an explicit build number (including + zero) or a non-empty changelog build tag for the version; otherwise the hook is skipped and no build tag is added. The callable must be deterministic and independent of wheel contents, build environment, or ELF metadata so fresh builds and cache lookups compute the same @@ -626,6 +627,33 @@ class PackageSettings(pydantic.BaseModel): build_dir: BuildDirectory | None = None """sub-directory with setup.py or pyproject.toml""" + build_number: dict[ + PackageVersion, typing.Annotated[int, Field(ge=0, strict=True)] + ] = Field(default_factory=dict) + """Explicit numeric wheel build tags, keyed by public version. + + The number applies to all variants; variant overrides are not supported. + Overrides both global and package changelog counts for configured versions. + Zero is an explicit tag, distinct from the legacy empty (unspecified) tag. + Pre-built wheels retain their original tags. ABI suffix hooks still apply. + + .. versionadded:: 0.102.0 + """ + + @pydantic.field_validator("build_number") + @classmethod + def validate_build_number_versions( + cls, value: dict[PackageVersion, int] + ) -> dict[PackageVersion, int]: + """Require public version keys so configured numbers cannot be ignored.""" + for version in value: + if version.local is not None: + raise ValueError( + f"build_number key {str(version)!r} must be a public version " + f"without a local version suffix; use {version.public!r}" + ) + return value + changelog: VariantChangelog = Field(default_factory=dict) """Changelog entries""" diff --git a/src/fromager/packagesettings/_pbi.py b/src/fromager/packagesettings/_pbi.py index 2e2620047..f119a5029 100644 --- a/src/fromager/packagesettings/_pbi.py +++ b/src/fromager/packagesettings/_pbi.py @@ -286,19 +286,35 @@ def get_changelog(self, version: Version) -> list[str]: package_changelog = self._ps.changelog.get(pv, []) return variant_changelog + package_changelog + def get_build_number(self, version: Version) -> int | None: + """Return the explicit public-version counter, or None for changelog defaults. + + .. versionadded:: 0.102.0 + """ + public = typing.cast(PackageVersion, Version(version.public)) + return self._ps.build_number.get(public) + def build_tag(self, version: Version) -> BuildTag: - """Build tag for version's changelog and this variant + """Numeric tag from an explicit number or this variant's changelog - .. versionchanged 0.54.0:: + .. versionchanged:: 0.54.0 Fromager ignores local version suffix of a package to determinate the build tag from changelog, e.g. version `1.0.3+local.suffix` uses `1.0.3`. + + .. versionchanged:: 0.102.0 + + Explicit ``build_number`` values override changelog counts. Zero + returns ``(0, "")`` rather than an omitted build tag. """ if self.pre_built: # pre-built wheels have no built tag return () - pv = typing.cast(PackageVersion, version) + pv = typing.cast(PackageVersion, Version(version.public)) + number = self.get_build_number(pv) + if number is not None: + return number, "" release = len(self.get_changelog(pv)) if release == 0: return () diff --git a/src/fromager/packagesettings/_settings.py b/src/fromager/packagesettings/_settings.py index f2077826c..ab273b1f7 100644 --- a/src/fromager/packagesettings/_settings.py +++ b/src/fromager/packagesettings/_settings.py @@ -211,6 +211,19 @@ def build_tag_hook(self) -> typing.Callable[..., typing.Any] | None: return None return self._settings.wheels.build_tag_hook + @property + def has_explicit_build_numbers(self) -> bool: + """Whether any loaded non-prebuilt package has explicit build numbers. + + Entries for unused versions also activate local-only installation. + + .. versionadded:: 0.102.0 + """ + return any( + package.build_number and not self.package_build_info(package.name).pre_built + for package in self._package_settings.values() + ) + def variant_changelog(self) -> list[str]: """Get global changelog for current variant""" return list(self._settings.changelog.get(self.variant, [])) diff --git a/tests/test_build_environment.py b/tests/test_build_environment.py index 28c504af1..9e557c95b 100644 --- a/tests/test_build_environment.py +++ b/tests/test_build_environment.py @@ -89,18 +89,33 @@ def test_get_distributions_valid_json() -> None: """get_distributions returns a mapping of package names to versions.""" env = Mock(spec=build_environment.BuildEnvironment) env.python = "/fake/python3" - env.run.return_value = json.dumps({"setuptools": "69.5.1", "pip": "24.0"}) + env._installed_distributions.return_value = [ + ("setuptools", "69.5.1", None), + ("pip", "24.0", "Wheel-Version: 1.0"), + ] result = build_environment.BuildEnvironment.get_distributions(env) assert result == {"pip": Version("24.0"), "setuptools": Version("69.5.1")} -def test_get_distributions_invalid_json_raises() -> None: - """get_distributions raises on invalid JSON instead of UnboundLocalError.""" +def test_installed_distributions_invalid_json_raises() -> None: + """Shared metadata introspection preserves invalid-JSON diagnostics.""" env = Mock(spec=build_environment.BuildEnvironment) env.python = "/fake/python3" env.run.return_value = "not valid json" with pytest.raises(json.JSONDecodeError): - build_environment.BuildEnvironment.get_distributions(env) + build_environment.BuildEnvironment._installed_distributions(env) + + +def test_installed_distributions_reads_wheel_metadata() -> None: + """The shared reader supplies metadata for validation and version reporting.""" + env = Mock(spec=build_environment.BuildEnvironment) + env.python = "/fake/python3" + installed = [["test-pkg", "1.0", "Build: 0\n"], ["legacy", "2.0", None]] + env.run.return_value = json.dumps(installed) + + result = build_environment.BuildEnvironment._installed_distributions(env) + + assert result == installed diff --git a/tests/test_build_number.py b/tests/test_build_number.py new file mode 100644 index 000000000..e2959c6df --- /dev/null +++ b/tests/test_build_number.py @@ -0,0 +1,511 @@ +"""Explicit numeric tags must work identically in fresh builds and caches.""" + +import logging +import pathlib +import subprocess +import threading +from concurrent.futures import ThreadPoolExecutor +from unittest.mock import patch + +import pytest +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name +from packaging.version import Version +from wheel.wheelfile import WheelFile + +from fromager import build_environment, context, packagesettings, server, wheels +from fromager.bootstrapper._cache import ( + _download_wheel_from_cache, + _look_for_existing_wheel, +) +from fromager.commands.build import _is_wheel_built + + +def _configure(ctx: context.WorkContext, counter: int) -> None: + package = packagesettings.PackageSettings.from_mapping( + "test-pkg", + {"build_number": {"1.0.1": counter}, "changelog": {"1.0.1": ["a", "b"]}}, + source="test", + has_config=True, + ) + ctx.settings = packagesettings.Settings( + settings=packagesettings.SettingsFile( + changelog={packagesettings.Variant("cpu"): ["global"]} + ), + package_settings=[package], + variant="cpu", + patches_dir=ctx.settings.patches_dir, + max_jobs=None, + ) + + +@pytest.mark.parametrize("counter", [0, 1, 7]) +def test_explicit_number_overrides_both_changelogs( + tmp_context: context.WorkContext, counter: int +) -> None: + _configure(tmp_context, counter) + pbi = tmp_context.package_build_info("Test.Pkg") + assert pbi.build_tag(Version("1.0.1")) == (counter, "") + assert pbi.build_tag(Version("1.0.1+local")) == (counter, "") + assert pbi.build_tag(Version("2.0")) == (1, "") + assert pbi.get_changelog(Version("1.0.1")) == ["global", "a", "b"] + assert pbi.serialize(mode="json")["build_number"] == {"1.0.1": counter} + + +@pytest.mark.parametrize("counter", [0, 1, 7]) +@pytest.mark.parametrize("cached_tag", ["", "0-", "1-", "2-", "7-"]) +def test_local_cache_requires_exact_explicit_number( + tmp_context: context.WorkContext, counter: int, cached_tag: str +) -> None: + _configure(tmp_context, counter) + cached = ( + tmp_context.wheels_downloads / f"test_pkg-1.0.1-{cached_tag}py3-none-any.whl" + ) + cached.touch() + with patch( + "fromager.bootstrapper._cache._extract_build_reqs_from_wheel", return_value=None + ): + result, _ = _look_for_existing_wheel( + tmp_context, + Requirement("test-pkg"), + Version("1.0.1"), + tmp_context.wheels_downloads, + ) + assert result == (cached if cached_tag == f"{counter}-" else None) + + +@pytest.mark.parametrize("counter", [0, 1, 7]) +@pytest.mark.parametrize("cached_tag", ["", "0-", "1-", "2-", "7-"]) +def test_remote_cache_requires_exact_explicit_number( + tmp_context: context.WorkContext, counter: int, cached_tag: str +) -> None: + _configure(tmp_context, counter) + cache_url = "https://cache.test/simple/" + tmp_context.wheel_server_url = cache_url + filename = f"test_pkg-1.0.1-{cached_tag}py3-none-any.whl" + cached = tmp_context.wheels_downloads / filename + url = cache_url + filename + with ( + patch( + "fromager.resolver.find_all_matching_from_provider", + return_value=[(url, Version("1.0.1"))], + ), + patch("fromager.wheels.download_wheel", return_value=cached) as download, + patch( + "fromager.bootstrapper._cache._extract_build_reqs_from_wheel", + return_value=None, + ), + ): + result, _ = _download_wheel_from_cache( + tmp_context, cache_url, Requirement("test-pkg"), Version("1.0.1") + ) + assert result == (cached if cached_tag == f"{counter}-" else None) + assert download.call_count == int(cached_tag == f"{counter}-") + + +def test_explicit_zero_preserves_suffix_hook(tmp_context: context.WorkContext) -> None: + _configure(tmp_context, 0) + + def suffix(**kwargs: object) -> list[str]: + return ["el9", "cuda13"] + + tmp_context.settings._settings = tmp_context.settings._settings.model_copy( + update={"wheels": packagesettings.WheelSettings(build_tag_hook=suffix)} + ) + identity = wheels.get_build_tag( + ctx=tmp_context, + req=Requirement("test-pkg"), + version=Version("1.0.1"), + wheel_tags=frozenset(), + ) + assert identity == (0, "_el9_cuda13") + + +@pytest.mark.parametrize("number", [-1, True, "1"]) +def test_invalid_explicit_number_is_rejected(number: object) -> None: + with pytest.raises(RuntimeError, match="failed to load settings"): + packagesettings.PackageSettings.from_mapping( + "test-pkg", + {"build_number": {"1.0": number}}, + source="test", + has_config=True, + ) + + +@pytest.mark.parametrize("include_public_key", [False, True]) +def test_build_number_rejects_local_version_keys(include_public_key: bool) -> None: + numbers = {"1.0+vendor": 7} + if include_public_key: + numbers["1.0"] = 1 + with pytest.raises( + RuntimeError, match=r"build_number key '1\.0\+vendor' must be a public version" + ): + packagesettings.PackageSettings.from_mapping( + "test-pkg", + {"build_number": numbers, "changelog": {"1.0": ["history"]}}, + source="test", + has_config=True, + ) + + +def test_public_build_number_applies_to_local_requested_version( + tmp_context: context.WorkContext, +) -> None: + _configure(tmp_context, 7) + pbi = tmp_context.package_build_info("test-pkg") + assert pbi.build_tag(Version("1.0.1+vendor")) == (7, "") + assert tmp_context.settings.has_explicit_build_numbers + + +def test_prebuilt_keeps_original_tag(tmp_context: context.WorkContext) -> None: + package = packagesettings.PackageSettings.from_mapping( + "test-pkg", + {"build_number": {"1.0": 7}, "variants": {"cpu": {"pre_built": True}}}, + source="test", + has_config=True, + ) + tmp_context.settings._package_settings[canonicalize_name("test-pkg")] = package + assert tmp_context.package_build_info("test-pkg").build_tag(Version("1.0")) == () + + +@pytest.mark.parametrize("counter", [0, 1, 7]) +@pytest.mark.parametrize("cached_tag", ["", "0-", "1-", "2-", "7-"]) +@pytest.mark.parametrize("local", [False, True]) +def test_build_sequence_cache_requires_exact_explicit_number( + tmp_context: context.WorkContext, counter: int, cached_tag: str, local: bool +) -> None: + _configure(tmp_context, counter) + cache_url = "https://cache.test/simple/" + tmp_context.wheel_server_url = cache_url if local else "" + filename = f"test_pkg-1.0.1-{cached_tag}py3-none-any.whl" + cached = tmp_context.wheels_downloads / filename + cached.touch() + with ( + patch( + "fromager.wheels.resolve_all_cached_wheels", + return_value=[(cache_url + filename, Version("1.0.1"))], + ), + patch("fromager.wheels.download_wheel", return_value=cached) as download, + ): + found = _is_wheel_built( + tmp_context, + "test-pkg", + Version("1.0.1"), + cache_wheel_server_url=None if local else cache_url, + ) + assert found == (cached if cached_tag == f"{counter}-" else None) + assert download.call_count == int(not local and cached_tag == f"{counter}-") + + +def test_legacy_build_sequence_keeps_untagged_zero_and_higher_error( + tmp_context: context.WorkContext, +) -> None: + cache_url = "https://cache.test/simple/" + assert ( + tmp_context.package_build_info("test-pkg").get_build_number(Version("1.0.1")) + is None + ) + assert not tmp_context.settings.has_explicit_build_numbers + with ( + patch( + "fromager.wheels.resolve_all_cached_wheels", + return_value=[ + (cache_url + "test_pkg-1.0.1-py3-none-any.whl", Version("1.0.1")) + ], + ), + patch( + "fromager.wheels.download_wheel", + return_value=tmp_context.wheels_downloads + / "test_pkg-1.0.1-py3-none-any.whl", + ), + ): + assert ( + _is_wheel_built( + tmp_context, + "test-pkg", + Version("1.0.1"), + cache_wheel_server_url=cache_url, + ) + is not None + ) + with patch( + "fromager.wheels.resolve_all_cached_wheels", + return_value=[ + (cache_url + "test_pkg-1.0.1-1-py3-none-any.whl", Version("1.0.1")) + ], + ): + with pytest.raises(ValueError, match=r"changelog.*inconsistent"): + _is_wheel_built( + tmp_context, + "test-pkg", + Version("1.0.1"), + cache_wheel_server_url=cache_url, + ) + + +def _installation_wheel( + directory: pathlib.Path, + name: str, + version: str, + build: str, + requires: str | None = None, +) -> pathlib.Path: + filename = f"{name}-{version}-{build}-py3-none-any.whl" + path = directory / filename + dist = f"{name}-{version}.dist-info" + metadata = f"Metadata-Version: 2.1\nName: {name}\nVersion: {version}\n" + if requires: + metadata += f"Requires-Dist: {requires}\n" + with WheelFile(path, "w") as archive: + archive.writestr(f"{name}/__init__.py", f"BUILD = {build!r}\n") + archive.writestr(f"{dist}/METADATA", metadata) + archive.writestr( + f"{dist}/WHEEL", + f"Wheel-Version: 1.0\nRoot-Is-Purelib: true\nBuild: {build}\nTag: py3-none-any\n", + ) + return path + + +def test_uv_install_enforces_transitive_counters_in_reused_environment( + tmp_context: context.WorkContext, +) -> None: + """Exercise successive installs against the same environment and uv cache. + + Keep this timeline together: fresh environments would miss stale installed + builds, failed-install recovery, and cache reuse across counter changes. + """ + # Reproduce the review: a shared index contains builds 0, 1 and 2. + # uv chooses 2, even though Fromager's desired build is zero. + _configure(tmp_context, 0) + for number in ("0", "1", "2"): + _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", number) + _installation_wheel( + tmp_context.wheels_downloads, "consumer", "1.0", "0", "test-pkg==1.0.1" + ) + server.update_wheel_mirror(tmp_context) + http_server, sock, thread = server.run_wheel_server(tmp_context) + source = tmp_context.work_dir / "consumer-src" + source.mkdir() + try: + environment = build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement("consumer"), + sdist_root_dir=source, + ) + environment.run( + [ + "uv", + "pip", + "install", + "--only-binary", + ":all:", + *tmp_context.pip_wheel_server_args, + "consumer==1.0", + ], + network_isolation=False, + ) + inspect_build = [ + str(environment.python), + "-c", + "import test_pkg; print(test_pkg.BUILD)", + ] + assert environment.run(inspect_build).strip() == "2" + # No permitted wheel must fail, even with a newer build already installed + # and cached by uv. It must not fall back to the broad HTTP index. + zero = tmp_context.wheels_downloads / "test_pkg-1.0.1-0-py3-none-any.whl" + zero.unlink() + with pytest.raises(subprocess.CalledProcessError): + environment.install([Requirement("consumer==1.0")]) + _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", "0") + for counter in (0, 1, 7): + _configure(tmp_context, counter) + if counter == 7: + _installation_wheel( + tmp_context.wheels_downloads, "test_pkg", "1.0.1", "7" + ) + environment.install([Requirement("consumer==1.0")]) + assert environment.run(inspect_build).strip() == str(counter) + installed = environment.run( + [ + str(environment.python), + "-c", + "from importlib.metadata import distribution; print(distribution('test-pkg').read_text('WHEEL'))", + ] + ) + assert f"Build: {counter}\n" in installed + assert len(list(tmp_context.wheels_downloads.glob("test_pkg-*.whl"))) == 4 + assert len(list(tmp_context.work_dir.glob("install-wheels-*"))) == 1 + # A transitive direct file reference bypasses find-links. Validate the + # installed WHEEL metadata so it cannot be used for compilation either. + _configure(tmp_context, 0) + foreign = tmp_context.wheels_downloads / "test_pkg-1.0.1-2-py3-none-any.whl" + _installation_wheel( + tmp_context.wheels_downloads, + "url_consumer", + "1.0", + "0", + f"test-pkg @ {foreign.as_uri()}", + ) + with pytest.raises(ValueError, match="installed wheel build '2'"): + environment.install( + [ + Requirement("url-consumer==1.0"), + Requirement(f"test-pkg @ {foreign.as_uri()}"), + ] + ) + finally: + http_server.should_exit = True + thread.join(timeout=5) + sock.close() + + +def test_installation_wheelhouse_checks_suffix_and_preserves_legacy_packages( + tmp_context: context.WorkContext, +) -> None: + _configure(tmp_context, 0) + + def suffix(**kwargs: object) -> list[str]: + return ["expected"] + + tmp_context.settings._settings = tmp_context.settings._settings.model_copy( + update={"wheels": packagesettings.WheelSettings(build_tag_hook=suffix)} + ) + for build in ("0", "0_expected", "0_other", "2_expected"): + _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", build) + _installation_wheel(tmp_context.wheels_downloads, "legacy", "1.0", "2") + with patch.object(build_environment.BuildEnvironment, "_createenv"): + environment = build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement("consumer"), + sdist_root_dir=tmp_context.work_dir, + ) + with environment._installation_wheel_args( + tmp_context.settings.has_explicit_build_numbers + ) as args: + assert "--no-index" in args and "--offline" in args and "--reinstall" in args + directory = pathlib.Path(args[args.index("--find-links") + 1]) + assert {path.name for path in directory.glob("*.whl")} == { + "test_pkg-1.0.1-0_expected-py3-none-any.whl", + "legacy-1.0-2-py3-none-any.whl", + } + assert directory.is_dir() # Reused by subsequent installs on this worker. + + +def test_legacy_dependency_installation_keeps_server_arguments( + tmp_context: context.WorkContext, +) -> None: + tmp_context.wheel_server_url = "https://legacy.test/simple/" + with patch.object(build_environment.BuildEnvironment, "_createenv"): + environment = build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement("consumer"), + sdist_root_dir=tmp_context.work_dir, + ) + with environment._installation_wheel_args( + tmp_context.settings.has_explicit_build_numbers + ) as args: + assert args == tmp_context.pip_wheel_server_args + + +def test_installation_wheel_view_reuses_links_and_refreshes_selection( + tmp_context: context.WorkContext, +) -> None: + _configure(tmp_context, 0) + zero = _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", "0") + _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", "1") + legacy = _installation_wheel(tmp_context.wheels_downloads, "legacy", "1.0", "2") + with patch.object(build_environment.BuildEnvironment, "_createenv"): + environments = [ + build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement(name), + sdist_root_dir=tmp_context.work_dir, + ) + for name in ("consumer", "another-consumer") + ] + with ( + patch.object( + pathlib.Path, + "symlink_to", + autospec=True, + side_effect=pathlib.Path.symlink_to, + ) as link, + patch.object( + build_environment, + "parse_wheel_filename", + wraps=build_environment.parse_wheel_filename, + ) as parse, + ): + with environments[0]._installation_wheel_args(True) as first: + directory = pathlib.Path(first[first.index("--find-links") + 1]) + assert {p.name for p in directory.iterdir()} == {zero.name, legacy.name} + with environments[1]._installation_wheel_args(True) as second: + assert first == second + assert link.call_count == 2 + assert parse.call_count == 3 + # Settings changes must invalidate selection even without new filenames. + _configure(tmp_context, 1) + with environments[0]._installation_wheel_args(True): + assert not (directory / zero.name).exists() + assert (directory / "test_pkg-1.0.1-1-py3-none-any.whl").exists() + assert link.call_count == 3 + assert parse.call_count == 3 + legacy.unlink() + added = _installation_wheel(tmp_context.wheels_prebuilt, "added", "1.0", "0") + with environments[1]._installation_wheel_args(True): + assert not (directory / legacy.name).is_symlink() + assert (directory / added.name).is_file() + assert link.call_count == 4 + assert parse.call_count == 4 + + +def test_installation_wheel_views_are_isolated_between_workers( + tmp_context: context.WorkContext, +) -> None: + _configure(tmp_context, 0) + wheel = _installation_wheel(tmp_context.wheels_downloads, "test_pkg", "1.0.1", "0") + barrier = threading.Barrier(2, timeout=10) + with patch.object(build_environment.BuildEnvironment, "_createenv"): + environment = build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement("consumer"), + sdist_root_dir=tmp_context.work_dir, + ) + + def inspect_view() -> pathlib.Path: + with environment._installation_wheel_args(True) as args: + directory = pathlib.Path(args[args.index("--find-links") + 1]) + barrier.wait() + assert (directory / wheel.name).is_file() + return directory + + with ThreadPoolExecutor(max_workers=2) as executor: + futures = [executor.submit(inspect_view) for _ in range(2)] + directories = [future.result() for future in futures] + assert directories[0] != directories[1] + # Thread-local view owners are released when their workers exit. + assert all(not directory.exists() for directory in directories) + + +def test_unused_counter_logs_global_installation_policy( + tmp_context: context.WorkContext, + caplog: pytest.LogCaptureFixture, +) -> None: + _configure(tmp_context, 0) + _installation_wheel(tmp_context.wheels_downloads, "unrelated", "1.0", "0") + tmp_context.wheel_server_url = "https://remote.test/simple/" + with patch.object(build_environment.BuildEnvironment, "_createenv"): + environment = build_environment.BuildEnvironment( + ctx=tmp_context, + req=Requirement("unrelated"), + sdist_root_dir=tmp_context.work_dir, + ) + with caplog.at_level(logging.INFO): + with environment._installation_wheel_args( + tmp_context.settings.has_explicit_build_numbers + ) as args: + assert "--offline" in args + assert "--index-url" not in args + assert "local-only" in caplog.text + assert "including unrelated versions" in caplog.text + assert "--wheel-server-url is not used" in caplog.text diff --git a/tests/test_packagesettings.py b/tests/test_packagesettings.py index 2c3f8f3fc..c3a1c8ee7 100644 --- a/tests/test_packagesettings.py +++ b/tests/test_packagesettings.py @@ -36,6 +36,7 @@ TEST_COOLDOWN_PKG = "test-cooldown-pkg" FULL_EXPECTED: dict[str, typing.Any] = { + "build_number": {}, "annotations": { "fromager.test.value": "somevalue", "fromager.test.override": "variant override", @@ -118,6 +119,7 @@ } EMPTY_EXPECTED: dict[str, typing.Any] = { + "build_number": {}, "name": "test-empty-pkg", "annotations": None, "build_dir": None, @@ -157,6 +159,7 @@ } PREBUILT_PKG_EXPECTED: dict[str, typing.Any] = { + "build_number": {}, "name": "test-prebuilt-pkg", "annotations": None, "build_dir": None,