go/packages: avoid unnecessary "realpath" on pwd

GOPACKAGESDRIVER commands absolutize filenames
relative to their current working directory. However,
os.Getwd may inadvertently expand out any symbolic
links in the path, causing files to have the "wrong"
path, and breaking various name-based equivalence tests
that are common in the go/packages domain.

This CL exploits the same trick used in gocommand to
prevent os.Getwd from expanding symbolic links:
if Stat(Getenv(PWD) returns the process's working
directory, then the iterated ".." search (which
inadvertently expands symlinks) is avoided.

It is unfortunate that driver writers must think about
this. Mostly it only shows up in tests, as that's
where the subprocess directory varies from the parent
directory.

Also, add -driver flag to gopackages debug helper,
which causes it to use drivertest instead of go list
directly.

Change-Id: Ibe12531fe565e74ca1d2565805b0f2458803f6b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/588767
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

go/internal/packagesdriver/sizes.go

@@ -13,6 +13,7 @@ import (
 	"golang.org/x/tools/internal/gocommand"
 )
 
+// TODO(adonovan): move back into go/packages.
 func GetSizesForArgsGolist(ctx context.Context, inv gocommand.Invocation, gocmdRunner *gocommand.Runner) (string, string, error) {
 	inv.Verb = "list"
 	inv.Args = []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"}

go/packages/external.go

@@ -119,7 +119,19 @@ func findExternalDriver(cfg *Config) driver {
 		stderr := new(bytes.Buffer)
 		cmd := exec.CommandContext(cfg.Context, tool, words...)
 		cmd.Dir = cfg.Dir
-		cmd.Env = cfg.Env
+		// The cwd gets resolved to the real path. On Darwin, where
+		// /tmp is a symlink, this breaks anything that expects the
+		// working directory to keep the original path, including the
+		// go command when dealing with modules.
+		//
+		// os.Getwd stdlib has a special feature where if the
+		// cwd and the PWD are the same node then it trusts
+		// the PWD, so by setting it in the env for the child
+		// process we fix up all the paths returned by the go
+		// command.
+		//
+		// (See similar trick in Invocation.run in ../../internal/gocommand/invoke.go)
+		cmd.Env = append(slicesClip(cfg.Env), "PWD="+cfg.Dir)
 		cmd.Stdin = bytes.NewReader(req)
 		cmd.Stdout = buf
 		cmd.Stderr = stderr
@@ -138,3 +150,7 @@ func findExternalDriver(cfg *Config) driver {
 		return &response, nil
 	}
 }
+
+// slicesClip removes unused capacity from the slice, returning s[:len(s):len(s)].
+// TODO(adonovan): use go1.21 slices.Clip.
+func slicesClip[S ~[]E, E any](s S) S { return s[:len(s):len(s)] }

go/packages/gopackages/main.go

@@ -14,16 +14,19 @@ import (
 	"flag"
 	"fmt"
 	"go/types"
+	"log"
 	"os"
 	"sort"
 	"strings"
 
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/drivertest"
 	"golang.org/x/tools/internal/tool"
 )
 
 func main() {
+	drivertest.RunIfChild()
 	tool.Main(context.Background(), &application{Mode: "imports"}, os.Args[1:])
 }
 
@@ -37,6 +40,7 @@ type application struct {
 	Private    bool            `flag:"private" help:"show non-exported declarations too (if -mode=syntax)"`
 	PrintJSON  bool            `flag:"json" help:"print package in JSON form"`
 	BuildFlags stringListValue `flag:"buildflag" help:"pass argument to underlying build system (may be repeated)"`
+	Driver     bool            `flag:"driver" help:"use golist passthrough driver (for debugging driver issues)"`
 }
 
 // Name implements tool.Application returning the binary name.
@@ -82,11 +86,17 @@ func (app *application) Run(ctx context.Context, args ...string) error {
 		return tool.CommandLineErrorf("not enough arguments")
 	}
 
+	env := os.Environ()
+	if app.Driver {
+		env = append(env, drivertest.Env(log.Default())...)
+	}
+
 	// Load, parse, and type-check the packages named on the command line.
 	cfg := &packages.Config{
 		Mode:       packages.LoadSyntax,
 		Tests:      app.Test,
 		BuildFlags: app.BuildFlags,
+		Env:        env,
 	}
 
 	// -mode flag

internal/drivertest/driver.go

@@ -17,7 +17,6 @@ import (
 	"flag"
 	"log"
 	"os"
-	"testing"
 
 	"golang.org/x/tools/go/packages"
 )
@@ -37,7 +36,9 @@ func RunIfChild() {
 
 // Env returns additional environment variables for use in [packages.Config]
 // to enable the use of drivertest as the driver.
-func Env(t *testing.T) []string {
+//
+// t abstracts a *testing.T or log.Default().
+func Env(t interface{ Fatal(...any) }) []string {
 	exe, err := os.Executable()
 	if err != nil {
 		t.Fatal(err)

internal/gocommand/invoke.go

@@ -259,12 +259,15 @@ func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error {
 		waitDelay.Set(reflect.ValueOf(30 * time.Second))
 	}
 
-	// On darwin the cwd gets resolved to the real path, which breaks anything that
-	// expects the working directory to keep the original path, including the
+	// The cwd gets resolved to the real path. On Darwin, where
+	// /tmp is a symlink, this breaks anything that expects the
+	// working directory to keep the original path, including the
 	// go command when dealing with modules.
-	// The Go stdlib has a special feature where if the cwd and the PWD are the
-	// same node then it trusts the PWD, so by setting it in the env for the child
-	// process we fix up all the paths returned by the go command.
+	//
+	// os.Getwd has a special feature where if the cwd and the PWD
+	// are the same node then it trusts the PWD, so by setting it
+	// in the env for the child process we fix up all the paths
+	// returned by the go command.
 	if !i.CleanEnv {
 		cmd.Env = os.Environ()
 	}
@@ -474,7 +477,7 @@ func cmdDebugStr(cmd *exec.Cmd) string {
 }
 
 // WriteOverlays writes each value in the overlay (see the Overlay
-// field of go/packages.Cfg) to a temporary file and returns the name
+// field of go/packages.Config) to a temporary file and returns the name
 // of a JSON file describing the mapping that is suitable for the "go
 // list -overlay" flag.
 //