From 1e9d12dd1f25735a6fcefd3665be4684ba23fc58 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Wed, 29 May 2024 11:31:30 -0400
Subject: [PATCH] go/packages: pass -overlay to all 'go list' invocations

Even in the trivial go list invocations (to enumerate
modules or type sizes), the complex behavior of Go modules
requires that the appropriate overlays are visible, since
they may define go.mod files.

Also, a test case suggested by Dominik Honnef.

Fixes golang/go#67644

Change-Id: I19348ae7270769de438a7f4ce69c3f7a55fb2f55
Reviewed-on: https://go-review.googlesource.com/c/tools/+/588936
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 go/packages/golist.go        | 13 +------------
 go/packages/packages.go      | 22 ++++++++++++++++++++++
 go/packages/packages_test.go | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/go/packages/golist.go b/go/packages/golist.go
index 71daa8bd4df..d9be410aa1a 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -841,6 +841,7 @@ func (state *golistState) cfgInvocation() gocommand.Invocation {
 		Env:        cfg.Env,
 		Logf:       cfg.Logf,
 		WorkingDir: cfg.Dir,
+		Overlay:    cfg.goListOverlayFile,
 	}
 }
 
@@ -849,18 +850,6 @@ func (state *golistState) invokeGo(verb string, args ...string) (*bytes.Buffer,
 	cfg := state.cfg
 
 	inv := state.cfgInvocation()
-
-	// Since go1.16, `go list` accepts overlays directly via the
-	// -overlay flag. (The check for "list" avoids unnecessarily
-	// writing the overlay file for a 'go env' command.)
-	if verb == "list" {
-		overlay, cleanup, err := gocommand.WriteOverlays(cfg.Overlay)
-		if err != nil {
-			return nil, err
-		}
-		defer cleanup()
-		inv.Overlay = overlay
-	}
 	inv.Verb = verb
 	inv.Args = args
 	gocmdRunner := cfg.gocmdRunner
diff --git a/go/packages/packages.go b/go/packages/packages.go
index ec4ade6540e..34306ddd390 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -133,7 +133,14 @@ const (
 
 // A Config specifies details about how packages should be loaded.
 // The zero value is a valid configuration.
+//
 // Calls to Load do not modify this struct.
+//
+// TODO(adonovan): #67702: this is currently false: in fact,
+// calls to [Load] do not modify the public fields of this struct, but
+// may modify hidden fields, so concurrent calls to [Load] must not
+// use the same Config. But perhaps we should reestablish the
+// documented invariant.
 type Config struct {
 	// Mode controls the level of information returned for each package.
 	Mode LoadMode
@@ -222,6 +229,10 @@ type Config struct {
 	// consistent package metadata about unsaved files. However,
 	// drivers may vary in their level of support for overlays.
 	Overlay map[string][]byte
+
+	// goListOverlayFile is the JSON file that encodes the Overlay
+	// mapping, used by 'go list -overlay=...'
+	goListOverlayFile string
 }
 
 // Load loads and returns the Go packages named by the given patterns.
@@ -316,6 +327,17 @@ func defaultDriver(cfg *Config, patterns ...string) (*DriverResponse, bool, erro
 		// (fall through)
 	}
 
+	// go list fallback
+	//
+	// Write overlays once, as there are many calls
+	// to 'go list' (one per chunk plus others too).
+	overlay, cleanupOverlay, err := gocommand.WriteOverlays(cfg.Overlay)
+	if err != nil {
+		return nil, false, err
+	}
+	defer cleanupOverlay()
+	cfg.goListOverlayFile = overlay
+
 	response, err := callDriverOnChunks(goListDriver, cfg, chunks)
 	if err != nil {
 		return nil, false, err
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 294f058e81a..2a2e5a01054 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -3024,3 +3024,38 @@ func TestLoadEitherSucceedsOrFails(t *testing.T) {
 		t.Errorf("Load returned %d packages (want 1) and no error", len(initial))
 	}
 }
+
+// TestLoadOverlayGoMod ensures that overlays containing go.mod files
+// are effective for all 'go list' calls made by go/packages (#67644).
+func TestLoadOverlayGoMod(t *testing.T) {
+	testenv.NeedsGoBuild(t)
+
+	cwd, _ := os.Getwd()
+
+	// This test ensures that the overlaid go.mod file is seen by
+	// all runs of 'go list', in particular the early run that
+	// enumerates the modules: if the go.mod file were absent,
+	// it would ascend to the parent directory (x/tools) and
+	// then (falsely) report inconsistent vendoring.
+	//
+	// (Ideally the testdata would be constructed from nothing
+	// rather than rely on the go/packages source tree, but it is
+	// turned out to a bigger project than bargained for.)
+	cfg := &packages.Config{
+		Mode: packages.LoadSyntax,
+		Overlay: map[string][]byte{
+			filepath.Join(cwd, "go.mod"): []byte("module example.com\ngo 1.0"),
+		},
+		Env: append(os.Environ(), "GOFLAGS=-mod=vendor", "GOWORK=off"),
+	}
+
+	pkgs, err := packages.Load(cfg, "./testdata")
+	if err != nil {
+		t.Fatal(err) // (would previously fail here with "inconsistent vendoring")
+	}
+	got := fmt.Sprint(pkgs)
+	want := `[./testdata]`
+	if got != want {
+		t.Errorf("Load: got %s, want %v", got, want)
+	}
+}