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
3 changes: 3 additions & 0 deletions docs/concepts/package-settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ layers override earlier ones:
│ 4. Variant overrides (within package YAML) │
│ (env vars, pre_built, wheel_server_url) │
│ │
│ 4a. Version-specific variant overrides │
│ (per-version pre_built, wheel_server_url) │
Comment thread
coderabbitai[bot] marked this conversation as resolved.
│ │
│ 5. Version-specific patches and changelog │
│ (patches/<pkg>-<version>/, changelog entries)│
│ │
Expand Down
26 changes: 23 additions & 3 deletions src/fromager/bootstrap_requirement_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from packaging.requirements import Requirement
from packaging.utils import NormalizedName, canonicalize_name
from packaging.version import Version
from packaging.version import InvalidVersion, Version

from . import finders, resolver, sources, wheels
from .dependency_graph import DependencyGraph
Expand All @@ -24,6 +24,21 @@
logger = logging.getLogger(__name__)


def _extract_pinned_version(req: Requirement) -> Version | None:
Comment thread
andre-motta marked this conversation as resolved.
"""Return the version if *req* is pinned to exactly one (``==``).

Returns ``None`` for range specifiers, wildcard pins (``==1.*``),
extras-only, or empty specifiers.
"""
specs = list(req.specifier)
if len(specs) == 1 and specs[0].operator == "==" and "*" not in specs[0].version:
try:
return Version(specs[0].version)
except InvalidVersion:
return None
return None


class BootstrapRequirementResolver:
"""Resolve package requirements from PyPI or dependency graph during bootstrap.

Expand Down Expand Up @@ -113,7 +128,8 @@ def resolve(
# Determine pre_built if not specified (needed for cache key)
if pre_built is None:
pbi = self.ctx.package_build_info(req)
pre_built = pbi.pre_built
pinned = _extract_pinned_version(req)
pre_built = pbi.is_pre_built(pinned)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
andre-motta marked this conversation as resolved.

rule_key = (str(req), pre_built)

Expand Down Expand Up @@ -181,8 +197,12 @@ def _resolve_and_extend(
results = cached_resolution
elif pre_built:
# Resolve prebuilt wheel
pinned = _extract_pinned_version(req)
wheel_server_urls = wheels.get_wheel_server_urls(
self.ctx, req, cache_wheel_server_url=resolver.PYPI_SERVER_URL
self.ctx,
req,
cache_wheel_server_url=resolver.PYPI_SERVER_URL,
version=pinned,
)
results = wheels.resolve_all_prebuilt_wheels(
ctx=self.ctx,
Expand Down
4 changes: 2 additions & 2 deletions src/fromager/bootstrapper/_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def _resolve_and_add_top_level(
req=req,
req_version=version,
download_url=source_url,
pre_built=pbi.pre_built,
pre_built=pbi.is_pre_built(version),
constraint=self.ctx.constraints.get_constraint(req.name),
)

Expand Down Expand Up @@ -600,7 +600,7 @@ def add_to_graph(
req=req,
req_version=req_version,
download_url=download_url,
pre_built=pbi.pre_built,
pre_built=pbi.is_pre_built(req_version),
constraint=self.ctx.constraints.get_constraint(req.name),
)
self._write_graph_async()
Expand Down
2 changes: 1 addition & 1 deletion src/fromager/bootstrapper/_process_install_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def run(self, bt: Bootstrapper) -> list[Phase]:
version=wi.resolved_version,
source_url=wi.source_url,
source_type=wi.build_result.source_type,
prebuilt=pbi.pre_built,
prebuilt=pbi.is_pre_built(wi.resolved_version),
constraint=constraint,
)

Expand Down
98 changes: 90 additions & 8 deletions src/fromager/bootstrapper/_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,67 @@
import logging
import typing

from packaging.requirements import Requirement
from packaging.version import Version

from .. import resolver, sources, wheels
from ..requirements_file import RequirementType
from ._phase import Phase
from ._prepare_source import PrepareSource
from ._types import BootstrapPhase

if typing.TYPE_CHECKING:
from .. import context
from ._bootstrapper import Bootstrapper

logger = logging.getLogger(__name__)


def _re_resolve_url(

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.

This function is the most important part of PR and has no test coverage.

Can we add tests for

(1) re-resolution when version-specific pre_built=True overrides variant default pre_built=False (prebuilt path),
(2) re-resolution when version-specific pre_built=False overrides variant default pr e_built=True (source path),
(3) re-resolution when only wheel_server_url differs,
(4) failure handling when re-resolution returns None (falls back to original URL with warning),
(5) no re-resolution when version-specific settings match variant defaults.

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.

Added 7 tests in TestReResolveUrl covering: prebuilt success, source success, ExceptionGroup failure, empty source results, wheel_server_url mismatch triggering re-resolution, no re-resolution when settings match, and fallback on failure.

ctx: context.WorkContext,
req: Requirement,
req_type: RequirementType,
resolved_version: Version,
pre_built: bool,
cache_wheel_server_url: str | None,
) -> str | None:
"""Re-resolve the download URL when version-specific pre_built differs.

Returns the new URL or ``None`` if re-resolution fails.
"""
pinned_req = Requirement(f"{req.name}=={resolved_version}")
if pre_built:

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.

This can raise an unhandled ExceptionGroup from wheels.resolve_prebuilt_wheel (which propagates from resolve_all_prebuilt_wheels at wheels.py:548) if no matching wheel is found on the version
-specific server. The source branch correctly returns None when no results are found (line 46-47), but the prebuilt branch has no exception handling. The caller at line 142-147 expects either a URL or None and logs a warning on None — an uncaught exception would instead crash the bootstrap run.

Can we wrap the prebuilt resolution in a try/except block and return None on failure, matching the source branch's behavior?

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.

Wrapped resolve_prebuilt_wheel in a try/except ExceptionGroup block that returns None, matching the source branch's behavior.

The caller already handles None by falling back to the original URL with a warning.

wheel_server_urls = wheels.get_wheel_server_urls(
ctx,
req,
cache_wheel_server_url=cache_wheel_server_url,
version=resolved_version,
)
try:
url, _ = wheels.resolve_prebuilt_wheel(
ctx=ctx,
req=pinned_req,
wheel_server_urls=wheel_server_urls,
req_type=req_type,
)
except ExceptionGroup:
return None
return str(url)
else:
pbi = ctx.package_build_info(req)
sdist_server = pbi.resolver_sdist_server_url(resolver.PYPI_SERVER_URL)
provider = sources.get_source_provider(
ctx=ctx,
req=pinned_req,
sdist_server_url=sdist_server,
req_type=req_type,
)
results = resolver.find_all_matching_from_provider(provider, pinned_req)
if results:
return str(results[0][0])
return None


class Start(Phase):
"""Record a resolved requirement in the dependency graph and deduplicate.

Expand Down Expand Up @@ -44,7 +94,46 @@ def run(self, bt: Bootstrapper) -> list[Phase]:
assert wi.resolved_version is not None
assert wi.source_url is not None

# Add to graph (skip TOP_LEVEL, already added in _resolve_and_add_top_level)
wi.build_sdist_only = bt.sdist_only and not wi.is_build_requirement_context()

# Must set pbi_pre_built before constructing PrepareSource so that
# PrepareSource.background_work() immediately sees the correct value.
pbi = bt.ctx.package_build_info(wi.req)
wi.pbi_pre_built = pbi.is_pre_built(wi.resolved_version)
wi.exclusive_build = pbi.exclusive_build

# Re-resolve URL before graph insertion so the graph stores the
# final URL, and before the seen-check so duplicate parents still
# get their edge recorded with the correct URL.
version_url = pbi.get_wheel_server_url(wi.resolved_version)
variant_url = pbi.wheel_server_url
needs_re_resolve = wi.pbi_pre_built != pbi.pre_built or (
wi.pbi_pre_built and version_url != variant_url
)
if needs_re_resolve:
logger.info(
f"{wi.req} {wi.resolved_version}: version-specific override "
f"(pre_built={wi.pbi_pre_built}, url={version_url}) differs "
f"from variant default, re-resolving URL"
)
new_url = _re_resolve_url(
bt.ctx,
wi.req,
wi.req_type,
wi.resolved_version,
wi.pbi_pre_built,
bt.cache_wheel_server_url,
)
if new_url is not None:
wi.source_url = new_url
else:
logger.warning(
f"{wi.req} {wi.resolved_version}: could not re-resolve URL "
f"for pre_built={wi.pbi_pre_built}, using original"
)

# Add to graph after URL finalization but before the seen-check
# so every parent-to-dep edge is recorded with the correct URL.
if wi.req_type != RequirementType.TOP_LEVEL:
bt.add_to_graph(
wi.req,
Expand All @@ -54,8 +143,6 @@ def run(self, bt: Bootstrapper) -> list[Phase]:
wi.parent,
)

wi.build_sdist_only = bt.sdist_only and not wi.is_build_requirement_context()

if bt.has_been_seen(wi.req, wi.resolved_version, wi.build_sdist_only):
logger.debug(
f"redundant {wi.req_type} dependency {wi.req} "
Expand All @@ -69,9 +156,4 @@ def run(self, bt: Bootstrapper) -> list[Phase]:
f"new {wi.req_type} dependency {wi.req} resolves to {wi.resolved_version}"
)

# Must set pbi_pre_built before constructing PrepareSource so that
# PrepareSource.background_work() immediately sees the correct value.
pbi = bt.ctx.package_build_info(wi.req)
wi.pbi_pre_built = pbi.pre_built
wi.exclusive_build = pbi.exclusive_build
return [PrepareSource(wi)]
7 changes: 5 additions & 2 deletions src/fromager/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,13 @@ def _build(

logger.info("starting processing")
pbi = wkctx.package_build_info(req)
prebuilt = pbi.pre_built
prebuilt = pbi.is_pre_built(resolved_version)

wheel_server_urls = wheels.get_wheel_server_urls(
wkctx, req, cache_wheel_server_url=cache_wheel_server_url
wkctx,
req,
cache_wheel_server_url=cache_wheel_server_url,
version=resolved_version,
)

# See if we can reuse an existing wheel.
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,
VersionSpecificSettings,
)
from ._pbi import PackageBuildInfo
from ._resolver import (
Expand Down Expand Up @@ -88,6 +89,7 @@
"Variant",
"VariantChangelog",
"VariantInfo",
"VersionSpecificSettings",
"default_update_extra_environ",
"get_extra_environ",
"pep440_tag_matcher",
Expand Down
54 changes: 54 additions & 0 deletions src/fromager/packagesettings/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
BuildDirectory,
EnvVars,
Package,
PackageVersion,
PurlType,
RawAnnotations,
Template,
Expand Down Expand Up @@ -450,6 +451,32 @@ def validate_update_build_requires(cls, v: list[str]) -> list[str]:
return v


class VersionSpecificSettings(pydantic.BaseModel):
"""Per-version overrides within a variant.

Allows overriding ``pre_built`` and ``wheel_server_url`` for
specific package versions. When a field is ``None``, the
variant-wide default is used.

.. versionadded:: 0.95.0

::

versions:
"2.9.0":
pre_built: true
wheel_server_url: https://gitlab.example.com/simple
"""

model_config = MODEL_CONFIG

wheel_server_url: str | None = None
"""Alternative package index for this version's pre-built wheel"""

pre_built: bool | None = None
"""Override pre-built flag for this version (None = inherit variant default)"""
Comment thread
andre-motta marked this conversation as resolved.


class VariantInfo(pydantic.BaseModel):
"""Variant information for a package

Expand All @@ -460,6 +487,10 @@ class VariantInfo(pydantic.BaseModel):
VAR2: "2.0
wheel_server_url: https://pypi.org/simple/
pre_built: False
versions:
"2.9.0":
pre_built: true
wheel_server_url: https://gitlab.example.com/simple
"""

model_config = MODEL_CONFIG
Expand All @@ -480,10 +511,33 @@ class VariantInfo(pydantic.BaseModel):
pre_built: bool = False
"""Use pre-built wheel from index server?"""

versions: Mapping[PackageVersion, VersionSpecificSettings] = Field(
default_factory=dict
)
"""Per-version overrides for ``pre_built`` and ``wheel_server_url``.

Version-specific settings take precedence over variant defaults
when present.

.. versionadded:: 0.95.0
"""

# TODO
# source: SourceResolver | None
# """Source resolver and downloader"""

@pydantic.field_validator("versions", mode="before")
@classmethod
def before_none_versions(
cls,
v: dict[str, typing.Any] | None,
info: core_schema.ValidationInfo,
) -> dict[str, typing.Any]:
"""Coerce ``None`` to empty dict for bare ``versions:`` YAML key."""
if v is None:
return {}
return v
Comment thread
coderabbitai[bot] marked this conversation as resolved.


class GitOptions(pydantic.BaseModel):
"""Git repository cloning options
Expand Down
Loading
Loading