go/packages: start with empty environment
go/packages is documented as starting with an empty environment if
Config.Env is populated. Fixing all uses of gocommand.Invocation was a
daunting task, so add a mode flag to it instead and set it just in
go/packages.
Clean up packagesdriver.GetSizes, which seems to be completely unused?
Fixes golang/go#42590.
Change-Id: Idac2e1a4798b4a2f5e7c8aa0a1a089a6bd3630ba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/270038
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/go/internal/packagesdriver/sizes.go b/go/internal/packagesdriver/sizes.go
index 35bc6a4..f4d73b2 100644
--- a/go/internal/packagesdriver/sizes.go
+++ b/go/internal/packagesdriver/sizes.go
@@ -6,12 +6,9 @@
package packagesdriver
import (
- "bytes"
"context"
- "encoding/json"
"fmt"
"go/types"
- "os/exec"
"strings"
"golang.org/x/tools/internal/gocommand"
@@ -19,67 +16,6 @@
var debug = false
-func GetSizes(ctx context.Context, buildFlags, env []string, gocmdRunner *gocommand.Runner, dir string) (types.Sizes, error) {
- // TODO(matloob): Clean this up. This code is mostly a copy of packages.findExternalDriver.
- const toolPrefix = "GOPACKAGESDRIVER="
- tool := ""
- for _, env := range env {
- if val := strings.TrimPrefix(env, toolPrefix); val != env {
- tool = val
- }
- }
-
- if tool == "" {
- var err error
- tool, err = exec.LookPath("gopackagesdriver")
- if err != nil {
- // We did not find the driver, so use "go list".
- tool = "off"
- }
- }
-
- if tool == "off" {
- inv := gocommand.Invocation{
- BuildFlags: buildFlags,
- Env: env,
- WorkingDir: dir,
- }
- return GetSizesGolist(ctx, inv, gocmdRunner)
- }
-
- req, err := json.Marshal(struct {
- Command string `json:"command"`
- Env []string `json:"env"`
- BuildFlags []string `json:"build_flags"`
- }{
- Command: "sizes",
- Env: env,
- BuildFlags: buildFlags,
- })
- if err != nil {
- return nil, fmt.Errorf("failed to encode message to driver tool: %v", err)
- }
-
- buf := new(bytes.Buffer)
- cmd := exec.CommandContext(ctx, tool)
- cmd.Dir = dir
- cmd.Env = env
- cmd.Stdin = bytes.NewReader(req)
- cmd.Stdout = buf
- cmd.Stderr = new(bytes.Buffer)
- if err := cmd.Run(); err != nil {
- return nil, fmt.Errorf("%v: %v: %s", tool, err, cmd.Stderr)
- }
- var response struct {
- // Sizes, if not nil, is the types.Sizes to use when type checking.
- Sizes *types.StdSizes
- }
- if err := json.Unmarshal(buf.Bytes(), &response); err != nil {
- return nil, err
- }
- return response.Sizes, nil
-}
-
func GetSizesGolist(ctx context.Context, inv gocommand.Invocation, gocmdRunner *gocommand.Runner) (types.Sizes, error) {
inv.Verb = "list"
inv.Args = []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"}
diff --git a/go/packages/golist.go b/go/packages/golist.go
index 81381fa..ba07cd3 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -827,6 +827,7 @@
BuildFlags: cfg.BuildFlags,
ModFile: cfg.modFile,
ModFlag: cfg.modFlag,
+ CleanEnv: cfg.Env != nil,
Env: cfg.Env,
Logf: cfg.Logf,
WorkingDir: cfg.Dir,
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index a5351ab..06b01f6 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -86,8 +86,8 @@
hash := initial[0]
// Even though the hash package has imports,
// they are not reported.
- got := fmt.Sprintf("iamashamedtousethedisabledqueryname=%s srcs=%v imports=%v", hash.Name, srcs(hash), hash.Imports)
- want := "iamashamedtousethedisabledqueryname=hash srcs=[hash.go] imports=map[]"
+ got := fmt.Sprintf("srcs=%v imports=%v", srcs(hash), hash.Imports)
+ want := "srcs=[hash.go] imports=map[]"
if got != want {
t.Fatalf("got %s, want %s", got, want)
}
@@ -2628,6 +2628,16 @@
}
}
+func TestEmptyEnvironment(t *testing.T) {
+ cfg := &packages.Config{
+ Env: []string{"FOO=BAR"},
+ }
+ _, err := packages.Load(cfg, "fmt")
+ if err == nil {
+ t.Fatal("Load with explicitly empty environment should fail")
+ }
+}
+
func errorMessages(errors []packages.Error) []string {
var msgs []string
for _, err := range errors {
diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go
index 8ba2253..c1373c6 100644
--- a/internal/gocommand/invoke.go
+++ b/internal/gocommand/invoke.go
@@ -133,6 +133,9 @@
ModFlag string
ModFile string
Overlay string
+ // If CleanEnv is set, the invocation will run only with the environment
+ // in Env, not starting with os.Environ.
+ CleanEnv bool
Env []string
WorkingDir string
Logf func(format string, args ...interface{})
@@ -207,7 +210,10 @@
// 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.
- cmd.Env = append(os.Environ(), i.Env...)
+ if !i.CleanEnv {
+ cmd.Env = os.Environ()
+ }
+ cmd.Env = append(cmd.Env, i.Env...)
if i.WorkingDir != "" {
cmd.Env = append(cmd.Env, "PWD="+i.WorkingDir)
cmd.Dir = i.WorkingDir