GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Fix Cargo source replacement being ignored (#338, #455) by mikolalysenko · Pull Request #1420 · SocketDev/socket-patch Conversation
|
| Issue | Regression tests (red before the fix → green) |
|---|---|
| #338 shape 1 (custom dir) | crawler_cargo_e2e::custom_vendor_dir_named_by_source_replacement_is_crawled; real cargo: e2e_safety_cargo_build::apply_patches_a_custom_named_vendor_dir |
| #338 shape 2 (member) | crawler_cargo_e2e::workspace_root_source_replacement_applies_to_a_member; real cargo: e2e_safety_cargo_build::apply_from_a_member_patches_the_workspace_vendor_dir |
#338 shape 3 + non-cargo vendor/ |
crawler_cargo_e2e::unwired_vendor_dir_falls_back_to_the_registry_cache |
| #338 fresh checkout / missing dir | crawler_cargo_e2e::missing_replacement_directory_crawls_nothing |
#338 $CARGO_HOME config |
crawler_cargo_e2e::cargo_home_source_replacement_is_honoured |
| #455 hosted redirect | patch::redirect::tests::cargo_directory_source_replacement_is_refused (both config spellings, CRLF, a multi-hop chain; mirror control still redirects) |
| #455 vex reads the wrong copy | covered by the crawler change: vex discovers through the same crawler (vex_consumed.rs); the crawler tests above |
| resolver | formats::cargo::source_replacement::tests (8 unit tests) |
Red before the fix: all 5 new crawler_cargo_e2e tests failed on origin/main
code. The hosted test failed with the Socket registry pin written into
Cargo.toml, Cargo.lock and .cargo/config.toml.
Fixtures that staged a bare vendor/ and relied on the old guess now write the
.cargo/config.toml that cargo vendor asks for (6 crawler unit/oracle tests,
crawlers_empty_paths_e2e, e2e_cargo ×2, e2e_vendor_cargo_build's #336
"behind a cargo vendor dir" rollback). None of their assertions were loosened.
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test -p socket-patch-core --all-features: all pass except 4copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_…).socket-patch-cli: --lib, e2e_cargo, in_process_cargo_apply,mode_migration_cargo, e2e_redirect_cargo_build, e2e_redirect_cargo_shapes,e2e_vendor_cargo_build (incl. both Cargo rollback after an agent→vendored takeover leaves the shared registry cache patched, reports success, and deletes the revert blobs #336 rollback tests with real cargo),vex_terminal_output, in_process_rollback_hosted, in_process_vendor,covgap_commands_scan_mod: passcargo test -p socket-patch-cli --all-features --test e2e_safety_cargo_build -- --ignored:cargo vendor uses a custom directory (or apply runs from a workspace member), yet reports success and VEX attests not_affected #338 shapescargo test --workspace couldn't finish locally: the sandbox rannpm/, pypi/, gem/) only dispatch to the binary, so nois_vendored (vendor/cargo.rs) still probes<root>/vendor for its already_vendored_in_tree refusal. That is avendor/. Cargo neverrollback now looksvendor/ copy stays.main crawls everyregistry/src/* dir the same way. Choosing the mirror's dir meansSourceId hash. Follow-up.[source.x] definitions:.. in--cwd is folded before the walk (ae078c1, b583db6 keeps the given cwd as the reported prefix). A symlinked cwd is walkedabsolute() is used rather thancanonicalize() so reported paths keep matching the cwd the user gave/var → /private/var).--config source replacement (CARGO_SOURCE_*) is notvendor::cargo_config::read_config_chain[patch]-specific (it drops files with no patch entries). Unifying the🤖 Generated with Claude Code
https://claude.ai/code/session_01TUHnkhA7W5gHh21jSMkY7P
Generated by Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
Agent-mode apply, scan and vex looked for a `cargo vendor` tree only at `<cwd>/vendor`. A custom vendor directory, a workspace root's vendor directory seen from a member, or a `vendor/` that no config wires all made apply patch a copy cargo ignores while reporting success, and made vex judge the wrong copy. The crawler now follows the `[source.crates-io] replace-with` chain of every cargo config cargo reads (each ancestor's `.cargo/config*`, then `$CARGO_HOME`'s) and crawls the directory source it ends on, resolved the way cargo resolves config paths. Without a directory replacement it crawls the registry cache, and a configured directory that is missing yields nothing rather than a copy cargo doesn't read. Refs #338, #455 Assisted-by: Claude Code:claude-opus-5-5
A project whose .cargo/config replaces crates.io with a directory source (`cargo vendor`) builds every crate from that directory and never reads a registry. `scan --mode hosted` still pinned the crate to a per-patch Socket registry and reported success, so every fresh `cargo build --frozen --offline` failed with "no matching package". The hosted cargo rewriter now follows the project config's `[source.crates-io] replace-with` chain and, when it ends on a directory source, skips every cargo dependency with nothing written and a `redirect_cargo_directory_source` warning that points at agent mode. A registry mirror replacement is still redirected. Refs #455 Assisted-by: Claude Code:claude-opus-5-5
Three fixtures staged a bare `vendor/` with no source replacement and relied on the crawler treating it as cargo's source. They now write the `.cargo/config.toml` that `cargo vendor` asks for, and the resolver strips a BOM through the shared text helper. Refs #338 Assisted-by: Claude Code:claude-opus-5-5
Rollback and prune look for an agent-mode patch left in the shared registry cache when the project crawl searches only a `cargo vendor` directory. That check still looked for `<cwd>/vendor`, so a custom or workspace-root vendor directory skipped it, and an unwired `vendor/` triggered it needlessly. It now asks the crawler for the directory source the project's cargo configs name. The hosted refusal's message now says why a Socket registry pin breaks an offline vendored build, instead of claiming cargo never reads it. Refs #338, #455 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The dispatch, all-ecosystem rollback and scan-scope tests staged a bare `vendor/` crate tree and relied on the crawler treating it as cargo's source. They now write the source replacement `cargo vendor` asks for, so they keep exercising the vendored-crate path. Refs #338 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ready for review (burn-down agent)
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Final review briefWhat it does: socket-patch now reads Cargo's source replacement ( Risk: low to medium. The resolver is small, pure and well tested: cycles and malformed TOML fall back to the registry, with no panic or loop. The risk is the intended behaviour change: a Look here:
Verified:
Changes I made: none. Open questions (none block merge; they could be follow-ups):
Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
A --cwd such as `b/../app` walked `b`'s ancestors and could pick up a sibling directory's `.cargo/config.toml`, so the crawl followed a source replacement cargo never sees from `app`. The path is folded lexically before the walk. Refs #338 Assisted-by: Claude Code:claude-opus-5-5
|
[agent] Thanks for the brief. On the open questions:
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Folding `..` before the config walk also moved the reported vendor directory onto the folded path, so path-scoped scan and rollback, which strip the given cwd as a prefix, saw the vendored crates as outside the project. The folded path now only picks the config files; a directory under it is reported back under the cwd as given. Refs #338 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
The new prefix test compared crawler output with a path that still held `..`, but on Windows `std::path::absolute` folds it, so the crawler (and path-scoped commands) report the folded path there. The test now builds its expectation with `absolute` too. Refs #338 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Final review brief (update for 747c561)The earlier brief covered everything up to 9083269. This one reviews the three commits since then. What it does: Before walking the ancestor Risk: low. The change is small and pure: +30/-4 in one function plus two tests. Look here:
Verified:
Changes I made: none. Open questions (non-blocking):
Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
The hosted refusal for a `cargo vendor` project only read the project's own `.cargo/config*`. A directory source set in a monorepo root's config or in `$CARGO_HOME` was missed, and a `cargo local-registry` was never recognised, so `scan --mode hosted` still pinned the Socket registry and every `cargo build --frozen --offline` broke. The hosted scan now resolves the source replacement from every config cargo reads for a build there, read-only, and passes it into the cargo rewriter. A directory source or local registry from any of them skips every cargo dependency with `redirect_cargo_directory_source`. Agent mode keeps crawling the registry cache for a local registry, which cargo unpacks there. The process-level resolve now lives in `source_replacement` so the crawler and the hosted scan share it. Refs #455 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
The format models are pure by rule (formats::architecture_tests), but the last commit put the file-reading resolve into formats/cargo/source_replacement.rs. It now lives in the cargo crawler as `CargoCrawler::crates_io_source_for`, which the hosted scan calls; the format module keeps only the pure chain resolution. Refs #455 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d81161b. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Final reviewer: disarmed auto-merge. My earlier brief covered 747c561; two non-merge commits have landed since (4032716 "Refuse hosted cargo pins on outer offline sources", d81161b "Keep cargo config reads out of formats"). I'll write a fresh brief for d81161b on a later run once the head has settled, then re-arm. Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Main's #1420 added a restored field to the e2e Shape struct; the declared-cargo-floor test this branch adds predates it and no longer compiled after merging main. Co-Authored-By: Claude <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Hosted cargo scan in acargo vendor project reports success but breaks every fresh cargo build --frozen --offline, and VEX then omits the patch
Agent-mode cargo apply patches the wrong copy when cargo vendor uses a custom directory (or apply runs from a workspace member), yet reports success and VEX attests not_affected
There was an error while loading. Please reload this page.