Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/fromager/bootstrapper/_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,22 +86,27 @@ def _look_for_existing_wheel(
search_in: pathlib.Path,
) -> tuple[pathlib.Path | None, pathlib.Path | None]:
pbi = 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
_, _, build_tag, _ = wheels.extract_info_from_wheel_file(req, wheel_filename)
if expected_build_tag and expected_build_tag != build_tag:
_, _, actual_build_tag, wheel_tags = wheels.extract_info_from_wheel_file(
req, wheel_filename
)
expected_build_tag = wheels.get_build_tag(
ctx=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
logger.info(f"found existing wheel {wheel_filename}")
Expand Down Expand Up @@ -129,16 +134,20 @@ def _download_wheel_from_cache(
results = resolver.find_all_matching_from_provider(provider, pinned_req)
wheel_url, _ = results[0]
wheelfile_name = pathlib.Path(urlparse(wheel_url).path)
_, _, actual_build_tag, wheel_tags = wheels.extract_info_from_wheel_file(
req, wheelfile_name
)
pbi = ctx.package_build_info(req)
expected_build_tag = pbi.build_tag(resolved_version)
expected_build_tag = wheels.get_build_tag(
ctx=ctx, req=req, version=resolved_version, wheel_tags=wheel_tags
)
logger.info(f"has expected build tag {expected_build_tag}")
changelogs = pbi.get_changelog(resolved_version)
logger.debug(f"has change logs {changelogs}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we are removing this log line?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. Will restore the changelog debug logging.


_, _, build_tag, _ = wheels.extract_info_from_wheel_file(req, wheelfile_name)
if expected_build_tag and expected_build_tag != build_tag:
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

Expand Down
33 changes: 18 additions & 15 deletions src/fromager/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,11 +485,25 @@ 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 = downloads.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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

url, wheel_basename, build_tag_from_name, and wheel_tags are assigned inside the try block and used after the except. The except block returns None, so the code is technically correct, but the flow is fragile. An else clause on the try would make the intent explicit and survive future refactoring more safely:

  try:
      ...
      _, _, build_tag_from_name, wheel_tags = parse_wheel_filename(wheel_basename)
  except Exception:
      ...
      return None
  else:
      # build tag comparison lives here
      expected_tag = wheels.get_build_tag(...)
      ...

except Exception:
logger.debug(
"could not locate prebuilt wheel %s-%s on %s",
dist_name,
resolved_version,
wheel_server_urls,
exc_info=True,
)
logger.info("could not locate prebuilt wheel")
return None
else:
# Compute expected build tag in the else clause so hook
# validation errors propagate instead of being swallowed.
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, "")
existing_build_tag = build_tag_from_name if build_tag_from_name else (0, "")
if (
existing_build_tag[0] > build_tag[0]
Expand All @@ -513,21 +527,10 @@ def _is_wheel_built(
wheel_filename = None

if not wheel_filename:
# if the found wheel was on an external server, then download it
logger.info("downloading wheel from %s", url)
wheel_filename = wheels.download_wheel(req, url, wkctx.wheels_downloads)

return wheel_filename
except Exception:
logger.debug(
"could not locate prebuilt wheel %s-%s on %s",
dist_name,
resolved_version,
wheel_server_urls,
exc_info=True,
)
logger.info("could not locate prebuilt wheel")
return None


def _build_parallel(
Expand Down
43 changes: 20 additions & 23 deletions src/fromager/finders.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,30 +162,27 @@ def find_wheel(
"""
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.
build_tag_prefixes: list[str] = []
if build_tag:
build_tag_prefixes.append(f"{build_tag[0]}{build_tag[1]}-")
if not build_tag[1]:
build_tag_prefixes.append(f"{build_tag[0]}_")
else:
build_tag_prefixes.append("")

name_variants = [
filename_prefix,
canonical_name,
req.name,
req.name.replace("-", "."),
]

candidate_bases: set[str] = set()
for name in name_variants:
for btp in build_tag_prefixes:
candidate_bases.add(f"{name}-{dist_version}-{btp}")

for base in candidate_bases:
logger.debug('looking for wheel as "%s"', base)
for filename in downloads_dir.glob("*.whl"):
Expand Down
2 changes: 2 additions & 0 deletions src/fromager/packagesettings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
ResolverDist,
SbomSettings,
VariantInfo,
WheelSettings,
)
from ._pbi import PackageBuildInfo
from ._resolver import (
Expand Down Expand Up @@ -88,6 +89,7 @@
"Variant",
"VariantChangelog",
"VariantInfo",
"WheelSettings",
"default_update_extra_environ",
"get_extra_environ",
"pep440_tag_matcher",
Expand Down
33 changes: 33 additions & 0 deletions src/fromager/packagesettings/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,39 @@
logger = logging.getLogger(__name__)


class WheelSettings(pydantic.BaseModel):
"""Global wheel build settings

::

wheels:
build_tag_hook: "mypackage.hooks:build_tag_hook"

.. versionadded:: 0.93.0
"""

model_config = MODEL_CONFIG

build_tag_hook: pydantic.ImportString[typing.Callable[..., typing.Any]] | None = (
None
)
"""Callable that returns suffix segments for the wheel build tag.

The callable receives keyword-only arguments ``ctx``, ``req``,
``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
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
tag.

.. versionadded:: 0.93.0
"""


class SbomSettings(pydantic.BaseModel):
"""Global SBOM generation settings

Expand Down
20 changes: 19 additions & 1 deletion src/fromager/packagesettings/_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pydantic import Field

from .. import overrides
from ._models import ExternalCommands, PackageSettings, SbomSettings
from ._models import ExternalCommands, PackageSettings, SbomSettings, WheelSettings
from ._pbi import PackageBuildInfo
from ._typedefs import MODEL_CONFIG, GlobalChangelog, Package, Variant

Expand Down Expand Up @@ -55,6 +55,14 @@ class SettingsFile(pydantic.BaseModel):
.. versionadded:: 0.92.0
"""

wheels: WheelSettings | None = None
"""Wheel build settings

Configures wheel build tag hooks and other wheel-specific options.

.. versionadded:: 0.93.0
"""

@classmethod
def from_string(
cls,
Expand Down Expand Up @@ -193,6 +201,16 @@ def external_commands(self) -> ExternalCommands:
"""
return self._settings.external_commands

@property
def build_tag_hook(self) -> typing.Callable[..., typing.Any] | None:
"""Get the wheel build tag hook callable, or None if not configured.

.. versionadded:: 0.93.0
"""
if self._settings.wheels is None:
return None
return self._settings.wheels.build_tag_hook

def variant_changelog(self) -> list[str]:
"""Get global changelog for current variant"""
return list(self._settings.changelog.get(self.variant, []))
Expand Down
63 changes: 61 additions & 2 deletions src/fromager/wheels.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
import os
import pathlib
import re
import shutil
import sys
import tempfile
Expand Down Expand Up @@ -38,12 +39,67 @@

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 _validate_build_tag_segments(segments: list[str]) -> None:
"""Validate that each segment matches ``[a-zA-Z0-9.]``."""
for seg in segments:
if not isinstance(seg, str):
raise ValueError(
f"build_tag_hook must return strings, got {type(seg).__name__}"
)
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 get_build_tag(
*,
ctx: context.WorkContext,
req: Requirement,
version: Version,
wheel_tags: frozenset[Tag],
) -> BuildTag:
"""Compute the full build tag including any hook-provided suffix.

Calls ``pbi.build_tag(version)`` for the numeric base, then invokes
the configured ``build_tag_hook`` (if any) to append environment
suffix segments.

.. versionadded:: 0.93.0
"""
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:
return base_tag

raw = hook(ctx=ctx, req=req, version=version, wheel_tags=wheel_tags)
if isinstance(raw, str | bytes):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test to verify bytes are also rejected? Currently, we only have test for str

raise ValueError(
"build_tag_hook must return a sequence of strings, not a single string"
)
segments = list(raw)
_validate_build_tag_segments(segments)

if not segments:
return base_tag

suffix = base_tag[1] + "_" + "_".join(segments)
return (base_tag[0], suffix)


def _log_existing_sboms(
req: Requirement,
dist_info_dir: pathlib.Path,
Expand Down Expand Up @@ -264,8 +320,11 @@ 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 = build_tag_from_settings if build_tag_from_settings else (0, "")
build_tag = get_build_tag(
ctx=ctx, req=req, version=version, wheel_tags=wheel_tags
)
if not build_tag:
build_tag = (0, "")

cmd = [
"wheel",
Expand Down
45 changes: 45 additions & 0 deletions tests/test_finders.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,48 @@ def test_pypi_cache_provider() -> None:
finders.PyPICacheProvider(
cache_server_url=url, include_sdists=False, include_wheels=False
)


class TestFindWheelBuildTagSuffix:
"""Tests for ``find_wheel`` with build tag suffixes."""

def test_find_wheel_with_exact_build_tag(self, tmp_path: pathlib.Path) -> None:
"""Plain build tag matches a plain-tagged wheel."""
downloads = tmp_path / "downloads"
downloads.mkdir()
wheel = downloads / "mypkg-1.0.0-2-py3-none-any.whl"
wheel.write_text("not-empty")
result = finders.find_wheel(downloads, Requirement("mypkg"), "1.0.0", (2, ""))
assert result == wheel

def test_find_wheel_matches_suffixed_with_base_tag(
self, tmp_path: pathlib.Path
) -> None:
"""Base tag (2, '') matches a suffixed wheel when no plain one exists."""
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")
result = finders.find_wheel(downloads, Requirement("mypkg"), "1.0.0", (2, ""))
assert result == wheel

def test_find_wheel_no_false_positive_on_higher_number(
self, tmp_path: pathlib.Path
) -> None:
"""Build tag 2 does 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")
result = finders.find_wheel(downloads, Requirement("mypkg"), "1.0.0", (2, ""))
assert result is None

def test_find_wheel_with_full_suffix(self, tmp_path: pathlib.Path) -> None:
"""Exact suffixed build tag 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")
result = finders.find_wheel(
downloads, Requirement("mypkg"), "1.0.0", (2, "_el9.6_cuda13.0")
)
assert result == wheel
Loading
Loading