Repository navigation
Stop attesting a Cargo pin a workspace member resolves from crates.io - #2019
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Conversation
This was referenced Nov 4, 2026
Cargo resolves each source on its own. When one declaration of a crate pins it to `socket-patch-<uuid>` but another (the root's or a workspace member's) names no Socket registry (crates.io, another registry, a path or git) with a requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. Discovery still attested the Cargo.lock ref because "some declaration pins the patch" was enough. The hosted rewriter pins every declaration, so this mixed state is a partial revert or a hand edit. The ref now stays (rollback, remove and the ledgers' liveness still see the pin) but is marked unattested with the new `UnattestedKind::CargoUnpinnedDeclaration`, naming the unpinned declaration's manifest. When Cargo.lock already locks a same-version copy from another source beside the Socket entry, the detail names that lock entry as direct evidence. A declaration whose requirement excludes the patched version, and a `[patch]` registry entry (which redirects declarations rather than adding one), do not withhold the statement. The CLI omits the ref as `vex_cargo_unpinned_declaration`; documented in CLI_CONTRACT.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
force-pushed
the
fix/cargo-mixed-member-pins
branch
from
November 5, 2026 19:57
06d306c to
77a4d2d
Compare
Review fixes for the unpinned-declaration mark. Each declaration of a Socket-locked crate is now classified once (`route`): routed to a Socket patch (its own `socket-patch-<uuid>` registry, or a Socket `[patch.<source>]` registry entry that redirects it), unpinned (cargo builds its own copy), or saying nothing about the locked version. - A crates.io (or registry) declaration that a Socket `[patch.<source>]` registry entry in the root manifest or project config redirects is no longer counted as unpinned. - A declaration naming `registry = "socket-patch-<uuid>"` beside `path`, `git` or `registry-index` is not a pin (the hosted rewriter's own pinned check): cargo builds that other source, so it now marks the ref. - Requirements go through `patch::redirect::cargo_req_selects` with the lock's other versions of the crate, and the detail says whether a re-scan can pin the declaration: only a crates.io declaration whose requirement matches no other locked version; otherwise the rewriter refuses (`redirect_cargo_toml_dep_unrewritable`), so the remedy is to change the declaration or roll back. The CLI note's remedy is now generic and the per-case remedy lives in the detail. - `registry-index` declarations are reported as such, not as crates.io. - A `[workspace.dependencies]` template counts only when some manifest inherits it (`workspace = true`). - When the lock already holds a same-version copy from another source, no mark is added: the twin rule (#679) withholds the ref and names it. - Root pinned to patch A with a member pinned to patch B (for the locked version, and B not resolved by the lock) now emits B's Cargo.toml ref beside A's, so the CLI's wiring-conflict gate runs. - `Decl` carries a `DeclSource` enum and a `live` flag; the origin text is formatted only for messages, and `[patch]` pins are built with `Decl::patch` (no dead version parameter). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Nov 6, 2026
`vendor` wires a crate through the root's `[patch.crates-io]`, which cargo applies only to declarations from crates.io. A workspace member (or the root) that declares the crate from another registry, a `registry-index` or git, at a requirement that may select the patched version, resolves its own unpatched copy. The vendored ref stayed attested beside it because the lock copy claim only reads sourceless entries. The ref now stays (rollback, remove and the ledgers still see the wiring) but is marked unattested with the new `UnattestedKind::CargoPatchBypassed`, naming the declaration. A same-version Cargo.lock entry from any registry or git source beside the copy is marked the same way, naming the lock entry. The check reads the declarations through #2019's classification: live declarations only, a registry dependency counts as covered when a `[patch.<registry>]` table pointing at the same copy redirects it (`Decl::patched_by`), a path declaration (the user's own fork) keeps the existing "ignored" rule, and a `socket-patch-*` registry dependency is hosted wiring. The CLI omits the ref as `vex_cargo_patch_bypassed`; documented in CLI_CONTRACT.md. The workspace member walk ran only when the lock held a Socket-sourced package (or, with no lock, a manifest named a `socket-patch-` registry). A vendored copy's lock entry has no source and it has no such pin, so the walk now also runs whenever the root manifest or the project config wires a Socket copy through `[patch]`, whatever the lock's state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Stacked on #2012 (base
fix/cargo-members-liveness-origin).Problem
#2012 made Cargo VEX discovery read workspace member manifests, but left one case open. When one declaration pins a crate to
socket-patch-<uuid>and another declaration of the same crate is not pinned (crates.io, another registry, a path or git) with a requirement that may select the locked version, discovery still emitted an attestedCargo.lockref. "Some declaration pins the patch" was enough. The other declaration can be the root's or a member's.Cargo resolves each source on its own. The unpinned declaration builds a separate, unpatched copy, and that copy can be the very version the pin covers. The hosted rewriter pins every declaration in the root and every member (
patch/redirect/mod.rs), so this mixed state comes from a partial revert or a hand edit.Fix
Each declaration of a Socket-locked crate is classified once (
route). It is one of three things:socket-patch-<uuid>registry, or by a Socket[patch.<source>]registry entry in the root manifest or project config that redirects it;[patch]entry itself, a[workspace.dependencies]template no manifest inherits, and a malformedsocket-patch-*name (diagnosed elsewhere).What the classification decides when some declaration names the lock's patch A:
UnattestedKind::CargoUnpinnedDeclaration.scancan pin the declaration. Only a crates.io declaration whose requirement matches no other locked version can be re-pinned; for anything else the rewriter refuses, so the remedy is to change the declaration or roll the patch back.registry = "socket-patch-<uuid>"besidepath,gitorregistry-indexis not a pin, because cargo builds that other source. This matches the hosted rewriter's own pinned check.Cargo.tomlref. The CLI then gates the package aswiring_conflict.Version requirements go through
patch::redirect::cargo_req_selects, with the lock's other versions of the crate, so "may select" and "ambiguous" agree with the rewriter.CLI: a new note arm
vex_cargo_unpinned_declarationinvex_sources.rs. Its remedy is generic ("not attested while that declaration stands"); the per-case remedy is in the detail. Both places inCLI_CONTRACT.mdare documented.Vendored mode
Vendored mode does not have this bug. Its wiring is a root
[patch.crates-io]entry, which cargo applies workspace-wide to every crates.io declaration the copy satisfies. The related gap (a member declaring the crate from another registry or git) is handled in #2024, which is stacked on this PR.Review findings (
/code-review high)All nine were confirmed and fixed. For the behaviour bugs I wrote the test first and saw it fail; 7 tests failed before the fix.
[patch]registry entry redirects was marked unpinned. Redirected declarations now count as pins. Addedout.unattested.is_empty()topatch_table_registry_pin_counts_as_the_declarationandconfig_patch_registry_pin_counts_for_a_declared_crate, plus a case where a declaration from another registry, not redirected by[patch.crates-io], is still marked.path/git/registry-indexcounted as pinned. Test:a_socket_registry_beside_another_source_is_not_a_pin. Next to a real pin, it marks the ref. As the only declaration, it keeps the ref because Socket's wiring is present, but marks it.the_remedy_says_whether_a_rescan_can_pin_it). A requirement that also matches another locked version names that version and saysscancannot pin it.a_locked_same_version_twin_is_left_to_the_twin_rule).[workspace.dependencies]template counts only when inherited (an_unpinned_workspace_template_counts_only_when_inherited).registry-indexdeclaration is reported as "the registry index ", with the roll-back remedy.a_member_pinned_to_another_patch_is_a_conflict_pair). No conflict is emitted when B's requirement excludes the locked version, or when the lock resolves the crate from B at another version (a separate, patched package with its own ref). This overlaps with the split-patch part of Withhold a vendored Cargo copy a member's other source bypasses #2024; see the note there.cargo_req_selectswith the lock's other versions;Decl::acceptsis gone.Declcarries aDeclSourceenum and aliveflag. The origin text is formatted only for messages, and[patch]pins useDecl::patch, which has no dead version parameter.Tests
cargo test -p socket-patch-core --lib -- vex::discover:: vendor::cargo patch::redirect:: utils::cargo_workspace: 1555 passed. The golden corpus test passes with no golden changes.cargo test -p socket-patch-cli --lib vex_sources: 15 passed.cargo test -p socket-patch-cli --test in_process_redirect cargo: 6 passed.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --checkare clean.Not done
unresolved_manifest_pins, from Read Cargo members in discovery; match liveness uuids by origin #2012) still treats asocket-patch-<uuid>registry besidepath/gitas pinning, and an uninherited template as a declaration. It is unchanged here.dependencieslists to tell which copy a declaration actually resolved.Generated by Claude Code