diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 863dbcbca..542209ae1 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1252,7 +1252,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | Tag | Action(s) | Context | |---------------------------|------------------|---------| | `already_patched` | `skipped` | apply: every file's hash already matches `afterHash`. | -| `package_not_installed` | `skipped` | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). | +| `package_not_installed` | `skipped` | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). **Cargo is never lockfile-only (v5.0, #616)**: `cargo fetch` unpacks every `Cargo.lock` entry (target-gated ones included), so a locked crate missing from `$CARGO_HOME/registry/src` was simply not fetched yet (a cold CI cache, a pruned `registry/src`) and the next `cargo build` would compile it unpatched; it counts as an unresolved purl (an all-miss run exits 1, as in 4.x) and its detail, the human error and the human warning say to run `cargo fetch` first. | | `apply_failed` | `failed` | apply: hash mismatch, write error, archive read error. | | `no_local_source` | `skipped`/`failed` | Agent patch application cannot obtain the required local or downloaded patch source. Vendored mode consumes complete server artifacts and no longer stages patch blobs. | | `offline_missing_sources` / `sources_download_failed` | apply run-level `warnings[]` | apply (additive): the patch sources were unavailable — `--offline` with no local source, or the download left a patch with no source — so nothing was attempted. The envelope keeps its pinned shape (`partialFailure`, empty `events[]`, zero summary, no top-level `error`); the warning is its machine-readable reason (the human path prints the staging `Error:` line on stderr instead, even under `--silent`). | diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 22ed44888..3c35b0ae2 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -1271,9 +1271,9 @@ fn collect_apply_failures( } for purl in unresolved_purls(unmatched, lockfile_only) { failures.push(ApplyFailure { - purl, code: "package_not_installed".to_string(), - error: "No installed package matches this PURL".to_string(), + error: not_installed_detail(&purl).to_string(), + purl, }); } failures @@ -1540,7 +1540,7 @@ pub(crate) async fn run_locked( let detail = if lockfile_only.contains(purl) { LOCKFILE_ONLY_DETAIL } else { - "No installed package matches this PURL" + not_installed_detail(purl) }; env.record( PatchEvent::new(PatchAction::Skipped, purl.clone()) @@ -2722,6 +2722,9 @@ async fn apply_patches_inner( for purl in &unresolved { eprintln!(" - {}", normalize_purl(purl)); } + if let Some(line) = cargo_fetch_hint(&unresolved) { + eprintln!("{line}"); + } } print_lockfile_only_note(args, &unmatched, &lockfile_only); @@ -3270,7 +3273,8 @@ const LOCKFILE_ONLY_DETAIL: &str = /// `@esbuild/-`), a devDependency under `npm ci --omit=dev`. The /// tree is in its correct end state, so they are calm skips, as `scan /// --mode agent` treats lockfile-only packages. Global runs have no project -/// lock, so nothing is lockfile-resolved there. +/// lock, so nothing is lockfile-resolved there, and neither is a Cargo +/// crate ([`lock_resolution_is_calm`]). async fn lockfile_resolved(common: &GlobalArgs, unmatched: &[String]) -> HashSet { if unmatched.is_empty() || common.is_global() { return HashSet::new(); @@ -3280,11 +3284,36 @@ async fn lockfile_resolved(common: &GlobalArgs, unmatched: &[String]) -> HashSet let lock_purls: HashSet = entries.iter().map(|e| PurlKey::new(&e.purl)).collect(); unmatched .iter() - .filter(|p| lock_purls.contains(&PurlKey::new(p))) + .filter(|p| lock_resolution_is_calm(p) && lock_purls.contains(&PurlKey::new(p))) .cloned() .collect() } +/// Whether a lock-resolved but uninstalled `purl` can be a calm skip. +/// Not for Cargo (#616): cargo leaves no locked crate out on purpose — +/// `cargo fetch` unpacks every `Cargo.lock` entry, target-gated ones +/// included — so a locked crate missing from `$CARGO_HOME/registry/src` +/// was just not fetched yet (a cold CI cache, a pruned `registry/src`), +/// and the next `cargo build` downloads or re-extracts it UNPATCHED. +fn lock_resolution_is_calm(purl: &str) -> bool { + Ecosystem::from_purl(purl) != Some(Ecosystem::Cargo) +} + +/// The `package_not_installed` detail of an unmatched purl the lock does +/// not calmly account for: Cargo's carries the `cargo fetch` remedy. +fn not_installed_detail(purl: &str) -> &'static str { + if Ecosystem::from_purl(purl) == Some(Ecosystem::Cargo) { + CARGO_NOT_FETCHED_DETAIL + } else { + "No installed package matches this PURL" + } +} + +/// [`not_installed_detail`] for a Cargo crate. +const CARGO_NOT_FETCHED_DETAIL: &str = + "No unpacked crate source matches this PURL; run `cargo fetch` first so the crate is in \ + the Cargo registry cache, then re-run apply"; + /// The `unmatched` purls with no lock evidence (sorted input, sorted /// output): the ones that can still fail an all-miss run. fn unresolved_purls(unmatched: &[String], lockfile_only: &HashSet) -> Vec { @@ -3345,9 +3374,24 @@ fn format_none_installed_error(unmatched: &[String]) -> Vec { "Check that the packages are installed and --cwd points to the right directory." .to_string(), ); + if let Some(line) = cargo_fetch_hint(unmatched) { + lines.push(line.to_string()); + } lines } +/// The `cargo fetch` remedy line when any of `unmatched` is a Cargo crate +/// (#616): cargo unpacks locked crates only when it fetches or builds. +fn cargo_fetch_hint(unmatched: &[String]) -> Option<&'static str> { + unmatched + .iter() + .any(|p| Ecosystem::from_purl(p) == Some(Ecosystem::Cargo)) + .then_some( + "Cargo crates are patched in the registry cache: run `cargo fetch` first so every \ + locked crate is unpacked there, then re-run apply.", + ) +} + #[cfg(test)] mod tests { //! Tests for `result_to_event` — the per-package → per-patch event diff --git a/crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs b/crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs index ac00c57b9..feffac100 100644 --- a/crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs +++ b/crates/socket-patch-cli/tests/apply/lockfile_only_skip.rs @@ -280,3 +280,113 @@ fn no_lockfile_all_miss_still_fails() { "{stderr}" ); } + +// ---- Cargo: lock-resolved but not fetched is NOT a calm skip (#616) ---- +// +// A Cargo.lock entry with no unpacked source in `$CARGO_HOME/registry/src` +// was not deliberately left out: cargo simply has not fetched it yet (a +// fresh CI runner / clone, or a pruned `registry/src`). The next `cargo +// build` downloads or re-extracts it UNPATCHED, so `apply` must fail the +// all-miss run like 4.0.0 did, and tell the user to `cargo fetch` first. + +const CARGO_PURL: &str = "pkg:cargo/cfg-if@1.0.0"; + +/// A binary crate locking `cfg-if 1.0.0` from crates.io. +fn write_cargo_project(root: &Path) { + std::fs::write( + root.join("Cargo.toml"), + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n\ + [dependencies]\ncfg-if = \"=1.0.0\"\n", + ) + .unwrap(); + std::fs::write( + root.join("Cargo.lock"), + "# This file is automatically @generated by Cargo.\n\ + # It is not intended for manual editing.\n\ + version = 4\n\n\ + [[package]]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\n \"cfg-if\",\n]\n\n\ + [[package]]\nname = \"cfg-if\"\nversion = \"1.0.0\"\n\ + source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\ + checksum = \"baf1de4339761588bc0d7c0e2b9b9d4ab8c4fa8c1f6c2fb5b9a6e7c3a7d2b0e1\"\n", + ) + .unwrap(); + std::fs::create_dir_all(root.join("src")).unwrap(); + std::fs::write(root.join("src/main.rs"), "fn main() {}\n").unwrap(); +} + +fn run_cargo_apply(cwd: &Path, cargo_home: &Path, args: &[&str]) -> (i32, String, String) { + let mut argv: Vec<&str> = vec!["apply"]; + argv.extend_from_slice(args); + let home = cargo_home.to_str().unwrap(); + run_with_env( + cwd, + &argv, + &[("SOCKET_TELEMETRY_DISABLED", "1"), ("CARGO_HOME", home)], + ) +} + +fn assert_cold_cargo_apply_fails(project: &Path, cargo_home: &Path) { + let (code, stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline", "--json"]); + let v = parse_json_envelope(&stdout); + assert_eq!( + code, 1, + "an unfetched cargo crate must fail apply, not skip calmly; {v}\n{stderr}" + ); + assert_eq!(v["status"], "partialFailure", "{v}"); + let ev = event(&v, CARGO_PURL); + assert_eq!(ev["errorCode"], "package_not_installed", "{ev}"); + let detail = ev.to_string(); + assert!( + detail.contains("cargo fetch") && !detail.contains("lockfile-only"), + "the event must carry the cargo fetch remedy, not the calm lockfile-only detail; {ev}" + ); + + // Human path: the exit-flipping error plus the actionable remedy. + let (code, _stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline"]); + assert_eq!(code, 1, "{stderr}"); + assert!( + stderr.contains("Error: The targeted manifest patch matched no installed package:") + && stderr.contains(&format!(" - {CARGO_PURL}")) + && stderr.contains("cargo fetch"), + "the error names the crate and tells the user to run `cargo fetch`; {stderr}" + ); + + // `--silent` (the hook / CI shape) still prints the error and exits 1. + let (code, _stdout, stderr) = run_cargo_apply(project, cargo_home, &["--offline", "--silent"]); + assert_eq!(code, 1, "{stderr}"); + assert!(stderr.contains("cargo fetch"), "{stderr}"); +} + +/// #616: an empty `$CARGO_HOME` (fresh CI runner): the crate is locked but +/// was never downloaded. +#[test] +fn cargo_crate_on_cold_registry_cache_fails_with_fetch_remedy() { + let tmp = tempfile::tempdir().unwrap(); + let project = tmp.path().join("app"); + std::fs::create_dir_all(&project).unwrap(); + write_cargo_project(&project); + write_manifest(&project, &[CARGO_PURL]); + let cargo_home = tmp.path().join("cargo-home"); + std::fs::create_dir_all(&cargo_home).unwrap(); + + assert_cold_cargo_apply_fails(&project, &cargo_home); +} + +/// #616 pruned variant: the `.crate` archive is cached but `registry/src` +/// was pruned, so cargo re-extracts the pristine source on the next build. +#[test] +fn cargo_crate_with_pruned_registry_src_fails_with_fetch_remedy() { + let tmp = tempfile::tempdir().unwrap(); + let project = tmp.path().join("app"); + std::fs::create_dir_all(&project).unwrap(); + write_cargo_project(&project); + write_manifest(&project, &[CARGO_PURL]); + let cargo_home = tmp.path().join("cargo-home"); + let cache = cargo_home.join("registry/cache/index.crates.io-1949cf8c6b5b557f"); + std::fs::create_dir_all(&cache).unwrap(); + std::fs::write(cache.join("cfg-if-1.0.0.crate"), b"not a real archive").unwrap(); + std::fs::create_dir_all(cargo_home.join("registry/src/index.crates.io-1949cf8c6b5b557f")) + .unwrap(); + + assert_cold_cargo_apply_fails(&project, &cargo_home); +} diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 759d7d182..c8cb647bf 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -947,6 +947,13 @@ crate that means the **shared** `$CARGO_HOME/registry` cache: the patch affects project on the machine, and is silently reset by `cargo clean` or a cache prune. Use `--mode vendored` for a project-local, committable patch. +Run `cargo fetch` before `apply` on a fresh checkout or CI runner. Cargo unpacks a +locked crate into `registry/src` only when it fetches or builds, so on a cold or pruned +cache there is nothing to patch, and the next `cargo build` would compile the pristine +crate. `apply` therefore treats a crate that `Cargo.lock` resolves but that is not +unpacked as not installed, not as a calm lockfile-only skip: a run where no targeted +patch matched exits 1 and names `cargo fetch` as the remedy. + ## Cargo: vendored wiring in Cargo.toml Vendored mode (v5+) wires a patched crate with a `[patch.crates-io]` path