diff --git a/crates/socket-patch-cli/tests/e2e_golang_workspace_build.rs b/crates/socket-patch-cli/tests/e2e_golang_workspace_build.rs index 9b4f52950..10a9cc7d3 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_workspace_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_workspace_build.rs @@ -1391,6 +1391,100 @@ fn shared_module_applied_once_in_go_work() { assert!(stdout.contains("A: PRISTINE"), "{stdout}\n{stderr}"); } +/// `GOWORK` naming another file beside the root's go.work (#1810): `apply` +/// never writes the replace into the root's go.work, which go doesn't +/// read. A root with no module refuses and writes nothing, with or without +/// a root go.work; a root module the workspace uses takes the replace in +/// its go.mod, and both members build the patched copy through it. +#[test] +fn apply_never_writes_go_work_when_gowork_names_a_sibling_file() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_workspace_build") + || !golang_e2e_matrix::supports_go_work() + { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let _guard = ChmodGuard(tmp.path().to_path_buf()); + let w = stage_members(tmp.path(), UVER, UVER); + let m = Machine::new(tmp.path(), "sibling"); + download(&w.root, &m, &w.proxy); + write_local_patch(&w.root, VENDOR_UUID); + let ci = w.root.join("ci.work"); + std::fs::write(&ci, "go 1.18\n\nuse (\n\t./a\n\t./b\n)\n").unwrap(); + std::fs::write(w.root.join("go.work"), "go 1.18\n\nuse ./a\n").unwrap(); + let gowork = [("GOWORK", ci.to_str().unwrap())]; + let cwd = w.root.to_str().unwrap(); + let apply = || { + socket_env( + &w.root, + &[ + "apply", + "--offline", + "--json", + "--ecosystems", + "golang", + "--cwd", + cwd, + ], + &m.modcache, + &gowork, + ) + }; + let files = |root: &Path| { + let mut all = snapshot(root); + all.push(("ci.work".into(), std::fs::read(root.join("ci.work")).ok())); + all + }; + + let before = files(&w.root); + let (code, out, stderr) = apply(); + assert_ne!(code, 0, "{out}\n{stderr}"); + let text = out.to_string(); + assert!(text.contains("has nowhere to go"), "{out}"); + assert!(text.contains("GOWORK selects"), "{out}"); + assert_snapshot(&w.root, &before, "no root module, root go.work"); + + std::fs::remove_file(w.root.join("go.work")).unwrap(); + let before = files(&w.root); + let (code, out, stderr) = apply(); + assert_ne!(code, 0, "{out}\n{stderr}"); + let text = out.to_string(); + assert!(text.contains("has nowhere to go"), "{out}"); + assert!(!text.contains("No such file"), "{out}"); + assert_snapshot(&w.root, &before, "no root module, no root go.work"); + + // A root module both workspace files use. + let root_mod = format!("module example.com/root\n\ngo 1.18\n\nrequire {UMOD} {UVER}\n"); + std::fs::write(w.root.join("go.mod"), &root_mod).unwrap(); + std::fs::write( + w.root.join("go.sum"), + format!( + "{UMOD} {UVER} {}\n{UMOD} {UVER}/go.mod {}\n", + w.upstream.0, w.upstream.1 + ), + ) + .unwrap(); + let work = "go 1.18\n\nuse (\n\t.\n\t./a\n\t./b\n)\n"; + std::fs::write(&ci, work).unwrap(); + std::fs::write(w.root.join("go.work"), work).unwrap(); + let (code, out, stderr) = apply(); + assert_eq!(code, 0, "{out}\n{stderr}"); + let copy = format!(".socket/go-patches/{UMOD}@{UVER}"); + assert_eq!( + read(&w.root.join("go.mod")), + format!("{root_mod}\nreplace {UMOD} {UVER} => ./{copy}\n") + ); + assert_eq!(read(&w.root.join("go.work")), work, "go.work untouched"); + assert_eq!(read(&ci), work, "ci.work untouched"); + for dir in ["a", "b"] { + let (stdout, stderr) = run_member(tmp.path(), &w.root, dir, &w.proxy, &gowork); + assert!( + stdout.contains(&format!("{}: PATCHED", dir.to_uppercase())), + "{dir}: {stdout}\n{stderr}" + ); + } +} + /// `get --mode hosted` from a go.work root, re-run: nothing changes, and /// the workspace builds the Socket module `-mod=readonly` from the /// committed go.work.sum on a fresh day-2 machine (bogus sumdb). diff --git a/crates/socket-patch-core/src/vendor/go_workspace.rs b/crates/socket-patch-core/src/vendor/go_workspace.rs index 50e3baf86..3c14447f5 100644 --- a/crates/socket-patch-core/src/vendor/go_workspace.rs +++ b/crates/socket-patch-core/src/vendor/go_workspace.rs @@ -294,9 +294,14 @@ pub enum WorkspaceConflict { /// The module is itself a `use` member (`member`): go builds it from /// that directory and ignores every replace of it. MainModule { work_file: String, member: String }, - /// The project is a go.work root with no module of its own: a replace - /// belongs in a member's go.mod (`members`, comma-separated). - NoModule { work_file: String, members: String }, + /// The project is in a workspace but has no module of its own: a + /// replace belongs in a member's go.mod (`members`, comma-separated). + /// `own_work`: the project holds a go.work that `GOWORK` passes over. + NoModule { + work_file: String, + members: String, + own_work: bool, + }, /// go refuses to load the workspace at all. Invalid { reason: String }, } @@ -364,11 +369,25 @@ impl WorkspaceConflict { builds {module} from that directory and ignores every replace of it; patch \ the source in {member}, or drop it from the `use` list of {work_file}" ), - WorkspaceConflict::NoModule { work_file, members } => format!( + WorkspaceConflict::NoModule { + work_file, + members, + own_work: false, + } => format!( "has nowhere to go: this directory is in the {work_file} workspace but holds \ neither go.work nor a go.mod of its own; run socket-patch in the workspace \ root or in a member ({members})" ), + WorkspaceConflict::NoModule { + work_file, + members, + own_work: true, + } => format!( + "has nowhere to go: GOWORK selects {work_file}, so go doesn't read this \ + directory's go.work, and the directory holds no go.mod of its own; unset \ + GOWORK (or point it at this go.work), or run socket-patch in a member \ + ({members})" + ), WorkspaceConflict::Invalid { reason } => { format!("cannot take effect: {reason}; fix the workspace setting and re-run") } @@ -638,9 +657,13 @@ impl GoWorkspace { [work, members] } - /// Whether go.work sits in `root`. + /// Whether this is `root`'s own `go.work`, the file the replace host + /// [`ReplaceHost::GoWork`] names. A `GOWORK` naming another file in + /// `root` (`ci.work`) is not: go doesn't read `root/go.work` then, so + /// a replace written there would do nothing (#1810). pub fn rooted_at(&self, root: &Path) -> bool { - same_dir_key(self.work_dir()) == same_dir_key(root) + self.work_file.file_name() == Some(std::ffi::OsStr::new("go.work")) + && same_dir_key(self.work_dir()) == same_dir_key(root) } /// Which file holds the Socket replace of `module` for the project at @@ -747,6 +770,21 @@ impl GoWorkspace { }); } let own = self.member_at(project_root); + // With no go.mod there is no file to write, even when the workspace + // already replaces the module the same way (#1810). + if own.is_none() && !project_root.join("go.mod").exists() { + let members: Vec = self + .members_within(project_root) + .into_iter() + .map(|(dir, _)| dir) + .filter(|dir| !dir.is_empty()) + .collect(); + return Some(WorkspaceConflict::NoModule { + work_file: display, + members: members.join(", "), + own_work: !self.rooted_at(project_root) && project_root.join("go.work").is_file(), + }); + } let our_target = lexical_target(own.map_or(project_root, |m| &m.dir), ours); // go reads go.work's replace for this exact version, then its @@ -778,18 +816,6 @@ impl GoWorkspace { } if own.is_none() { - if !project_root.join("go.mod").exists() { - let members: Vec = self - .members_within(project_root) - .into_iter() - .map(|(dir, _)| dir) - .filter(|dir| !dir.is_empty()) - .collect(); - return Some(WorkspaceConflict::NoModule { - work_file: display, - members: members.join(", "), - }); - } return Some(WorkspaceConflict::NotAMember { work_file: display }); } let project = same_dir_key(project_root); @@ -976,7 +1002,9 @@ pub async fn replace_host(project_root: &Path, module: &str) -> ReplaceHost { } /// The `go.work` in effect when it sits in `project_root`: the one whose -/// replaces are the project's own. +/// replaces are the project's own. Readers use it, so it includes a +/// `GOWORK` file beside the root's go.work (go reads its replaces); the +/// writers only ever write the root's go.work ([`GoWorkspace::rooted_at`]). pub fn root_work_file(project_root: &Path) -> Option { match resolve(project_root) { GoWork::File(file) @@ -1711,6 +1739,87 @@ mod tests { ); } + /// `GOWORK` naming another file beside the root's go.work (#1810): the + /// root's go.work is not the workspace go reads, so the replace never + /// goes there. A root that is a member keeps its go.mod; one with no + /// module is refused. The root's go.work named by path still is it. + #[tokio::test] + async fn a_sibling_gowork_file_is_not_the_roots_go_work() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let ci = root.join("ci.work"); + let ci_env = Some(ci.to_str().unwrap()); + write(root, "ci.work", "go 1.18\n\nuse (\n\t./a\n\t./b\n)\n"); + write(root, "go.work", "go 1.18\n\nuse ./a\n"); + member(root, "a", "1.21", &require("v1.0.0")); + member(root, "b", "1.21", &require("v1.0.0")); + let loaded = load_with(root, ci_env).await; + let Loaded::Workspace(ws) = &loaded else { + panic!("no workspace: {loaded:?}"); + }; + assert_eq!(same_dir_key(&ws.work_file), same_dir_key(&ci)); + assert!(!ws.rooted_at(root)); + assert_eq!(loaded.replace_host(root, false, DEP), ReplaceHost::GoMod); + let ours = replace(&format!( + "{DEP} v1.0.0 => ./.socket/go-patches/{DEP}@v1.0.0" + )); + assert!(matches!( + loaded.host_conflict(ReplaceHost::GoMod, root, DEP, "v1.0.0", &ours), + Some(WorkspaceConflict::NoModule { own_work: true, .. }) + )); + // Refused even when ci.work already replaces it the same way: + // there is no go.mod to write. + write( + root, + "ci.work", + &format!( + "go 1.18\n\nuse (\n\t./a\n\t./b\n)\n\nreplace {DEP} v1.0.0 => \ + ./.socket/go-patches/{DEP}@v1.0.0\n" + ), + ); + let loaded = load_with(root, ci_env).await; + assert!(matches!( + loaded.host_conflict(ReplaceHost::GoMod, root, DEP, "v1.0.0", &ours), + Some(WorkspaceConflict::NoModule { own_work: true, .. }) + )); + + // Without the root's go.work the same holds. + std::fs::remove_file(root.join("go.work")).unwrap(); + let loaded = load_with(root, ci_env).await; + assert_eq!(loaded.replace_host(root, false, DEP), ReplaceHost::GoMod); + assert!(matches!( + loaded.host_conflict(ReplaceHost::GoMod, root, DEP, "v1.0.0", &ours), + Some(WorkspaceConflict::NoModule { + own_work: false, + .. + }) + )); + + // A root module ci.work uses keeps (and is patched by) its go.mod. + write(root, "go.work", "go 1.18\n\nuse (\n\t.\n\t./a\n\t./b\n)\n"); + write(root, "ci.work", "go 1.18\n\nuse (\n\t.\n\t./a\n\t./b\n)\n"); + write( + root, + "go.mod", + &format!("module example.com/root\n\ngo 1.21\n{}", require("v1.0.0")), + ); + let loaded = load_with(root, ci_env).await; + assert_eq!(loaded.replace_host(root, true, DEP), ReplaceHost::GoMod); + assert_eq!( + load_with(root, None).await.replace_host(root, true, DEP), + ReplaceHost::GoWork, + "the root's own go.work shares it" + ); + let own = root.join("go.work"); + assert_eq!( + load_with(root, Some(own.to_str().unwrap())) + .await + .replace_host(root, true, DEP), + ReplaceHost::GoWork, + "GOWORK naming the root's go.work is the same workspace" + ); + } + /// A go.work replace overrides every member's: a user replace of the /// version (or of every version) in go.work or in a member refuses it; /// one of another version, or a Socket one, does not. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index feb2028eb..433f7cbc9 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1522,6 +1522,11 @@ module moves into `go.work` the next time `apply` or `vendor` writes it (a `vendor` takeover then deletes the `.socket/go-patches/` copy no file names any more); a hosted pin there stays, its sum lines beside it, and in workspace mode the `go.work` replace overrides it. +Only the directory's own `go.work` counts: when `GOWORK` names another file +in it (say `ci.work`), go doesn't read `go.work`, so socket-patch never +writes the replace there. It treats that file like a workspace file in +another directory: a module it `use`s takes the replace in its own `go.mod`, +and a directory with no module is refused ("has nowhere to go"). One `go.work` replace applies to every member. Member replaces of one project-local copy would not: go resolves each against its own member directory, sees two targets and refuses the workspace. The target is