Skip to content

Commit 1413d3c

Browse files
committed
Check go.work before copying a Go patch
Review follow-ups on the go.work check: - apply and vendor now check before copying or fetching anything, so apply --dry-run predicts the refusal, a re-run over an inert replace fails instead of reporting it in sync, and a refused apply leaves no orphan copy under .socket/go-patches. vendor refuses with its own vendor_go_workspace_conflict code. - A go.work replace counts as ours only when it sends this version to the project's own copy, however the path is spelled; when it does, other members' replaces are ignored, as go ignores them. - Replacement targets are compared lexically, as go compares them, and GOWORK set with `go env -w` is honoured. - The workspace is read once per audit and shared with the modules.txt check. Assisted-by: Claude Code:claude-opus-5-5
1 parent 957dd30 commit 1413d3c

5 files changed

Lines changed: 250 additions & 75 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_golang_build.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,13 +810,37 @@ fn go_work_replace_overrides_the_socket_replace() {
810810
assert!(go_out(&consumer, &env).contains("OUT: FORK"));
811811

812812
write_patch(&consumer);
813+
// The dry run predicts the refusal.
814+
let (code, so, se) = run_socket(
815+
&consumer,
816+
&[
817+
"apply",
818+
"--offline",
819+
"--dry-run",
820+
"--ecosystems",
821+
"golang",
822+
"--json",
823+
"--cwd",
824+
consumer.to_str().unwrap(),
825+
],
826+
&modcache,
827+
);
828+
assert_ne!(
829+
code, 0,
830+
"apply --dry-run must predict the refusal:\n{so}\n{se}"
831+
);
832+
assert!(so.contains("go.work replaces"), "{so}");
813833
let (code, so, se) = apply_json(&consumer, &modcache);
814834
assert_ne!(
815835
code, 0,
816836
"apply must refuse an overridden replace:\n{so}\n{se}"
817837
);
818838
assert!(so.contains("go.work replaces"), "{so}");
819839
assert!(replace_lines(&consumer.join("go.mod")).is_empty());
840+
assert!(
841+
!consumer.join(".socket/go-patches").exists(),
842+
"a refused apply leaves no copy behind"
843+
);
820844
assert!(go_out(&consumer, &env).contains("OUT: FORK"));
821845

822846
// Applied before the go.work replace existed: the replace is inert.
@@ -825,6 +849,9 @@ fn go_work_replace_overrides_the_socket_replace() {
825849
assert_eq!(code, 0, "apply without go.work:\n{so}\n{se}");
826850
std::fs::write(consumer.join("go.work"), &go_work).unwrap();
827851
assert!(go_out(&consumer, &env).contains("OUT: FORK"));
852+
// A re-run doesn't call the inert replace in sync.
853+
let (code, so, se) = apply_json(&consumer, &modcache);
854+
assert_ne!(code, 0, "re-run apply must refuse:\n{so}\n{se}");
828855
let (code, so, se) = check(&consumer, &modcache);
829856
assert_eq!(
830857
code, 1,

‎crates/socket-patch-core/src/patch/redirect/golang_local.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,16 @@ pub async fn apply_go_redirect<'a>(
238238
);
239239
}
240240

241+
// A replace the go.work in effect would override, ignore or reject
242+
// (#393, #458, #531) leaves the build unpatched or broken: refuse before
243+
// copying anything, dry run and in-sync re-run alike.
244+
if let Some(refusal) =
245+
crate::vendor::go_consumer_sync::replace_refusal(project_root, module, version, base_rel)
246+
.await
247+
{
248+
return synthesized_result(purl, &copy_dir, Vec::new(), false, Some(refusal));
249+
}
250+
241251
if dry_run {
242252
// Verify (read-only) against the pristine source for an accurate
243253
// "would patch" report, without creating the copy or editing go.mod.

‎crates/socket-patch-core/src/vendor/go_consumer_sync.rs‎

Lines changed: 197 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -280,41 +280,110 @@ fn modules_txt_location(project_root: &Path) -> (PathBuf, &'static str) {
280280
)
281281
}
282282

283-
/// The `go.work` go would use for `project_root`: `GOWORK` when it names a
283+
/// The `go.work` go would use for `project_root`: `GOWORK` (from the
284+
/// environment, else the go env file `go env -w` writes) when it names a
284285
/// file (`off` disables workspaces), else the nearest `go.work` at or above
285286
/// the project.
286287
fn workspace_file(project_root: &Path) -> Option<PathBuf> {
287288
let base = std::env::current_dir()
288289
.unwrap_or_default()
289290
.join(project_root);
290-
match std::env::var("GOWORK") {
291-
Ok(v) if v == "off" => return None,
292-
Ok(v) if !v.is_empty() && v != "auto" => return Some(base.join(v)),
291+
let gowork = match std::env::var("GOWORK") {
292+
Ok(v) => Some(v),
293+
Err(_) => go_env_file_value("GOWORK"),
294+
};
295+
match gowork.as_deref() {
296+
Some("off") => return None,
297+
Some(v) if !v.is_empty() && v != "auto" => return Some(base.join(v)),
293298
_ => {}
294299
}
295300
base.ancestors()
296301
.map(|dir| dir.join("go.work"))
297302
.find(|file| file.is_file())
298303
}
299304

300-
/// The replaces a workspace `modules.txt` records: `go work vendor` writes
301-
/// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns
302-
/// `(go.work replaces, every member's replaces)`.
303-
async fn workspace_replaces(project_root: &Path) -> (Vec<ReplaceEntry>, Vec<ReplaceEntry>) {
304-
let Some(work_file) = workspace_file(project_root) else {
305-
return (Vec::new(), Vec::new());
305+
/// `key` from the go env file (`go env -w KEY=value`): `$GOENV`, else
306+
/// `go/env` under the user config directory (`os.UserConfigDir`). `None`
307+
/// when unset, unreadable or disabled (`GOENV=off`).
308+
fn go_env_file_value(key: &str) -> Option<String> {
309+
let file = match std::env::var_os("GOENV") {
310+
Some(v) if v == "off" => return None,
311+
Some(v) if !v.is_empty() => PathBuf::from(v),
312+
_ => user_config_dir()?.join("go").join("env"),
306313
};
307-
let Ok(text) = read_regular_to_string(&work_file).await else {
308-
return (Vec::new(), Vec::new());
314+
let text = std::fs::read_to_string(file).ok()?;
315+
text.lines()
316+
.rev()
317+
.filter_map(|line| line.trim().split_once('='))
318+
.find(|(k, _)| k.trim() == key)
319+
.map(|(_, v)| v.trim().to_string())
320+
}
321+
322+
/// go's `os.UserConfigDir`.
323+
fn user_config_dir() -> Option<PathBuf> {
324+
let var = |k: &str| {
325+
std::env::var_os(k)
326+
.filter(|v| !v.is_empty())
327+
.map(PathBuf::from)
309328
};
310-
let work_dir = work_file.parent().unwrap_or(Path::new(""));
329+
if cfg!(windows) {
330+
var("AppData")
331+
} else if cfg!(target_os = "macos") {
332+
var("HOME").map(|h| h.join("Library").join("Application Support"))
333+
} else {
334+
var("XDG_CONFIG_HOME").or_else(|| var("HOME").map(|h| h.join(".config")))
335+
}
336+
}
337+
338+
/// The `go.work` in effect, read once: its own replaces and each `use`
339+
/// member's go.mod replaces.
340+
struct Workspace {
341+
work_file: PathBuf,
342+
replaces: Vec<ReplaceEntry>,
343+
members: Vec<Member>,
344+
}
345+
346+
struct Member {
347+
/// The member directory as go.work spells it, joined to go.work's
348+
/// directory (go resolves the member's relative replaces against it).
349+
dir: PathBuf,
350+
replaces: Vec<ReplaceEntry>,
351+
}
352+
353+
async fn load_workspace(project_root: &Path) -> Option<Workspace> {
354+
let work_file = workspace_file(project_root)?;
355+
let text = read_regular_to_string(&work_file).await.ok()?;
356+
let text = go_mod_edit::normalize_for_read(&text);
357+
let work_dir = work_file.parent().unwrap_or(Path::new("")).to_path_buf();
311358
let mut members = Vec::new();
312359
for dir in go_mod_edit::parse_use_dirs(&text) {
313-
if let Ok(go_mod) = read_regular_to_string(&work_dir.join(&dir).join("go.mod")).await {
314-
members.extend(go_mod_edit::parse_replace_entries(&go_mod));
360+
let dir = work_dir.join(dir);
361+
let replaces = match read_regular_to_string(&dir.join("go.mod")).await {
362+
Ok(go_mod) => {
363+
go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod))
364+
}
365+
Err(_) => Vec::new(),
366+
};
367+
members.push(Member { dir, replaces });
368+
}
369+
Some(Workspace {
370+
replaces: go_mod_edit::parse_replace_entries(&text),
371+
work_file,
372+
members,
373+
})
374+
}
375+
376+
/// The replaces a workspace `modules.txt` records: `go work vendor` writes
377+
/// every `use` member's `go.mod` replaces plus the `go.work` ones. Returns
378+
/// `(go.work replaces, every member's replaces)`.
379+
async fn workspace_replaces(project_root: &Path) -> (Vec<ReplaceEntry>, Vec<ReplaceEntry>) {
380+
match load_workspace(project_root).await {
381+
Some(ws) => {
382+
let members = ws.members.into_iter().flat_map(|m| m.replaces).collect();
383+
(ws.replaces, members)
315384
}
385+
None => (Vec::new(), Vec::new()),
316386
}
317-
(go_mod_edit::parse_replace_entries(&text), members)
318387
}
319388

320389
/// Whether the `go.work` in effect for `project_root` keeps a project
@@ -329,49 +398,75 @@ pub async fn workspace_conflict(
329398
version: &str,
330399
ours: &ReplaceEntry,
331400
) -> Option<WorkspaceConflict> {
332-
let work_file = workspace_file(project_root)?;
333-
let text = read_regular_to_string(&work_file).await.ok()?;
334-
let work_dir = work_file.parent().unwrap_or(Path::new(""));
401+
let ws = load_workspace(project_root).await?;
402+
conflict_in(&ws, project_root, module, version, ours)
403+
}
404+
405+
/// Why `apply` / `vendor` must not write `replace module version =>
406+
/// ./<base_rel>/module@version` into the project's go.mod, if the `go.work`
407+
/// in effect would override, ignore or reject it: the refusal message.
408+
pub async fn replace_refusal(
409+
project_root: &Path,
410+
module: &str,
411+
version: &str,
412+
base_rel: &str,
413+
) -> Option<String> {
414+
let target = go_mod_edit::replace_target_path(base_rel, module, version);
415+
let ours = go_mod_edit::parse_replace_body(&format!("{module} {version} => {target}"))?;
416+
let conflict = workspace_conflict(project_root, module, version, &ours).await?;
417+
Some(format!(
418+
"refusing to write `replace {module} {version} => {target}` to go.mod: it {}",
419+
conflict.consequence(module, version)
420+
))
421+
}
422+
423+
fn conflict_in(
424+
ws: &Workspace,
425+
project_root: &Path,
426+
module: &str,
427+
version: &str,
428+
ours: &ReplaceEntry,
429+
) -> Option<WorkspaceConflict> {
430+
let work_dir = ws.work_file.parent().unwrap_or(Path::new(""));
431+
let display = display_path(project_root, &ws.work_file);
335432
let project = same_dir_key(project_root);
336-
let display = display_path(project_root, &work_file);
433+
let own = ws.members.iter().find(|m| same_dir_key(&m.dir) == project);
434+
let our_target = lexical_target(own.map_or(project_root, |m| &m.dir), ours);
337435

338436
// go drops every member replace of a module path that go.work replaces
339-
// (any version), so only our own replace moved there, for this very
340-
// version, keeps the patch.
341-
if let Some(w) = go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&text))
342-
.into_iter()
343-
.find(|w| {
344-
w.module == module && !(w.socket_owned() && w.version.as_deref() == Some(version))
345-
})
346-
{
347-
return Some(WorkspaceConflict::WorkReplace {
348-
work_file: display,
349-
target: target_display(&w),
350-
});
437+
// (any version). The build links the patched copy only if go.work's own
438+
// replace for this version (else its version-less one) sends it there.
439+
let work: Vec<&ReplaceEntry> = ws.replaces.iter().filter(|w| w.module == module).collect();
440+
if let Some(first) = work.first() {
441+
let effective = work
442+
.iter()
443+
.find(|w| w.version.as_deref() == Some(version))
444+
.or_else(|| work.iter().find(|w| w.version.is_none()));
445+
return match effective {
446+
Some(w) if lexical_target(work_dir, w) == our_target => None,
447+
_ => Some(WorkspaceConflict::WorkReplace {
448+
work_file: display,
449+
target: target_display(effective.unwrap_or(first)),
450+
}),
451+
};
351452
}
352453

353-
let members: Vec<PathBuf> = go_mod_edit::parse_use_dirs(&text)
354-
.iter()
355-
.map(|dir| work_dir.join(dir))
356-
.collect();
357-
if !members.iter().any(|m| same_dir_key(m) == project) {
454+
if own.is_none() {
358455
return Some(WorkspaceConflict::NotAMember { work_file: display });
359456
}
360-
361-
let our_target = resolved_target(project_root, ours);
362-
for member in members.iter().filter(|m| same_dir_key(m) != project) {
363-
let Ok(go_mod) = read_regular_to_string(&member.join("go.mod")).await else {
364-
continue;
365-
};
366-
let theirs = go_mod_edit::parse_replace_entries(&go_mod_edit::normalize_for_read(&go_mod));
367-
if let Some(other) = theirs.iter().find(|e| {
457+
for member in ws
458+
.members
459+
.iter()
460+
.filter(|m| same_dir_key(&m.dir) != project)
461+
{
462+
if let Some(other) = member.replaces.iter().find(|e| {
368463
e.module == module
369464
&& e.version.as_deref() == Some(version)
370-
&& resolved_target(member, e) != our_target
465+
&& lexical_target(&member.dir, e) != our_target
371466
}) {
372467
return Some(WorkspaceConflict::MemberConflict {
373468
work_file: display,
374-
member: display_path(project_root, member),
469+
member: display_path(project_root, &member.dir),
375470
target: target_display(other),
376471
});
377472
}
@@ -380,16 +475,20 @@ pub async fn workspace_conflict(
380475
}
381476

382477
/// [`workspace_conflict`] for every versioned socket replace in the
383-
/// project's go.mod.
478+
/// project's go.mod, reading the workspace once.
384479
async fn workspace_issues(project_root: &Path, replaces: &[ReplaceEntry]) -> Vec<GoSyncIssue> {
480+
if !replaces.iter().any(ReplaceEntry::socket_owned) {
481+
return Vec::new();
482+
}
483+
let Some(ws) = load_workspace(project_root).await else {
484+
return Vec::new();
485+
};
385486
let mut issues = Vec::new();
386487
for entry in replaces.iter().filter(|e| e.socket_owned()) {
387488
let Some(version) = entry.version.as_deref() else {
388489
continue;
389490
};
390-
if let Some(conflict) =
391-
workspace_conflict(project_root, &entry.module, version, entry).await
392-
{
491+
if let Some(conflict) = conflict_in(&ws, project_root, &entry.module, version, entry) {
393492
issues.push(GoSyncIssue::Workspace {
394493
module: entry.module.clone(),
395494
version: version.to_string(),
@@ -400,21 +499,29 @@ async fn workspace_issues(project_root: &Path, replaces: &[ReplaceEntry]) -> Vec
400499
issues
401500
}
402501

403-
/// A directory as go compares workspace members: absolute, with symlinks
404-
/// resolved when it exists, else lexically normalized.
502+
/// A directory as go matches a module root against the workspace's `use`
503+
/// list: absolute, with symlinks resolved when it exists, else lexically
504+
/// normalized.
405505
fn same_dir_key(dir: &Path) -> PathBuf {
406506
let abs = std::env::current_dir().unwrap_or_default().join(dir);
407507
std::fs::canonicalize(&abs)
408508
.unwrap_or_else(|_| crate::utils::relpath::normalize_lexically_keeping_escapes(&abs))
409509
}
410510

411-
/// Where a replace sends the module: its directory (resolved against the
412-
/// go.mod's own directory) or its `module version`.
413-
fn resolved_target(go_mod_dir: &Path, entry: &ReplaceEntry) -> String {
511+
/// Where a replace sends the module, compared as go compares replacements
512+
/// (`filepath.Join` of the go.mod's directory and the path: lexical, no
513+
/// symlink resolution), or its `module version`.
514+
fn lexical_target(go_mod_dir: &Path, entry: &ReplaceEntry) -> String {
414515
match &entry.path {
415-
Some(path) => same_dir_key(&go_mod_dir.join(path.replace('\\', "/")))
416-
.to_string_lossy()
417-
.into_owned(),
516+
Some(path) => {
517+
let abs = std::env::current_dir()
518+
.unwrap_or_default()
519+
.join(go_mod_dir)
520+
.join(path.replace('\\', "/"));
521+
crate::utils::relpath::normalize_lexically_keeping_escapes(&abs)
522+
.to_string_lossy()
523+
.into_owned()
524+
}
418525
None => target_display(entry),
419526
}
420527
}
@@ -795,6 +902,37 @@ mod tests {
795902
assert_eq!(audit(root, &none).await, vec![]);
796903
}
797904

905+
/// A go.work replace counts as ours only when it sends this version to
906+
/// the project's own copy, however it spells the path; then go drops
907+
/// every member replace of the module, so another member's can't
908+
/// conflict either.
909+
#[tokio::test]
910+
async fn go_work_replace_is_judged_by_where_it_points() {
911+
let dir = tempfile::tempdir().unwrap();
912+
let ws = dir.path();
913+
let (app, b) = (ws.join("app"), ws.join("b"));
914+
consumer(&app, REPLACE);
915+
consumer(&b, "\nreplace example.com/upstream v1.0.0 => ../fork\n");
916+
let none = HashMap::new();
917+
let work = |replace: &str| {
918+
write(
919+
ws,
920+
"go.work",
921+
&format!("go 1.21\n\nuse (\n\t./app\n\t./b\n)\n\nreplace {replace}\n"),
922+
)
923+
};
924+
work(&format!("example.com/upstream v1.0.0 => ./app/{COPY}"));
925+
assert_eq!(audit(&app, &none).await, vec![]);
926+
work(&format!("example.com/upstream => ./app/{COPY}"));
927+
assert_eq!(audit(&app, &none).await, vec![]);
928+
// A copy under the workspace root is not the project's.
929+
work(&format!("example.com/upstream v1.0.0 => ./{COPY}"));
930+
assert_eq!(codes(&audit(&app, &none).await), vec![WORKSPACE_CODE]);
931+
// Without the go.work replace, b's fork conflicts with ours.
932+
write(ws, "go.work", "go 1.21\n\nuse (\n\t./app\n\t./b\n)\n");
933+
assert_eq!(codes(&audit(&app, &none).await), vec![WORKSPACE_CODE]);
934+
}
935+
798936
/// #458: go ignores the go.mod of a module go.work doesn't `use`, so its
799937
/// replace is inert; also when the go.work sits in an ancestor.
800938
#[tokio::test]

0 commit comments

Comments
 (0)