diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index d53f6de28..1cebcc8a5 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -60,6 +60,20 @@ fn go_env<'a>(modcache: &'a str, proxy_url: &'a str) -> Vec<(&'a str, &'a str)> ] } +/// [`go_env`] with `GOFLAGS=-mod=readonly`: a build that proves the +/// committed go.mod and go.sum are complete. +fn readonly_env<'a>(env: &[(&'a str, &'a str)]) -> Vec<(&'a str, &'a str)> { + env.iter() + .map(|&(k, v)| { + if k == "GOFLAGS" { + (k, "-mod=readonly") + } else { + (k, v) + } + }) + .collect() +} + /// Run socket-patch through the shared hermetic runner with the fixture /// GOMODCACHE (the go crawler resolves installed modules through it). fn run_socket(cwd: &Path, args: &[&str], modcache: &Path) -> (i32, String, String) { @@ -84,13 +98,20 @@ fn go(dir: &Path, args: &[&str], env: &[(&str, &str)]) -> std::process::Output { /// Build the upstream module into a file-proxy and `go mod download` it into a /// temp GOMODCACHE. Returns (consumer_dir, modcache, proxy_url). fn stage(tmp: &Path) -> (std::path::PathBuf, std::path::PathBuf, String) { + stage_with(tmp, golang_e2e_matrix::go_directive()) +} + +/// [`stage`] with the upstream module's `go` directive version. `go mod +/// tidy` raises the consumer's directive to its dependencies' highest, so +/// an unpruned (go 1.16) consumer needs a go 1.16 upstream. +fn stage_with(tmp: &Path, upstream_go: &str) -> (std::path::PathBuf, std::path::PathBuf, String) { let godir = golang_e2e_matrix::go_directive(); // Staging dir holding `@/` for zipping. let stage = tmp.join("stage").join(format!("{UMOD}@{UVER}")); std::fs::create_dir_all(&stage).unwrap(); std::fs::write( stage.join("go.mod"), - format!("module {UMOD}\n\ngo {godir}\n"), + format!("module {UMOD}\n\ngo {upstream_go}\n"), ) .unwrap(); std::fs::write(stage.join("lib.go"), PRISTINE_LIB).unwrap(); @@ -105,7 +126,7 @@ fn stage(tmp: &Path) -> (std::path::PathBuf, std::path::PathBuf, String) { .unwrap(); std::fs::write( pxv.join(format!("{UVER}.mod")), - format!("module {UMOD}\n\ngo {godir}\n"), + format!("module {UMOD}\n\ngo {upstream_go}\n"), ) .unwrap(); let zip_out = pxv.join(format!("{UVER}.zip")); @@ -1671,16 +1692,7 @@ fn module_built_only_as_a_replace_target_is_not_patched() { let tmp = tempfile::tempdir().unwrap(); let (consumer, modcache, proxy_url) = stage(tmp.path()); let env = go_env(modcache.to_str().unwrap(), &proxy_url); - let readonly: Vec<(&str, &str)> = env - .iter() - .map(|&(k, v)| { - if k == "GOFLAGS" { - (k, "-mod=readonly") - } else { - (k, v) - } - }) - .collect(); + let readonly = readonly_env(&env); // Applied while the consumer still required the upstream directly. write_patch(&consumer); @@ -1753,6 +1765,190 @@ fn module_built_only_as_a_replace_target_is_not_patched() { } } +/// Point the consumer at `example.com/mid`, which requires `FORK`, with the +/// root go.mod replacing `FORK` by the upstream (`replace_line`). In an +/// unpruned (go 1.16) graph tidy doesn't list `FORK` in the root go.mod, +/// but the root's replace still applies to it. +fn mid_fork_consumer(consumer: &Path, extra: &str, env: &[(&str, &str)]) { + std::fs::write( + consumer.join("go.mod"), + format!("module example.com/consumer\n\ngo 1.16\n\nrequire {MID} {UVER}\n\n{extra}\n"), + ) + .unwrap(); + std::fs::write( + consumer.join("main.go"), + format!( + "package main\n\nimport (\n\t\"fmt\"\n\t\"{MID}\"\n)\n\nfunc main() {{ fmt.Println(\"OUT:\", mid.Hello()) }}\n" + ), + ) + .unwrap(); + let tidy = go(consumer, &["mod", "tidy"], env); + assert!( + tidy.status.success(), + "go mod tidy: {}", + String::from_utf8_lossy(&tidy.stderr) + ); +} + +const MID: &str = "example.com/mid"; + +/// #1840: the #1823 shape where only a dependency requires the fork. In a +/// go 1.16 graph the root go.mod never lists the fork, yet its `replace` +/// still sends the build to the upstream under the fork's path, so a +/// replace of the upstream itself is inert. apply must refuse, and a +/// Socket replace left from before must fail `apply --check` and be left +/// out of VEX. Covers the versioned and versionless fork replace. +#[test] +fn fork_required_only_by_a_dependency_is_a_replace_target() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(transitive-replace-target)") { + return; + } + for replace_line in [ + format!("replace {FORK} => {UMOD} {UVER}"), + format!("replace {FORK} {UVER} => {UMOD} {UVER}"), + ] { + let case = replace_line.clone(); + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16"); + publish( + tmp.path(), + MID, + UVER, + &[ + ( + "go.mod", + &format!("module {MID}\n\ngo 1.16\n\nrequire {FORK} {UVER}\n"), + ), + ( + "mid.go", + &format!( + "package mid\n\nimport u \"{FORK}\"\n\nfunc Hello() string {{ return u.Greeting() }}\n" + ), + ), + ], + ); + let env = go_env(modcache.to_str().unwrap(), &proxy_url); + let readonly = readonly_env(&env); + + // Applied while the consumer still required the upstream directly. + write_patch(&consumer); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!(code, 0, "{case}: direct apply:\n{so}\n{se}"); + assert!( + go_out(&consumer, &readonly).contains("OUT: PATCHED"), + "{case}" + ); + let socket_replace = replace_lines(&consumer.join("go.mod")); + assert_eq!(socket_replace.len(), 1, "{case}: {socket_replace:?}"); + + // Now the upstream is only the fork's replacement, and the fork is + // only the dependency's requirement: the Socket replace is inert. + mid_fork_consumer( + &consumer, + &format!("{replace_line}\n\n{}", socket_replace[0]), + &env, + ); + let go_mod = std::fs::read_to_string(consumer.join("go.mod")).unwrap(); + assert!( + !go_mod.contains(&format!("require {FORK}")) && !go_mod.contains(&format!("\t{FORK} ")), + "{case}: the root go.mod must not require the fork:\n{go_mod}" + ); + let sum = std::fs::read_to_string(consumer.join("go.sum")).unwrap(); + assert!(sum.contains(&format!("{UMOD} {UVER} h1:")), "{case}: {sum}"); + assert!( + go_out(&consumer, &readonly).contains("OUT: PRISTINE"), + "{case}" + ); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!( + code, 1, + "{case}: apply --check must flag the inert replace:\n{so}\n{se}" + ); + assert!( + so.contains(&format!("only as the replacement of {FORK}")) + || se.contains(&format!("only as the replacement of {FORK}")), + "{case}:\n{so}\n{se}" + ); + let r = VexRun { + offline: true, + product: Some("pkg:golang/example.com/consumer".into()), + ..VexRun::default() + } + .env("GOMODCACHE", &modcache) + .env("GOFLAGS", ""); + let out = run_vex(&vex_e2e_common::binary(), &consumer, &r); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_version_not_selected"); + + // A fresh apply over the transitive fork shape refuses and writes + // nothing. + mid_fork_consumer(&consumer, &replace_line, &env); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_ne!( + code, 0, + "{case}: apply must refuse an inert replace:\n{so}\n{se}" + ); + assert!( + so.contains(&format!("only as the replacement of {FORK}")), + "{case}:\n{so}\n{se}" + ); + assert_eq!( + replace_lines(&consumer.join("go.mod")), + vec![replace_line.clone()], + "{case}" + ); + assert!( + go_out(&consumer, &readonly).contains("OUT: PRISTINE"), + "{case}" + ); + + chmod_writable(tmp.path()); + } +} + +/// #1840 positive control: `go mod tidy` keeps a fork replace nothing +/// requires. With the dependency requiring the upstream itself, the build +/// links it under its own path, so the patch applies, builds and checks +/// in sync: the module graph read from the module cache shows the fork is +/// not in the build. +#[test] +fn stale_fork_replace_keeps_a_transitive_patch() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(stale-fork-replace)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage_with(tmp.path(), "1.16"); + publish( + tmp.path(), + MID, + UVER, + &[ + ( + "go.mod", + &format!("module {MID}\n\ngo 1.16\n\nrequire {UMOD} {UVER}\n"), + ), + ( + "mid.go", + &format!( + "package mid\n\nimport u \"{UMOD}\"\n\nfunc Hello() string {{ return u.Greeting() }}\n" + ), + ), + ], + ); + let env = go_env(modcache.to_str().unwrap(), &proxy_url); + let readonly = readonly_env(&env); + mid_fork_consumer(&consumer, &format!("replace {FORK} => {UMOD} {UVER}"), &env); + assert!(go_out(&consumer, &readonly).contains("OUT: PRISTINE")); + write_patch(&consumer); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!(code, 0, "apply:\n{so}\n{se}"); + assert!(go_out(&consumer, &readonly).contains("OUT: PATCHED")); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!(code, 0, "apply --check:\n{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/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index f214e686d..61153c5d8 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -495,7 +495,7 @@ async fn read_go_workspace( out: &mut CandidateFiles, ) { use crate::vendor::go_workspace::{self, GoWorkspace, Loaded, Member}; - let (root, loaded, modfile) = match view { + let (root, loaded, modfile, module_graph) = match view { ProjectView::Memory(_) => 'memory: { let Some(work) = read_advisory(view, unreadable, "go.work").await else { return; @@ -508,6 +508,7 @@ async fn read_go_workspace( std::path::PathBuf::new(), Loaded::Invalid(format!("go.work: {problem}, so go refuses to build")), None, + None, ); } let mut members = Vec::new(); @@ -540,7 +541,7 @@ async fn read_go_workspace( members, graph: None, }; - (std::path::PathBuf::new(), Loaded::Workspace(ws), None) + (std::path::PathBuf::new(), Loaded::Workspace(ws), None, None) } ProjectView::Disk(_) | ProjectView::Snapshot(_) => { let Some(root) = view.disk_root_reading(std::iter::empty::<&std::path::Path>()) else { @@ -560,19 +561,39 @@ async fn read_go_workspace( touched.push(member.dir.join("go.sum")); } } + // Module mode: whether a module-to-module replace is in effect + // can turn on modules only the dependencies require (#1840). + let mut module_graph = None; + if loaded == Loaded::None { + touched.push(root.join("go.mod")); + let go_mod = crate::utils::fs::read_regular_to_string(&root.join("go.mod")).await; + if let Ok(go_mod) = go_mod { + let go_mod = crate::vendor::go_mod_edit::normalize_for_read(&go_mod); + if crate::vendor::go_build_list::may_need_module_graph(&go_mod) { + touched.extend( + crate::vendor::go_build_list::module_graph_reads(&go_mod) + .into_iter() + .map(|rel| root.join(rel)), + ); + module_graph = + Some(crate::vendor::go_build_list::module_graph(&root, &go_mod).await); + } + } + } view.disk_root_reading(&touched); // `-modfile` in GOFLAGS sends the build to another go.mod (#1799). let modfile = go_workspace::modfile_setting(&root); - (root, loaded, modfile) + (root, loaded, modfile, module_graph) } }; - if loaded == Loaded::None && modfile.is_none() { + if loaded == Loaded::None && modfile.is_none() && module_graph.is_none() { return; } let hosted = crate::patch::redirect::HostedGoWorkspace { root, loaded, modfile, + module_graph, }; if let Ok(json) = serde_json::to_string(&hosted) { out.files diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index f209cd7a1..765a1cc21 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -9392,6 +9392,10 @@ pub struct HostedGoWorkspace { /// [`crate::vendor::go_workspace::modfile_setting`] words it (#1799). #[serde(default)] pub modfile: Option, + /// The root module's unpruned module graph from the module cache, when + /// a module-to-module replace makes it matter (#1840). + #[serde(default)] + pub module_graph: Option, } fn rewrite_golang( @@ -9431,6 +9435,9 @@ fn rewrite_golang( return; } let mut mod_target = has_go_mod.then(|| GoTarget::new(files, "go.mod", "go.sum", true)); + if let Some(target) = mod_target.as_mut() { + target.graph = workspace.as_ref().and_then(|ws| ws.module_graph.clone()); + } let mut work_target = root_work.map(|_| GoTarget::new(files, "go.work", "go.work.sum", false)); for dep in &golang { @@ -9612,6 +9619,9 @@ struct GoTarget { go_sum: crate::vendor::go_sum_edit::GoSumEditor, mod_changed: bool, sum_changed: bool, + /// The module graph [`HostedGoWorkspace::module_graph`] read for this + /// go.mod. + graph: Option, } impl GoTarget { @@ -9635,6 +9645,7 @@ impl GoTarget { go_sum, mod_changed: false, sum_changed: false, + graph: None, } } @@ -9675,6 +9686,7 @@ fn rewrite_golang_target( go_sum, mod_changed, sum_changed, + graph, .. } = target; let in_workspace = selection.is_some(); @@ -9715,7 +9727,14 @@ fn rewrite_golang_target( let selection = selection.unwrap_or_else(|| { let zips = go_sum.zip_versions(fname); let zips: Vec<&str> = zips.iter().map(String::as_str).collect(); - go_build_list::select(go_mod, fname, scan.required.as_deref(), &zips, &dep.version) + go_build_list::select_in( + go_mod, + graph.as_ref(), + fname, + scan.required.as_deref(), + &zips, + &dep.version, + ) }); // A pin the build provably doesn't use: a selected version other than // the patched one, or a module go builds only as the target of another @@ -9749,9 +9768,16 @@ fn rewrite_golang_target( }), _ => None, }; + // A replace target the files only suspect (a go 1.16 graph whose + // go.mod files the module cache lacks, #1840) refuses a new pin but + // never removes one that may be working. + let proven = !matches!(selection, go_build_list::Selection::ReplaceTarget(_)) + || go_build_list::proves_replace_target(go_mod, graph.as_ref()); if let Some(warning) = inert { result.warnings.push(warning); - let stale_hosted = prior.filter(|e| e.owner == Some(go_mod_edit::ReplaceOwner::Hosted)); + let stale_hosted = prior + .filter(|e| e.owner == Some(go_mod_edit::ReplaceOwner::Hosted)) + .filter(|_| proven); if let Some(stale) = stale_hosted { if let Ok(Some(new)) = go_mod_edit::remove_replace_entry(go_mod, fname, go_mod_edit::ReplaceOwner::Hosted) @@ -24019,6 +24045,7 @@ packages: ), }), modfile: None, + module_graph: None, }; ( GO_WORKSPACE_KEY.to_string(), @@ -24241,6 +24268,7 @@ packages: graph: None, }), modfile: None, + module_graph: None, }; again.insert( GO_WORKSPACE_KEY.to_string(), @@ -24257,6 +24285,126 @@ packages: ); } + /// #1840: in a go 1.16 module the fork replace applies to a fork only a + /// dependency requires. The module graph read from the module cache + /// decides: with the fork in it the pin is refused; with a stale fork + /// replace nothing requires, the module is built in its own right and + /// the pin is written. + #[test] + fn golang_go116_module_graph_decides_the_fork_replace() { + use crate::vendor::go_build_list::ModuleGraph; + use crate::vendor::go_workspace::Loaded; + let go_mod = "module example.com/app\n\ngo 1.16\n\nrequire example.com/mid v1.0.0\n\n\ + replace other.com/fork => github.com/foo/bar v1.4.2\n"; + let graph = |modules: &[(&str, &str)]| ModuleGraph { + selected: modules + .iter() + .map(|(m, v)| (m.to_string(), v.to_string())) + .collect(), + complete: true, + }; + for (module_graph, inert) in [ + (None, true), + ( + Some(graph(&[ + ("example.com/mid", "v1.0.0"), + ("other.com/fork", "v1.4.2"), + ])), + true, + ), + ( + Some(graph(&[ + ("example.com/mid", "v1.0.0"), + ("github.com/foo/bar", "v1.4.2"), + ])), + false, + ), + ] { + let mut files = golang_files(); + files.insert("go.mod".to_string(), go_mod.to_string()); + let hosted = HostedGoWorkspace { + root: std::path::PathBuf::new(), + loaded: Loaded::None, + modfile: None, + module_graph: module_graph.clone(), + }; + files.insert( + GO_WORKSPACE_KEY.to_string(), + serde_json::to_string(&hosted).unwrap(), + ); + let ovr = golang_override(); + let out = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); + if inert { + assert!(out.files.is_empty(), "{module_graph:?}: {:?}", out.files); + assert_eq!( + out.warnings + .iter() + .map(|w| w.code.as_str()) + .collect::>(), + ["redirect_golang_not_in_module_graph"], + "{module_graph:?}" + ); + } else { + assert!(out.warnings.is_empty(), "{:?}", out.warnings); + assert!(out.files["go.mod"].contains("replace github.com/foo/bar v1.4.2 =>")); + } + } + } + + /// #1840: without a complete module graph a go 1.16 fork replace is + /// only suspected to make the hosted pin inert, so the pin is neither + /// confirmed nor removed; a graph that shows the fork removes it. + #[test] + fn golang_go116_suspected_replace_target_keeps_the_hosted_pin() { + use crate::vendor::go_build_list::ModuleGraph; + use crate::vendor::go_workspace::Loaded; + let pin = format!( + "replace github.com/foo/bar v1.4.2 => {} v1.4.2-socketpatch.1", + golang_socket_module() + ); + let go_mod = format!( + "module example.com/app\n\ngo 1.16\n\nrequire example.com/mid v1.0.0\n\n\ + replace other.com/fork => github.com/foo/bar v1.4.2\n\n{pin}\n" + ); + let fork = ModuleGraph { + selected: [("other.com/fork".to_string(), "v1.4.2".to_string())].into(), + complete: true, + }; + let partial = ModuleGraph { + selected: Default::default(), + complete: false, + }; + for (module_graph, removed) in [(None, false), (Some(partial), false), (Some(fork), true)] { + let mut files = golang_files(); + files.insert("go.mod".to_string(), go_mod.clone()); + let hosted = HostedGoWorkspace { + root: std::path::PathBuf::new(), + loaded: Loaded::None, + modfile: None, + module_graph: module_graph.clone(), + }; + files.insert( + GO_WORKSPACE_KEY.to_string(), + serde_json::to_string(&hosted).unwrap(), + ); + let ovr = golang_override(); + let out = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); + assert!(out.confirmed_golang_uuids.is_empty(), "{module_graph:?}"); + assert_eq!( + out.warnings[0].code, "redirect_golang_not_in_module_graph", + "{module_graph:?}: {:?}", + out.warnings + ); + match out.files.get("go.mod") { + Some(text) => { + assert!(removed, "{module_graph:?}: {text}"); + assert!(!text.contains(&pin), "{text}"); + } + None => assert!(!removed, "{module_graph:?}"), + } + } + } + /// #1799: `-modfile` in GOFLAGS sends the build to another go.mod, so /// the hosted pin is refused, nothing written or confirmed. #[test] @@ -24268,6 +24416,7 @@ packages: root: std::path::PathBuf::new(), loaded: Loaded::None, modfile: Some("GOFLAGS sets -modfile=go.alt.mod, so go builds from go.alt.mod".into()), + module_graph: None, }; files.insert( GO_WORKSPACE_KEY.to_string(), 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 bc4e310f6..e3f9bcaf0 100644 --- a/crates/socket-patch-core/src/vendor/go_build_list.rs +++ b/crates/socket-patch-core/src/vendor/go_build_list.rs @@ -36,13 +36,24 @@ //! module@version under two module paths, so the zip can't be from the //! target being built in its own right. Such a module is //! [`Selection::ReplaceTarget`] (#1823). +//! +//! Whether that replace is in effect turns on the version of the replaced +//! module the build selects. A pruned go.mod lists it. An unpruned one +//! omits a module only a dependency requires, yet its replace still +//! applies there (#1840), so that answer comes from the module graph read +//! from the module cache ([`module_graph`]). use std::cmp::Ordering; +use std::collections::BTreeMap; use std::path::Path; +use serde::{Deserialize, Serialize}; + use crate::utils::fs::read_regular_to_string; -use super::go_mod_edit::{parse_replace_entries, parse_required_versions, prunes_module_graph}; +use super::go_mod_edit::{ + parse_replace_entries, parse_required_versions, prunes_module_graph, ReplaceEntry, +}; use super::go_sum_edit::{go_semver_cmp, go_sum_lines}; /// Whether the build list selects the patched version of a module. @@ -75,9 +86,31 @@ impl Selection { } } +/// What MVS selects over a module-mode project's whole module graph, +/// read from the module cache ([`module_graph`]). +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ModuleGraph { + /// The selected version of each module the graph requires. + pub selected: BTreeMap, + /// Every go.mod the graph needed was read, so a module missing from + /// `selected` is not in the build. + pub complete: bool, +} + /// [`Selection`] for `module@version` from the text of `go.mod` and (when /// present) `go.sum`. pub fn selection(go_mod: &str, go_sum: Option<&str>, module: &str, version: &str) -> Selection { + selection_in(go_mod, go_sum, None, module, version) +} + +/// [`selection`] with the project's [`ModuleGraph`], when read. +pub fn selection_in( + go_mod: &str, + go_sum: Option<&str>, + graph: Option<&ModuleGraph>, + module: &str, + version: &str, +) -> Selection { let required = parse_required_versions(go_mod).remove(module); let zips: Vec<&str> = go_sum .map(|text| { @@ -86,7 +119,7 @@ pub fn selection(go_mod: &str, go_sum: Option<&str>, module: &str, version: &str .collect() }) .unwrap_or_default(); - select(go_mod, module, required.as_deref(), &zips, version) + select_in(go_mod, graph, module, required.as_deref(), &zips, version) } /// [`selection`] from already-parsed parts: the root go.mod's text (whether @@ -98,10 +131,22 @@ pub fn select( required: Option<&str>, zips: &[&str], version: &str, +) -> Selection { + select_in(go_mod, None, module, required, zips, version) +} + +/// [`select`] with the project's [`ModuleGraph`], when read. +pub fn select_in( + go_mod: &str, + graph: Option<&ModuleGraph>, + module: &str, + required: Option<&str>, + zips: &[&str], + version: &str, ) -> Selection { // A zip line at the version a module-to-module replace targets is the // replacement's, not the module's own (see the module docs). - let replaced = replaced_module(go_mod, module, version); + let replaced = replaced_module(go_mod, graph, module, version); let zips: Vec<&str> = zips .iter() .copied() @@ -137,33 +182,136 @@ pub fn select( } } +/// The module-to-module replaces in `go_mod` whose target is exactly +/// `module@version`. +fn replaces_targeting<'a>( + entries: &'a [ReplaceEntry], + module: &'a str, + version: &'a str, +) -> impl Iterator { + entries.iter().filter(move |e| { + !e.socket_owned() + && e.module != module + && e.rhs_module.as_deref() == Some(module) + && e.rhs_version.as_deref() == Some(version) + }) +} + +/// Whether [`select`]'s answer for `module@version` can turn on the +/// [`ModuleGraph`]: an unpruned go.mod with a module-to-module replace +/// targeting it. Only then is the graph worth reading. +pub fn needs_module_graph(go_mod: &str, module: &str, version: &str) -> bool { + !prunes_module_graph(go_mod) + && replaces_targeting(&parse_replace_entries(go_mod), module, version) + .next() + .is_some() +} + +/// Whether any answer of [`select`] over `go_mod` can turn on the +/// [`ModuleGraph`] ([`needs_module_graph`] for every module). +pub fn may_need_module_graph(go_mod: &str) -> bool { + !prunes_module_graph(go_mod) + && parse_replace_entries(go_mod) + .iter() + .any(|e| !e.socket_owned() && e.rhs_module.is_some() && e.rhs_version.is_some()) +} + /// The module whose module-to-module `replace` in `go_mod` targets exactly -/// `module@version`, if that replace is the one in effect: go.mod requires -/// the replaced module, and no replace pinned to the required version -/// shadows a version-less one (go prefers the versioned replace). `go mod -/// tidy` keeps unused replaces, so one of a module the build doesn't use, or -/// pinned to a version it doesn't require, explains nothing. -fn replaced_module(go_mod: &str, module: &str, version: &str) -> Option { +/// `module@version`, if that replace is the one in effect: the build +/// selects the replaced module, and no replace pinned to its selected +/// version shadows a version-less one (go prefers the versioned replace). +/// `go mod tidy` keeps unused replaces, so one of a module the build +/// doesn't use, or pinned to a version it doesn't select, explains nothing. +/// +/// A pruned go.mod's `require` holds the selected version. An unpruned +/// one's holds only lower bounds and omits modules only a dependency +/// requires, so a complete `graph` answers there (#1840). Without one, a +/// replace that may be in effect counts as in effect (the safe side: no +/// patch is reported built that isn't), unless go.mod requires +/// `module@version` itself: that puts it in the build under its own path, +/// and go refuses one module@version under two. +fn replaced_module( + go_mod: &str, + graph: Option<&ModuleGraph>, + module: &str, + version: &str, +) -> Option { + /// What the files show of the replaced module's selected version. + enum Fork { + Selected(String), + Absent, + AtLeast(String), + Unknown, + } + let pruned = prunes_module_graph(go_mod); let required = parse_required_versions(go_mod); let entries = parse_replace_entries(go_mod); - entries - .iter() - .filter(|e| e.module != module) - .filter(|e| { - e.rhs_module.as_deref() == Some(module) && e.rhs_version.as_deref() == Some(version) - }) - .find(|e| { - let Some(req) = required.get(&e.module) else { - return false; - }; - match e.version.as_deref() { - Some(v) => v == req, - None => !entries - .iter() - .any(|o| o.module == e.module && o.version.as_deref() == Some(req.as_str())), - } + let graph = graph.filter(|g| g.complete && !pruned); + let fork = |m: &str| match (graph, required.get(m)) { + (Some(g), _) => g + .selected + .get(m) + .cloned() + .map_or(Fork::Absent, Fork::Selected), + (None, Some(r)) if pruned => Fork::Selected(r.clone()), + (None, None) if pruned => Fork::Absent, + (None, Some(r)) => Fork::AtLeast(r.clone()), + (None, None) => Fork::Unknown, + }; + let built_itself = required.get(module).map(String::as_str) == Some(version); + let in_effect = replaces_targeting(&entries, module, version) + .find(|e| match (fork(&e.module), e.version.as_deref()) { + (Fork::Selected(sel), Some(v)) => v == sel, + (Fork::Selected(sel), None) => !entries + .iter() + .any(|o| o.module == e.module && o.version.as_deref() == Some(sel.as_str())), + (Fork::Absent, _) => false, + // MVS never selects below go.mod's own require. + (Fork::AtLeast(min), Some(v)) => go_semver_cmp(v, &min).is_ge() && !built_itself, + (Fork::AtLeast(_), None) | (Fork::Unknown, _) => !built_itself, }) - .map(|e| e.module.clone()) + .map(|e| e.module.clone()); + in_effect +} + +/// Whether a [`Selection::ReplaceTarget`] from [`select_in`] over `go_mod` +/// is proven, not only the safe guess for an unpruned graph read without +/// every go.mod it needs. +pub fn proves_replace_target(go_mod: &str, graph: Option<&ModuleGraph>) -> bool { + prunes_module_graph(go_mod) || graph.is_some_and(|g| g.complete) +} + +/// The project files [`super::go_workspace::module_graph`] reads for +/// `go_mod` besides the module cache: each directory replacement's go.mod, +/// root-relative. +pub fn module_graph_reads(go_mod: &str) -> Vec { + parse_replace_entries(go_mod) + .into_iter() + .filter_map(|e| e.path) + .map(|dir| format!("{}/go.mod", dir.replace('\\', "/").trim_end_matches('/'))) + .collect() +} + +/// The [`ModuleGraph`] of the module-mode project at `project_root`, read +/// from the module cache, when [`needs_module_graph`] says the answer for +/// `module@version` can turn on it. +pub async fn module_graph_for( + project_root: &Path, + go_mod: &str, + module: &str, + version: &str, +) -> Option { + if !needs_module_graph(go_mod, module, version) { + return None; + } + Some(module_graph(project_root, go_mod).await) +} + +/// [`super::go_workspace::module_graph`] of the module-mode project at +/// `project_root` against the module cache go uses. +pub async fn module_graph(project_root: &Path, go_mod: &str) -> ModuleGraph { + let cache = crate::crawlers::go_crawler::GoCrawler::get_gomodcache(); + super::go_workspace::module_graph(project_root, go_mod, cache.as_deref()).await } /// [`selection`] for a project directory. In workspace mode the build @@ -191,7 +339,14 @@ pub async fn project_selection( let go_sum = read_regular_to_string(&project_root.join("go.sum")) .await .ok(); - Some(selection(&go_mod, go_sum.as_deref(), module, version)) + let graph = module_graph_for(project_root, &go_mod, module, version).await; + Some(selection_in( + &go_mod, + go_sum.as_deref(), + graph.as_ref(), + module, + version, + )) } #[cfg(test)] @@ -322,20 +477,27 @@ mod tests { fn inactive_replace_target_keeps_the_zip_as_evidence() { let s = sum(&[&format!("{UP} v1.0.0")]); for go in ["go 1.21", "go 1.16"] { - for (require, replace) in [ - ("", format!("replace other.com/fork => {UP} v1.0.0")), - ( - "require other.com/fork v1.0.0\n", - format!("replace other.com/fork v0.5.0 => {UP} v1.0.0"), - ), + // A pruned graph lists every module it builds, so a fork the + // root go.mod doesn't require is not in the build. + let unrequired = + (go == "go 1.21").then(|| ("", format!("replace other.com/fork => {UP} v1.0.0"))); + // A pruned go.mod's require is the selected version, so the + // versioned replace pinned there shadows the version-less one. + // An unpruned one's is a lower bound: see + // `unpruned_module_graph_settles_the_fork_replace`. + let shadowed = (go == "go 1.21").then(|| { ( "require other.com/fork v1.0.0\n", format!( "replace other.com/fork => {UP} v1.0.0\n\ replace other.com/fork v1.0.0 => ../fork" ), - ), - ] { + ) + }); + for (require, replace) in unrequired.into_iter().chain(shadowed).chain([( + "require other.com/fork v1.0.0\n", + format!("replace other.com/fork v0.5.0 => {UP} v1.0.0"), + )]) { let m = format!("module c\n\n{go}\n\n{require}\n{replace}\n"); assert_eq!( selection(&m, Some(&s), UP, "v1.0.0"), @@ -346,6 +508,182 @@ mod tests { } } + /// #1840: an unpruned graph (go 1.16, or no `go` directive) omits from + /// the root go.mod a fork only a dependency requires, but the root's + /// replace still applies to it, so the zip line is the fork's. + #[test] + fn unpruned_replace_of_a_transitive_fork_is_a_replace_target() { + let s = sum(&[&format!("{UP} v1.0.0"), &format!("{UP} v1.0.0/go.mod")]); + let via = Selection::ReplaceTarget("other.com/fork".into()); + for go in ["go 1.16", ""] { + for replace in [ + format!("replace other.com/fork => {UP} v1.0.0"), + format!("replace other.com/fork v1.0.0 => {UP} v1.0.0"), + format!("replace (\n\tother.com/fork => {UP} v1.0.0\n)"), + ] { + let m = + format!("module c\n\n{go}\n\nrequire example.com/mid v1.0.0\n\n{replace}\n"); + assert_eq!( + selection(&m, Some(&s), UP, "v1.0.0"), + via, + "{go} / {replace}" + ); + // No zip line at all: still built only as the replacement. + assert_eq!(selection(&m, None, UP, "v1.0.0"), via, "{go} / {replace}"); + } + } + } + + fn graph(selected: &[(&str, &str)], complete: bool) -> ModuleGraph { + ModuleGraph { + selected: selected + .iter() + .map(|(m, v)| (m.to_string(), v.to_string())) + .collect(), + complete, + } + } + + /// With the module graph read, the replaced module's selected version + /// settles a go 1.16 fork replace either way: one `go mod tidy` kept + /// though nothing requires the fork explains nothing (the module is + /// built in its own right), and one a dependency raised the fork to + /// applies although go.mod requires a lower version. + #[test] + fn unpruned_module_graph_settles_the_fork_replace() { + let s = sum(&[&format!("{UP} v1.0.0")]); + let via = Selection::ReplaceTarget("other.com/fork".into()); + let m = format!( + "module c\n\ngo 1.16\n\nrequire example.com/mid v1.0.0\n\n\ + replace other.com/fork => {UP} v1.0.0\n" + ); + let stale = graph(&[("example.com/mid", "v1.0.0"), (UP, "v1.0.0")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&stale), UP, "v1.0.0"), + Selection::Selected + ); + let forked = graph( + &[("example.com/mid", "v1.0.0"), ("other.com/fork", "v1.0.0")], + true, + ); + assert_eq!(selection_in(&m, Some(&s), Some(&forked), UP, "v1.0.0"), via); + // A graph missing go.mod files can't show the fork is absent. + let partial = graph(&[("example.com/mid", "v1.0.0")], false); + assert_eq!( + selection_in(&m, Some(&s), Some(&partial), UP, "v1.0.0"), + via + ); + + // go.mod requires fork v0.9.0, a dependency raises it to v1.0.0, + // where the versioned replace applies. Without a complete graph + // go.mod's lower bound can't rule that out. + let m = format!( + "module c\n\ngo 1.16\n\nrequire other.com/fork v0.9.0\n\n\ + replace other.com/fork v1.0.0 => {UP} v1.0.0\n" + ); + let raised = graph(&[("other.com/fork", "v1.0.0")], true); + assert_eq!(selection_in(&m, Some(&s), Some(&raised), UP, "v1.0.0"), via); + let partial = graph(&[("other.com/fork", "v0.9.0")], false); + assert_eq!( + selection_in(&m, Some(&s), Some(&partial), UP, "v1.0.0"), + via + ); + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), via); + let kept = graph(&[("other.com/fork", "v0.9.0")], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&kept), UP, "v1.0.0"), + Selection::Selected + ); + // A versioned replace pinned at the selected version wins over the + // version-less one. + for (require, selected) in [ + ("example.com/mid v1.0.0", "v1.1.0"), + ("other.com/fork v1.1.0", "v1.1.0"), + ] { + let m = format!( + "module c\n\ngo 1.16\n\nrequire {require}\n\n\ + replace other.com/fork => {UP} v1.0.0\n\ + replace other.com/fork {selected} => ../fork\n" + ); + let shadowed = graph(&[("other.com/fork", selected)], true); + assert_eq!( + selection_in(&m, Some(&s), Some(&shadowed), UP, "v1.0.0"), + Selection::Selected, + "{require}" + ); + // go.mod alone can't tell a dependency didn't raise the fork + // past the shadowing version. + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), via, "{require}"); + } + // A pruned go.mod lists what it builds: the graph isn't consulted. + let m = format!("module c\n\ngo 1.21\n\nreplace other.com/fork => {UP} v1.0.0\n"); + assert_eq!( + selection_in(&m, Some(&s), Some(&forked), UP, "v1.0.0"), + Selection::Selected + ); + } + + #[test] + fn only_a_pruned_go_mod_or_a_complete_graph_proves_a_replace_target() { + let m16 = "module c\n\ngo 1.16\n"; + assert!(proves_replace_target("module c\n\ngo 1.21\n", None)); + assert!(!proves_replace_target(m16, None)); + assert!(!proves_replace_target(m16, Some(&graph(&[], false)))); + assert!(proves_replace_target(m16, Some(&graph(&[], true)))); + } + + #[test] + fn module_graph_is_needed_only_for_an_unpruned_fork_replace() { + let fork = format!("replace other.com/fork => {UP} v1.0.0\n"); + for (go, replace, needed) in [ + ("go 1.16", fork.as_str(), true), + ("", fork.as_str(), true), + ("go 1.21", fork.as_str(), false), + ("go 1.16", "replace other.com/fork => ../fork\n", false), + ("go 1.16", "", false), + ( + "go 1.16", + "replace other.com/fork v1.0.0 => patch.socket.dev/gopatch/4d5e6f70-8192-4a1b-8c2d-0123456789ab v1.0.0-socketpatch.1\n", + false, + ), + ] { + let m = format!("module c\n\n{go}\n\n{replace}"); + assert_eq!(needs_module_graph(&m, UP, "v1.0.0"), needed, "{m}"); + assert_eq!(may_need_module_graph(&m), needed, "{m}"); + } + let m = format!("module c\n\ngo 1.16\n\n{fork}"); + assert!(!needs_module_graph(&m, UP, "v1.1.0")); + } + + /// Without the graph, go.mod's require of a lower version of the + /// module doesn't put `module@version` itself in the build: go builds + /// v0.9.0 under its own path and v1.0.0 only as the fork. + #[test] + fn unpruned_root_require_of_another_version_keeps_the_fork_replace() { + let m = format!( + "module c\n\ngo 1.16\n\nrequire (\n\t{UP} v0.9.0\n\texample.com/mid v1.0.0\n)\n\n\ + replace other.com/fork => {UP} v1.0.0\n" + ); + let s = sum(&[&format!("{UP} v1.0.0")]); + assert_eq!( + selection(&m, Some(&s), UP, "v1.0.0"), + Selection::ReplaceTarget("other.com/fork".into()) + ); + } + + /// A root require of the module puts it in the build under its own + /// path, and go refuses one module@version under two paths, so a + /// replace of an unrequired fork can't explain its zip line. + #[test] + fn unpruned_root_require_of_the_module_keeps_its_zip() { + let m = format!( + "module c\n\ngo 1.16\n\nrequire {UP} v1.0.0\n\n\ + replace other.com/fork => {UP} v1.0.0\n" + ); + let s = sum(&[&format!("{UP} v1.0.0")]); + assert_eq!(selection(&m, Some(&s), UP, "v1.0.0"), Selection::Selected); + } + #[test] fn replace_target_at_another_version_keeps_its_own_evidence() { // The fork targets v1.0.0; a zip line at v1.1.0 is the module's own. diff --git a/crates/socket-patch-core/src/vendor/go_workspace.rs b/crates/socket-patch-core/src/vendor/go_workspace.rs index b452124ae..2c2218035 100644 --- a/crates/socket-patch-core/src/vendor/go_workspace.rs +++ b/crates/socket-patch-core/src/vendor/go_workspace.rs @@ -41,7 +41,7 @@ use serde::{Deserialize, Serialize}; use crate::utils::fs::read_regular_to_string; -use super::go_build_list::Selection; +use super::go_build_list::{ModuleGraph, Selection}; use super::go_mod_edit::{self, ReplaceEntry}; use super::go_sum_edit::{go_semver_cmp, go_sum_lines}; @@ -502,6 +502,30 @@ fn selected_versions( selected } +/// The module graph of the main module at `dir` (module mode, go.mod text +/// `go_mod`), read as [`GoWorkspace::read_graph_from`] reads a one-member +/// workspace's: what MVS selects for each module, and whether every go.mod +/// it needed was read. +pub async fn module_graph(dir: &Path, go_mod: &str, cache: Option<&Path>) -> ModuleGraph { + let mut ws = GoWorkspace { + work_file: dir.join("go.work"), + work: String::new(), + work_sum: None, + members: vec![Member { + dir: dir.to_path_buf(), + go_mod: Some(go_mod_edit::normalize_for_read(go_mod)), + go_sum: None, + }], + graph: None, + }; + ws.read_graph_from(cache).await; + let graph = ws.graph.unwrap_or_default(); + ModuleGraph { + complete: graph.values().all(Option::is_some), + selected: selected_versions(&ws.members, &graph), + } +} + impl GoWorkspace { /// go.work's own `replace` directives. pub fn replaces(&self) -> Vec { @@ -1308,6 +1332,35 @@ mod tests { ); } + /// #1840: a go 1.16 module's graph reaches a fork only a dependency + /// requires, which the root go.mod never lists. + #[tokio::test] + async fn module_graph_reaches_a_fork_only_a_dependency_requires() { + let tmp = tempfile::tempdir().unwrap(); + let (root, cache) = (tmp.path().join("app"), tmp.path().join("cache")); + let go_mod = "module example.com/app\n\ngo 1.16\n\nrequire example.com/mid v1.0.0\n\n\ + replace other.com/fork => example.com/upstream v1.0.0\n"; + cached( + &cache, + "example.com/mid", + "v1.0.0", + "1.16", + "\nrequire other.com/fork v1.0.0\n", + ); + cached(&cache, "example.com/upstream", "v1.0.0", "1.16", ""); + let graph = module_graph(&root, go_mod, Some(&cache)).await; + assert!(graph.complete, "{graph:?}"); + assert_eq!( + graph.selected.get("other.com/fork").map(String::as_str), + Some("v1.0.0") + ); + // Without the dependency's go.mod the graph is incomplete. + let empty = tmp.path().join("empty"); + let graph = module_graph(&root, go_mod, Some(&empty)).await; + assert!(!graph.complete); + assert!(!graph.selected.contains_key("other.com/fork")); + } + #[tokio::test] async fn selection_spans_every_member() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/vex/discover/golang.rs b/crates/socket-patch-core/src/vex/discover/golang.rs index 9e17d02f3..a44a3dbc6 100644 --- a/crates/socket-patch-core/src/vex/discover/golang.rs +++ b/crates/socket-patch-core/src/vex/discover/golang.rs @@ -124,7 +124,7 @@ use super::{ }; use crate::patch::path_safety::is_safe_single_segment; use crate::utils::go_env::{go_env, goflags_modfile, GoflagsModfile}; -use crate::vendor::go_build_list::{self, Selection}; +use crate::vendor::go_build_list::{self, ModuleGraph, Selection}; use crate::vendor::go_mod_edit::{ self, block_structure_error, hosted_module_uuid, normalize_for_read, ReplaceEntry, ReplaceOwner, HOSTED_GO_MODULE_PREFIX, @@ -283,7 +283,21 @@ async fn extract_with(ctx: &DiscoverCtx<'_>, out: &mut Discovery, workspaces: bo .iter() .flat_map(|(_, entries)| entries) .any(|e| e.owner == Some(ReplaceOwner::Hosted)); - let graph = Graph::new(go_mod.as_deref(), workspace.as_ref().map(|ws| &ws.model)); + // Module mode: whether a module-to-module replace is in effect can turn + // on modules only the dependencies require (#1840). + let mut module_graph = None; + if let (Some(text), None) = (&go_mod, &workspace) { + if go_build_list::may_need_module_graph(text) { + if let Some(root) = ctx.disk_root_reading(go_build_list::module_graph_reads(text)) { + module_graph = Some(go_build_list::module_graph(root, text).await); + } + } + } + let graph = Graph::new( + go_mod.as_deref(), + workspace.as_ref().map(|ws| &ws.model), + module_graph.as_ref(), + ); let mut sums = SumPins::default(); if any_hosted { let sum_files = match &workspace { @@ -304,10 +318,16 @@ struct Graph<'a> { required: HashMap, root: Option<&'a str>, workspace: Option<&'a GoWorkspace>, + /// The root module's unpruned graph from the module cache, when read. + module_graph: Option<&'a ModuleGraph>, } impl<'a> Graph<'a> { - fn new(go_mod: Option<&'a str>, workspace: Option<&'a GoWorkspace>) -> Self { + fn new( + go_mod: Option<&'a str>, + workspace: Option<&'a GoWorkspace>, + module_graph: Option<&'a ModuleGraph>, + ) -> Self { let required = match workspace { Some(ws) => { let mut max: HashMap = HashMap::new(); @@ -331,6 +351,7 @@ impl<'a> Graph<'a> { required, root: go_mod, workspace, + module_graph, } } @@ -342,8 +363,9 @@ impl<'a> Graph<'a> { Some(ws) => ws.selection(module, version), None => { let root = self.root.unwrap_or_default(); - go_build_list::select( + go_build_list::select_in( root, + self.module_graph, module, self.required.get(module).map(String::as_str), zips, @@ -2105,7 +2127,7 @@ mod tests { .collect(), ), }; - let graph = Graph::new(None, Some(&ws)); + let graph = Graph::new(None, Some(&ws), None); let entry = |version: &str| ReplaceEntry { module: MODULE.to_string(), version: Some(version.to_string()), @@ -2128,7 +2150,7 @@ mod tests { assert!(out.diagnostics[0].detail.contains("is inert")); // Without the graph nothing proves the raise. let ws = GoWorkspace { graph: None, ..ws }; - let graph = Graph::new(None, Some(&ws)); + let graph = Graph::new(None, Some(&ws), None); let mut out = Discovery::default(); assert_eq!( replaced_purl(&entry("v1.5.0"), &graph, &HashSet::new(), GO_WORK, &mut out),