diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index aff3ef6d8..a1c28e1a1 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -500,6 +500,113 @@ fn committed_vendor_dir_needs_go_mod_vendor_after_apply_and_rollback() { chmod_writable(tmp.path()); } +/// #1766: `go work vendor` records a non-root member's replace relative to +/// the workspace root (`./consumer/.socket/go-patches/…`). After rollback +/// or remove in that member every workspace build fails "inconsistent +/// vendoring", so both must warn and `apply --check` must report it. +#[test] +fn go_work_vendor_member_needs_go_work_vendor_after_rollback_and_remove() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-vendor)") { + return; + } + if golang_e2e_matrix::go_minor() < 22 { + eprintln!("SKIP: `go work vendor` needs Go 1.22+"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage(tmp.path()); + let ws = tmp.path(); + std::fs::write(ws.join("go.work"), "go 1.22\n\nuse ./consumer\n").unwrap(); + let cs = consumer.to_str().unwrap(); + let mc = modcache.to_str().unwrap(); + let mut goenv = go_env(mc, &proxy_url); + goenv.retain(|(k, _)| *k != "GOFLAGS"); + goenv.push(("GOFLAGS", "")); + let vendor = |what: &str| { + let out = go(ws, &["work", "vendor"], &goenv); + assert!( + out.status.success(), + "go work vendor ({what}): {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let warns = |args: &[&str], what: &str| { + let (code, so, se) = run_socket(&consumer, args, &modcache); + assert_eq!(code, 0, "{what} failed.\n{so}\n{se}"); + assert!( + warnings_of(&so).contains(&"go_vendor_modules_txt_out_of_sync".to_string()), + "{what} must name the vendor/modules.txt regeneration:\n{so}" + ); + }; + let apply = ["apply", "--offline", "--json", "--cwd", cs]; + vendor("before apply"); + + write_patch(&consumer); + warns(&apply, "apply"); + vendor("after apply"); + let run = go(ws, &["run", "./consumer"], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: PATCHED"), + "{}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + let modules_txt = std::fs::read_to_string(ws.join("vendor/modules.txt")).unwrap(); + assert!( + modules_txt.contains("=> ./consumer/.socket/go-patches/"), + "the fixture records the member replace relative to go.work:\n{modules_txt}" + ); + + for (what, args) in [ + ( + "rollback", + vec!["rollback", "--offline", "--json", "--cwd", cs], + ), + ( + "remove", + vec!["remove", UPURL, "--offline", "--json", "--cwd", cs], + ), + ] { + warns(&args, what); + let broken = go(ws, &["build", "./consumer"], &goenv); + assert!( + !broken.status.success() + && String::from_utf8_lossy(&broken.stderr).contains("inconsistent vendoring"), + "{what}: the fixture reproduces the broken build: {}", + String::from_utf8_lossy(&broken.stderr) + ); + let (code, _so, se) = run_socket( + &consumer, + &["apply", "--check", "--ecosystems", "golang", "--cwd", cs], + &modcache, + ); + assert_eq!(code, 1, "{what}: stale vendor/modules.txt is drift:\n{se}"); + assert!( + se.contains("no longer has") && se.contains("go work vendor"), + "{what}: {se}" + ); + + // The step the warning names fixes the build. + vendor(what); + let run = go(ws, &["run", "./consumer"], &goenv); + assert!( + String::from_utf8_lossy(&run.stdout).contains("OUT: PRISTINE"), + "{what}: {}{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + if what == "rollback" { + // Re-apply and re-vendor so `remove` starts from a patched, + // in-sync workspace (rollback dropped the manifest entry). + write_patch(&consumer); + warns(&apply, "re-apply"); + vendor("after re-apply"); + } + } + + chmod_writable(tmp.path()); +} + /// #618: a patch whose go.mod raises a requirement (the common security-fix /// shape) leaves the default `-mod=readonly` build failing with "updates /// to go.mod needed". apply names `go mod tidy`, `apply --check` reports diff --git a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs index b2b7e3c37..d2bdab85e 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -338,6 +338,9 @@ struct Member { /// directory (go resolves the member's relative replaces against it). dir: PathBuf, replaces: Vec, + /// Whether the member's go.mod could be read (else `replaces` is empty + /// because it is unknown, not because there are none). + readable: bool, } async fn load_workspace(project_root: &Path) -> Option { @@ -345,13 +348,18 @@ async fn load_workspace(project_root: &Path) -> Option { let text = read_work_file(&work_file).await?; let mut members = Vec::new(); for dir in use_dirs(&work_file, &text) { - let replaces = match read_regular_to_string(&dir.join("go.mod")).await { - Ok(go_mod) => { - go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod)) - } - Err(_) => Vec::new(), + let (replaces, readable) = match read_regular_to_string(&dir.join("go.mod")).await { + Ok(go_mod) => ( + go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod)), + true, + ), + Err(_) => (Vec::new(), false), }; - members.push(Member { dir, replaces }); + members.push(Member { + dir, + replaces, + readable, + }); } Some(Workspace { replaces: go_mod_edit::parse_replace_entries(&text), @@ -360,19 +368,6 @@ async fn load_workspace(project_root: &Path) -> Option { }) } -/// The replaces a workspace `modules.txt` records: `go work vendor` writes -/// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns -/// `(go.work replaces, every member's replaces)`. -async fn workspace_replaces(project_root: &Path) -> (Vec, Vec) { - match load_workspace(project_root).await { - Some(ws) => { - let members = ws.members.into_iter().flat_map(|m| m.replaces).collect(); - (ws.replaces, members) - } - None => (Vec::new(), Vec::new()), - } -} - /// Whether the `go.work` in effect for `project_root` keeps a project /// `go.mod` replace of `module@version` => `ours` (a replace as parsed /// from, or about to be written to, that go.mod) from taking effect. @@ -567,11 +562,52 @@ async fn vendor_modules_txt_issues( .collect(); // In a workspace, a `go.work` replace of the same module overrides the - // member's, and a recorded replacement may come from any member. - let (work_replaces, member_replaces) = if workspace { - workspace_replaces(project_root).await + // member's, and a recorded replacement may come from any member: + // `go work vendor` writes every `use` member's `go.mod` replaces plus + // the `go.work` ones, each resolved against its own file's directory + // and recorded relative to the go.work directory. Outside a workspace + // `go mod vendor` records the go.mod's replaces verbatim. + let ws = if workspace { + load_workspace(project_root).await } else { - (Vec::new(), Vec::new()) + None + }; + // modules.txt sits in `/vendor/`. + let work_dir = path.parent().and_then(Path::parent).unwrap_or(project_root); + let project = same_dir_key(project_root); + let project_dir = ws + .iter() + .flat_map(|w| &w.members) + .find(|m| same_dir_key(&m.dir) == project) + .map_or(project_root, |m| m.dir.as_path()); + let work_replaces: &[ReplaceEntry] = ws.as_ref().map_or(&[], |w| &w.replaces); + // Every replace modules.txt may record, with the directory go resolves + // it against. + let sources: Vec<(&Path, &ReplaceEntry)> = replaces + .iter() + .map(|e| (project_dir, e)) + .chain( + ws.iter() + .flat_map(|w| &w.members) + .flat_map(|m| m.replaces.iter().map(|e| (m.dir.as_path(), e))), + ) + .chain(work_replaces.iter().map(|e| (work_dir, e))) + .collect(); + // Members whose socket copies a recorded line may point into; one whose + // go.mod can't be read is left out, since its replaces are unknown. + let member_dirs: Vec = ws + .iter() + .flat_map(|w| &w.members) + .filter(|m| m.readable) + .map(|m| { + let abs = std::env::current_dir().unwrap_or_default().join(&m.dir); + crate::utils::relpath::normalize_lexically_keeping_escapes(&abs) + }) + .collect(); + let records = |dir: &Path, entry: &ReplaceEntry, rec: &ReplaceEntry| { + entry.module == rec.module + && entry.version == rec.version + && lexical_target(dir, entry) == lexical_target(work_dir, rec) }; let overridden_by_work = |entry: &ReplaceEntry| { work_replaces.iter().any(|w| { @@ -581,11 +617,7 @@ async fn vendor_modules_txt_issues( let mut issues = Vec::new(); for entry in replaces.iter().filter(|e| e.socket_owned()) { - if !overridden_by_work(entry) - && !recorded - .iter() - .any(|r| same_replacement(entry, r, workspace)) - { + if !overridden_by_work(entry) && !recorded.iter().any(|r| records(project_dir, entry, r)) { issues.push(GoSyncIssue::VendorModulesTxt { module: entry.module.clone(), modules_txt: display.clone(), @@ -594,13 +626,11 @@ async fn vendor_modules_txt_issues( }); } } - for rec in recorded.iter().filter(|r| r.socket_owned()) { - if !replaces - .iter() - .chain(&member_replaces) - .chain(&work_replaces) - .any(|e| same_replacement(e, rec, workspace)) - { + for rec in recorded + .iter() + .filter(|r| r.socket_owned() || member_owned(&member_dirs, work_dir, r)) + { + if !sources.iter().any(|(dir, e)| records(dir, e, rec)) { issues.push(GoSyncIssue::VendorModulesTxt { module: rec.module.clone(), modules_txt: display.clone(), @@ -612,28 +642,21 @@ async fn vendor_modules_txt_issues( issues } -/// Whether a `modules.txt` replacement records the `go.mod` one. go writes -/// the target verbatim from `go.mod`; a workspace vendor directory may -/// record a module's directory replacement relative to the workspace root, -/// so there the recorded path only has to end with the module's. -fn same_replacement(go_mod: &ReplaceEntry, recorded: &ReplaceEntry, workspace: bool) -> bool { - let norm = |p: &str| { - let p = p.replace('\\', "/"); - p.strip_prefix("./").unwrap_or(&p).to_string() - }; - go_mod.module == recorded.module - && go_mod.version == recorded.version - && match (&go_mod.path, &recorded.path) { - (Some(a), Some(b)) => { - let (a, b) = (norm(a), norm(b)); - a == b || (workspace && b.ends_with(&format!("/{a}"))) - } - (None, None) => { - go_mod.rhs_module == recorded.rhs_module - && go_mod.rhs_version == recorded.rhs_version - } - _ => false, - } +/// Whether a workspace `modules.txt` replacement points into a `use` +/// member's socket copies. `go work vendor` records a member's directory +/// replace relative to the go.work directory (`./a/.socket/go-patches/…`), +/// which [`go_mod_edit::detect_owner`] alone, anchored at one go.mod's +/// directory, reads as user-authored. +fn member_owned(member_dirs: &[PathBuf], work_dir: &Path, recorded: &ReplaceEntry) -> bool { + if recorded.path.is_none() { + return false; + } + let target = PathBuf::from(lexical_target(work_dir, recorded)); + member_dirs.iter().any(|dir| { + target + .strip_prefix(dir) + .is_ok_and(|rel| go_mod_edit::detect_owner(&rel.to_string_lossy()).is_some()) + }) } async fn requirement_issues( @@ -851,6 +874,113 @@ mod tests { assert_eq!(codes, vec![WORKSPACE_CODE]); } + /// #1766: `go work vendor` records a non-root member's replace relative + /// to the workspace root (`./a/.socket/…`). After a rollback or remove + /// in that member, the recorded line is stale and the workspace build + /// fails "inconsistent vendoring", so the audit must say so, for either + /// socket backend and a member nested or outside the go.work directory. + #[tokio::test] + async fn workspace_member_rollback_leaves_modules_txt_stale() { + let vendored = ".socket/vendor/golang/11111111-1111-4111-8111-111111111111/\ + example.com/upstream@v1.0.0"; + for (use_dir, copy) in [ + ("a", COPY), + ("a", vendored), + ("sub/a", COPY), + ("../a", COPY), + ] { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + let member = ws.join(use_dir); + // How go spells a path relative to the go.work directory. + let rel = if use_dir.starts_with("../") { + use_dir.to_string() + } else { + format!("./{use_dir}") + }; + consumer(&member, ""); + write(&ws, "go.work", &format!("go 1.22\n\nuse {rel}\n")); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => {rel}/{copy}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + let issues = audit(&member, &HashMap::new()).await; + assert!( + matches!( + &issues[..], + [GoSyncIssue::VendorModulesTxt { stale: true, command: "go work vendor", module, .. }] + if module == "example.com/upstream" + ), + "{use_dir} {copy}: {issues:?}" + ); + + // Applied again: the recorded line matches the member's replace. + consumer( + &member, + &format!("\nreplace example.com/upstream v1.0.0 => ./{copy}\n"), + ); + assert_eq!( + audit(&member, &HashMap::new()).await, + vec![], + "{use_dir} {copy}" + ); + } + + // Rolled back in `a`, applied in `b`, not re-vendored: `b`'s replace + // has the same spelling as `a`'s recorded one but another target, + // so the line is stale and `b`'s is not recorded. + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + consumer(&ws.join("a"), ""); + consumer(&ws.join("b"), REPLACE); + write(&ws, "go.work", "go 1.22\n\nuse (\n\t./a\n\t./b\n)\n"); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => ./a/{COPY}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + let mut stale: Vec = audit(&ws.join("b"), &HashMap::new()) + .await + .iter() + .filter_map(|i| match i { + GoSyncIssue::VendorModulesTxt { stale, .. } => Some(*stale), + _ => None, + }) + .collect(); + stale.sort(); + assert_eq!(stale, vec![false, true]); + + // A member whose go.mod can't be read: its recorded line is not + // judged stale from another member, since its replaces are unknown. + std::fs::remove_file(ws.join("a/go.mod")).unwrap(); + std::fs::create_dir(ws.join("a/go.mod")).unwrap(); + consumer(&ws.join("b"), ""); + assert_eq!(audit(&ws.join("b"), &HashMap::new()).await, vec![]); + + // A user's replace onto another checkout's copy is not ours, even + // when the path has a `.socket/go-patches/` segment. + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path().join("ws"); + consumer(&ws.join("a"), ""); + write(&ws, "go.work", "go 1.22\n\nuse ./a\n"); + write( + &ws, + "vendor/modules.txt", + &format!( + "# example.com/upstream v1.0.0 => ../sibling/{COPY}\n\ + ## explicit; go 1.21\nexample.com/upstream\n" + ), + ); + assert_eq!(audit(&ws.join("a"), &HashMap::new()).await, vec![]); + } + fn codes(issues: &[GoSyncIssue]) -> Vec<&'static str> { issues.iter().map(GoSyncIssue::code).collect() }