Skip to content

feat: source resolver configuration #1052

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.

Sign up for GitHub

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

Open
tiran wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: source resolver configuration #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 41 additions & 0 deletions docs/reference/config-reference.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Configuration Reference
=======================

.. currentmodule:: fromager.packagesettings

Per-package Settings
--------------------

Expand All @@ -24,6 +26,45 @@ For example `flash_attn.yaml`.

.. autopydantic_model:: fromager.packagesettings.SbomSettings

Source Resolver
^^^^^^^^^^^^^^^

.. autopydantic_model:: PyPISDistResolver
:inherited-members: AbstractPyPIResolver, CooldownMixin

.. autopydantic_model:: PyPIPrebuiltResolver
:inherited-members: AbstractPyPIResolver, CooldownMixin

.. autopydantic_model:: PyPIDownloadResolver
:inherited-members: AbstractPyPIResolver, CooldownMixin

.. autopydantic_model:: PyPIGitResolver
:inherited-members: AbstractPyPIResolver, CooldownMixin

.. autopydantic_model:: VersionMapResolver

.. autopydantic_model:: GitHubTagDownloadResolver
:inherited-members: AbstractGitSourceResolver, CooldownMixin

.. autopydantic_model:: GitHubTagCloneResolver
:inherited-members: AbstractGitSourceResolver, CooldownMixin

.. autopydantic_model:: GitLabTagDownloadResolver
:inherited-members: AbstractGitSourceResolver, CooldownMixin

.. autopydantic_model:: GitLabTagCloneResolver
:inherited-members: AbstractGitSourceResolver, CooldownMixin

.. autopydantic_model:: NotAvailableResolver

.. autopydantic_model:: HookResolver

.. autoclass:: BuildSDist

.. autoattribute:: pep517
.. autoattribute:: tarball


Global Settings
---------------

Expand Down
28 changes: 28 additions & 0 deletions src/fromager/packagesettings/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,21 @@
VariantInfo,
)
from ._pbi import PackageBuildInfo
from ._resolver import (
DEFAULT_TAG_MATCHER,
BuildSDist,
GitHubTagCloneResolver,
GitHubTagDownloadResolver,
GitLabTagCloneResolver,
GitLabTagDownloadResolver,
HookResolver,
NotAvailableResolver,
PyPIDownloadResolver,
PyPIGitResolver,
PyPIPrebuiltResolver,
PyPISDistResolver,
VersionMapResolver,
)
from ._settings import Settings, SettingsFile
from ._templates import substitute_template
from ._typedefs import (
Expand All @@ -32,22 +47,34 @@
)

__all__ = (
"DEFAULT_TAG_MATCHER",
"MODEL_CONFIG",
"Annotations",
"BuildDirectory",
"BuildOptions",
"BuildSDist",
"DownloadSource",
"EnvKey",
"EnvVars",
"GitHubTagCloneResolver",
"GitHubTagDownloadResolver",
"GitLabTagCloneResolver",
"GitLabTagDownloadResolver",
"GitOptions",
"GlobalChangelog",
"HookResolver",
"NotAvailableResolver",
"Package",
"PackageBuildInfo",
"PackageSettings",
"PackageVersion",
"PatchMap",
"ProjectOverride",
"PurlConfig",
"PyPIDownloadResolver",
"PyPIGitResolver",
"PyPIPrebuiltResolver",
"PyPISDistResolver",
"RawAnnotations",
"ResolverDist",
"SbomSettings",
Expand All @@ -57,6 +84,7 @@
"Variant",
"VariantChangelog",
"VariantInfo",
"VersionMapResolver",
"default_update_extra_environ",
"get_extra_environ",
"substitute_template",
Expand Down
15 changes: 14 additions & 1 deletion src/fromager/packagesettings/_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from pydantic import Field
from pydantic_core import core_schema

from ._resolver import SourceResolver
from ._typedefs import (
MODEL_CONFIG,
BuildDirectory,
Expand Down Expand Up @@ -313,7 +314,7 @@ class VariantInfo(pydantic.BaseModel):
VAR1: "value 1"
VAR2: "2.0
wheel_server_url: https://pypi.org/simple/
pre_build: False
pre_built: False
"""

model_config = MODEL_CONFIG
Expand All @@ -334,6 +335,12 @@ class VariantInfo(pydantic.BaseModel):
pre_built: bool = False
"""Use pre-built wheel from index server?"""

source: typing.Annotated[
SourceResolver | None,
pydantic.Field(default=None, discriminator="provider"),
]
"""Source resolver and downloader"""
Comment on lines +338 to +342
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make source mutually exclusive with the legacy source settings.

These models now allow source alongside download_source, resolver_dist, wheel_server_url, and pre_built. Since src/fromager/packagesettings/_pbi.py Line 181 reads the new resolver while the older accessors still exist, mixed configs can silently disagree about how a package should be resolved or downloaded.

Proposed fix 
 class VariantInfo(pydantic.BaseModel):
@@
     source: typing.Annotated[
         SourceResolver | None,
         pydantic.Field(default=None, discriminator="provider"),
     ]
     """Source resolver and downloader"""
+
+    `@pydantic.model_validator`(mode="after")
+    def validate_source_conflicts(self) -> typing.Self:
+        if self.source is not None and (
+            self.wheel_server_url is not None or self.pre_built
+        ):
+            raise ValueError(
+                "'source' cannot be combined with 'wheel_server_url' or 'pre_built'"
+            )
+        return self
@@
 class PackageSettings(pydantic.BaseModel):
@@
     source: typing.Annotated[
         SourceResolver | None,
         pydantic.Field(default=None, discriminator="provider"),
     ]
     """Source resolver and downloader"""
+
+    `@pydantic.model_validator`(mode="after")
+    def validate_source_conflicts(self) -> typing.Self:
+        if self.source is not None:
+            if (
+                self.download_source.url is not None
+                or self.download_source.destination_filename is not None
+                or self.resolver_dist != ResolverDist()
+            ):
+                raise ValueError(
+                    "'source' cannot be combined with legacy source settings"
+                )
+        return self

Also applies to: 468-472

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fromager/packagesettings/_models.py` around lines 338 - 342, The new
"source" field must be made mutually exclusive with the legacy fields
(download_source, resolver_dist, wheel_server_url, pre_built): add a model-level
validator (e.g., a pydantic root_validator or `@model_validator`) on the class
that declares the "source" field to check if source is not None and any of
download_source/resolver_dist/wheel_server_url/pre_built are also set, and if so
raise a ValueError with a clear message; implement the same validator logic for
the other occurrence of the "source" field in this file so both model variants
reject mixed configs.



class GitOptions(pydantic.BaseModel):
"""Git repository cloning options
Expand Down Expand Up @@ -458,6 +465,12 @@ class PackageSettings(pydantic.BaseModel):
project_override: ProjectOverride = Field(default_factory=ProjectOverride)
"""Patch project settings"""

source: typing.Annotated[
SourceResolver | None,
pydantic.Field(default=None, discriminator="provider"),
]
"""Source resolver and downloader"""

variants: Mapping[Variant, VariantInfo] = Field(default_factory=dict)
"""Variant configuration"""

Expand Down
9 changes: 9 additions & 0 deletions src/fromager/packagesettings/_pbi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
PurlConfig,
VariantInfo,
)
from ._resolver import SourceResolver
from ._templates import _resolve_template, substitute_template
from ._typedefs import Annotations, PackageVersion, PatchMap, Template, Variant

Expand Down Expand Up @@ -176,6 +177,14 @@ def pre_built(self) -> bool:
return vi.pre_built
return False

@property
def source_resolver(self) -> SourceResolver | None:
"""Get source resolver settings (variant or global)"""
vi = self._ps.variants.get(self.variant)
if vi is not None and vi.source is not None:
return vi.source
return self._ps.source

@property
def wheel_server_url(self) -> str | None:
"""Alternative package index for pre-build wheel"""
Expand Down
Loading
Loading