From 6e52befac5f839ad9cd43a7f18ba167799cb4406 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:22:28 +0000 Subject: [PATCH 1/7] Start fix for #1751 Assisted-by: Claude Code:claude-opus-5-5 From 3d994be43fa79a3feeb3b515dbca9b8f0bc016c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:35:10 +0000 Subject: [PATCH 2/7] Check other go.work members before Go patching In a go.work workspace Go builds the highest version any member requires. The version check behind apply, apply --check, vendor and vex read only the current module's go.mod, so when another member required a newer version, apply wrote a replace the build ignored, apply --check said "in sync" and vex attested not_affected for a module the workspace still built unpatched. The check now reads every `use` member's go.mod and go.sum and treats a higher version anywhere in the workspace as the one built. apply and vendor refuse, apply --check fails and vex leaves the patch out (go_version_not_selected). Fixes #1751. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_golang_build.rs | 94 ++++++++++++ .../src/vendor/go_build_list.rs | 139 ++++++++++++++++-- .../src/vendor/go_consumer_sync.rs | 15 ++ 3 files changed, 233 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index ad8515f65..aff3ef6d8 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -983,6 +983,100 @@ fn go_work_member_replaces_must_not_conflict() { chmod_writable(tmp.path()); } +/// #1751: a go.work build list spans every `use` member's requirements, so +/// when member b requires a newer upstream than a, the workspace builds a +/// with b's version and a replace pinned to a's is inert. apply in a must +/// refuse, and a replace written before the go.work appeared fails +/// `apply --check` and is left out of VEX (`go_version_not_selected`). +#[test] +fn go_work_member_requiring_a_newer_version_makes_the_replace_inert() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-newer-member)") { + return; + } + const NEWER: &str = "v1.1.0"; + let tmp = tempfile::tempdir().unwrap(); + let (_consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + publish( + tmp.path(), + UMOD, + NEWER, + &[ + ("go.mod", &format!("module {UMOD}\n\ngo 1.21\n")), + ( + "lib.go", + "package upstream\n\nfunc Greeting() string { return \"PRISTINE-v1.1.0\" }\n", + ), + ], + ); + let ws = tmp.path().join("ws"); + let (a, b) = (ws.join("a"), ws.join("b")); + consumer_module(&a, "example.com/a", mc, &proxy_url); + consumer_module(&b, "example.com/b", mc, &proxy_url); + let go_mod = std::fs::read_to_string(b.join("go.mod")).unwrap(); + std::fs::write( + b.join("go.mod"), + go_mod.replace(&format!("{UMOD} {UVER}"), &format!("{UMOD} {NEWER}")), + ) + .unwrap(); + let mut tidy_env = go_env(mc, &proxy_url); + tidy_env.push(("GOWORK", "off")); + let tidy = go(&b, &["mod", "tidy"], &tidy_env); + assert!( + tidy.status.success(), + "go mod tidy: {}", + String::from_utf8_lossy(&tidy.stderr) + ); + write_patch(&a); + let go_work = "go 1.21\n\nuse (\n\t./a\n\t./b\n)\n"; + std::fs::write(ws.join("go.work"), go_work).unwrap(); + let env = workspace_env(mc, &proxy_url); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + + let (code, so, se) = apply_json(&a, &modcache); + assert_ne!(code, 0, "apply must refuse an inert replace:\n{so}\n{se}"); + assert!( + so.contains(&format!("the build selects {UMOD} {NEWER}")), + "{so}" + ); + assert!(replace_lines(&a.join("go.mod")).is_empty()); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + + // Applied before the go.work existed: the replace is inert. + std::fs::remove_file(ws.join("go.work")).unwrap(); + let (code, so, se) = apply_json(&a, &modcache); + assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); + assert!(go_out(&a, &tidy_env).contains("OUT: PATCHED")); + std::fs::write(ws.join("go.work"), go_work).unwrap(); + assert!(go_out(&a, &env).contains("OUT: PRISTINE-v1.1.0")); + let (code, so, se) = check(&a, &modcache); + assert_eq!( + code, 1, + "apply --check must flag the inert replace:\n{so}\n{se}" + ); + let r = VexRun { + offline: true, + product: Some("pkg:golang/example.com/a@v0.0.1".into()), + ..VexRun::default() + } + .env("GOMODCACHE", &modcache) + .env("GOFLAGS", ""); + let out = run_vex(&vex_e2e_common::binary(), &a, &r); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected"); + + // Positive control: once b is back on a's version, the replace patches + // the whole workspace and nothing is flagged. + std::fs::write(b.join("go.mod"), &go_mod).unwrap(); + let tidy = go(&b, &["mod", "tidy"], &tidy_env); + assert!(tidy.status.success()); + assert!(go_out(&b, &env).contains("OUT: PATCHED")); + let (code, so, se) = check(&a, &modcache); + assert_eq!(code, 0, "{so}\n{se}"); + + chmod_writable(tmp.path()); +} + // ── global mode: tools already built with `go install` (#422) ── const TOOL: &str = "example.com/tool"; diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index 45a8f881e..4aee4a4f3 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -106,26 +106,64 @@ pub fn select(pruned: bool, required: Option<&str>, zips: &[&str], version: &str } /// [`selection`] for a project directory: reads `/go.mod` and -/// `go.sum` (`None` when there is no readable go.mod). In workspace mode the -/// build list also takes other members' requirements, so a module absent -/// from this go.mod may still be built: [`Selection::NotInGraph`] becomes -/// [`Selection::Unknown`] there. +/// `go.sum` (`None` when there is no readable go.mod). +/// +/// In workspace mode the build list also takes every `use` member's +/// requirements, and MVS selects the highest version any of them asks for +/// (#1751). So another member's higher version is the one built, and a +/// module absent from this go.mod may still be built: +/// [`Selection::NotInGraph`] becomes [`Selection::Unknown`] there unless a +/// member proves the version. pub async fn project_selection( project_root: &Path, module: &str, version: &str, ) -> Option { - let go_mod = read_regular_to_string(&project_root.join("go.mod")) - .await - .ok()?; - let go_sum = read_regular_to_string(&project_root.join("go.sum")) - .await - .ok(); - let sel = selection(&go_mod, go_sum.as_deref(), module, version); - if sel == Selection::NotInGraph && super::go_consumer_sync::workspace_in_effect(project_root) { - return Some(Selection::Unknown); - } - Some(sel) + let own = dir_selection(project_root, module, version).await?; + let Some(members) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { + if own == Selection::NotInGraph + && super::go_consumer_sync::workspace_in_effect(project_root) + { + return Some(Selection::Unknown); + } + return Some(own); + }; + let mut highest = evidence(&own, version); + for dir in members { + let Some(member) = dir_selection(&dir, module, version).await else { + continue; + }; + if let Some(v) = evidence(&member, version) { + if highest + .as_deref() + .is_none_or(|h| go_semver_cmp(&v, h) == Ordering::Greater) + { + highest = Some(v); + } + } + } + Some(match highest { + Some(h) if go_semver_cmp(&h, version) == Ordering::Greater => Selection::Other(h), + Some(h) if h == version => Selection::Selected, + _ if own == Selection::NotInGraph => Selection::Unknown, + _ => own, + }) +} + +/// [`selection`] from `/go.mod` and `go.sum`, ignoring any workspace. +async fn dir_selection(dir: &Path, module: &str, version: &str) -> Option { + let go_mod = read_regular_to_string(&dir.join("go.mod")).await.ok()?; + let go_sum = read_regular_to_string(&dir.join("go.sum")).await.ok(); + Some(selection(&go_mod, go_sum.as_deref(), module, version)) +} + +/// The version a module's own go.mod and go.sum prove it builds, if any. +fn evidence(sel: &Selection, version: &str) -> Option { + match sel { + Selection::Selected => Some(version.to_string()), + Selection::Other(v) => Some(v.clone()), + Selection::NotInGraph | Selection::Unknown => None, + } } #[cfg(test)] @@ -226,6 +264,77 @@ mod tests { assert!(!prunes_module_graph("module c\ngodebug default=go1.21\n")); } + fn module(dir: &Path, name: &str, version: &str) { + std::fs::create_dir_all(dir).unwrap(); + let m = format!("module example.com/{name}\n\ngo 1.21\n\nrequire {UP} {version}\n"); + std::fs::write(dir.join("go.mod"), m).unwrap(); + let s = sum(&[ + &format!("{UP} {version}"), + &format!("{UP} {version}/go.mod"), + ]); + std::fs::write(dir.join("go.sum"), s).unwrap(); + } + + /// #1751: a go.work build list takes every `use` member's requirements, + /// so another member's higher version is the one built, and a replace + /// pinned to this member's lower version is inert. + #[tokio::test] + async fn workspace_member_requiring_a_higher_version_wins() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b) = (ws.join("a"), ws.join("b")); + module(&a, "a", "v1.0.0"); + module(&b, "b", "v1.1.0"); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Selected), + "outside a workspace only a's go.mod counts" + ); + std::fs::write(ws.join("go.work"), "go 1.21\n\nuse (\n\t./a\n\t./b\n)\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + // b's own version is the workspace's. + assert_eq!( + project_selection(&b, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + // A member that doesn't require the module is built at the version + // another member selects. + std::fs::write(a.join("go.mod"), "module example.com/a\n\ngo 1.21\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + } + + /// A member requiring a lower version doesn't move the workspace off + /// this member's higher one, and a member go.work doesn't `use` counts + /// for nothing. + #[tokio::test] + async fn workspace_lower_or_unused_members_do_not_change_the_selection() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); + module(&a, "a", "v1.1.0"); + module(&b, "b", "v1.0.0"); + module(&c, "c", "v1.2.0"); + std::fs::write(ws.join("go.work"), "go 1.21\n\nuse ./a\nuse ./b\n").unwrap(); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Selected) + ); + assert_eq!( + project_selection(&b, UP, "v1.0.0").await, + Some(Selection::Other("v1.1.0".into())) + ); + } + #[test] fn incompatible_versions_compare_exactly() { let m = format!("module c\n\ngo 1.21\n\nrequire {UP} v2.0.0+incompatible\n"); 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 b51638281..10d46d8d8 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -305,6 +305,21 @@ fn workspace_file(project_root: &Path) -> Option { .find(|file| file.is_file()) } +/// The `use` member directories of the `go.work` in effect for +/// `project_root` (joined to go.work's directory), or `None` when no +/// workspace is in effect or its go.work can't be read. +pub(crate) async fn workspace_member_dirs(project_root: &Path) -> Option> { + let work_file = workspace_file(project_root)?; + let text = read_regular_to_string(&work_file).await.ok()?; + let work_dir = work_file.parent().unwrap_or(Path::new("")); + Some( + go_mod_edit::parse_use_dirs(&go_mod_edit::normalize_for_read(&text)) + .into_iter() + .map(|dir| work_dir.join(dir)) + .collect(), + ) +} + /// The `go.work` in effect, read once: its own replaces and each `use` /// member's go.mod replaces. struct Workspace { From 10f5e26313eac26007240024efda750a3a4e61b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:41:24 +0000 Subject: [PATCH 3/7] Weigh lower workspace versions in the Go check When every go.work member proves a version below the patched one and none is undecided, the workspace builds the highest of them, so the refusal now names that version instead of this module's own or "unknown". Reading go.work's members is shared with the conflict check, and the Go workspace docs say which commands read the other members. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/vendor/go_build_list.rs | 56 ++++++++++++++++--- .../src/vendor/go_consumer_sync.rs | 37 +++++++----- docs/ecosystems.md | 6 +- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index 4aee4a4f3..fce0b9f43 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -110,8 +110,8 @@ pub fn select(pruned: bool, required: Option<&str>, zips: &[&str], version: &str /// /// In workspace mode the build list also takes every `use` member's /// requirements, and MVS selects the highest version any of them asks for -/// (#1751). So another member's higher version is the one built, and a -/// module absent from this go.mod may still be built: +/// (#1751). So a version another member proves is weighed with this one's, +/// and a module absent from this go.mod may still be built: /// [`Selection::NotInGraph`] becomes [`Selection::Unknown`] there unless a /// member proves the version. pub async fn project_selection( @@ -121,18 +121,17 @@ pub async fn project_selection( ) -> Option { let own = dir_selection(project_root, module, version).await?; let Some(members) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { - if own == Selection::NotInGraph - && super::go_consumer_sync::workspace_in_effect(project_root) - { - return Some(Selection::Unknown); - } return Some(own); }; let mut highest = evidence(&own, version); + // A member whose files can't tell may build a higher version than the + // highest one proven, so then only a proof at or above ours decides. + let mut unknown = own == Selection::Unknown; for dir in members { let Some(member) = dir_selection(&dir, module, version).await else { continue; }; + unknown |= member == Selection::Unknown; if let Some(v) = evidence(&member, version) { if highest .as_deref() @@ -143,8 +142,10 @@ pub async fn project_selection( } } Some(match highest { - Some(h) if go_semver_cmp(&h, version) == Ordering::Greater => Selection::Other(h), Some(h) if h == version => Selection::Selected, + Some(h) if !unknown || go_semver_cmp(&h, version) == Ordering::Greater => { + Selection::Other(h) + } _ if own == Selection::NotInGraph => Selection::Unknown, _ => own, }) @@ -313,6 +314,45 @@ mod tests { ); } + /// A version below the patched one counts when it is the highest any + /// member proves, unless a member's files can't tell. + #[tokio::test] + async fn workspace_lower_versions_decide_only_without_unknown_members() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); + module(&a, "a", "v1.0.0"); + module(&b, "b", "v1.0.5"); + std::fs::create_dir_all(&c).unwrap(); + std::fs::write(c.join("go.mod"), "module example.com/c\n\ngo 1.21\n").unwrap(); + std::fs::write( + ws.join("go.work"), + "go 1.21\n\nuse (\n\t./a\n\t./b\n\t./c\n)\n", + ) + .unwrap(); + for project in [&a, &c] { + assert_eq!( + project_selection(project, UP, "v1.1.0").await, + Some(Selection::Other("v1.0.5".into())) + ); + } + // An old-style member with only a lower bound may build anything. + std::fs::write( + b.join("go.mod"), + format!("module example.com/b\n\ngo 1.16\n\nrequire {UP} v1.0.5\n"), + ) + .unwrap(); + std::fs::write(b.join("go.sum"), "").unwrap(); + assert_eq!( + project_selection(&c, UP, "v1.1.0").await, + Some(Selection::Unknown) + ); + assert_eq!( + project_selection(&a, UP, "v1.1.0").await, + Some(Selection::Other("v1.0.0".into())) + ); + } + /// A member requiring a lower version doesn't move the workspace off /// this member's higher one, and a member go.work doesn't `use` counts /// for nothing. 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 10d46d8d8..c7978c2ae 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -306,18 +306,30 @@ fn workspace_file(project_root: &Path) -> Option { } /// The `use` member directories of the `go.work` in effect for -/// `project_root` (joined to go.work's directory), or `None` when no -/// workspace is in effect or its go.work can't be read. +/// `project_root`: `None` when no workspace is in effect, empty when its +/// go.work can't be read. pub(crate) async fn workspace_member_dirs(project_root: &Path) -> Option> { let work_file = workspace_file(project_root)?; - let text = read_regular_to_string(&work_file).await.ok()?; + Some(match read_work_file(&work_file).await { + Some(text) => use_dirs(&work_file, &text), + None => Vec::new(), + }) +} + +/// `work_file`'s text, normalized for parsing. +async fn read_work_file(work_file: &Path) -> Option { + let text = read_regular_to_string(work_file).await.ok()?; + Some(go_mod_edit::normalize_for_read(&text)) +} + +/// The `use` directories of go.work `text`, joined to `work_file`'s +/// directory. +fn use_dirs(work_file: &Path, text: &str) -> Vec { let work_dir = work_file.parent().unwrap_or(Path::new("")); - Some( - go_mod_edit::parse_use_dirs(&go_mod_edit::normalize_for_read(&text)) - .into_iter() - .map(|dir| work_dir.join(dir)) - .collect(), - ) + go_mod_edit::parse_use_dirs(text) + .into_iter() + .map(|dir| work_dir.join(dir)) + .collect() } /// The `go.work` in effect, read once: its own replaces and each `use` @@ -337,12 +349,9 @@ struct Member { async fn load_workspace(project_root: &Path) -> Option { let work_file = workspace_file(project_root)?; - let text = read_regular_to_string(&work_file).await.ok()?; - let text = go_mod_edit::normalize_for_read(&text); - let work_dir = work_file.parent().unwrap_or(Path::new("")).to_path_buf(); + let text = read_work_file(&work_file).await?; let mut members = Vec::new(); - for dir in go_mod_edit::parse_use_dirs(&text) { - let dir = work_dir.join(dir); + 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)) diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 62b0b8556..b840a0058 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1513,7 +1513,11 @@ and `go.sum`: the `require` line in a go 1.17+ module, or, in an older module (whose `require` lists only lower bounds and leaves out transitive modules), the highest of the required version and the versions `go.sum` has a module zip line for. `apply` and `scan --mode hosted` refuse to write a replace for -any other version. After an upgrade such as `go get M@v1.0.1`, `apply --check` +any other version. In a workspace the build takes the highest version any +`use` member selects, so `apply`, `vendor`, `apply --check` and `vex` read +every member's `go.mod` and `go.sum` too: a member that requires a newer +version makes a replace pinned to the older one inert. `scan --mode hosted` +still reads only the module it runs in (#1753). After an upgrade such as `go get M@v1.0.1`, `apply --check` fails and `vex` leaves the patch out (`go_version_not_selected`) until you get a patch for the new version. From ac7071b5ab761cc8e6897d6fb89be1c52f1f3ff6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:46:00 +0000 Subject: [PATCH 4/7] Drop the unused Go workspace_in_effect helper The workspace member list now answers whether a go.work is in effect, so the separate check has no callers left. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vendor/go_consumer_sync.rs | 7 ------- 1 file changed, 7 deletions(-) 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 c7978c2ae..b2b7e3c37 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -280,13 +280,6 @@ fn modules_txt_location(project_root: &Path) -> (PathBuf, &'static str) { ) } -/// Whether the go command builds `project_root` in workspace mode (a -/// `go.work` from `GOWORK` or at or above the project), so its build list -/// spans every member's requirements, not only the project go.mod's. -pub(crate) fn workspace_in_effect(project_root: &Path) -> bool { - workspace_file(project_root).is_some() -} - /// The `go.work` go would use for `project_root`: `GOWORK` (resolved as go /// resolves it: the environment, else the `go env -w` file) when it names a /// file (`off` disables workspaces), else the nearest `go.work` at or above From 4a3d52cef58b31ac84f923f089f51a32d5faabeb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:23:36 +0000 Subject: [PATCH 5/7] Start fix for #1753 Assisted-by: Claude Code:claude-opus-5-5 From 18fac8a3273bd5e3570ce145639d9c619f77403a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:41:06 +0000 Subject: [PATCH 6/7] Refuse hosted Go pins other go.work members beat In a go.work workspace the build uses the highest version any member requires. `scan --mode hosted` only read the module it ran in, so in member a it pinned v1.0.0 while member b's v1.1.0 is what gets built, and reported the module as redirected though the pin did nothing. The hosted Go rewriter now also weighs the other members' go.mod and go.sum, the same way agent apply, apply --check and vex do. It refuses such a pin with redirect_golang_version_mismatch, and a rescan removes a pin it wrote before another member moved ahead. The in-memory engine has no workspace on disk and is unchanged. Fixes #1753 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/commands/scan/hosted.rs | 5 + .../tests/e2e_golang_hosted_state.rs | 66 ++++++++++++ crates/socket-patch-core/src/hosted/engine.rs | 24 +++++ .../src/hosted/memory/stages.rs | 1 + .../redirect/golang_equivalence_tests.rs | 2 +- .../patch/redirect/group_equivalence_tests.rs | 3 +- .../src/patch/redirect/mod.rs | 100 +++++++++++++++++- .../src/vendor/go_build_list.rs | 96 ++++++++++++++--- .../src/vendor/go_consumer_sync.rs | 12 +++ docs/ecosystems.md | 13 +-- 10 files changed, 298 insertions(+), 24 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index edbbc1f99..8ccbe7f54 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -1358,6 +1358,10 @@ async fn run_redirect_selected_untracked( // parent directory's and `$CARGO_HOME`'s): a directory source or local // registry in any of them refuses the cargo rewrite like a project one. let cargo_outer = || CargoCrawler::crates_io_source_for(&common.cwd); + // The other members of the go.work in effect: the workspace build + // selects the highest version any of them requires. + let go_workspace = + || socket_patch_core::vendor::go_build_list::other_workspace_members(&common.cwd); // The scan runs in the install's environment: a pnpm setting there // (`pnpm_config_trust_lockfile`) overrides the project's files. let pnpm_env = |var: &str| std::env::var(var).ok(); @@ -1375,6 +1379,7 @@ async fn run_redirect_selected_untracked( npm_outer: &npm_outer, yarn_classic_outer: &yarn_classic_outer, cargo_outer: &cargo_outer, + go_workspace: &go_workspace, pnpm_env: &pnpm_env, blocking: true, takeover_uuids: takeover_uuids.clone(), diff --git a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs index 2c2aed542..c27a0e023 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs @@ -267,6 +267,72 @@ async fn leftover_go_sum_lines_do_not_confirm_a_refused_rewrite() { assert!(!ledger.contains(UPURL), "{ledger}"); } +/// #1753: in a go.work workspace the build selects the highest version any +/// `use` member requires. Once another member requires v1.1.0, a hosted +/// pin for this member's v1.0.0 is inert: the scan must refuse to write it +/// and take back the one it wrote before, never report it as redirected. +#[tokio::test(flavor = "multi_thread")] +async fn hosted_pin_another_go_work_member_outranks_is_refused() { + let tmp = tempfile::tempdir().unwrap(); + let ws = tmp.path().join("ws"); + let consumer = ws.join("consumer"); + write_consumer(&consumer, &format!("require {UMOD} {UVER}\n"), UPSTREAM_SUM); + let other = ws.join("other"); + let other_mod = + |v: &str| format!("module example.com/other\n\ngo 1.21\n\nrequire {UMOD} {v}\n"); + std::fs::create_dir_all(&other).unwrap(); + std::fs::write(other.join("go.mod"), other_mod(UVER)).unwrap(); + std::fs::write( + ws.join("go.work"), + "go 1.21\n\nuse (\n\t./consumer\n\t./other\n)\n", + ) + .unwrap(); + let server = MockServer::start().await; + mount_hosted_grant(&server).await; + let modcache = tmp.path().join("modcache"); + let mismatch = |env: &serde_json::Value| { + env["warnings"] + .as_array() + .unwrap() + .iter() + .any(|w| w["code"] == "redirect_golang_version_mismatch") + }; + + // Control: the other member requires the same version, so the pin is + // the one the workspace builds. + let env = get_hosted(&consumer, &server, &modcache); + assert_eq!(hosted_pinned(&env), 1, "envelope: {env}"); + let go_mod = std::fs::read_to_string(consumer.join("go.mod")).unwrap(); + assert!( + go_mod.contains(&format!("replace {UMOD} {UVER} => {}", smod())), + "{go_mod}" + ); + + // The other member moves to v1.1.0: re-running takes the inert pin back. + std::fs::write(other.join("go.mod"), other_mod("v1.1.0")).unwrap(); + let env = get_hosted(&consumer, &server, &modcache); + assert_eq!(hosted_pinned(&env), 0, "envelope: {env}"); + assert!(mismatch(&env), "{env}"); + let go_mod = std::fs::read_to_string(consumer.join("go.mod")).unwrap(); + assert!( + !go_mod.contains(&smod()), + "the inert pin is removed: {go_mod}" + ); + let go_sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap(); + assert!(!go_sum.contains(&smod()), "{go_sum}"); + + // A fresh project in that state never gets the pin. + write_consumer(&consumer, &format!("require {UMOD} {UVER}\n"), UPSTREAM_SUM); + let _ = std::fs::remove_dir_all(consumer.join(".socket")); + let env = get_hosted(&consumer, &server, &modcache); + assert_eq!(hosted_pinned(&env), 0, "envelope: {env}"); + assert!(mismatch(&env), "{env}"); + assert_eq!( + std::fs::read_to_string(consumer.join("go.mod")).unwrap(), + format!("module example.com/consumer\n\ngo 1.21\n\nrequire {UMOD} {UVER}\n") + ); +} + /// vendored → hosted takeover: the hosted run must revert the vendored /// state (ledger entry, committed copy, marker) before redirecting, so the /// project is fully hosted — never a hosted go.mod beside a vendor ledger diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index ead398946..148fdada8 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -42,6 +42,7 @@ use crate::patch::redirect::{ use crate::utils::pnpm_workspace::governing_workspace_file; use crate::utils::purl::purl_parts; use crate::utils::redact::url_host; +use crate::vendor::go_build_list::ModuleFiles; use crate::vendor::lock_inventory::{bun_text_lock_drives, MemoryEntry, ProjectView}; use super::guidance::{ @@ -1546,6 +1547,11 @@ pub struct RewriteOptions<'a> { /// resolved only when a cargo dep is rewritten (the cargo rewriter's /// offline-source refusal). pub cargo_outer: &'a (dyn Fn() -> CratesIoSource + Send + Sync), + /// The other `use` members of the go.work in effect for the project, + /// resolved only when a golang dep is rewritten: the Go rewriter weighs + /// their requirements, since the workspace build selects the highest + /// version any member asks for (#1753). + pub go_workspace: &'a (dyn Fn() -> Vec + Send + Sync), /// A variable of the environment pnpm installs the project in, by /// name: a `pnpm_config_trust_lockfile` there beats the /// pnpm-workspace.yaml key the trust auto-config writes (#1514). @@ -2272,6 +2278,11 @@ async fn rewrite_once( } else { CratesIoSource::Registry }; + let go_members = if rewrite_overrides.iter().any(|o| o.ecosystem == "golang") { + (options.go_workspace)() + } else { + Vec::new() + }; let (files, mut rewrite) = if options.blocking { // Pure CPU over every lock text (the independent rewriter groups // run concurrently inside), so it runs on the blocking pool rather @@ -2288,6 +2299,7 @@ async fn rewrite_once( &gradle_unreadable, &yarn_outer, &cargo_outer, + &go_members, ); (files, rewrite) }) @@ -2307,6 +2319,7 @@ async fn rewrite_once( &gradle_unreadable, &yarn_outer, &cargo_outer, + &go_members, ); (files, rewrite) }; @@ -3800,6 +3813,7 @@ mod tests { npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -3873,6 +3887,7 @@ mod tests { npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -4188,6 +4203,7 @@ mod tests { npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: env, blocking: false, takeover_uuids: Default::default(), @@ -4624,6 +4640,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -4729,6 +4746,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -4905,6 +4923,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -5246,6 +5265,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -5462,6 +5482,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -5517,6 +5538,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -5715,6 +5737,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), @@ -6124,6 +6147,7 @@ snapshots: npm_outer: &outer, yarn_classic_outer: &OuterYarnMirror::default, cargo_outer: &no_cargo_outer, + go_workspace: &Vec::new, pnpm_env: &no_env, blocking: false, takeover_uuids: Default::default(), diff --git a/crates/socket-patch-core/src/hosted/memory/stages.rs b/crates/socket-patch-core/src/hosted/memory/stages.rs index 63811b36b..192b5097e 100644 --- a/crates/socket-patch-core/src/hosted/memory/stages.rs +++ b/crates/socket-patch-core/src/hosted/memory/stages.rs @@ -377,6 +377,7 @@ pub(crate) async fn rewrite( yarn_classic_outer: &yarn_classic_outer, // The in-memory engine has no project on disk to read. cargo_outer: &|| CratesIoSource::Registry, + go_workspace: &Vec::new, // Nor the environment the project's installs run in. pnpm_env: &|_| None, blocking: false, diff --git a/crates/socket-patch-core/src/patch/redirect/golang_equivalence_tests.rs b/crates/socket-patch-core/src/patch/redirect/golang_equivalence_tests.rs index e14ac1610..800a71cf0 100644 --- a/crates/socket-patch-core/src/patch/redirect/golang_equivalence_tests.rs +++ b/crates/socket-patch-core/src/patch/redirect/golang_equivalence_tests.rs @@ -16,7 +16,7 @@ fn run( overrides: &[DepOverride], ) -> RewriteResult { let mut got = RewriteResult::default(); - rewrite_golang(files, overrides, &mut got); + rewrite_golang(files, overrides, &[], &mut got); g.next(&(files, overrides), &got); got } diff --git a/crates/socket-patch-core/src/patch/redirect/group_equivalence_tests.rs b/crates/socket-patch-core/src/patch/redirect/group_equivalence_tests.rs index 7bbf2bed5..763cf6c3b 100644 --- a/crates/socket-patch-core/src/patch/redirect/group_equivalence_tests.rs +++ b/crates/socket-patch-core/src/patch/redirect/group_equivalence_tests.rs @@ -40,7 +40,7 @@ fn serial_oracle( rewrite_nuget(files, overrides, &mut result); rewrite_gem(files, overrides, &mut result); rewrite_maven_pom(files, overrides, &mut result); - rewrite_golang(files, overrides, &mut result); + rewrite_golang(files, overrides, &[], &mut result); result } @@ -139,6 +139,7 @@ fn assert_same_with_metadata( &no_gradle_unreadable, &no_yarn_outer, &CratesIoSource::Registry, + &[], ); let merged = merge_group_outputs(&prefix, run_groups_concurrently(&prefix, &groups)).map( |mut merged| { diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0b1e6e400..bdb58b406 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -604,6 +604,7 @@ pub fn rewrite_registry_redirect_with_pipenv_version( &std::collections::BTreeSet::new(), &yarnrc::OuterYarnMirror::default(), &CratesIoSource::Registry, + &[], ) } @@ -684,6 +685,7 @@ pub fn rewrite_registry_redirect_withholding_vlt( gradle_unreadable: &std::collections::BTreeSet, yarn_outer: &yarnrc::OuterYarnMirror, cargo_outer: &CratesIoSource, + go_members: &[crate::vendor::go_build_list::ModuleFiles], ) -> RewriteResult { let mut result = RewriteResult::default(); let overrides = withhold_pypi_platform_wheels(overrides, &mut result); @@ -711,6 +713,7 @@ pub fn rewrite_registry_redirect_withholding_vlt( gradle_unreadable, yarn_outer, cargo_outer, + go_members, ); let mut result = rewrite_groups_parallel(result, &groups); result.vlt_drives = vlt::vlt_drives(files, bun_lockb_present); @@ -739,6 +742,7 @@ fn rewriter_groups<'a>( gradle_unreadable: &'a std::collections::BTreeSet, yarn_outer: &'a yarnrc::OuterYarnMirror, cargo_outer: &'a CratesIoSource, + go_members: &'a [crate::vendor::go_build_list::ModuleFiles], ) -> Vec> { vec![ Box::new(move |result| { @@ -765,7 +769,7 @@ fn rewriter_groups<'a>( Box::new(move |result| { gradle::rewrite_gradle_hosted(files, gradle_unreadable, overrides, result) }), - Box::new(move |result| rewrite_golang(files, overrides, result)), + Box::new(move |result| rewrite_golang(files, overrides, go_members, result)), Box::new(move |result| { sbt::rewrite_sbt(files, overrides, result); scala_guidance::warn(files, overrides, result); @@ -9121,6 +9125,7 @@ fn go_token_safe(s: &str) -> bool { fn rewrite_golang( files: &BTreeMap, overrides: &[DepOverride], + go_members: &[crate::vendor::go_build_list::ModuleFiles], result: &mut RewriteResult, ) { use crate::vendor::go_build_list; @@ -9265,17 +9270,29 @@ fn rewrite_golang( // (`go_build_list`): `require` for a go >= 1.17 graph; for an older // one, the highest of `require` and go.sum's zip lines, since a // `/go.mod`-only line is written for every version MVS reads, not - // only the one it selects (#509). + // only the one it selects (#509). In a go.work workspace the other + // members' files are weighed too: MVS builds the highest version + // any of them asks for (#1753). let zips = go_sum.zip_versions(&fname); let zips: Vec<&str> = zips.iter().map(String::as_str).collect(); - let selection = go_build_list::select( + let own = go_build_list::select( go_mod_edit::prunes_module_graph(&go_mod), scan.required.as_deref(), &zips, &dep.version, ); + let selection = if go_members.is_empty() { + own.clone() + } else { + let members = go_members.iter().map(|m| { + go_build_list::selection(&m.go_mod, m.go_sum.as_deref(), &fname, &dep.version) + }); + go_build_list::workspace_selection(own.clone(), members, &dep.version) + }; if let Some(selected) = selection.other_version() { - let basis = if scan.required.as_deref() == Some(selected) { + let basis = if own.other_version() != Some(selected) { + "another go.work workspace member makes the build select" + } else if scan.required.as_deref() == Some(selected) { "go.mod requires" } else { "go.sum shows the build selects" @@ -12789,6 +12806,7 @@ mod tests { &std::collections::BTreeSet::new(), &refusing[3], &CratesIoSource::Registry, + &[], ); assert!(!r.files.contains_key("yarn.lock")); assert!(r @@ -22999,6 +23017,80 @@ packages: assert!(out.warnings[0].detail.contains("v1.5.0")); } + /// A go.work member that is not the scanned module. + fn go_member(require: &str) -> crate::vendor::go_build_list::ModuleFiles { + crate::vendor::go_build_list::ModuleFiles { + go_mod: format!("module example.com/other\n\ngo 1.21\n\n{require}"), + go_sum: None, + } + } + + /// #1753: in a go.work workspace the build selects the highest version + /// any member requires, so a pin for this module's v1.4.2 is inert when + /// another member requires v1.5.0. + #[test] + fn golang_workspace_member_requiring_a_higher_version_skips() { + let files = golang_files(); + let ovr = golang_override(); + let members = [go_member("require github.com/foo/bar v1.5.0\n")]; + let mut out = RewriteResult::default(); + rewrite_golang(&files, std::slice::from_ref(&ovr), &members, &mut out); + assert!(out.files.is_empty(), "nothing written: {:?}", out.files); + assert_eq!( + out.warnings + .iter() + .map(|w| w.code.as_str()) + .collect::>(), + ["redirect_golang_version_mismatch"] + ); + assert!( + out.warnings[0] + .detail + .contains("another go.work workspace member makes the build select v1.5.0"), + "{}", + out.warnings[0].detail + ); + } + + /// #1753: a member requiring a lower version, or not requiring the + /// module at all, leaves the build on this module's v1.4.2. A member + /// that proves the patched version puts a module this go.mod omits in + /// the build. + #[test] + fn golang_workspace_members_that_do_not_outrank_still_redirect() { + let ovr = golang_override(); + for members in [ + vec![go_member("require github.com/foo/bar v1.4.1\n")], + vec![go_member("require example.com/unrelated v1.0.0\n")], + vec![go_member("")], + ] { + let mut out = RewriteResult::default(); + rewrite_golang( + &golang_files(), + std::slice::from_ref(&ovr), + &members, + &mut out, + ); + assert!(out.warnings.is_empty(), "{members:?}: {:?}", out.warnings); + assert!(out.files["go.mod"].contains("replace github.com/foo/bar v1.4.2 =>")); + } + + let mut files = golang_files(); + files.insert( + "go.mod".to_string(), + "module example.com/app\n\ngo 1.21\n\nrequire example.com/direct v2.0.0\n".to_string(), + ); + files.insert("go.sum".to_string(), String::new()); + let mut out = RewriteResult::default(); + rewrite_golang(&files, std::slice::from_ref(&ovr), &[], &mut out); + assert_eq!(out.warnings[0].code, "redirect_golang_not_in_module_graph"); + let members = [go_member("require github.com/foo/bar v1.4.2\n")]; + let mut out = RewriteResult::default(); + rewrite_golang(&files, std::slice::from_ref(&ovr), &members, &mut out); + assert!(out.warnings.is_empty(), "{:?}", out.warnings); + assert!(out.files["go.mod"].contains("replace github.com/foo/bar v1.4.2 =>")); + } + /// A module NOT in the main go.mod's require block may still be resolved /// (transitively) at the patched version — the cross-check only fires on a /// positive mismatch, never on absence. diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index fce0b9f43..ab46fbe9a 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -109,28 +109,72 @@ pub fn select(pruned: bool, required: Option<&str>, zips: &[&str], version: &str /// `go.sum` (`None` when there is no readable go.mod). /// /// In workspace mode the build list also takes every `use` member's -/// requirements, and MVS selects the highest version any of them asks for -/// (#1751). So a version another member proves is weighed with this one's, -/// and a module absent from this go.mod may still be built: -/// [`Selection::NotInGraph`] becomes [`Selection::Unknown`] there unless a -/// member proves the version. +/// requirements, so their go.mod/go.sum are weighed too +/// ([`workspace_selection`]). pub async fn project_selection( project_root: &Path, module: &str, version: &str, ) -> Option { let own = dir_selection(project_root, module, version).await?; - let Some(members) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { + let Some(dirs) = super::go_consumer_sync::workspace_member_dirs(project_root).await else { return Some(own); }; + let mut members = Vec::new(); + for dir in dirs { + members.extend(dir_selection(&dir, module, version).await); + } + Some(workspace_selection(own, members, version)) +} + +/// The text of one go.work member's `go.mod` and (when present) `go.sum`. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ModuleFiles { + pub go_mod: String, + pub go_sum: Option, +} + +/// The files of every other `use` member of the go.work in effect for +/// `project_root` (empty outside a workspace), read on the calling thread. +/// A member whose go.mod can't be read is left out, as +/// [`project_selection`] skips it. +pub fn other_workspace_members(project_root: &Path) -> Vec { + use crate::utils::fs::read_regular_to_string_sync; + let Some(dirs) = super::go_consumer_sync::workspace_member_dirs_sync(project_root) else { + return Vec::new(); + }; + let base = std::env::current_dir() + .unwrap_or_default() + .join(project_root); + let root = std::fs::canonicalize(&base).unwrap_or(base); + dirs.into_iter() + .filter(|dir| std::fs::canonicalize(dir).map_or(true, |dir| dir != root)) + .filter_map(|dir| { + Some(ModuleFiles { + go_mod: read_regular_to_string_sync(&dir.join("go.mod")).ok()?, + go_sum: read_regular_to_string_sync(&dir.join("go.sum")).ok(), + }) + }) + .collect() +} + +/// The selection for the whole workspace, from this module's own +/// [`Selection`] (`own`) and each other member's. +/// +/// MVS selects the highest version any member asks for (#1751). So a +/// version another member proves is weighed with this one's, and a module +/// absent from this go.mod may still be built: [`Selection::NotInGraph`] +/// becomes [`Selection::Unknown`] unless a member proves the version. +pub fn workspace_selection( + own: Selection, + members: impl IntoIterator, + version: &str, +) -> Selection { let mut highest = evidence(&own, version); // A member whose files can't tell may build a higher version than the // highest one proven, so then only a proof at or above ours decides. let mut unknown = own == Selection::Unknown; - for dir in members { - let Some(member) = dir_selection(&dir, module, version).await else { - continue; - }; + for member in members { unknown |= member == Selection::Unknown; if let Some(v) = evidence(&member, version) { if highest @@ -141,14 +185,14 @@ pub async fn project_selection( } } } - Some(match highest { + match highest { Some(h) if h == version => Selection::Selected, Some(h) if !unknown || go_semver_cmp(&h, version) == Ordering::Greater => { Selection::Other(h) } _ if own == Selection::NotInGraph => Selection::Unknown, _ => own, - }) + } } /// [`selection`] from `/go.mod` and `go.sum`, ignoring any workspace. @@ -314,6 +358,34 @@ mod tests { ); } + /// #1753: the hosted rewriter gets every member but the scanned one, + /// with its go.mod and go.sum text; a member without a go.mod is left + /// out, and outside a workspace there are none. + #[test] + fn other_workspace_members_reads_every_member_but_this_one() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); + module(&a, "a", "v1.0.0"); + module(&b, "b", "v1.1.0"); + std::fs::remove_file(a.join("go.sum")).unwrap(); + std::fs::create_dir_all(&c).unwrap(); + assert!(other_workspace_members(&a).is_empty()); + std::fs::write( + ws.join("go.work"), + "go 1.21\n\nuse (\n\t./a\n\t./b/\n\t./c\n)\n", + ) + .unwrap(); + let members = other_workspace_members(&a); + assert_eq!(members.len(), 1, "{members:?}"); + assert!(members[0].go_mod.contains("module example.com/b")); + assert!(members[0].go_sum.as_deref().unwrap().contains("v1.1.0")); + let members = other_workspace_members(&b); + assert_eq!(members.len(), 1, "{members:?}"); + assert!(members[0].go_mod.contains("module example.com/a")); + assert_eq!(members[0].go_sum, None); + } + /// A version below the patched one counts when it is the highest any /// member proves, unless a member's files can't tell. #[tokio::test] 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..89062845c 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -309,6 +309,18 @@ pub(crate) async fn workspace_member_dirs(project_root: &Path) -> Option Option> { + let work_file = workspace_file(project_root)?; + Some( + match crate::utils::fs::read_regular_to_string_sync(&work_file) { + Ok(text) => use_dirs(&work_file, &go_mod_edit::normalize_for_read(&text)), + Err(_) => Vec::new(), + }, + ) +} + /// `work_file`'s text, normalized for parsing. async fn read_work_file(work_file: &Path) -> Option { let text = read_regular_to_string(work_file).await.ok()?; diff --git a/docs/ecosystems.md b/docs/ecosystems.md index b840a0058..fce21858a 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1514,12 +1514,13 @@ and `go.sum`: the `require` line in a go 1.17+ module, or, in an older module the highest of the required version and the versions `go.sum` has a module zip line for. `apply` and `scan --mode hosted` refuse to write a replace for any other version. In a workspace the build takes the highest version any -`use` member selects, so `apply`, `vendor`, `apply --check` and `vex` read -every member's `go.mod` and `go.sum` too: a member that requires a newer -version makes a replace pinned to the older one inert. `scan --mode hosted` -still reads only the module it runs in (#1753). After an upgrade such as `go get M@v1.0.1`, `apply --check` -fails and `vex` leaves the patch out (`go_version_not_selected`) until you -get a patch for the new version. +`use` member selects, so `apply`, `vendor`, `apply --check`, `vex` and +`scan --mode hosted` read every member's `go.mod` and `go.sum` too: a member +that requires a newer version makes a replace pinned to the older one inert. +A hosted rescan removes a pin it wrote once another member outranks it. +After an upgrade such as `go get M@v1.0.1`, `apply --check` fails and `vex` +leaves the patch out (`go_version_not_selected`) until you get a patch for the +new version. Paid hosted Go references are unsupported: embedding credentials in module paths would expose them to module proxies and change module identity. Use vendored mode From d882a618c98ccf98f5304b9f2a4aeccce977dd78 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 00:49:31 +0000 Subject: [PATCH 7/7] Read go.work members once per hosted Go scan Review follow-ups for the workspace check: - Parse each member's go.mod and go.sum once, not once per patched module, so a large workspace does not slow the scan. - Match the scanned module against the use list the way the rest of the go.work code does (same_dir_key). - Say "no go.work member" in the not-in-graph warning when the workspace was weighed, instead of blaming this module alone. - Pin GOWORK and GOENV in the hosted Go state tests so a machine setting GOWORK=off can't hide the workspace. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_golang_hosted_state.rs | 8 +- crates/socket-patch-core/src/hosted/engine.rs | 4 +- .../src/patch/redirect/mod.rs | 32 ++++---- .../src/vendor/go_build_list.rs | 82 +++++++++++++------ .../src/vendor/go_consumer_sync.rs | 2 +- 5 files changed, 83 insertions(+), 45 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs index c27a0e023..95778f16b 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_hosted_state.rs @@ -132,10 +132,16 @@ fn get_hosted_with( "fake", ]; args.extend_from_slice(extra); + // The go.work in effect is the one above the project (#1753), whatever + // GOWORK the machine running the tests sets. let (code, stdout, stderr) = run_with_prebuilt( consumer, &args, - &[("GOMODCACHE", modcache.to_str().unwrap())], + &[ + ("GOMODCACHE", modcache.to_str().unwrap()), + ("GOWORK", "auto"), + ("GOENV", "off"), + ], ); assert_eq!( code, 0, diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 148fdada8..05bd4c14a 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -42,7 +42,7 @@ use crate::patch::redirect::{ use crate::utils::pnpm_workspace::governing_workspace_file; use crate::utils::purl::purl_parts; use crate::utils::redact::url_host; -use crate::vendor::go_build_list::ModuleFiles; +use crate::vendor::go_build_list::ModuleEvidence; use crate::vendor::lock_inventory::{bun_text_lock_drives, MemoryEntry, ProjectView}; use super::guidance::{ @@ -1551,7 +1551,7 @@ pub struct RewriteOptions<'a> { /// resolved only when a golang dep is rewritten: the Go rewriter weighs /// their requirements, since the workspace build selects the highest /// version any member asks for (#1753). - pub go_workspace: &'a (dyn Fn() -> Vec + Send + Sync), + pub go_workspace: &'a (dyn Fn() -> Vec + Send + Sync), /// A variable of the environment pnpm installs the project in, by /// name: a `pnpm_config_trust_lockfile` there beats the /// pnpm-workspace.yaml key the trust auto-config writes (#1514). diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index bdb58b406..9da4f74d0 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -685,7 +685,7 @@ pub fn rewrite_registry_redirect_withholding_vlt( gradle_unreadable: &std::collections::BTreeSet, yarn_outer: &yarnrc::OuterYarnMirror, cargo_outer: &CratesIoSource, - go_members: &[crate::vendor::go_build_list::ModuleFiles], + go_members: &[crate::vendor::go_build_list::ModuleEvidence], ) -> RewriteResult { let mut result = RewriteResult::default(); let overrides = withhold_pypi_platform_wheels(overrides, &mut result); @@ -742,7 +742,7 @@ fn rewriter_groups<'a>( gradle_unreadable: &'a std::collections::BTreeSet, yarn_outer: &'a yarnrc::OuterYarnMirror, cargo_outer: &'a CratesIoSource, - go_members: &'a [crate::vendor::go_build_list::ModuleFiles], + go_members: &'a [crate::vendor::go_build_list::ModuleEvidence], ) -> Vec> { vec![ Box::new(move |result| { @@ -9125,7 +9125,7 @@ fn go_token_safe(s: &str) -> bool { fn rewrite_golang( files: &BTreeMap, overrides: &[DepOverride], - go_members: &[crate::vendor::go_build_list::ModuleFiles], + go_members: &[crate::vendor::go_build_list::ModuleEvidence], result: &mut RewriteResult, ) { use crate::vendor::go_build_list; @@ -9284,9 +9284,7 @@ fn rewrite_golang( let selection = if go_members.is_empty() { own.clone() } else { - let members = go_members.iter().map(|m| { - go_build_list::selection(&m.go_mod, m.go_sum.as_deref(), &fname, &dep.version) - }); + let members = go_members.iter().map(|m| m.selection(&fname, &dep.version)); go_build_list::workspace_selection(own.clone(), members, &dep.version) }; if let Some(selected) = selection.other_version() { @@ -9346,14 +9344,16 @@ fn rewrite_golang( // outside this project's graph (local discovery crawls the whole // module cache). Its replace would be inert, and confirming it // would attest a patch no build links. + let detail = if go_members.is_empty() { + "neither go.mod's require nor a go.sum zip line shows the build selecting \ + this version — it is not in this project's build graph" + } else { + "no go.work workspace member's go.mod require or go.sum zip line shows the \ + build selecting this version" + }; result.warnings.push(RewriteWarning { code: "redirect_golang_not_in_module_graph".into(), - detail: format!( - "{fname}@{}: neither go.mod's require nor a go.sum zip line shows the \ - build selecting this version — it is not in this project's build graph; \ - nothing redirected", - dep.version - ), + detail: format!("{fname}@{}: {detail}; nothing redirected", dep.version), }); continue; } @@ -23018,11 +23018,9 @@ packages: } /// A go.work member that is not the scanned module. - fn go_member(require: &str) -> crate::vendor::go_build_list::ModuleFiles { - crate::vendor::go_build_list::ModuleFiles { - go_mod: format!("module example.com/other\n\ngo 1.21\n\n{require}"), - go_sum: None, - } + fn go_member(require: &str) -> crate::vendor::go_build_list::ModuleEvidence { + let go_mod = format!("module example.com/other\n\ngo 1.21\n\n{require}"); + crate::vendor::go_build_list::ModuleEvidence::from_files(&go_mod, None) } /// #1753: in a go.work workspace the build selects the highest version diff --git a/crates/socket-patch-core/src/vendor/go_build_list.rs b/crates/socket-patch-core/src/vendor/go_build_list.rs index ab46fbe9a..ba20567e9 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -25,6 +25,7 @@ //! the original's go.sum lines, so absence proves nothing. use std::cmp::Ordering; +use std::collections::HashMap; use std::path::Path; use crate::utils::fs::read_regular_to_string; @@ -127,33 +128,66 @@ pub async fn project_selection( Some(workspace_selection(own, members, version)) } -/// The text of one go.work member's `go.mod` and (when present) `go.sum`. +/// What one go.work member's `go.mod` and `go.sum` prove about the +/// versions it builds, parsed once for every module asked about. #[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct ModuleFiles { - pub go_mod: String, - pub go_sum: Option, +pub struct ModuleEvidence { + pruned: bool, + required: HashMap, + zips: HashMap>, } -/// The files of every other `use` member of the go.work in effect for +impl ModuleEvidence { + pub fn from_files(go_mod: &str, go_sum: Option<&str>) -> Self { + let mut zips: HashMap> = HashMap::new(); + for line in go_sum.into_iter().flat_map(go_sum_lines) { + if !line.go_mod { + let versions = zips.entry(line.module.to_string()).or_default(); + versions.push(line.version.to_string()); + } + } + Self { + pruned: prunes_module_graph(go_mod), + required: parse_required_versions(go_mod), + zips, + } + } + + /// [`selection`] for `module@version` from these files. + pub fn selection(&self, module: &str, version: &str) -> Selection { + let zips: Vec<&str> = self + .zips + .get(module) + .into_iter() + .flatten() + .map(String::as_str) + .collect(); + select( + self.pruned, + self.required.get(module).map(String::as_str), + &zips, + version, + ) + } +} + +/// The evidence of every other `use` member of the go.work in effect for /// `project_root` (empty outside a workspace), read on the calling thread. /// A member whose go.mod can't be read is left out, as /// [`project_selection`] skips it. -pub fn other_workspace_members(project_root: &Path) -> Vec { +pub fn other_workspace_members(project_root: &Path) -> Vec { + use super::go_consumer_sync::{same_dir_key, workspace_member_dirs_sync}; use crate::utils::fs::read_regular_to_string_sync; - let Some(dirs) = super::go_consumer_sync::workspace_member_dirs_sync(project_root) else { + let Some(dirs) = workspace_member_dirs_sync(project_root) else { return Vec::new(); }; - let base = std::env::current_dir() - .unwrap_or_default() - .join(project_root); - let root = std::fs::canonicalize(&base).unwrap_or(base); + let root = same_dir_key(project_root); dirs.into_iter() - .filter(|dir| std::fs::canonicalize(dir).map_or(true, |dir| dir != root)) + .filter(|dir| same_dir_key(dir) != root) .filter_map(|dir| { - Some(ModuleFiles { - go_mod: read_regular_to_string_sync(&dir.join("go.mod")).ok()?, - go_sum: read_regular_to_string_sync(&dir.join("go.sum")).ok(), - }) + let go_mod = read_regular_to_string_sync(&dir.join("go.mod")).ok()?; + let go_sum = read_regular_to_string_sync(&dir.join("go.sum")).ok(); + Some(ModuleEvidence::from_files(&go_mod, go_sum.as_deref())) }) .collect() } @@ -358,9 +392,9 @@ mod tests { ); } - /// #1753: the hosted rewriter gets every member but the scanned one, - /// with its go.mod and go.sum text; a member without a go.mod is left - /// out, and outside a workspace there are none. + /// #1753: the hosted rewriter gets every member but the scanned one; a + /// member without a go.mod is left out, and outside a workspace there + /// are none. #[test] fn other_workspace_members_reads_every_member_but_this_one() { let dir = tempfile::tempdir().unwrap(); @@ -368,7 +402,6 @@ mod tests { let (a, b, c) = (ws.join("a"), ws.join("b"), ws.join("c")); module(&a, "a", "v1.0.0"); module(&b, "b", "v1.1.0"); - std::fs::remove_file(a.join("go.sum")).unwrap(); std::fs::create_dir_all(&c).unwrap(); assert!(other_workspace_members(&a).is_empty()); std::fs::write( @@ -378,12 +411,13 @@ mod tests { .unwrap(); let members = other_workspace_members(&a); assert_eq!(members.len(), 1, "{members:?}"); - assert!(members[0].go_mod.contains("module example.com/b")); - assert!(members[0].go_sum.as_deref().unwrap().contains("v1.1.0")); + assert_eq!( + members[0].selection(UP, "v1.0.0"), + Selection::Other("v1.1.0".into()) + ); let members = other_workspace_members(&b); assert_eq!(members.len(), 1, "{members:?}"); - assert!(members[0].go_mod.contains("module example.com/a")); - assert_eq!(members[0].go_sum, None); + assert_eq!(members[0].selection(UP, "v1.0.0"), Selection::Selected); } /// A version below the patched one counts when it is the highest any 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 89062845c..9fe44e6aa 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -503,7 +503,7 @@ async fn workspace_issues(project_root: &Path, replaces: &[ReplaceEntry]) -> Vec /// exists once folded (`./x/../app` with no `x`, which go's `filepath.Join` /// folds) is folded lexically and then resolved, so it still matches a /// project under a symlinked dir (macOS `/var` -> `/private/var`). -fn same_dir_key(dir: &Path) -> PathBuf { +pub(crate) fn same_dir_key(dir: &Path) -> PathBuf { let abs = std::env::current_dir().unwrap_or_default().join(dir); std::fs::canonicalize(&abs).unwrap_or_else(|_| { let folded = crate::utils::relpath::normalize_lexically_keeping_escapes(&abs);