Fix Go replace written to unused go.work (#1810)

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).

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<String> = 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<String> = 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<PathBuf> {
     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.

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