Skip to content

feat(resolver): add the declarative source: configuration - #1414

Merged
LalatenduMohanty merged 70 commits into
mainfrom
new-resolver-config
Nov 4, 2026
Merged

LalatenduMohanty merged 70 commits into
mainfrom
new-resolver-config

Conversation

@LalatenduMohanty

@LalatenduMohanty LalatenduMohanty commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Merges the new-resolver-config feature branch into main. It implements the new resolver config proposal. Epic: #1254.

What it does

A package can now use one source: block instead of plugins and scattered settings. The block picks:

  • how fromager finds versions (PyPI index, GitHub or GitLab tags)
  • what it downloads (sdist, prebuilt wheel, tarball or git checkout)
  • how it makes the sdist (pep517 or tarball)

Providers: pypi-sdist, pypi-prebuilt, pypi-download, pypi-git, github-tag-git, github-tag-download, gitlab-tag-git, gitlab-tag-download, hook-sdist, hook-prebuilt, not-available.

Packages without source: work as before.

Breaking change

sources.download_source() now returns DownloadedSource (path and kind) instead of a Path, for all packages. Code that calls it must use .path.

Downstream plugins are not affected: plugins that implement the download_source hook still return a Path. Only one downstream script calls sources.download_source(), and the fix is to add .path to the call.

Pending, not blocking

These only affect packages that use source:.

Decide before 0.103.0 ships:

Issue Work
#1440 Configured prebuilt wheels end up in wheels-repo/prebuilt instead of downloads in build commands

Follow-ups:

Issue Work
#1416 Reject a non-hook source: when the package has resolver or download hooks
#1432 Prebuilt replay can leave the mirror serving an older wheel with the same name
#1388 Deprecation warnings for old settings
#1197 versionmap-git provider (open PR, needs rebase)
#1419, #1421, #1422, #1423 Smaller behavior fixes
#1420, #1424, #1425 Tests and docs
#1434 Shared Git checkout path helper

build-parallel does not check the recorded source_provider yet; the approach is still open.

Also tracked: #1395 and #1396 (prebuilt follow-ups) and #1257 (deferred).

Fixed since the last review: #1418 (#1433), #1430 (#1438) and #1431 (#1437).

🤖 Generated with Claude Code

LalatenduMohanty and others added 30 commits August 14, 2026 09:51
…er` PBI property

Uncomment the `source: SourceResolver | None` field on both
`PackageSettings` and `VariantInfo`, add `SourceResolver` to the
public API, and wire up a `PackageBuildInfo.source_resolver` property
with variant-then-package fallback.

This is the foundation for all downstream resolver-config wiring
(Phases 2-9 of the new resolver config epic).

Closes: #1291
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…lidator_nrc

feat(packagesettings): activate source field and add source_resolver PBI property
…th set

Prevent confusing configs where the new `source` field silently
overrides legacy fields. Package-level `source` now conflicts with
`download_source` and `resolver_dist`; variant-level `source`
conflicts with `wheel_server_url` and `pre_built`.

Closes: #1207
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…lidator_nrc

feat(packagesettings): error when source and legacy settings are both set
Updating new-resolver-config branch
Route packages with source resolver configuration through the configured
provider while preserving legacy override behavior. Widen
`AbstractResolver.resolver_provider` to accept `RequirementType | None`
and remove the `typing.cast` that hid the mismatch. Add coverage for
source and bootstrap resolution paths and document the dispatch.

Refs: #1292
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…lvers

Source resolvers bypassed `resolve_package_cooldown()`, so the global
`--min-release-age` flag was silently ignored and top-level `==` pins
did not disable the cooldown.

Extract `_effective_cooldown()` from `resolve_package_cooldown()` and
call it in the source-resolver dispatch path so both legacy and
configured resolvers share the same cooldown merge logic.

Refs: #1292
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…ver-dispatch

feat(resolver): dispatch configured source resolvers
Route hook-backed source resolver profiles through `get_resolver_provider`, forwarding resolver context and server URL while preserving legacy hook signatures. Validate returned providers and cover cooldown and error behavior.

Refs: #1324
Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
fix(resolver): delegate hook source providers
New `source:` profiles can download more than sdists. Dispatch
configured downloads through their resolver and return
`(Path, DownloadKind)` so later phases can handle each artifact.
Keep the legacy hook path for packages without `source:`.

Closes: #1293

Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
Bring the feature branch up to date with main. Preserve the source resolver and wheel settings tests, and keep the bootstrap tests free of network requests.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
chore(sync): merge main into new-resolver-config
Replace `tuple[Path, DownloadKind]` with a frozen `DownloadedSource`
dataclass so the download contract is stable before Phase 4 consumes
the artifact kind to select prepare/build strategies.

Implement `HookSDistResolver.download()` and
`HookPrebuiltResolver.download()` through the `download_source` override
hook. For legacy hooks returning a bare `Path`, `HookSDistResolver`
treats `.tar.gz` and `.zip` files as `sdist`, while `HookPrebuiltResolver`
treats `.whl` files as `prebuilt_wheel`. Bare directories and other path
forms require `DownloadedSource` with an explicit supported kind.
Reject `any_source`, `not_available`, and kinds incompatible with the
selected profile.

Closes: #1331
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…ad-result

Feat/1331 typed download result
Reject conflicting source and legacy settings during package settings loading, including explicit pre_built: false. Cover both configuration directions with focused tests and document the rule.

Co-Authored-By: Codex <codex@openai.com>
Closes: #1368
Signed-off-by: Shanmukh Pawan <smoparth@redhat.com>
…idation

fix(packagesettings): reject cross-level (package level and variant level) settings conflicts for source
Honor configured PEP 517 and tarball strategies, warning when an installed build hook is ignored. Require resolvers to declare their strategy and hook-sdist to choose it explicitly. Keep build overrides for legacy settings.

Co-Authored-By: Codex <codex@openai.com>

Closes: #1206

Related: #1256
Signed-off-by: Justin Larkin <jlarkin@redhat.com>
fix(sources): honor resolver sdist build strategy
Carry DownloadedSource.kind through bootstrap and build entry points. Unpack ordinary archives; prepare clean Git checkouts in place so patches, project overrides, and Rust vendoring run.

Keep prepare_source override arguments and path-only callers compatible. Document the public Git helper and no-cleanup behavior, and reject dirty nested submodules before preparation.

Reject archive input paths that cleanup would remove before extraction, preserving the input and reporting an actionable error.

Closes: #1205
Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: Shanmukh Pawan <smoparth@redhat.com>
…heckout

fix(sources): prepare Git checkouts by download kind
Resolve and download fresh prebuilt wheels through the selected source
resolver. Preserve local wheel cache reuse, legacy prebuilt downloads,
prebuilt hooks, and the test-mode fallback.

Add focused coverage for both configured profiles, build-order replay,
and legacy behavior.

Closes #1208
Co-Authored-By: Codex <noreply@openai.com>

Signed-off-by: Shanmukh Pawan <smoparth@redhat.com>
…lvers

fix(resolver): honor configured prebuilt wheel resolvers
`downloads/` and `prebuilt/` can hold different wheels with the same
filename. On a local mirror hit, bootstrap reused the same-named file
from the other folder and then switched the mirror to serve it.

Add `server.find_mirror_wheel()` to follow the mirror link to the real
file. `download_prebuilt_wheel()` returns that file for configured
packages, `bg_prepare_prebuilt()` switches the mirror only after a fresh
download, and `_is_wheel_built()` uses the same lookup.

Co-Authored-By: Claude <claude@anthropic.com>
Closes: #1397
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…d-wheel

fix(wheels): reuse the exact wheel the local mirror serves
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…ig-from-main-20261027T183147Z

chore(sync): merge main into new-resolver-config
Hook profiles now have an `index_url` setting. `hook-prebuilt` must set
it. `hook-sdist` uses PyPI if it is not set. Fromager passes this URL to
the `get_resolver_provider` hook, so existing hooks keep working.

Profiles in `source:` now always use their own index. If a command
passes a different URL, Fromager ignores it and logs a warning.

Co-Authored-By: Claude <claude@anthropic.com>
Closes: #1373
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
@LalatenduMohanty

Copy link
Copy Markdown
Member Author

@EmilienM Thanks. The find_source_dir() finding is confirmed. The same can happen on main with legacy archives, but #1441 made it possible for Git checkouts too. Fix in #1449: it tries names in a fixed order and only returns a nested directory that exists.

LalatenduMohanty and others added 5 commits November 3, 2026 21:05
Read the first origin URL from the checkout config without URL rewrites or credential logging. Cover global configuration, rewrites, and multiple origins; isolate tests from user Git settings.

Find existing source roots in a consistent order and infer the sole supported source kind for preparation. Keep the sdist fallback for legacy and ambiguous settings, excluding prebuilt wheels from inference. Update the tests and usage documentation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
Use `_display_url()` to remove user information and fragments while preserving query parameter names and masking every value. Apply it to configured and legacy index, wheel server, and download URLs.

Cover duplicate and blank query parameters, credential-bearing output, and the original URLs passed to resolution.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
#1393 dropped the "resolver returned no results" warning when it
restructured source resolution. Restore it and add a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
Describe clean Git checkout reuse and provider validation for every build-order entry. Use the exported get_resolver_provider hook name consistently.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
LalatenduMohanty and others added 2 commits November 4, 2026 09:14
Add a requirement-to-build overview and separate provider selection, source preparation, Git checkout reuse, and build-order replay into focused sections. Remove repeated explanations, clarify session-cache behavior, and tighten the wording throughout.

Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
…ture-readability

docs(resolver): improve architecture guide readability

@dhellmann dhellmann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The implementation of this change is pretty complex. I think a lot of that is caused by not setting defaults "early" in the process of loading settings for a package, and so every point in the code that needs to figure out what to do for any aspect of processing a package has to manage all of the edge cases locally, instead of centralizing that logic. That also means it is harder to use patterns like Strategy to isolate each edge case, so we end up with a lot of if/then/elif statements or match statements to call variations of functions or instantiate classes.

If instead we took an approach that said the source: section of the config must be populated before we start processing a package, then we could inject the default settings into the config object. For packages that have no config file, we could inject them into a fake config object used at runtime. All of the rest of the logic throughout the other layers would then be simpler, because it could just instantiate the right kind of handler(s) and invoke them with a consistent API.

It is possible that this feature branch could be evolved in this direction, I don't know. I'll leave it up to the team to decide that.

This change also needs more user-facing documentation, with some how-to instructions with example configuration for different cases handled by the different types of providers and resolvers.

Comment thread docs/concepts/resolver-architecture.rst Resolved
Comment thread docs/using.md Resolved
Comment thread src/fromager/commands/build_order.py Resolved
@LalatenduMohanty

Copy link
Copy Markdown
Member Author

Thanks Doug — I think your diagnosis is right, and the review history of this PR is the best evidence for it. find-updates (#1408), list-versions / list-overrides (#1413) and package resolve (#1407) each needed a separate fix because every command re-implements the "is there a source: config?" branch, and each new entry point could miss it. There are about 15 of those branches in the final code.

The root cause is that we're deliberately supporting two systems at once. We can't drop the legacy settings yet because existing plugins and downstream configs depend on them, and the safety contract of this PR is "packages without source: work exactly as before." That's why hook-sdist and hook-prebuilt exist as bridge profiles — they let a plugin keep its hooks while moving its config to source:, one package at a time.

The good news is that most of those ~15 branches exist only to serve the legacy path. Once downstream plugins and settings have migrated to source:, we can delete the legacy branches and what's left is close to the model you're describing: every package has a populated source:, and the dispatch becomes "instantiate the handler for the profile" instead of per-site fallback checks. At that point the Strategy-style refactor is much smaller than it would be today.

So the plan I'd propose:

I'll answer the inline threads (step commands, provider match on replay) separately.

@LalatenduMohanty

Copy link
Copy Markdown
Member Author

I have discussed with maintainers and we all agree to merge this and continue the development in main branch.

ryanpetrello
ryanpetrello previously approved these changes Nov 4, 2026
@mergify

mergify Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

mprpic and others added 2 commits November 4, 2026 16:45
`_parse_spdx()` skipped validation only when the whole expression
started with `LicenseRef-`. A valid PEP 639 expression such as
`MIT OR LicenseRef-Proprietary` was rejected, while
`LicenseRef-Foo OR Apache 2` and malformed ids like `LicenseRef-a_b`
were accepted.

It now parses without validation and rejects every unknown key that is
not a well-formed `LicenseRef-` id, raising the same `ExpressionError`
as before. This is needed to populate SBOM license fields (#1378).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Martin Prpič <mprpic@redhat.com>
Validate with `packaging.licenses.canonicalize_license_expression()`
so `_parse_spdx()` follows PEP 639 exactly, and use license-expression
only to build the expression object. `Apache-2.0 WITH LicenseRef-x` is
now rejected and `licenseref-foo` accepted, matching `packaging`. The
exception type stays `ExpressionError`; its message now comes from
`packaging`.

The invalid-expression cases move to their own test that checks which
key was rejected, and new cases cover an exception next to a
`LicenseRef` and an unknown `WITH` exception.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Martin Prpič <mprpic@redhat.com>

This branch was previously deployed

1 inactive deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.