From 38d1265e1a015add1d0b8651f5c2ea3f06199765 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 4 Sep 2026 17:29:06 -0400 Subject: [PATCH] cmd/go: preserve vet export data in memory for dependent actions on failure When "go test" is applied to multiple packages, it runs cmd/vet without -json (in contrast to "go vet"). If a package has diagnostics, the vet tool prints them to stderr and exits nonzero. Vet actions in the build graph set IgnoreFail so that upstream diagnostics do not suppress vetting and testing of downstream packages. However, when sh.run returns an error, Builder.vet returns immediately without recording a.built. Previously this was benign because unitchecker imported types from the compiler's export data (cfg.PackageFile), treating missing .vetx entries in cfg.PackageVetx as merely lacking analyzer facts. However, with unitchecker transitioning to doing its own type export through .vetx files, a missing .vetx file causes downstream typechecking to fail entirely ("no package vetx file for ..."), causing downstream test runs to fail with "[build failed]". This change ensures that if the vet tool exited non-zero but nonetheless successfully produced its output file (vcfg.VetxOutput), a.built is still recorded in memory so that downstream actions in the same build graph can locate the dependency's types and facts. To preserve cache integrity: - The .vetx file is only written to the persistent cache (cache.Default) when runErr == nil. - runErr is still returned, ensuring that the failing package is still reported as a failure and its own test binary is not run. Updates #81188 Change-Id: I26d36c0d1874dc1f78162cc84b6eeebe513b643d Reviewed-on: https://go-review.googlesource.com/c/go/+/827884 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Michael Matloob Auto-Submit: Alan Donovan Reviewed-by: Michael Matloob --- src/cmd/go/internal/work/exec.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index c711a906202495..b7370ec4cd28a6 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -1745,18 +1745,31 @@ cachemiss: panic("VetTool unset") } - if err := sh.run(p.Dir, p.ImportPath, env, cfg.BuildToolexec, tool, vetFlags, a.Objdir+"vet.cfg"); err != nil { - return err - } - - // Vet tool succeeded, possibly with facts, fixes, or JSON stdout. - // Save all in cache. - - // Save facts. + runErr := sh.run(p.Dir, p.ImportPath, env, cfg.BuildToolexec, tool, vetFlags, a.Objdir+"vet.cfg") + + // Save facts and export data. + // Even if vet reported diagnostics and exited non-zero, it may have + // successfully produced export data (vet.out). Record a.built so that + // downstream actions in this build that ignore dependency failures can + // still typecheck and analyze packages that import this one. + // However, do not save to the persistent cache on failure. + // + // TODO(adonovan): This is unsafe if runErr was caused by an I/O + // error (e.g. EMFILE, ENOSPC) or crash that left vet.out truncated + // or corrupt, which downstream actions will then fail trying to parse. + // The principled fix is for 'go test' to run vet in -json mode (like + // 'go vet' does), where exit code 0 unambiguously indicates a clean + // run (with diagnostics in stdout), while non-zero indicates tool failure. if f, err := os.Open(vcfg.VetxOutput); err == nil { defer f.Close() // ignore error a.built = vcfg.VetxOutput - cache.Default().Put(id, f) // ignore error + if runErr == nil { + cache.Default().Put(id, f) // ignore error + } + } + + if runErr != nil { + return runErr } // Save fix archive (if any).