Repository navigation
Fix Go module cache default read two ways (#1834) - #1876
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
The crawler and the installed-tool audit each worked out the Go module cache from GOMODCACHE and GOPATH, and the two copies disagreed with each other and with go. With GOPATH=":/a" the crawler patched /a/pkg/mod, and with GOPATH=":" it fell back to ~/go/pkg/mod, but go itself reads only the first GOPATH entry, so both settings leave go with no module cache at all. A relative GOMODCACHE, which go refuses, was crawled under the working directory. Both callers now use go_env::gomodcache and gopath_entries, which follow go's cfg.gopathDir (checked against go 1.24's go env and go mod download). Assisted-by: Claude Code:claude-opus-5-5
go exits when any GOPATH entry is relative or starts with ~, even with GOMODCACHE set, so there is no module cache to crawl or audit then. Empty settings count as unset in the shared helpers and in the installed-tool audit, whatever lookup the caller passes. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit cc81e3b. Configure here.
|
Ready for review (burn-down agent).
Generated by Claude Code |
|
Burn-down agent: labeled Ready for review at
Generated by Claude Code |
Final review brief (
|
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1834
Summary
Before this PR, the Go crawler (
GoCrawler::get_gomodcache, which apply, scan and the workspace graph reader use) and the stale-installed-tool audit (go_installed_tools::bin_dirs_with) each worked out the module cache fromGOMODCACHE/GOPATHthemselves. The two copies disagreed with each other and with go:GOMODCACHEGOPATH=/a:/b/a/pkg/mod/a/pkg/mod/a/pkg/mod/a/pkg/modGOPATH=:/a/a/pkg/mod/a/pkg/modGOPATH=:$HOME/go/pkg/modGOMODCACHE=rel./relunder CWDrelGOPATH=/a:relor/a:~/x/a/pkg/mod/a/pkg/mod$HOME/go/pkg/modEvery "go" cell was checked with real go 1.24.7, using
go env GOMODCACHEand ago mod downloadfrom a file GOPROXY.The issue proposed falling back to the home default for a GOPATH of only empty entries. go doesn't do that.
cfg.gopathDirreads onlylist[0], and an empty entry there means go has no module cache. So this PR follows go instead.Change
utils/go_env.rs:gopath_entries(go's GOPATH list, with the$HOME/godefault) andgomodcache(the rules in the table) are now the only implementation.GoCrawler::get_gomodcacheandbin_dirs_withcall them. The redundant empty filters ongo_envresults are gone.bin_dirs_withkeeps one filter at its injected-lookup boundary, because it'spub.npm/,pypi/andgem/only dispatch to the binary.Tests (red → green)
:/a,:) gives no cache, and a relative GOMODCACHE gives no cachego_crawler::tests::test_get_gomodcache_empty_env_fallback_chainleft: Some("/tmp/.tmp…/pkg/mod") right: NoneGOPATH=:/bputs no copy in the user's cache, so GOBIN isn't searchedgo_installed_tools::tests::bin_dirs_follow_gobin_then_gopath_then_home_and_the_cache_rootleft: ["/gobin", "/b/bin"] right: ["/b/bin"]~entriesgo_env::tests::gomodcache_follows_goCase A of the crawler test used to assert that an empty first entry is skipped in favor of the next one. That was the divergence the issue describes, so the test now asserts what go does. Its original guarantee, never a path relative to CWD, still holds.
Commands run locally:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test -p socket-patch-core --all-features --lib: 6995 pass. The 4 failures are the known root-sandbox permission tests the other agent PRs also see (copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_…,pypi_requirements::wire_failure_rolls_back_…). None of them touches Go.cargo test -p socket-patch-cli --all-features --lib --bins: 940 passcargo test -p socket-patch-core --test crawler_go_e2e: 19 passe2e_golang_build31,e2e_golang_hosted_build25,e2e_golang_workspace_build31,e2e_vendor_golang_build44,e2e_vex_lockfile golang::24, all pass.Per-issue checklist
utils/go_env.rs, used by both callers, with the redundant filters removed. Covered bygomodcache_follows_go,test_get_gomodcache_empty_env_fallback_chainandbin_dirs_follow_gobin_then_gopath_then_home_and_the_cache_root./code-review high
Fixed in cc81e3b:
GOBINorGOMODCACHE/GOPATHfrom an injected lookup counted as set. The helpers now treat it as unset whatever lookup they're given.~GOPATH entry after the first was accepted, though go refuses the whole setting.tests/crawler_go_e2e.rs.Not changed:
GOPATH=:/bnow crawls nothing instead of/b/pkg/mod. go itself can't download into or build from a cache there, so nothing in the build came from it. Adding a warning would be a separate UX change.binis still searched for stale tools. That predates this PR, and searching more directories only widens the audit.bin_dirs_withresolves GOPATH twice, once directly and once insidegomodcache. That's one small extra env-file read per audit, which I kept in exchange for a single shared rule.https://claude.ai/code/session_011f9aePZXiQWURg2M6sbh88
Generated by Claude Code