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..fce0b9f43 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,65 @@ 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 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( 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 { + 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() + .is_none_or(|h| go_semver_cmp(&v, h) == Ordering::Greater) + { + highest = Some(v); + } + } + } + Some(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. +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 +265,116 @@ 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 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. + #[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..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 @@ -305,6 +298,33 @@ 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`: `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)?; + 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("")); + 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` /// member's go.mod replaces. struct Workspace { @@ -322,12 +342,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.