Conversation
weihanglo
force-pushed
the
pubgrub
branch
2 times, most recently
from
June 19, 2026 15:32
a2a2827 to
09389d8
Compare
Track missing Cargo.toml state using Option<Option<Package>>, where outer None means unloaded and inner None means loaded but missing. This allows the resolver to produce standard diagnostics instead of raw IO error chains.
- Scan subdirectories when a path dependency is missing or invalid. - Show helpful hints suggesting packages that exist nearby.
…7236) ### What does this PR try to resolve? This is another optimization to reduce the amount of args we pass to rustc when the new build-dir layout is enabled. Discussed in rust-lang#17183 (comment), this reduce the total `-L` arg length by ~16% in the case of Zed. ### How to test and review this PR? * Tested this on Zed on windows and was able to compile it fine * Created a testsuite case for this.
report entries This captures the current behavior of the report.
…#17238) # What Does This PR Try to Resolve? Fixes rust-lang#17212 # How to Test and Review This PR? The first commit adds tests pinning the current behavior. The second commit fixes the underlying issues and updates the tests as needed.
This is to be consistent with the remote http registry (sparse registry)
…ang#17240) ### What does this PR try to resolve? Make the registry source's backend naming more consistent. Found this confusing when working in this area. I don't remember what I was doing here before the weekend. Maybe related to rust-lang#17226? ### How to test and review this PR?
The main benefit I see to `git fetch`s output is the progress reporting. The motivating reason to remove it unless progress is being reported is that this is making it annoying to update the test suite to `net.git-fetch-with-cli = true`.
This gives context for what the bool means
Fixes fresh resolution of large graphs (including Cargo itself), which previously failed or diverged because optional-dependency edges were handled incorrectly. Two coupled fixes: * Gate optional-dependency edges on activation again (a prior change had made them feature-agnostic, which drew unactivated optional edges such as schemars -> url and produced dependency cycles). * Activate the optional dependency for ANY `dep/feat` reference in an enabled feature, including weak `dep?/feat` ones. Cargo's v1 lock resolver always records the optional dependency in the graph for such references (the `weak` flag only gates the dependency's own implicit feature). Previously weak references pulled the target into the lock but dropped the edge to it (e.g. bstr -> serde via `serde?/std`). With both fixes, `-Zpubgrub-resolver generate-lockfile` run from scratch (no pre-existing lock) produces a byte-identical Cargo.lock to the default resolver for Cargo's own ~5944-line dependency tree. Adds graph-level (edge) comparison tests against the default resolver, including regressions for the cycle and weak-dependency cases that the package-set/SAT tests did not catch.
Captures architecture, the encoding, build/test instructions (Nix dev shell), verified status (fresh byte-identical lockfile of Cargo itself), the hard-won correctness insights (member seeding, activation-gated edges, weak-dependency edge semantics, edge-level testing gap), reused reference material, known limitations, and prioritized next steps for a takeover.
Add CARGO_TEST_PUBGRUB env switch to the resolver-tests harness so the
convenience helpers (resolve / resolve_and_validated) route through
-Zpubgrub-resolver. This lets the entire curated suite (tests/resolve.rs,
tests/pubgrub.rs) be re-run against the PubGrub resolver for differential
validation:
CARGO_TEST_PUBGRUB=1 cargo test -p resolver-tests
Default behavior is unchanged when the variable is unset.
Current results through pubgrub: tests/pubgrub.rs 28/28 pass;
tests/resolve.rs 35/37 pass, the 2 failures being error-message text
only (PubGrub's DefaultStringReporter vs Cargo-native messages), not
resolution-outcome differences.
Two curated tests assert Cargo's native conflict/no-match error text. The PubGrub resolver formats errors via its derivation-tree reporter, so these messages differ while the resolution outcome (error) is identical. Gate just the message assertions on CARGO_TEST_PUBGRUB so the full curated suite is a clean green gate on both resolvers (resolve.rs 37/37, pubgrub.rs 28/28).
Update the handoff doc: the curated resolver suites now run through pubgrub via CARGO_TEST_PUBGRUB (resolve.rs 37/37, pubgrub.rs 28/28), mark next-step #1 done, and document the command.
Emit a tracing::debug! at the top of the pubgrub resolve path so its activation can be confirmed via `CARGO_LOG=cargo::core::resolver::pubgrub=debug` without temporary instrumentation. Useful when verifying that -Zpubgrub-resolver actually dispatches (a byte-identical lockfile alone cannot distinguish a working resolver from a no-op flag).
Record the pristine-clone parity test (avoids this branch's Cargo.toml confound), the dispatch proof via the new CARGO_LOG trace, the single-resolve-pass note, and refresh the commit history.
…ve helper Extract resolve_with_prefs_raw from resolve_with_global_context_raw so a caller can supply its own VersionPreferences instead of always starting from default(). Add prefs_from_lock to build preferences that reproduce a previous resolution. This is the resolver-level slice of the conservative-update flows (building against an existing lock, cargo update -p, --precise), which all reach the resolver as VersionPreferences. Pure refactor: the existing helper now delegates with VersionPreferences::default(), so behavior is unchanged.
Eight deterministic offline tests covering the conservative-update paths: building against an untouched lock, cargo update -p <crate>, cargo update (free everything), adding a dependency, shared transitive pinning, stale-lock override, --precise, and a wildcard requirement. Each resolves a manifest fresh, derives VersionPreferences from that resolution, mutates one input, and asserts the pubgrub graph (nodes and edges) matches the default resolver run with the same preferences. The default resolver is the oracle, so the tests pin down that pubgrub honors preferences exactly like Cargo already does rather than re-encoding the logic under test.
Add prop_pubgrub_locked_reresolve_passes_validation: resolve fresh, feed the result back as VersionPreferences, then re-resolve both with everything kept (building against the lock) and with the requested crate freed (cargo update -p). Each re-resolution must produce a SAT-valid solution and agree with the default resolver on solvability. Preferences only reorder the candidates pubgrub considers; they must never let it accept an invalid solution nor fail when one exists. This is the randomized counterpart to the deterministic pubgrub_update.rs suite.
Route every resolution through the PubGrub resolver when CARGO_TEST_PUBGRUB is set, independent of the nightly-gated -Zpubgrub-resolver flag. This lets the full integration testsuite (which shells out to a real cargo binary, mostly on the stable channel) be re-run on PubGrub for differential validation; child cargo processes inherit the env var. Never set in production.
Update the handoff doc for the work in this batch: - sections 2/8/9.3: conservative-update paths are now verified at the resolver level (pubgrub_update.rs + pubgrub_prop.rs), with the remaining end-to-end gap (ops::resolve glue, --precise registry pinning) called out; - section 3: document the CARGO_TEST_PUBGRUB dispatch hook and how to run the full integration testsuite on PubGrub; - section 9.1: proptests pass 5/5 under the env var at 256 cases; - section 10: refresh the commit ledger; - section 12 (new): record the first full-testsuite survey -- 3872 passed, 4 failed, of which 3 are pre-existing env/snapshot failures and 1 is the known error-reporting limitation (typed ResolveError), not a misresolution.
The cargo::z_help::case snapshot predates the -Zpubgrub-resolver flag, so the added help line (and the y-coordinate shifts it caused in the SVG) failed the testsuite. Regenerated with SNAPSHOTS=overwrite.
The pubgrub resolver module and its resolver-tests were never run through `cargo fmt`; CI's `cargo fmt --all --check` flagged them. Pure formatting, no behavior change.
Cargo's clippy config disallows `std::env::var_os` (rust-lang#11588); route the env reads through `GlobalContext::get_env_os` at the resolver dispatch fork and in the resolver-tests helper. Two reads in tests/resolve.rs have no GlobalContext in scope, so they keep direct env access under a scoped `#[expect(clippy::disallowed_methods, reason = ...)]`, matching the idiom used elsewhere in cargo (e.g. context/environment.rs). Also rename the variable to `__CARGO_TEST_PUBGRUB` to match cargo's existing test-only env convention (`__CARGO_TEST_*`), updating the doc-comment and the design doc in lockstep so the documented and runtime names agree. `GlobalContext` snapshots the process environment at construction, so child cargo processes spawned by the testsuite still observe the variable.
`cargo doc` with `-D warnings` (CI's intra-doc-link gate) flagged two links: `Provider::query` (no such method; the blocking poll loop lives in `Provider::candidates`) and the ambiguous `super::resolve` (both a function and a module -- disambiguate to the function with `()`).
The pubgrub solution reconstruction built each node as `PackageId::new(name, version, bucket.source)`, using the source the dependency named. But `[patch]` redirects a query to a summary from a different source (e.g. a `crates-io` requirement satisfied by a path patch); `PackageRegistry::query` returns that patched summary while the bucket still names the registry source. The result was a lockfile entry pointing at the registry for a package the patch actually provided, which surfaced as "patch ... was not used in the crate graph" plus a checksum mismatch against the patched (sourceless) package. Use the selected summary's real `PackageId` as the node identity via the new `bucket_pid` helper, threading it through the activation/edge/checksum passes so the patched source is preserved. `resolve_child` now keys on the bucket `(name, source)` but returns the actually-selected `PackageId`. Reduces `patch::` testsuite failures under `__CARGO_TEST_PUBGRUB` from 43 to 25 (remaining failures are a separate spurious-index-update issue and error-message formatting, not resolution). The bug only manifests with the real `PackageRegistry` patch machinery, so it is covered by the existing `tests/testsuite/patch.rs` suite rather than the bare resolver-tests harness.
The pubgrub solution recorded the `[replace]` map (`from -> to`) but never added the replacement target as a resolved package. Because `Resolve::deps` redirects dependency edges through `replacement()`, the build then walked to the replacement `PackageId` and failed with an internal "couldn't find `<pkg>` in package set" panic. Mirror the default resolver, which activates the replacement summary so it becomes a graph node: for each node with a recorded replacement, add the replacement target to the graph, summaries, and checksums. Reduces `replace::` testsuite failures under `__CARGO_TEST_PUBGRUB` from 20 to 9; the remaining failures are the shared spurious-index-update and error-message-context issues, not resolution. Covered by the existing `tests/testsuite/replace.rs` suite (the bare resolver-tests harness cannot model `[replace]`).
A feature listing itself (e.g. `default = ["default"]`) produced a self-dependency in the pubgrub encoding, which the solver treats as trivially satisfiable -- so the cycle was silently accepted and resolution proceeded, diverging from the default resolver's `cyclic feature dependency: feature `X` depends on itself` error. Detect the self-reference while expanding a feature's values and return it as an `Unavailable` reason, matching the default resolver's `RequirementError::Cycle` (which fires only on `*dep_feat == feat`, i.e. direct self-reference; mutual feature cycles like `A -> B -> A` remain legal and are unaffected -- see resolver-tests `test_04_cyclic_features`). The error *outcome* now matches; the exact message text is still wrapped by pubgrub's derivation-tree reporter pending Cargo-native error reporting.
Widen `RequirementError` and `RequirementError::into_activate_error` to `pub(in crate::core::resolver)` and derive `Clone`/`Debug`/`PartialEq`/`Eq` plus a `Display` impl, so the pubgrub error-reporting bridge can reuse the v1 resolver's exact message rendering rather than re-implementing it. No behavior change; the type is still private to the resolver module.
Introduce a standalone `pubgrub/error.rs` module that turns a PubGrub failure into a Cargo `ResolveError` instead of a generic `anyhow` wrapper over PubGrub's `DefaultStringReporter`. The bridge is deliberately thin and reuses the v1 resolver's own rendering: - PubGrub's custom incompatibility metadata `M` changes from `String` to a structured `UnavailableReason`, so the provider records *why* a package is unusable (carrying the v1 `RequirementError`) rather than baking prose at the throw site; - for the common single-cause shape (missing/cyclic feature, missing dependency) the bridge recovers the failing summary and calls `RequirementError::into_activate_error`, producing byte-identical text; - it reconstructs a best-effort `package_path` from the derivation tree so member-level diagnostics work (fixes `member_errors`); - anything else falls back to PubGrub's reporter, still wrapped as a typed `ResolveError` so downcasting callers keep working. All user-facing prose now lives in this one module, which can be dropped or rewritten without touching resolution. Fixes the `member_errors` downcast failures and byte-matches the root-level feature errors; conflict-family messages (which need full `activation_error` context) still fall back and are left for a later change. No regressions in the deterministic or curated suites.
Split the "no candidates found" branch of `activation_error` (no version exists / yanked / typo'd name / etc.) into a standalone `no_candidates_error` that takes the resolved `package_path` and a pre-rendered `required_by` dependency-chain string instead of a `ResolverContext`. This is a pure extract-method refactor with no behavior change for the default resolver; it lets the experimental PubGrub error bridge reuse the exact same rendering (it recovers `package_path`/`required_by` from its own derivation tree). Marked `!` and kept standalone so it can be cherry-picked upstream independently of the PubGrub work.
Extend the PubGrub error bridge to the "no candidates found" family (no matching package / no matching version / yanked / typo'd name). When the derivation tree reduces to a `parent depends on a missing child` edge, the bridge recovers the original `Dependency` from the parent's summary and the parent path, then delegates to the v1 resolver's `no_candidates_error` for byte-identical output. Reconstruction is guarded: it only triggers when the child crate genuinely has no candidate versions (not a version/feature conflict), otherwise it falls back to PubGrub's reporter, so it never emits a wrong error. Under `__CARGO_TEST_PUBGRUB`: `registry::` failures drop 36 -> 23 and `package::` is fully green; no regressions in the deterministic or curated suites. The offline-mode hint is omitted (the provider carries no `GlobalContext`); conflict-family messages (candidates exist) still fall back and are left for a later change.
- §4.2: add the `error.rs` bridge module to the layout table. - §6: record the patch/replace node-identity and self-cyclic-feature insights. - §8: mark `[patch]`/`[replace]` as handled (with the remaining spurious-`[UPDATING]` caveat) and rewrite the error-reporting entry to describe the standalone bridge and what still falls back. - §9: error reporting is now in-progress; add the conflict-family and spurious-`[UPDATING]` follow-ups. - §10: extend the commit ledger with this session's work. - §12: replace the invalid survey number (it had run the default resolver via a misspelled env var) with a corrected before→after under nightly (233→177 failures), plus a methodology warning.
Generalize the no-candidates bridge from "crate is absent" to "no candidate version satisfies the requirement". The trigger now recovers the failing `Dependency` and checks whether any candidate matches its version req; if none do, it delegates to `no_candidates_error` (which re-queries via `alt_versions`/`rejected_versions`), producing byte-identical text for the `candidate versions found which didn't match` / yanked / unavailable families too. Cases where a candidate matches but conflicts with another selection are deliberately excluded and still fall back. Under `__CARGO_TEST_PUBGRUB`: `registry::` drops 20->15, with smaller drops in `directory`/`update`; no regressions in the deterministic or curated suites.
…rror Split the "candidates exist but conflict" branch of `activation_error` (semver clash / `links` collision / requested feature the dependency lacks) into a standalone `version_conflict_error` that takes the resolved `package_path`, the pre-rendered `required_by` parent chain, and a map of `conflict_paths` for the conflicting packages, instead of a `ResolverContext`. Pure extract-method refactor with no behavior change for the default resolver; it lets the experimental PubGrub error bridge reuse the exact same rendering. Marked `!` and kept standalone so it can be cherry-picked upstream independently of the PubGrub work. The now-unused `to_resolve_err` closure in `activation_error` is removed (both branches are extracted).
When a dependency requests a feature its target lacks (e.g. `foo` depends on `bar` with feature `bar` but `bar` has no such feature), render the v1 resolver's "... depends on X with feature Y but X does not have that feature" conflict instead of the root-level "does not have the feature" form. The bridge locates the `Custom(child, Requirement(..))` leaf and the `FromDependencyOf(parent, child)` edge (which sit at different tree depths, so they are searched independently and matched by package), reuses `into_activate_error(Some(parent), …)` to get Cargo's own `ConflictReason` classification (missing / required-dep / `dep:`-syntax), and delegates to `version_conflict_error`. Byte-identical for all five `features::dependency_activates_*` cases plus `test::cyclical_dep_with_missing_feature`. The remaining conflict families (semver / links) still fall back. No regressions.
The branch was rebased onto upstream master after rust-lang#17230 and rust-lang#17231, whose `src/` flattening moved every path this document referenced: `src/cargo/core/resolver/` -> `src/resolver/`, `src/cargo/core/features.rs` -> `src/workspace/features.rs`, and `src/doc/` -> `doc/`. Refresh the stale paths, the `CARGO_LOG` target (now `cargo::resolver::pubgrub`), and the unit-test filter, and replace the section 10 hashes, which the rebase invalidated wholesale. Add section 13 recording how the rebase was actually done. A plain `rebase --onto` does not survive this move: git relocates modified files but leaves a newly added directory at its dead path. The working recipe rewrites paths with `filter-branch --index-filter` first, and must leave upstream-owned files alone to preserve the merge base. Both remaining conflicts were in generated artifacts that have to be regenerated rather than hand-merged -- taking `--theirs` on the `-Z help` snapshot silently drops upstream new flags.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.