diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index c2de0c6d..5f60a9bf 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -926,26 +926,34 @@ def _look_for_existing_wheel( search_in: pathlib.Path, ) -> tuple[pathlib.Path | None, pathlib.Path | None]: pbi = self.ctx.package_build_info(req) - expected_build_tag = pbi.build_tag(resolved_version) + base_build_tag = pbi.build_tag(resolved_version) logger.info( - f"looking for existing wheel for version {resolved_version} with build tag {expected_build_tag} in {search_in}" + f"looking for existing wheel for version {resolved_version} with build tag {base_build_tag} in {search_in}" ) wheel_filename = finders.find_wheel( downloads_dir=search_in, req=req, dist_version=str(resolved_version), - build_tag=expected_build_tag, + build_tag=base_build_tag, ) if not wheel_filename: return None, None - # Re-validate build tag from the actual wheel metadata because - # finders.find_wheel matches by filename prefix, which may not - # enforce an exact build tag match. - _, _, build_tag, _ = wheels.extract_info_from_wheel_file(req, wheel_filename) - if expected_build_tag and expected_build_tag != build_tag: + # Re-validate build tag from the actual wheel metadata. + # Compute the full expected tag (including any hook suffix) + # now that we have the wheel's tags. + _, _, actual_build_tag, wheel_tags = wheels.extract_info_from_wheel_file( + req, wheel_filename + ) + expected_build_tag = wheels.get_build_tag( + ctx=self.ctx, + req=req, + version=resolved_version, + wheel_tags=wheel_tags, + ) + if expected_build_tag and expected_build_tag != actual_build_tag: logger.info( - f"found wheel for {resolved_version} in {wheel_filename} but build tag does not match. Got {build_tag} but expected {expected_build_tag}" + f"found wheel for {resolved_version} in {wheel_filename} but build tag does not match. Got {actual_build_tag} but expected {expected_build_tag}" ) return None, None @@ -973,19 +981,23 @@ def _download_wheel_from_cache( wheel_url, _ = results[0] wheelfile_name = pathlib.Path(urlparse(wheel_url).path) pbi = self.ctx.package_build_info(req) - expected_build_tag = pbi.build_tag(resolved_version) - # Log the expected build tag for debugging - logger.info(f"has expected build tag {expected_build_tag}") - # Get changelogs for debug info + base_build_tag = pbi.build_tag(resolved_version) + logger.info(f"has expected base build tag {base_build_tag}") changelogs = pbi.get_changelog(resolved_version) logger.debug(f"has change logs {changelogs}") - _, _, build_tag, _ = wheels.extract_info_from_wheel_file( + _, _, actual_build_tag, wheel_tags = wheels.extract_info_from_wheel_file( req, wheelfile_name ) - if expected_build_tag and expected_build_tag != build_tag: + expected_build_tag = wheels.get_build_tag( + ctx=self.ctx, + req=req, + version=resolved_version, + wheel_tags=wheel_tags, + ) + if expected_build_tag and expected_build_tag != actual_build_tag: logger.info( - f"found wheel for {resolved_version} in cache but build tag does not match. Got {build_tag} but expected {expected_build_tag}" + f"found wheel for {resolved_version} in cache but build tag does not match. Got {actual_build_tag} but expected {expected_build_tag}" ) return None, None diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 204f01ac..2bc368f8 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -492,12 +492,16 @@ def _is_wheel_built( wheel_server_urls=wheel_server_urls, ) logger.info("found candidate wheel %s", url) - pbi = wkctx.package_build_info(req) - build_tag_from_settings = pbi.build_tag(resolved_version) - build_tag = build_tag_from_settings if build_tag_from_settings else (0, "") wheel_basename = resolver.extract_filename_from_url(url) - _, _, build_tag_from_name, _ = parse_wheel_filename(wheel_basename) + _, _, build_tag_from_name, wheel_tags = parse_wheel_filename(wheel_basename) existing_build_tag = build_tag_from_name if build_tag_from_name else (0, "") + expected_tag = wheels.get_build_tag( + ctx=wkctx, + req=req, + version=resolved_version, + wheel_tags=wheel_tags, + ) + build_tag = expected_tag if expected_tag else (0, "") if ( existing_build_tag[0] > build_tag[0] and existing_build_tag[1] == build_tag[1] diff --git a/src/fromager/finders.py b/src/fromager/finders.py index 69c31e33..d15c2841 100644 --- a/src/fromager/finders.py +++ b/src/fromager/finders.py @@ -159,33 +159,36 @@ def find_wheel( and dotted), each suffixed with the build tag when present. Uses case-insensitive ``startswith`` matching rather than exact base comparison because wheel filenames include platform/Python tags after the version. + + When the build tag has an empty suffix, also matches wheels whose + build tag contains a hook-provided suffix (e.g. ``2_el9.6``). + Callers should validate the full build tag after finding a match. """ filename_prefix = _dist_name_to_filename(req.name) canonical_name = canonicalize_name(req.name) - # if build tag is 0 then we can ignore to handle non tagged wheels for backward compatibility - candidate_bases_build_tag = f"{build_tag[0]}{build_tag[1]}-" if build_tag else "" - candidate_bases = set( - [ - # First check if the file is there using the canonically - # transformed name. - f"{filename_prefix}-{dist_version}-{candidate_bases_build_tag}", - # If that didn't work, try the canonical dist name. That's not - # "correct" but we do see it. (charset-normalizer-3.3.2- - # and setuptools-scm-8.0.4-) for example - f"{canonical_name}-{dist_version}-{candidate_bases_build_tag}", - # If *that* didn't work, try the dist name we've been - # given as a dependency. That's not "correct", either but we do - # see it. (oslo.messaging-14.7.0-) for example - f"{req.name}-{dist_version}-{candidate_bases_build_tag}", - # Sometimes the sdist uses '.' instead of '-' in the - # package name portion. - f"{req.name.replace('-', '.')}-{dist_version}-{candidate_bases_build_tag}", - ] - ) - # Case-insensitive globbing was added to Python 3.12, but we - # have to run with older versions, too, so do our own name - # comparison. + if build_tag: + suffix = build_tag[1] + if suffix: + # Exact match including suffix: "2_el9.6-" + build_tag_prefixes = [f"{build_tag[0]}{suffix}-"] + else: + # No suffix: match "2-" (plain) or "2_" (hook-suffixed) + build_tag_prefixes = [f"{build_tag[0]}-", f"{build_tag[0]}_"] + else: + build_tag_prefixes = [""] + + candidate_bases = set() + for bt_prefix in build_tag_prefixes: + candidate_bases.update( + [ + f"{filename_prefix}-{dist_version}-{bt_prefix}", + f"{canonical_name}-{dist_version}-{bt_prefix}", + f"{req.name}-{dist_version}-{bt_prefix}", + f"{req.name.replace('-', '.')}-{dist_version}-{bt_prefix}", + ] + ) + for base in candidate_bases: logger.debug('looking for wheel as "%s"', base) for filename in downloads_dir.glob("*.whl"): diff --git a/src/fromager/packagesettings/__init__.py b/src/fromager/packagesettings/__init__.py index 2e023925..b3066728 100644 --- a/src/fromager/packagesettings/__init__.py +++ b/src/fromager/packagesettings/__init__.py @@ -11,6 +11,7 @@ ResolverDist, SbomSettings, VariantInfo, + WheelSettings, ) from ._pbi import PackageBuildInfo from ._resolver import ( @@ -82,6 +83,7 @@ "Variant", "VariantChangelog", "VariantInfo", + "WheelSettings", "default_update_extra_environ", "get_extra_environ", "pep440_tag_matcher", diff --git a/src/fromager/packagesettings/_models.py b/src/fromager/packagesettings/_models.py index 4e982e18..ad210489 100644 --- a/src/fromager/packagesettings/_models.py +++ b/src/fromager/packagesettings/_models.py @@ -72,6 +72,36 @@ class SbomSettings(pydantic.BaseModel): """ +class WheelSettings(pydantic.BaseModel): + """Global wheel settings + + :: + + wheels: + build_tag_hook: "mypackage.hooks:build_tag_hook" + + .. versionadded:: 0.82.0 + """ + + model_config = MODEL_CONFIG + + build_tag_hook: pydantic.ImportString[typing.Callable[..., typing.Any]] | None = ( + None + ) + """Dotted import path to a callable that returns build tag suffix segments. + + The callable receives keyword-only arguments ``ctx``, ``req``, + ``version``, and ``wheel_tags``, and returns ``Sequence[str]``. + """ + + @pydantic.field_validator("build_tag_hook", mode="before") + @classmethod + def _colon_to_dot(cls, v: typing.Any) -> typing.Any: + if isinstance(v, str) and ":" in v: + return v.replace(":", ".", 1) + return v + + class PurlConfig(pydantic.BaseModel): """Per-package purl configuration for SBOM generation. diff --git a/src/fromager/packagesettings/_settings.py b/src/fromager/packagesettings/_settings.py index 1d76c948..ee2de320 100644 --- a/src/fromager/packagesettings/_settings.py +++ b/src/fromager/packagesettings/_settings.py @@ -13,7 +13,7 @@ from pydantic import Field from .. import overrides -from ._models import PackageSettings, SbomSettings +from ._models import PackageSettings, SbomSettings, WheelSettings from ._pbi import PackageBuildInfo from ._typedefs import MODEL_CONFIG, GlobalChangelog, Package, Variant @@ -45,6 +45,12 @@ class SettingsFile(pydantic.BaseModel): are generated. """ + wheels: WheelSettings | None = None + """Wheel build settings + + .. versionadded:: 0.82.0 + """ + @classmethod def from_string( cls, @@ -175,6 +181,13 @@ def sbom_settings(self) -> SbomSettings | None: """Get global SBOM settings, or None if SBOM generation is disabled.""" return self._settings.sbom + @property + def build_tag_hook(self) -> typing.Callable[..., typing.Any] | None: + """Get the configured build tag hook callable, or None.""" + if self._settings.wheels is not None: + return self._settings.wheels.build_tag_hook + return None + def variant_changelog(self) -> list[str]: """Get global changelog for current variant""" return list(self._settings.changelog.get(self.variant, [])) diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py index 0998a9de..484ffd93 100644 --- a/src/fromager/wheels.py +++ b/src/fromager/wheels.py @@ -4,6 +4,7 @@ import logging import os import pathlib +import re import shutil import sys import tempfile @@ -39,12 +40,60 @@ logger = logging.getLogger(__name__) +_BUILD_TAG_SEGMENT_RE = re.compile(r"^[a-zA-Z0-9.]+$") + FROMAGER_BUILD_SETTINGS = "fromager-build-settings" FROMAGER_ELF_PROVIDES = "fromager-elf-provides.txt" FROMAGER_ELF_REQUIRES = "fromager-elf-requires.txt" FROMAGER_BUILD_REQ_PREFIX = "fromager" +def get_build_tag( + *, + ctx: context.WorkContext, + req: Requirement, + version: Version, + wheel_tags: frozenset[Tag] | None = None, +) -> BuildTag: + """Compute the full build tag, including any hook-provided suffix. + + When *wheel_tags* is provided and a ``build_tag_hook`` is configured + in global settings, the hook is called to obtain suffix segments + that are joined with ``_`` and appended to the base build tag. + + When *wheel_tags* is ``None`` or no hook is configured, returns the + base build tag from changelog settings (identical to + ``pbi.build_tag(version)``). + """ + pbi = ctx.package_build_info(req) + base_tag = pbi.build_tag(version) + if not base_tag: + return base_tag + + hook = ctx.settings.build_tag_hook + if hook is None or wheel_tags is None: + return base_tag + + segments = hook(ctx=ctx, req=req, version=version, wheel_tags=wheel_tags) + _validate_build_tag_segments(segments) + + if not segments: + return base_tag + + suffix = base_tag[1] + "_" + "_".join(segments) + return (base_tag[0], suffix) + + +def _validate_build_tag_segments(segments: typing.Sequence[str]) -> None: + """Validate that each segment matches ``[a-zA-Z0-9.]``.""" + for seg in segments: + if not _BUILD_TAG_SEGMENT_RE.match(seg): + raise ValueError( + f"build tag hook returned invalid segment {seg!r}: " + "each segment must match [a-zA-Z0-9.]" + ) + + def _log_existing_sboms( req: Requirement, dist_info_dir: pathlib.Path, @@ -274,7 +323,12 @@ def add_extra_metadata_to_wheels( ) sbom.write_sbom(sbom=sbom_doc, dist_info_dir=dist_info_dir) - build_tag_from_settings = pbi.build_tag(version) + build_tag_from_settings = get_build_tag( + ctx=ctx, + req=req, + version=version, + wheel_tags=wheel_tags, + ) build_tag = build_tag_from_settings if build_tag_from_settings else (0, "") cmd = [ diff --git a/tests/test_finders.py b/tests/test_finders.py index 110ccfa1..a0d05e7f 100644 --- a/tests/test_finders.py +++ b/tests/test_finders.py @@ -146,3 +146,50 @@ def test_pypi_cache_provider() -> None: finders.PyPICacheProvider( cache_server_url=url, include_sdists=False, include_wheels=False ) + + +def test_find_wheel_with_build_tag(tmp_path: pathlib.Path) -> None: + downloads = tmp_path / "downloads" + downloads.mkdir() + wheel = downloads / "mypkg-1.0.0-2-py3-none-any.whl" + wheel.write_text("not-empty") + + req = Requirement("mypkg") + actual = finders.find_wheel(downloads, req, "1.0.0", (2, "")) + assert actual == wheel + + +def test_find_wheel_with_build_tag_suffix(tmp_path: pathlib.Path) -> None: + """find_wheel with an empty suffix should also match suffixed filenames.""" + downloads = tmp_path / "downloads" + downloads.mkdir() + wheel = downloads / "mypkg-1.0.0-2_el9.6-cp312-cp312-linux_x86_64.whl" + wheel.write_text("not-empty") + + req = Requirement("mypkg") + # Search with base tag (no suffix) — should still find the suffixed wheel + actual = finders.find_wheel(downloads, req, "1.0.0", (2, "")) + assert actual == wheel + + +def test_find_wheel_with_exact_suffix(tmp_path: pathlib.Path) -> None: + """find_wheel with an exact suffix matches the right wheel.""" + downloads = tmp_path / "downloads" + downloads.mkdir() + wheel = downloads / "mypkg-1.0.0-2_el9.6_cuda13.0-cp312-cp312-linux_x86_64.whl" + wheel.write_text("not-empty") + + req = Requirement("mypkg") + actual = finders.find_wheel(downloads, req, "1.0.0", (2, "_el9.6_cuda13.0")) + assert actual == wheel + + +def test_find_wheel_suffix_no_false_positive(tmp_path: pathlib.Path) -> None: + """Build tag 2 should not match build tag 20.""" + downloads = tmp_path / "downloads" + downloads.mkdir() + (downloads / "mypkg-1.0.0-20-py3-none-any.whl").write_text("not-empty") + + req = Requirement("mypkg") + actual = finders.find_wheel(downloads, req, "1.0.0", (2, "")) + assert actual is None diff --git a/tests/test_packagesettings.py b/tests/test_packagesettings.py index f65c1d67..160050ae 100644 --- a/tests/test_packagesettings.py +++ b/tests/test_packagesettings.py @@ -932,3 +932,61 @@ def test_version_none_no_reference( result = pbi.get_extra_environ(template_env={}, version=None) assert result["FOO"] == "bar" assert "__version__" not in result + + +def test_wheel_settings_default() -> None: + gs = SettingsFile.from_string("") + assert gs.wheels is None + + +def test_wheel_settings_empty() -> None: + gs = SettingsFile.from_string("wheels: {}") + assert gs.wheels is not None + assert gs.wheels.build_tag_hook is None + + +def test_wheel_settings_build_tag_hook_dot_syntax() -> None: + gs = SettingsFile.from_string("wheels:\n build_tag_hook: os.path.join") + assert gs.wheels is not None + assert gs.wheels.build_tag_hook is not None + import os.path + + assert gs.wheels.build_tag_hook is os.path.join + + +def test_wheel_settings_build_tag_hook_colon_syntax() -> None: + gs = SettingsFile.from_string("wheels:\n build_tag_hook: 'os.path:join'") + assert gs.wheels is not None + import os.path + + assert gs.wheels.build_tag_hook is os.path.join + + +def test_wheel_settings_build_tag_hook_invalid_import() -> None: + with pytest.raises(RuntimeError, match="failed to load global settings"): + SettingsFile.from_string("wheels:\n build_tag_hook: 'nonexistent.module.func'") + + +def test_settings_build_tag_hook_property_none() -> None: + settings = Settings( + settings=SettingsFile(), + package_settings=[], + variant="cpu", + patches_dir=pathlib.Path("/tmp"), + max_jobs=1, + ) + assert settings.build_tag_hook is None + + +def test_settings_build_tag_hook_property_with_hook() -> None: + sf = SettingsFile.from_string("wheels:\n build_tag_hook: os.path.join") + settings = Settings( + settings=sf, + package_settings=[], + variant="cpu", + patches_dir=pathlib.Path("/tmp"), + max_jobs=1, + ) + import os.path + + assert settings.build_tag_hook is os.path.join diff --git a/tests/test_wheels.py b/tests/test_wheels.py index 2b5f7f33..a4051737 100644 --- a/tests/test_wheels.py +++ b/tests/test_wheels.py @@ -1,4 +1,5 @@ import pathlib +import typing import zipfile from unittest.mock import Mock, patch @@ -6,10 +7,17 @@ import wheel.wheelfile # type: ignore from conftest import make_sbom_ctx from packaging.requirements import Requirement +from packaging.tags import Tag from packaging.version import Version from fromager import build_environment, context, sources, wheels -from fromager.packagesettings import SbomSettings +from fromager.packagesettings import ( + SbomSettings, + Settings, + SettingsFile, + Variant, + WheelSettings, +) @patch("fromager.sources.download_url") @@ -353,3 +361,169 @@ def test_validate_wheel_file( else: with pytest.raises(ValueError): wheels.validate_wheel_filename(req, version, wheel_file) + + +# -- get_build_tag and build tag hook tests -- + + +def _make_ctx_with_hook( + tmp_path: pathlib.Path, + hook: typing.Callable[..., typing.Any] | None = None, +) -> context.WorkContext: + """Create a WorkContext with an optional build_tag_hook.""" + if hook is not None: + sf = SettingsFile( + wheels=WheelSettings(build_tag_hook=hook), + changelog={Variant("cpu"): ["variant change"]}, + ) + else: + sf = SettingsFile(changelog={Variant("cpu"): ["variant change"]}) + settings = Settings( + settings=sf, + package_settings=[], + variant="cpu", + patches_dir=tmp_path / "patches", + max_jobs=1, + ) + ctx = context.WorkContext( + active_settings=settings, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + ) + ctx.setup() + return ctx + + +_PLATLIB_TAGS: frozenset[Tag] = frozenset({Tag("cp312", "cp312", "linux_x86_64")}) +_PURELIB_TAGS: frozenset[Tag] = frozenset({Tag("py3", "none", "any")}) + + +def test_get_build_tag_no_hook(tmp_path: pathlib.Path) -> None: + ctx = _make_ctx_with_hook(tmp_path, hook=None) + req = Requirement("some-pkg==1.0.0") + tag = wheels.get_build_tag( + ctx=ctx, req=req, version=Version("1.0.0"), wheel_tags=_PLATLIB_TAGS + ) + pbi = ctx.package_build_info(req) + assert tag == pbi.build_tag(Version("1.0.0")) + + +def test_get_build_tag_no_wheel_tags(tmp_path: pathlib.Path) -> None: + def hook(**kwargs: typing.Any) -> list[str]: + return ["el9.6"] + + ctx = _make_ctx_with_hook(tmp_path, hook=hook) + req = Requirement("some-pkg==1.0.0") + tag = wheels.get_build_tag( + ctx=ctx, req=req, version=Version("1.0.0"), wheel_tags=None + ) + pbi = ctx.package_build_info(req) + assert tag == pbi.build_tag(Version("1.0.0")) + + +def test_get_build_tag_with_hook_platlib(tmp_path: pathlib.Path) -> None: + def hook(**kwargs: typing.Any) -> list[str]: + wt = kwargs["wheel_tags"] + if any(t.platform != "any" for t in wt): + return ["el9.6", "cuda13.0"] + return [] + + ctx = _make_ctx_with_hook(tmp_path, hook=hook) + req = Requirement("some-pkg==1.0.0") + tag = wheels.get_build_tag( + ctx=ctx, req=req, version=Version("1.0.0"), wheel_tags=_PLATLIB_TAGS + ) + assert tag == (1, "_el9.6_cuda13.0") + + +def test_get_build_tag_with_hook_purelib(tmp_path: pathlib.Path) -> None: + def hook(**kwargs: typing.Any) -> list[str]: + wt = kwargs["wheel_tags"] + if any(t.platform != "any" for t in wt): + return ["el9.6"] + return [] + + ctx = _make_ctx_with_hook(tmp_path, hook=hook) + req = Requirement("some-pkg==1.0.0") + tag = wheels.get_build_tag( + ctx=ctx, req=req, version=Version("1.0.0"), wheel_tags=_PURELIB_TAGS + ) + # Hook returns [] for purelib → base tag unchanged + assert tag == (1, "") + + +def test_get_build_tag_no_changelog(tmp_path: pathlib.Path) -> None: + def hook(**kwargs: typing.Any) -> list[str]: + return ["el9.6"] + + sf = SettingsFile(wheels=WheelSettings(build_tag_hook=hook)) + settings = Settings( + settings=sf, + package_settings=[], + variant="cpu", + patches_dir=tmp_path / "patches", + max_jobs=1, + ) + ctx = context.WorkContext( + active_settings=settings, + patches_dir=tmp_path / "patches", + sdists_repo=tmp_path / "sdists-repo", + wheels_repo=tmp_path / "wheels-repo", + work_dir=tmp_path / "work-dir", + ) + ctx.setup() + req = Requirement("some-pkg==1.0.0") + # No changelog → empty build tag, hook is not called + tag = wheels.get_build_tag( + ctx=ctx, req=req, version=Version("1.0.0"), wheel_tags=_PLATLIB_TAGS + ) + assert tag == () + + +def test_validate_build_tag_segments_valid() -> None: + wheels._validate_build_tag_segments(["el9.6", "cuda13.0", "torch2.10.0"]) + wheels._validate_build_tag_segments([]) + wheels._validate_build_tag_segments(["fc43"]) + + +def test_validate_build_tag_segments_invalid() -> None: + with pytest.raises(ValueError, match="invalid segment"): + wheels._validate_build_tag_segments(["el-9.6"]) + + with pytest.raises(ValueError, match="invalid segment"): + wheels._validate_build_tag_segments(["has space"]) + + with pytest.raises(ValueError, match="invalid segment"): + wheels._validate_build_tag_segments(["has_underscore"]) + + +def test_get_build_tag_hook_raises(tmp_path: pathlib.Path) -> None: + def bad_hook(**kwargs: typing.Any) -> list[str]: + raise RuntimeError("hook failed") + + ctx = _make_ctx_with_hook(tmp_path, hook=bad_hook) + req = Requirement("some-pkg==1.0.0") + with pytest.raises(RuntimeError, match="hook failed"): + wheels.get_build_tag( + ctx=ctx, + req=req, + version=Version("1.0.0"), + wheel_tags=_PLATLIB_TAGS, + ) + + +def test_get_build_tag_hook_invalid_segment(tmp_path: pathlib.Path) -> None: + def bad_hook(**kwargs: typing.Any) -> list[str]: + return ["valid", "in-valid"] + + ctx = _make_ctx_with_hook(tmp_path, hook=bad_hook) + req = Requirement("some-pkg==1.0.0") + with pytest.raises(ValueError, match="invalid segment"): + wheels.get_build_tag( + ctx=ctx, + req=req, + version=Version("1.0.0"), + wheel_tags=_PLATLIB_TAGS, + )