diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 1a0c56081..0b053a4b7 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -544,6 +544,11 @@ are patched by `socket-patch apply` like every other ecosystem: `go.sum`-verified, so in-place patching can't build). Commit `go.mod` + `.socket/go-patches/` + your `.socket/` patches so a clone builds the patched bytes with no further step. `socket-patch apply --check` is a read-only audit of the committed redirect. + When a `go.work` is in effect, `apply` and `vendor` refuse a `replace` that it would override (a + go.work `replace` of the module), ignore (the project isn't a `use` member) or reject (another member + replaces the same `module@version` elsewhere); one already written fails `apply --check` / + `vendor --check` and warns `go_workspace_replace_ineffective`, and `vex` omits the patch with + `go_workspace_override` (#393, #458, #531). ### sbt / Mill / scala-cli caches in agent mode diff --git a/crates/socket-patch-cli/src/commands/vex.rs b/crates/socket-patch-cli/src/commands/vex.rs index 5de8c3542..d980ae907 100644 --- a/crates/socket-patch-cli/src/commands/vex.rs +++ b/crates/socket-patch-cli/src/commands/vex.rs @@ -814,6 +814,37 @@ async fn generate_vex( ); } + // A Go replace that the go.work in effect overrides, ignores, or makes + // conflict with another member's (#393, #458, #531) is not what the + // build links, however intact its copy or pin: never attest it. + if !common.is_global() { + use socket_patch_core::utils::purl::{build_golang_purl, strip_purl_qualifiers}; + for issue in socket_patch_core::vendor::go_consumer_sync::workspace_audit(&common.cwd).await + { + let socket_patch_core::vendor::go_consumer_sync::GoSyncIssue::Workspace { + module, + version, + .. + } = &issue + else { + continue; + }; + let purl = build_golang_purl(module, version); + let before = outcome.applied.len(); + outcome + .applied + .retain(|p| strip_purl_qualifiers(p) != purl.as_str()); + if outcome.applied.len() == before { + continue; + } + note_warning(warnings, common, issue.code(), format!("{purl}: {issue}")); + outcome.failed.push(FailedPatch { + purl, + reason: GO_WORKSPACE_OVERRIDE.to_string(), + }); + } + } + // The plan's gate omissions (record_unavailable / record_mismatch / // vendor_unwired / redirect_unwired / wiring_conflict) join the omission // channel so they surface as per-purl `skipped` events like any @@ -1504,6 +1535,10 @@ fn join_and(items: &[&str]) -> String { } } +/// Omission tag for a Go patch whose go.mod replace the `go.work` in effect +/// overrides, ignores or rejects (#393, #458, #531). +const GO_WORKSPACE_OVERRIDE: &str = "go_workspace_override"; + /// Synthesize go-patches redirect targets for `vex`: for every socket-owned (`.socket/go-patches/`) `replace` in /// `go.mod` whose module+version maps to a golang PURL of the record view /// (`manifest` — for `vex` the merged manifest + ledger + lockfile view, so @@ -1634,6 +1669,10 @@ fn omission_phrase(reason: &str) -> &'static str { "the hosted ledger records it, but no lockfile wires its hosted patch to this \ package any more" } + GO_WORKSPACE_OVERRIDE => { + "the go.work in effect overrides, ignores or conflicts with its go.mod replace, \ + so the build does not link the patched module" + } WIRING_CONFLICT => { "the lockfiles wire this package to different patches, so which one the build \ installs cannot be determined" @@ -1816,6 +1855,7 @@ mod tests { VENDOR_UNWIRED, REDIRECT_UNWIRED, WIRING_CONFLICT, + GO_WORKSPACE_OVERRIDE, ] { assert_ne!( omission_phrase(tag), diff --git a/crates/socket-patch-cli/tests/e2e_golang_build.rs b/crates/socket-patch-cli/tests/e2e_golang_build.rs index c765bc39d..2369e65e3 100644 --- a/crates/socket-patch-cli/tests/e2e_golang_build.rs +++ b/crates/socket-patch-cli/tests/e2e_golang_build.rs @@ -675,3 +675,307 @@ fn patched_go_mod_requirement_bump_needs_go_mod_tidy() { chmod_writable(tmp.path()); } + +// ── the go.work in effect (#393, #458, #531) ───────────────────────── + +/// Workspace-mode go env: `-mod=mod` is rejected in workspace mode, so +/// `GOFLAGS` is cleared (the consumers carry a tidy go.sum instead). +fn workspace_env<'a>(modcache: &'a str, proxy_url: &'a str) -> Vec<(&'a str, &'a str)> { + let mut env = go_env(modcache, proxy_url); + env.retain(|(k, _)| *k != "GOFLAGS"); + env.push(("GOFLAGS", "")); + env +} + +/// A module at `dir` that requires the upstream and prints its greeting, +/// with the go.sum `go mod tidy` writes (outside any workspace). +fn consumer_module(dir: &Path, module: &str, modcache: &str, proxy_url: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("go.mod"), + format!("module {module}\n\ngo 1.21\n\nrequire {UMOD} {UVER}\n"), + ) + .unwrap(); + std::fs::write( + dir.join("main.go"), + format!( + "package main\n\nimport (\n\t\"fmt\"\n\t\"{UMOD}\"\n)\n\nfunc main() {{ fmt.Println(\"OUT:\", upstream.Greeting()) }}\n" + ), + ) + .unwrap(); + let mut env = go_env(modcache, proxy_url); + env.push(("GOWORK", "off")); + let tidy = go(dir, &["mod", "tidy"], &env); + assert!( + tidy.status.success(), + "go mod tidy: {}", + String::from_utf8_lossy(&tidy.stderr) + ); +} + +fn go_out(dir: &Path, env: &[(&str, &str)]) -> String { + let out = go(dir, &["run", "."], env); + format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) +} + +fn apply_json(dir: &Path, modcache: &Path) -> (i32, String, String) { + run_socket( + dir, + &[ + "apply", + "--offline", + "--ecosystems", + "golang", + "--json", + "--cwd", + dir.to_str().unwrap(), + ], + modcache, + ) +} + +fn check(dir: &Path, modcache: &Path) -> (i32, String, String) { + run_socket( + dir, + &[ + "apply", + "--check", + "--ecosystems", + "golang", + "--cwd", + dir.to_str().unwrap(), + ], + modcache, + ) +} + +/// Standalone `vex --json` over `dir`: must not attest the upstream patch +/// and must say why (`go_workspace_override`). +fn assert_vex_omits(dir: &Path, modcache: &Path, product: &str) { + let r = VexRun { + offline: true, + product: Some(product.into()), + ..VexRun::default() + } + .env("GOMODCACHE", modcache) + .env("GOFLAGS", ""); + let out = run_vex(&vex_e2e_common::binary(), dir, &r); + assert_absent(out.doc.as_ref(), UPURL); + vex_e2e_common::assert_not_attested(&out.envelope, UPURL, "go_workspace_override"); + assert!( + warnings_of(&out.stdout).contains(&"go_workspace_replace_ineffective".to_string()), + "{out}" + ); +} + +fn replace_lines(go_mod: &Path) -> Vec { + std::fs::read_to_string(go_mod) + .unwrap() + .lines() + .filter(|l| l.starts_with("replace ")) + .map(str::to_string) + .collect() +} + +/// #393: a user `replace` of the module in go.work overrides any member +/// replace. apply must refuse instead of reporting a patch the build +/// doesn't link; a replace written before the go.work appeared fails +/// `apply --check` and is left out of VEX. +#[test] +fn go_work_replace_overrides_the_socket_replace() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-replace)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + let mut tidy_env = go_env(mc, &proxy_url); + tidy_env.push(("GOWORK", "off")); + assert!(go(&consumer, &["mod", "tidy"], &tidy_env).status.success()); + let fork = tmp.path().join("fork"); + std::fs::create_dir_all(&fork).unwrap(); + std::fs::write(fork.join("go.mod"), format!("module {UMOD}\n\ngo 1.21\n")).unwrap(); + std::fs::write( + fork.join("lib.go"), + "package upstream\n\nfunc Greeting() string { return \"FORK\" }\n", + ) + .unwrap(); + let go_work = format!("go 1.21\n\nuse .\n\nreplace {UMOD} {UVER} => ../fork\n"); + std::fs::write(consumer.join("go.work"), &go_work).unwrap(); + let env = workspace_env(mc, &proxy_url); + assert!(go_out(&consumer, &env).contains("OUT: FORK")); + + write_patch(&consumer); + // The dry run predicts the refusal. + let (code, so, se) = run_socket( + &consumer, + &[ + "apply", + "--offline", + "--dry-run", + "--ecosystems", + "golang", + "--json", + "--cwd", + consumer.to_str().unwrap(), + ], + &modcache, + ); + assert_ne!( + code, 0, + "apply --dry-run must predict the refusal:\n{so}\n{se}" + ); + assert!(so.contains("go.work replaces"), "{so}"); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_ne!( + code, 0, + "apply must refuse an overridden replace:\n{so}\n{se}" + ); + assert!(so.contains("go.work replaces"), "{so}"); + assert!(replace_lines(&consumer.join("go.mod")).is_empty()); + assert!( + !consumer.join(".socket/go-patches").exists(), + "a refused apply leaves no copy behind" + ); + assert!(go_out(&consumer, &env).contains("OUT: FORK")); + + // Applied before the go.work replace existed: the replace is inert. + std::fs::remove_file(consumer.join("go.work")).unwrap(); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); + std::fs::write(consumer.join("go.work"), &go_work).unwrap(); + assert!(go_out(&consumer, &env).contains("OUT: FORK")); + // A re-run doesn't call the inert replace in sync. + let (code, so, se) = apply_json(&consumer, &modcache); + assert_ne!(code, 0, "re-run apply must refuse:\n{so}\n{se}"); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!( + code, 1, + "apply --check must flag the inert replace:\n{so}\n{se}" + ); + assert!( + se.contains("go_workspace_replace_ineffective") || se.contains("go.work replaces"), + "{se}" + ); + assert_vex_omits( + &consumer, + &modcache, + "pkg:golang/example.com/consumer@v0.0.1", + ); + + chmod_writable(tmp.path()); +} + +/// #458: a go.work that doesn't `use` the project makes every workspace +/// build ignore the project's go.mod, replace included. +#[test] +fn go_work_without_the_project_ignores_its_replace() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-not-member)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + let mut tidy_env = go_env(mc, &proxy_url); + tidy_env.push(("GOWORK", "off")); + assert!(go(&consumer, &["mod", "tidy"], &tidy_env).status.success()); + let svc = consumer.join("svc"); + consumer_module(&svc, "example.com/svc", mc, &proxy_url); + let go_work = "go 1.21\n\nuse ./svc\n"; + std::fs::write(consumer.join("go.work"), go_work).unwrap(); + let env = workspace_env(mc, &proxy_url); + assert!(go_out(&svc, &env).contains("OUT: PRISTINE")); + + write_patch(&consumer); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_ne!(code, 0, "apply must refuse an ignored replace:\n{so}\n{se}"); + assert!(so.contains("does not `use` this module"), "{so}"); + assert!(replace_lines(&consumer.join("go.mod")).is_empty()); + + // Applied before the go.work existed. + std::fs::remove_file(consumer.join("go.work")).unwrap(); + let (code, so, se) = apply_json(&consumer, &modcache); + assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); + std::fs::write(consumer.join("go.work"), go_work).unwrap(); + assert!(go_out(&svc, &env).contains("OUT: PRISTINE")); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!( + code, 1, + "apply --check must flag the ignored replace:\n{so}\n{se}" + ); + assert_vex_omits( + &consumer, + &modcache, + "pkg:golang/example.com/consumer@v0.0.1", + ); + + // Positive control: once go.work uses the project, the same replace + // works and nothing is flagged. + std::fs::write( + consumer.join("go.work"), + "go 1.21\n\nuse (\n\t.\n\t./svc\n)\n", + ) + .unwrap(); + assert!(go_out(&svc, &env).contains("OUT: PATCHED")); + let (code, so, se) = check(&consumer, &modcache); + assert_eq!(code, 0, "{so}\n{se}"); + + chmod_writable(tmp.path()); +} + +/// #531: two members replacing the same module@version with different +/// directories break the whole workspace build ("conflicting +/// replacements"). The second apply must refuse (the first member's +/// replace already patches the whole workspace); two replaces written +/// before the go.work appeared fail `apply --check` and VEX. +#[test] +fn go_work_member_replaces_must_not_conflict() { + if !golang_e2e_matrix::toolchain_ready("e2e_golang_build(go-work-conflict)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let (_consumer, modcache, proxy_url) = stage(tmp.path()); + let mc = modcache.to_str().unwrap(); + 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); + write_patch(&a); + write_patch(&b); + 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); + + let (code, so, se) = apply_json(&a, &modcache); + assert_eq!(code, 0, "first member apply:\n{so}\n{se}"); + assert!(go_out(&b, &env).contains("OUT: PATCHED")); + let (code, so, se) = apply_json(&b, &modcache); + assert_ne!(code, 0, "second member apply must refuse:\n{so}\n{se}"); + assert!(so.contains("conflicting"), "{so}"); + assert!(replace_lines(&b.join("go.mod")).is_empty()); + let out = go_out(&b, &env); + assert!( + out.contains("OUT: PATCHED"), + "the workspace still builds: {out}" + ); + let (code, so, se) = check(&a, &modcache); + assert_eq!(code, 0, "{so}\n{se}"); + + // Both applied before the go.work existed: the workspace can't build. + std::fs::remove_file(ws.join("go.work")).unwrap(); + let (code, so, se) = apply_json(&b, &modcache); + assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}"); + std::fs::write(ws.join("go.work"), go_work).unwrap(); + assert!(go_out(&b, &env).contains("conflicting replacements")); + for member in [&a, &b] { + let (code, so, se) = check(member, &modcache); + assert_eq!(code, 1, "apply --check must flag the conflict:\n{so}\n{se}"); + } + assert_vex_omits(&b, &modcache, "pkg:golang/example.com/b@v0.0.1"); + + chmod_writable(tmp.path()); +} diff --git a/crates/socket-patch-core/src/patch/redirect/golang_local.rs b/crates/socket-patch-core/src/patch/redirect/golang_local.rs index 42a95d1e3..981249b6b 100644 --- a/crates/socket-patch-core/src/patch/redirect/golang_local.rs +++ b/crates/socket-patch-core/src/patch/redirect/golang_local.rs @@ -238,6 +238,16 @@ pub async fn apply_go_redirect<'a>( ); } + // A replace the go.work in effect would override, ignore or reject + // (#393, #458, #531) leaves the build unpatched or broken: refuse before + // copying anything, dry run and in-sync re-run alike. + if let Some(refusal) = + crate::vendor::go_consumer_sync::replace_refusal(project_root, module, version, base_rel) + .await + { + return synthesized_result(purl, ©_dir, Vec::new(), false, Some(refusal)); + } + if dry_run { // Verify (read-only) against the pristine source for an accurate // "would patch" report, without creating the copy or editing go.mod. 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 5599e40f9..ce7649ab9 100644 --- a/crates/socket-patch-core/src/vendor/go_consumer_sync.rs +++ b/crates/socket-patch-core/src/vendor/go_consumer_sync.rs @@ -22,6 +22,15 @@ //! "missing go.sum entry" for an added requirement) until `go mod tidy` //! updates `go.mod` and `go.sum`. //! +//! * **The `go.work` in effect** (#393, #458, #531). In workspace mode go +//! reads `replace` directives from `go.work` and from the go.mod files of +//! the modules it `use`s. A project `go.mod` replace does nothing when +//! `go.work` replaces the same module itself, or when `go.work` doesn't +//! `use` the project, and it breaks the build ("conflicting replacements") +//! when another member replaces the same `module@version` elsewhere. +//! [`workspace_conflict`] says which; the writers refuse such a replace, +//! and [`audit`] reports one already written. +//! //! socket-patch does not regenerate either file itself: `go mod vendor` and //! `go mod tidy` need the go toolchain and every module in the graph, and //! rewrite files the user owns. Instead [`audit`] reports each disagreement @@ -40,6 +49,27 @@ pub const VENDOR_MODULES_TXT_CODE: &str = "go_vendor_modules_txt_out_of_sync"; /// Warning code for a patched module whose `go.mod` requires a newer (or a /// new) dependency than the consumer's `go.mod` lists. pub const REQUIREMENTS_CODE: &str = "go_requirements_out_of_sync"; +/// Warning code for a socket `replace` that the `go.work` in effect +/// overrides, ignores, or makes conflict with another member's. +pub const WORKSPACE_CODE: &str = "go_workspace_replace_ineffective"; + +/// Why the `go.work` in effect keeps a project `go.mod` replace from +/// taking effect. Paths are as displayed to the user. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WorkspaceConflict { + /// `go.work` replaces the module itself (`target`), which overrides + /// every member's replace of it. + WorkReplace { work_file: String, target: String }, + /// `go.work` doesn't `use` the project, so go ignores its go.mod. + NotAMember { work_file: String }, + /// Another `use` member (`member`) replaces the same `module@version` + /// with a different `target`; go refuses to build the workspace. + MemberConflict { + work_file: String, + member: String, + target: String, + }, +} /// One way the consumer's derived Go state disagrees with the socket /// `replace` directives. @@ -64,6 +94,14 @@ pub enum GoSyncIssue { required: String, listed: Option, }, + /// The socket `replace` for `module@version` in the project's `go.mod` + /// has no effect, or breaks the build, because of the `go.work` in + /// effect. + Workspace { + module: String, + version: String, + conflict: WorkspaceConflict, + }, } impl GoSyncIssue { @@ -72,6 +110,7 @@ impl GoSyncIssue { match self { GoSyncIssue::VendorModulesTxt { .. } => VENDOR_MODULES_TXT_CODE, GoSyncIssue::Requirement { .. } => REQUIREMENTS_CODE, + GoSyncIssue::Workspace { .. } => WORKSPACE_CODE, } } @@ -79,7 +118,8 @@ impl GoSyncIssue { pub fn module(&self) -> &str { match self { GoSyncIssue::VendorModulesTxt { module, .. } - | GoSyncIssue::Requirement { module, .. } => module, + | GoSyncIssue::Requirement { module, .. } + | GoSyncIssue::Workspace { module, .. } => module, } } } @@ -128,6 +168,45 @@ impl std::fmt::Display for GoSyncIssue { updated; run `go mod tidy`, then commit go.mod and go.sum" ) } + GoSyncIssue::Workspace { + module, + version, + conflict, + } => write!( + f, + "the go.mod `replace` for {module} {version} {}", + conflict.consequence(module, version) + ), + } + } +} + +impl WorkspaceConflict { + /// What the conflict does to a go.mod replace of `module@version`, and + /// how to fix it (completes "the go.mod `replace` for M V …"). + pub fn consequence(&self, module: &str, version: &str) -> String { + match self { + WorkspaceConflict::WorkReplace { work_file, target } => format!( + "has no effect: {work_file} replaces {module} with {target}, and a go.work \ + replace overrides every module's, so the build links {target} instead of \ + the patched copy; remove that replace from {work_file}, or move the \ + socket-patch replace there" + ), + WorkspaceConflict::NotAMember { work_file } => format!( + "has no effect: {work_file} does not `use` this module, so workspace builds \ + ignore its go.mod and link the unpatched {module}; add this module to \ + {work_file} (`go work use .`), or run socket-patch in a module it uses" + ), + WorkspaceConflict::MemberConflict { + work_file, + member, + target, + } => format!( + "conflicts with {member}/go.mod, which replaces {module} {version} with \ + {target}: go refuses to build the {work_file} workspace (\"conflicting \ + replacements\"); keep one replace for the whole workspace, in {work_file} \ + or in one member's go.mod" + ), } } } @@ -150,7 +229,8 @@ pub async fn audit( return Vec::new(); }; let replaces = go_mod_edit::parse_replace_entries(&go_mod); - let mut issues = vendor_modules_txt_issues(project_root, &replaces).await; + let mut issues = workspace_issues(project_root, &replaces).await; + issues.extend(vendor_modules_txt_issues(project_root, &replaces).await); issues.extend(requirement_issues(project_root, &go_mod, &replaces, pristine_go_mods).await); issues } @@ -168,6 +248,19 @@ pub async fn audit_warnings( .collect() } +/// Only the [`GoSyncIssue::Workspace`] issues of [`audit`]: the socket +/// replaces in the project's `go.mod` that the `go.work` in effect +/// overrides, ignores or makes conflict. For `vex`, which must not attest +/// a patch the build does not link. +pub async fn workspace_audit(project_root: &Path) -> Vec { + match read_regular_to_string(&project_root.join("go.mod")).await { + Ok(go_mod) => { + workspace_issues(project_root, &go_mod_edit::parse_replace_entries(&go_mod)).await + } + Err(_) => Vec::new(), + } +} + /// Where go looks for the vendor directory's `modules.txt`, and the command /// that regenerates it: the workspace root's `vendor/` (`go work vendor`) /// when a `go.work` is in effect, else the module's own (`go mod vendor`). @@ -187,41 +280,238 @@ fn modules_txt_location(project_root: &Path) -> (PathBuf, &'static str) { ) } -/// The `go.work` go would use for `project_root`: `GOWORK` when it names a +/// 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 /// the project. fn workspace_file(project_root: &Path) -> Option { - match std::env::var("GOWORK") { - Ok(v) if v == "off" => return None, - Ok(v) if !v.is_empty() => return Some(PathBuf::from(v)), - _ => {} - } let base = std::env::current_dir() .unwrap_or_default() .join(project_root); + match crate::utils::go_env::go_env("GOWORK").as_deref() { + Some("off") => return None, + Some(v) if v != "auto" => return Some(base.join(v)), + _ => {} + } base.ancestors() .map(|dir| dir.join("go.work")) .find(|file| file.is_file()) } +/// The `go.work` in effect, read once: its own replaces and each `use` +/// member's go.mod replaces. +struct Workspace { + work_file: PathBuf, + replaces: Vec, + members: Vec, +} + +struct Member { + /// The member directory as go.work spells it, joined to go.work's + /// directory (go resolves the member's relative replaces against it). + dir: PathBuf, + replaces: Vec, +} + +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 mut members = Vec::new(); + for dir in go_mod_edit::parse_use_dirs(&text) { + let dir = work_dir.join(dir); + 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)) + } + Err(_) => Vec::new(), + }; + members.push(Member { dir, replaces }); + } + Some(Workspace { + replaces: go_mod_edit::parse_replace_entries(&text), + work_file, + members, + }) +} + /// The replaces a workspace `modules.txt` records: `go work vendor` writes /// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns /// `(go.work replaces, every member's replaces)`. async fn workspace_replaces(project_root: &Path) -> (Vec, Vec) { - let Some(work_file) = workspace_file(project_root) else { - return (Vec::new(), Vec::new()); - }; - let Ok(text) = read_regular_to_string(&work_file).await else { - return (Vec::new(), Vec::new()); + match load_workspace(project_root).await { + Some(ws) => { + let members = ws.members.into_iter().flat_map(|m| m.replaces).collect(); + (ws.replaces, members) + } + None => (Vec::new(), Vec::new()), + } +} + +/// Whether the `go.work` in effect for `project_root` keeps a project +/// `go.mod` replace of `module@version` => `ours` (a replace as parsed +/// from, or about to be written to, that go.mod) from taking effect. +/// `None` when no workspace is in effect or the replace works. Read-only; +/// an unreadable `go.work` or member `go.mod` is skipped (go would fail +/// loudly on it anyway). +pub async fn workspace_conflict( + project_root: &Path, + module: &str, + version: &str, + ours: &ReplaceEntry, +) -> Option { + let ws = load_workspace(project_root).await?; + conflict_in(&ws, project_root, module, version, ours) +} + +/// Why `apply` / `vendor` must not write `replace module version => +/// .//module@version` into the project's go.mod, if the `go.work` +/// in effect would override, ignore or reject it: the refusal message. +pub async fn replace_refusal( + project_root: &Path, + module: &str, + version: &str, + base_rel: &str, +) -> Option { + let target = go_mod_edit::replace_target_path(base_rel, module, version); + let ours = go_mod_edit::parse_replace_body(&format!("{module} {version} => {target}"))?; + let conflict = workspace_conflict(project_root, module, version, &ours).await?; + Some(format!( + "refusing to write `replace {module} {version} => {target}` to go.mod: it {}", + conflict.consequence(module, version) + )) +} + +fn conflict_in( + ws: &Workspace, + project_root: &Path, + module: &str, + version: &str, + ours: &ReplaceEntry, +) -> Option { + let work_dir = ws.work_file.parent().unwrap_or(Path::new("")); + let display = display_path(project_root, &ws.work_file); + let project = same_dir_key(project_root); + let own = ws.members.iter().find(|m| same_dir_key(&m.dir) == project); + let our_target = lexical_target(own.map_or(project_root, |m| &m.dir), ours); + + // go drops every member replace of a module path that go.work replaces + // (any version). The build links the patched copy only if go.work's own + // replace for this version (else its version-less one) sends it there. + let work: Vec<&ReplaceEntry> = ws.replaces.iter().filter(|w| w.module == module).collect(); + if let Some(first) = work.first() { + let effective = work + .iter() + .find(|w| w.version.as_deref() == Some(version)) + .or_else(|| work.iter().find(|w| w.version.is_none())); + return match effective { + Some(w) if lexical_target(work_dir, w) == our_target => None, + _ => Some(WorkspaceConflict::WorkReplace { + work_file: display, + target: target_display(effective.unwrap_or(first)), + }), + }; + } + + if own.is_none() { + return Some(WorkspaceConflict::NotAMember { work_file: display }); + } + for member in ws + .members + .iter() + .filter(|m| same_dir_key(&m.dir) != project) + { + if let Some(other) = member.replaces.iter().find(|e| { + e.module == module + && e.version.as_deref() == Some(version) + && lexical_target(&member.dir, e) != our_target + }) { + return Some(WorkspaceConflict::MemberConflict { + work_file: display, + member: display_path(project_root, &member.dir), + target: target_display(other), + }); + } + } + None +} + +/// [`workspace_conflict`] for every versioned socket replace in the +/// project's go.mod, reading the workspace once. +async fn workspace_issues(project_root: &Path, replaces: &[ReplaceEntry]) -> Vec { + if !replaces.iter().any(ReplaceEntry::socket_owned) { + return Vec::new(); + } + let Some(ws) = load_workspace(project_root).await else { + return Vec::new(); }; - let work_dir = work_file.parent().unwrap_or(Path::new("")); - let mut members = Vec::new(); - for dir in go_mod_edit::parse_use_dirs(&text) { - if let Ok(go_mod) = read_regular_to_string(&work_dir.join(&dir).join("go.mod")).await { - members.extend(go_mod_edit::parse_replace_entries(&go_mod)); + let mut issues = Vec::new(); + for entry in replaces.iter().filter(|e| e.socket_owned()) { + let Some(version) = entry.version.as_deref() else { + continue; + }; + if let Some(conflict) = conflict_in(&ws, project_root, &entry.module, version, entry) { + issues.push(GoSyncIssue::Workspace { + module: entry.module.clone(), + version: version.to_string(), + conflict, + }); } } - (go_mod_edit::parse_replace_entries(&text), members) + issues +} + +/// A directory as go matches a module root against the workspace's `use` +/// list: absolute, with symlinks resolved when it exists, else lexically +/// normalized. +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(|_| crate::utils::relpath::normalize_lexically_keeping_escapes(&abs)) +} + +/// Where a replace sends the module, compared as go compares replacements +/// (`filepath.Join` of the go.mod's directory and the path: lexical, no +/// symlink resolution), or its `module version`. +fn lexical_target(go_mod_dir: &Path, entry: &ReplaceEntry) -> String { + match &entry.path { + Some(path) => { + let abs = std::env::current_dir() + .unwrap_or_default() + .join(go_mod_dir) + .join(path.replace('\\', "/")); + crate::utils::relpath::normalize_lexically_keeping_escapes(&abs) + .to_string_lossy() + .into_owned() + } + None => target_display(entry), + } +} + +fn target_display(entry: &ReplaceEntry) -> String { + match (&entry.path, &entry.rhs_module, &entry.rhs_version) { + (Some(path), _, _) => path.clone(), + (None, Some(m), Some(v)) => format!("{m} {v}"), + (None, Some(m), None) => m.clone(), + (None, None, _) => "".to_string(), + } +} + +/// `path` relative to `project_root` when it is inside it (`go.work`, +/// `tools`), else absolute. +fn display_path(project_root: &Path, path: &Path) -> String { + let base = same_dir_key(project_root); + let abs = std::env::current_dir().unwrap_or_default().join(path); + let abs = match (abs.parent(), abs.file_name()) { + (Some(parent), Some(name)) => same_dir_key(parent).join(name), + _ => abs, + }; + match abs.strip_prefix(&base) { + Ok(rel) if !rel.as_os_str().is_empty() => rel.to_string_lossy().replace('\\', "/"), + Ok(_) => ".".to_string(), + Err(_) => abs.display().to_string(), + } } async fn vendor_modules_txt_issues( @@ -527,7 +817,155 @@ mod tests { example.com/upstream\n\ # example.com/other v1.0.0 => ./tools/.socket/go-patches/example.com/other@v1.0.0\n", ); + // modules.txt agrees with go.work; the root's own replace is the + // problem now (#393), not the vendor directory. + let codes: Vec<&str> = audit(root, &none).await.iter().map(|i| i.code()).collect(); + assert_eq!(codes, vec![WORKSPACE_CODE]); + } + + fn codes(issues: &[GoSyncIssue]) -> Vec<&'static str> { + issues.iter().map(GoSyncIssue::code).collect() + } + + /// #393: a user replace of the module in go.work (versioned, version-less, + /// or for another version) overrides the member's socket replace. Our own + /// replace moved into go.work for this version does not. + #[tokio::test] + async fn go_work_replace_of_the_module_makes_the_replace_inert() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + consumer(root, REPLACE); + let none = HashMap::new(); + for user in [ + "replace example.com/upstream v1.0.0 => ../fork", + "replace example.com/upstream => ../fork", + "replace example.com/upstream v0.9.0 => ../fork", + "replace \"example.com/upstream\" => example.com/fork v1.2.3", + ] { + write(root, "go.work", &format!("go 1.21\n\nuse .\n\n{user}\n")); + let issues = audit(root, &none).await; + assert_eq!(codes(&issues), vec![WORKSPACE_CODE], "{user}"); + let text = issues[0].to_string(); + assert!( + text.contains("go.work replaces example.com/upstream"), + "{text}" + ); + } + write( + root, + "go.work", + &format!("go 1.21\n\nuse .\n\nreplace example.com/upstream v1.0.0 => ./{COPY}\n"), + ); assert_eq!(audit(root, &none).await, vec![]); + write( + root, + "go.work", + "go 1.21\n\nuse .\n\nreplace example.com/other => ../o\n", + ); + assert_eq!(audit(root, &none).await, vec![]); + } + + /// A go.work replace counts as ours only when it sends this version to + /// the project's own copy, however it spells the path; then go drops + /// every member replace of the module, so another member's can't + /// conflict either. + #[tokio::test] + async fn go_work_replace_is_judged_by_where_it_points() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (app, b) = (ws.join("app"), ws.join("b")); + consumer(&app, REPLACE); + consumer(&b, "\nreplace example.com/upstream v1.0.0 => ../fork\n"); + let none = HashMap::new(); + let work = |replace: &str| { + write( + ws, + "go.work", + &format!("go 1.21\n\nuse (\n\t./app\n\t./b\n)\n\nreplace {replace}\n"), + ) + }; + work(&format!("example.com/upstream v1.0.0 => ./app/{COPY}")); + assert_eq!(audit(&app, &none).await, vec![]); + work(&format!("example.com/upstream => ./app/{COPY}")); + assert_eq!(audit(&app, &none).await, vec![]); + // A copy under the workspace root is not the project's. + work(&format!("example.com/upstream v1.0.0 => ./{COPY}")); + assert_eq!(codes(&audit(&app, &none).await), vec![WORKSPACE_CODE]); + // Without the go.work replace, b's fork conflicts with ours. + write(ws, "go.work", "go 1.21\n\nuse (\n\t./app\n\t./b\n)\n"); + assert_eq!(codes(&audit(&app, &none).await), vec![WORKSPACE_CODE]); + } + + /// #458: go ignores the go.mod of a module go.work doesn't `use`, so its + /// replace is inert; also when the go.work sits in an ancestor. + #[tokio::test] + async fn go_work_that_does_not_use_the_project_makes_the_replace_inert() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let root = ws.join("app"); + consumer(&root, REPLACE); + let none = HashMap::new(); + write(&root, "go.work", "go 1.21\n\nuse ./svc\n"); + let issues = audit(&root, &none).await; + assert_eq!(codes(&issues), vec![WORKSPACE_CODE]); + assert!(issues[0].to_string().contains("does not `use` this module")); + + std::fs::remove_file(root.join("go.work")).unwrap(); + write(ws, "go.work", "go 1.21\n\nuse ./other\n"); + assert_eq!(codes(&audit(&root, &none).await), vec![WORKSPACE_CODE]); + // Spellings of the same member all count. + for use_dir in ["./app", "app", "./app/", "\"./app\"", "./x/../app"] { + write( + ws, + "go.work", + &format!("go 1.21\n\nuse (\n\t{use_dir}\n)\n"), + ); + assert_eq!(audit(&root, &none).await, vec![], "{use_dir}"); + } + } + + /// #531: two members replacing one module@version with different + /// directories make go refuse the workspace. The same hosted module in + /// both, or the same directory, is fine, and so is a replace of + /// another version. + #[tokio::test] + async fn member_replaces_of_one_module_version_must_agree() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let (a, b) = (ws.join("a"), ws.join("b")); + consumer(&a, REPLACE); + consumer(&b, REPLACE); + write(ws, "go.work", "go 1.21\n\nuse (\n\t./a\n\t./b\n)\n"); + let none = HashMap::new(); + let issues = audit(&b, &none).await; + assert_eq!(codes(&issues), vec![WORKSPACE_CODE]); + assert!( + issues[0].to_string().contains("conflicting"), + "{}", + issues[0] + ); + assert_eq!(codes(&audit(&a, &none).await), vec![WORKSPACE_CODE]); + + consumer(&b, "\nreplace example.com/upstream v1.0.0 => ../a/.socket/go-patches/example.com/upstream@v1.0.0\n"); + assert_eq!(audit(&a, &none).await, vec![]); + consumer(&b, "\nreplace example.com/upstream v0.9.0 => ../fork\n"); + assert_eq!(audit(&a, &none).await, vec![]); + + let hosted = "\nreplace example.com/upstream v1.0.0 => \ + patch.socket.dev/gopatch/11111111-2222-4333-8444-555555555555 \ + v1.0.0-socketpatch.1\n"; + consumer(&a, hosted); + consumer(&b, hosted); + assert_eq!(audit(&a, &none).await, vec![]); + assert_eq!(audit(&b, &none).await, vec![]); + } + + /// No go.work in effect: nothing to report. + #[tokio::test] + async fn no_workspace_no_workspace_issue() { + let dir = tempfile::tempdir().unwrap(); + consumer(dir.path(), REPLACE); + assert_eq!(workspace_audit(dir.path()).await, vec![]); } /// A user-authored replace is never ours to report, and a hosted diff --git a/crates/socket-patch-core/src/vendor/go_mod_edit.rs b/crates/socket-patch-core/src/vendor/go_mod_edit.rs index 16fdafe7a..4a9b49de8 100644 --- a/crates/socket-patch-core/src/vendor/go_mod_edit.rs +++ b/crates/socket-patch-core/src/vendor/go_mod_edit.rs @@ -171,7 +171,10 @@ pub async fn read_required_versions(project_root: &Path) -> Option Result { + if let Some(refusal) = + crate::vendor::go_consumer_sync::replace_refusal(project_root, module, version, base_rel) + .await + { + return Err(refusal); + } edit_go_mod(project_root, dry_run, |c| { upsert_replace_entry(c, module, version, base_rel) }) @@ -1676,6 +1685,53 @@ replace ( assert!(read_replace_entries(dir.path()).await.is_empty()); } + /// #393 / #458 / #531: a replace the go.work in effect would override, + /// ignore or reject is refused, dry run included, and go.mod is left + /// as it was. + #[tokio::test] + async fn test_ensure_refuses_a_replace_go_work_defeats() { + let dir = tempfile::tempdir().unwrap(); + let ws = dir.path(); + let root = ws.join("app"); + let go_mod = "module example.com/app\n\ngo 1.21\n\nrequire example.com/m v1.0.0\n"; + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("go.mod"), go_mod).unwrap(); + std::fs::create_dir_all(ws.join("b")).unwrap(); + std::fs::write( + ws.join("b/go.mod"), + "module example.com/b\n\nreplace example.com/m v1.0.0 => ./.socket/go-patches/example.com/m@v1.0.0\n", + ) + .unwrap(); + for (work, says) in [ + ( + "use ./app\n\nreplace example.com/m => ../fork\n", + "go.work replaces", + ), + ("use ./b\n", "does not `use` this module"), + ("use (\n\t./app\n\t./b\n)\n", "conflicting replacements"), + ] { + std::fs::write(ws.join("go.work"), format!("go 1.21\n\n{work}")).unwrap(); + for dry_run in [true, false] { + let err = + ensure_replace_entry(&root, "example.com/m", "v1.0.0", GO_PATCHES_DIR, dry_run) + .await + .expect_err(work); + assert!(err.contains(says), "{err}"); + } + assert_eq!( + std::fs::read_to_string(root.join("go.mod")).unwrap(), + go_mod + ); + } + // A workspace that uses the module and nothing else in the way. + std::fs::write(ws.join("go.work"), "go 1.21\n\nuse ./app\n").unwrap(); + assert!( + ensure_replace_entry(&root, "example.com/m", "v1.0.0", GO_PATCHES_DIR, false) + .await + .unwrap() + ); + } + #[tokio::test] async fn test_ensure_dry_run_does_not_write() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/vendor/golang.rs b/crates/socket-patch-core/src/vendor/golang.rs index ee546a84f..7507fd7b7 100644 --- a/crates/socket-patch-core/src/vendor/golang.rs +++ b/crates/socket-patch-core/src/vendor/golang.rs @@ -234,6 +234,16 @@ pub async fn vendor_go_module<'a>( }; let (module, version) = (module.as_str(), version.as_str()); + // A replace the go.work in effect would override, ignore or reject + // (#393, #458, #531): refuse before fetching or wiring anything. + if !record.files.is_empty() { + if let Some(refusal) = + super::go_consumer_sync::replace_refusal(project_root, module, version, &base_rel).await + { + return refused("vendor_go_workspace_conflict", refusal); + } + } + let mut warnings: Vec = Vec::new(); // A healthy committed copy requires neither a local module cache nor the service. diff --git a/docs/ecosystems.md b/docs/ecosystems.md index d79318605..a53ef53ce 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1296,6 +1296,30 @@ them (that needs the go toolchain and the whole module graph); `apply`, `vendor` | --- | --- | --- | | `go_vendor_modules_txt_out_of_sync` | A committed `vendor/` directory (`go mod vendor`; `go work vendor` for a workspace) whose `vendor/modules.txt` does not record a socket `replace`, or still records one that was removed. go builds with `-mod=vendor` and fails with "inconsistent vendoring" | `go mod vendor` (or `go work vendor`), then commit `vendor/` | | `go_requirements_out_of_sync` | The patched module's own `go.mod` requires a dependency at a version above the one the project's `go.mod` lists (agent and vendored modes; `apply` also reports a requirement the patch added). The default `-mod=readonly` build fails with "updates to go.mod needed" or "missing go.sum entry" | `go mod tidy`, then commit `go.mod` and `go.sum` | + +### Go workspaces (go.work) + +In workspace mode go reads `replace` directives from `go.work` and from the +`go.mod` of each module it `use`s. socket-patch writes its `replace` into the +project's own `go.mod`, so it first checks the `go.work` go would use (`GOWORK`, +else the nearest `go.work` at or above the project). A project `go.mod` replace +does nothing, or breaks the build, when: + +- `go.work` replaces the same module itself (any version). A go.work replace + overrides every module's, so the build links the go.work target; +- `go.work` doesn't `use` the project, so workspace builds ignore its `go.mod`; +- another `use` member already replaces the same `module@version` with a + different target, so go refuses the workspace ("conflicting replacements"). + A hosted replace names the same Socket module in every member and doesn't + conflict. + +`apply` and `vendor` refuse to write such a replace and say which case applies. +A replace written before the `go.work` changed is reported as +`go_workspace_replace_ineffective` by `apply`, `vendor` and +`scan --mode hosted`, fails `apply --check` and `vendor --check`, and `vex` +leaves the patch out (`go_workspace_override`). To patch the whole workspace, +apply in one member it uses, or move the Socket `replace` into `go.work`. + Paid hosted Go references are unsupported: embedding credentials in module paths would expose them to module proxies and change module identity. Use vendored mode for those patches; the CLI reports `redirect_golang_unsupported` when the required