Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions crates/socket-patch-cli/src/commands/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,9 +1295,15 @@ async fn revert_vendored_matches(
let mut leg = RemoveVendorLeg::default();
// Stops at the first hard failure: remove aborts there, leaving the
// remaining matches (and the manifest) untouched.
let reverted = VendoredBackend::new(&args.common, None)
.revert(keys, state, opts, true)
.await;
let backend = VendoredBackend::new(&args.common, None);
let reverted = backend.revert(keys, state, opts, true).await;
for (key, w) in backend.go_undo_warnings(&reverted).await {
if loud {
eprintln!("Warning: {}", w.detail);
}
leg.skipped
.push(PatchEvent::new(PatchAction::Skipped, key).with_reason(w.code, w.detail));
}
for RevertedEntry {
key,
warnings,
Expand Down
11 changes: 8 additions & 3 deletions crates/socket-patch-cli/src/commands/rollback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -989,9 +989,14 @@ async fn run_vendored_leg(
keep_artifact: preserve,
};
let loud = !common.json && !common.silent;
let reverted = VendoredBackend::new(common, None)
.revert(keys, state, opts, false)
.await;
let backend = VendoredBackend::new(common, None);
let reverted = backend.revert(keys, state, opts, false).await;
for (_, w) in backend.go_undo_warnings(&reverted).await {
if loud {
eprintln!("Warning: {}", w.detail);
}
out.warnings.push((w.code.to_string(), w.detail));
}
for RevertedEntry {
key,
warnings,
Expand Down
54 changes: 47 additions & 7 deletions crates/socket-patch-cli/src/commands/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,8 @@ async fn run_check(args: &VendorArgs) -> i32 {
} else {
None
};
let go_sync_issues = tokio::sync::OnceCell::new();
let go_sync_issues =
socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new()).await;
for (key, entry) in entries {
let record = entry.record.as_ref().or_else(|| manifest.patches.get(key));
let mut failure = match record {
Expand Down Expand Up @@ -1280,11 +1281,6 @@ async fn run_check(args: &VendorArgs) -> i32 {
if failure.is_none() && entry.ecosystem == "golang" {
if let Some((module, _)) = socket_patch_core::utils::purl::parse_golang_purl(key) {
let issues: Vec<String> = go_sync_issues
.get_or_init(|| async {
socket_patch_core::vendor::go_consumer_sync::audit(root, &HashMap::new())
.await
})
.await
.iter()
.filter(|issue| issue.module() == module)
.map(ToString::to_string)
Expand Down Expand Up @@ -1362,6 +1358,46 @@ async fn run_check(args: &VendorArgs) -> i32 {
.with_details(serde_json::json!({ "ecosystem": eco, "path": rel })),
);
}
// A committed vendor/modules.txt still recording a socket `replace`
// that go.mod no longer has breaks every default build until it is
// regenerated (#1777). The ledger entry it came from is gone after a
// revert, so the per-entry check above cannot see it.
let ledger_modules: HashSet<String> = state
.entries
.iter()
.filter(|(_, entry)| entry.ecosystem == "golang")
.filter_map(|(key, _)| socket_patch_core::utils::purl::parse_golang_purl(key))
.map(|(module, _)| module.into_owned())
.collect();
let stale_records: Vec<(String, String)> = go_sync_issues
.iter()
.filter(|issue| {
matches!(
issue,
socket_patch_core::vendor::go_consumer_sync::GoSyncIssue::VendorModulesTxt {
stale: true,
..
}
) && !ledger_modules.contains(issue.module())
})
.map(|issue| (issue.module().to_string(), issue.to_string()))
.collect();
for (module, detail) in stale_records {
if !args.common.json {
eprintln!(
"{}: {detail}",
socket_patch_core::vendor::go_consumer_sync::VENDOR_MODULES_TXT_CODE
);
}
env.record(
PatchEvent::artifact(PatchAction::Failed)
.with_error(
socket_patch_core::vendor::go_consumer_sync::VENDOR_MODULES_TXT_CODE,
detail,
)
.with_details(serde_json::json!({ "ecosystem": "golang", "module": module })),
);
}
if args.common.json {
println!("{}", env.to_pretty_json());
}
Expand Down Expand Up @@ -4436,14 +4472,18 @@ async fn run_revert(args: &VendorArgs, env: &mut Envelope) -> i32 {
// (rollback's vendored leg, both of remove's paths, the manifest
// reconcile): dispatch → drift-keep → per-entry ledger save. Only the
// event vocabulary and the human lines are this command's.
let reverted = VendoredBackend::new(common, None)
let backend = VendoredBackend::new(common, None);
let reverted = backend
.revert(
&recorded,
&mut state,
RevertOpts::new(common.dry_run),
false,
)
.await;
for (purl, w) in backend.go_undo_warnings(&reverted).await {
record_warning(env, &purl, &w, common);
}
for RevertedEntry {
key: purl,
flavor,
Expand Down
43 changes: 43 additions & 0 deletions crates/socket-patch-cli/src/commands/vendored_backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,49 @@ impl<'a> VendoredBackend<'a> {
}
out
}

/// Dropping a vendored Go `replace` leaves a committed
/// vendor/modules.txt still recording it, which breaks every default
/// build until `go mod vendor` runs (#343, #1777). The undo commands
/// call this after [`Self::revert`]: it audits the Go consumer state
/// once and returns the issues of the Go modules `reverted` unwired,
/// each keyed by the ledger key of its entry. Not part of `revert`
/// itself, whose forward-run caller (the manifest reconcile) audits
/// once at the end of the run instead.
pub(crate) async fn go_undo_warnings(
&self,
reverted: &[RevertedEntry],
) -> Vec<(String, VendorWarning)> {
let unwired: Vec<(&str, String)> = reverted
.iter()
.filter(|e| {
matches!(
e.step,
VendorRevertStep::Reverted
| VendorRevertStep::Preserved
| VendorRevertStep::LedgerWriteFailed(_)
)
})
.filter_map(|e| {
socket_patch_core::utils::purl::parse_golang_purl(&e.key)
.map(|(module, _)| (e.key.as_str(), module.into_owned()))
})
.collect();
if unwired.is_empty() {
return Vec::new();
}
socket_patch_core::vendor::go_consumer_sync::audit(&self.common.cwd, &HashMap::new())
.await
.into_iter()
.filter_map(|issue| {
let (key, _) = unwired.iter().find(|(_, m)| m == issue.module())?;
Some((
key.to_string(),
VendorWarning::new(issue.code(), issue.to_string()),
))
})
.collect()
}
}

/// Name a patched `<name>-<version>.gem` left in Bundler's cache dir once
Expand Down
173 changes: 173 additions & 0 deletions crates/socket-patch-cli/tests/e2e_vendor_golang_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,3 +1035,176 @@ fn go_vendor_reports_applied_event() {

chmod_writable(tmp.path());
}

// ── #1777: vendored undo vs a committed vendor/ ───────────────────────

/// #1777: in a project with a committed `vendor/`, a vendored patch plus
/// the `go mod vendor` its forward warning asks for leaves
/// `vendor/modules.txt` recording the socket `replace`. Every vendored undo
/// (`vendor --revert`, `rollback`, `remove <purl>`) drops that `replace`, so
/// the next default build fails with "inconsistent vendoring" until
/// `go mod vendor` runs again. Each undo must name that regeneration (as
/// agent-mode rollback does, #343), and `apply --check` must report the
/// stale modules.txt as drift until it is run.
fn vendored_undo_needs_go_mod_vendor(label: &str, undo: &[&str], workspace: bool) {
if !golang_e2e_matrix::toolchain_ready(&format!("e2e_vendor_golang_build({label})")) {
return;
}
let tmp = tempfile::tempdir().unwrap();
let (consumer, modcache, proxy) = stage(tmp.path());
let mc = modcache.to_str().unwrap();
let mut goenv = go_env(mc, &proxy);
// In a workspace, `go work vendor` owns vendor/modules.txt.
let regen_cmd: &[&str] = if workspace {
std::fs::write(consumer.join("go.work"), "go 1.21\n\nuse .\n").unwrap();
&["work", "vendor"]
} else {
&["mod", "vendor"]
};
if workspace {
// `-mod=mod` is not allowed in workspace mode.
goenv.retain(|(k, _)| *k != "GOFLAGS");
goenv.push(("GOFLAGS", ""));
}
let vendored = go(&consumer, regen_cmd, &goenv);
assert!(
vendored.status.success(),
"{regen_cmd:?}: {}",
String::from_utf8_lossy(&vendored.stderr)
);
// Default flags from here on: go builds from vendor/.
goenv.retain(|(k, _)| *k != "GOFLAGS");
goenv.push(("GOFLAGS", ""));

write_patch(&consumer);
let (code, stdout, stderr) = run_socket(
&consumer,
&[
"vendor",
"--json",
"--offline",
"--cwd",
consumer.to_str().unwrap(),
],
&modcache,
);
assert_eq!(
code, 0,
"vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}"
);
let revendor = go(&consumer, regen_cmd, &goenv);
assert!(
revendor.status.success(),
"{}",
String::from_utf8_lossy(&revendor.stderr)
);
let run = go(&consumer, &["run", "."], &goenv);
assert!(
String::from_utf8_lossy(&run.stdout).contains("OUT: PATCHED"),
"{}{}",
String::from_utf8_lossy(&run.stdout),
String::from_utf8_lossy(&run.stderr)
);

{
let cs = consumer.to_str().unwrap();
let mut args = undo.to_vec();
args.extend(["--cwd", cs]);
let (code, so, se) = run_socket(&consumer, &args, &modcache);
assert_eq!(code, 0, "[{label}] failed.\n{so}\n{se}");
let renv = parse_json_envelope(&so);
assert!(
!std::fs::read_to_string(consumer.join("go.mod"))
.unwrap()
.contains("replace "),
"[{label}] the socket replace is gone"
);
let broken = go(&consumer, &["build", "./..."], &goenv);
assert!(
!broken.status.success()
&& String::from_utf8_lossy(&broken.stderr).contains("inconsistent vendoring"),
"[{label}] the fixture reproduces the broken build: {}",
String::from_utf8_lossy(&broken.stderr)
);
assert!(
common::envelope::all_codes(&renv)
.contains(&"go_vendor_modules_txt_out_of_sync".to_string()),
"[{label}] must name the vendor/modules.txt regeneration:\n{renv}"
);
let (code, so, se) = run_socket(
&consumer,
&["apply", "--check", "--ecosystems", "golang", "--cwd", cs],
&modcache,
);
assert_eq!(
code, 1,
"[{label}] stale vendor/modules.txt is drift:\n{so}\n{se}"
);
let named = format!("go {}", regen_cmd.join(" "));
assert!(se.contains(&named), "[{label}] must name `{named}`: {se}");
let (code, so, se) = run_socket(&consumer, &["vendor", "--check", "--cwd", cs], &modcache);
assert_eq!(
code, 1,
"[{label}] vendor --check: stale vendor/modules.txt is drift:\n{so}\n{se}"
);
assert!(se.contains(&named), "[{label}] must name `{named}`: {se}");

// The step the warning names fixes the build.
let regen = go(&consumer, regen_cmd, &goenv);
assert!(
regen.status.success(),
"[{label}] {}",
String::from_utf8_lossy(&regen.stderr)
);
let run = go(&consumer, &["run", "."], &goenv);
assert!(
String::from_utf8_lossy(&run.stdout).contains("OUT: PRISTINE"),
"[{label}] {}{}",
String::from_utf8_lossy(&run.stdout),
String::from_utf8_lossy(&run.stderr)
);
// (`vendor --revert` keeps the manifest record, which `--check`
// still reports as unledgered; only the modules.txt drift is gone.)
let (_code, so, se) = run_socket(&consumer, &["vendor", "--check", "--cwd", cs], &modcache);
assert!(
!se.contains("go_vendor_modules_txt_out_of_sync") && !so.contains("modules.txt"),
"[{label}] in sync after {named}:\n{so}\n{se}"
);
}

chmod_writable(tmp.path());
}

const REVERT: &[&str] = &["vendor", "--revert", "--json"];
const ROLLBACK: &[&str] = &["rollback", "--offline", "--json"];
const REMOVE: &[&str] = &["remove", UPURL, "--offline", "--json"];

#[test]
fn vendor_revert_needs_go_mod_vendor_with_committed_vendor_dir() {
vendored_undo_needs_go_mod_vendor("vendor --revert", REVERT, false);
}

#[test]
fn vendored_rollback_needs_go_mod_vendor_with_committed_vendor_dir() {
vendored_undo_needs_go_mod_vendor("rollback", ROLLBACK, false);
}

#[test]
fn vendored_remove_needs_go_mod_vendor_with_committed_vendor_dir() {
vendored_undo_needs_go_mod_vendor("remove", REMOVE, false);
}

#[test]
fn vendor_revert_needs_go_work_vendor_in_workspace() {
vendored_undo_needs_go_mod_vendor("go.work vendor --revert", REVERT, true);
}

#[test]
fn vendored_rollback_needs_go_work_vendor_in_workspace() {
vendored_undo_needs_go_mod_vendor("go.work rollback", ROLLBACK, true);
}

#[test]
fn vendored_remove_needs_go_work_vendor_in_workspace() {
vendored_undo_needs_go_mod_vendor("go.work remove", REMOVE, true);
}
5 changes: 3 additions & 2 deletions docs/ecosystems.md
Original file line number Diff line number Diff line change
Expand Up @@ -1475,8 +1475,9 @@ for it until `rollback -g`.

Two files the go command derives from `go.mod` can disagree with a new or removed
socket `replace`, and go then refuses to build. socket-patch does not regenerate
them (that needs the go toolchain and the whole module graph); `apply`, `vendor`,
`rollback` and `scan --mode hosted` warn with the command that does, and
them (that needs the go toolchain and the whole module graph); `apply`, `vendor`
(including `vendor --revert`), `rollback`, `remove` and `scan --mode hosted` warn
with the command that does, and
`apply --check` / `vendor --check` report the disagreement as drift until it is run:

| Warning code | When | Fix |
Expand Down
Loading