-
Notifications
You must be signed in to change notification settings - Fork 50
feat(wheels): add configurable build tag hook for wheel filenames #1217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
| ) | ||
|
Comment on lines
+992
to
+997
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Don’t downgrade hook validation failures to a cache miss.
Also applies to: 1027-1031 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
except ResolverException:
logger.info(...)
return None, None
except requests.exceptions.RequestException as err:
logger.warning(...)
return None, None
except Exception as err: # ← this one
logger.warning(
f"unexpected error checking wheel cache for {resolved_version} "
f"at {self.cache_wheel_server_url}: {err}"
)
return None, NoneA You're right that 🧠 Learnings used |
||
| 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 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, "") | ||
|
Comment on lines
+498
to
+504
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Let
Also applies to: 532-541 🤖 Prompt for AI Agents |
||
| if ( | ||
| existing_build_tag[0] > build_tag[0] | ||
| and existing_build_tag[1] == build_tag[1] | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The finder API is an old, badly designed part of Fromager. It tries to be clever, but gets it wrong in some cases. We may need to reconsider and redesign this part of Fromager, first. :/ |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fromager 0.88.0 is out, the new feature will land in 0.89.0 or later.
Suggested change
|
||||||
| """ | ||||||
|
|
||||||
| model_config = MODEL_CONFIG | ||||||
|
|
||||||
| build_tag_hook: pydantic.ImportString[typing.Callable[..., typing.Any]] | None = ( | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's define a more detailed callable: BuildTagHookCallable = typing.Callable[
["context.WorkContext", Requirement, Version, frozenset[Tag]],
typing.Sequence[str],
]the define the hook as: |
||||||
| 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 | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
+97
to
+102
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to modify the string. >>> import pydantic
>>> class WheelSettings(pydantic.BaseModel):
... build_tag_hook: pydantic.ImportString
...
>>> WheelSettings(build_tag_hook="fromager.context:WorkContext")
WheelSettings(build_tag_hook=<class 'fromager.context.WorkContext'>) |
||||||
|
|
||||||
|
|
||||||
| class PurlConfig(pydantic.BaseModel): | ||||||
| """Per-package purl configuration for SBOM generation. | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under which circumstances is |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> 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.]" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Materialize and type-check the hook result before validating it. Right now a hook that returns Suggested fix- segments = hook(ctx=ctx, req=req, version=version, wheel_tags=wheel_tags)
- _validate_build_tag_segments(segments)
+ raw_segments = hook(ctx=ctx, req=req, version=version, wheel_tags=wheel_tags)
+ if isinstance(raw_segments, (str, bytes)):
+ raise ValueError(
+ "build_tag_hook must return a sequence of segment strings"
+ )
+ try:
+ segments = list(raw_segments)
+ except TypeError as err:
+ raise ValueError(
+ "build_tag_hook must return a sequence of segment strings"
+ ) from err
+ if any(not isinstance(seg, str) for seg in segments):
+ raise ValueError("build_tag_hook must return only strings")
+ _validate_build_tag_segments(segments)A regression test for a single-string return and a generator return would lock this down. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Comment on lines
+151
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Add the coexistence case for plain and suffixed wheels. These tests only cover plain and suffixed filenames in isolation. The regression shows up when both are present in the same directory and the caller rejects the first candidate after revalidation. Please add a case with both 🤖 Prompt for AI AgentsSource: Path instructions |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Normalize empty build tags before comparing.
These checks only reject mismatches when
expected_build_tagis truthy, so()will silently accept a cached wheel whose filename carries a real build tag. That is inconsistent withsrc/fromager/commands/build.py, which normalizes falsy tags to(0, "")and compares exactly. As written, the bootstrapper can reuse a wheel that the build verification path would reject.Suggested fix
Also applies to: 992-1000
🤖 Prompt for AI Agents