From 8ef127344c684f0eb23e5eea40f303b74de097b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 12 Mar 2021 18:06:33 +0000 Subject: [PATCH] Revert "gotooltest: proxy the new GOMODCACHE env var too" (#136) We started making gotooltest share the host's module download cache with test scripts, since we did it with GOCACHE and it initially made sense. However, the upside is significantly smaller for GOMODCACHE compared to GOCACHE. The build cache can save a significant amount of time, since many tools have to load or build Go packages as part of their tests. In contrast, few tests download modules, and those which do tend to download those modules from a local proxy like goproxytest, which is very fast already. The downsides of sharing the module download cache are a few: * We don't share GOPATH, and since the default GOMODCACHE is GOPATH/pkg/mod, sharing one and not the other is unexpected and inconsistent. * Upstream testscript shares GOCACHE, but does not share GOMODCACHE. See https://github.com/golang/go/issues/42017. * Losing a degree of isolation in the tests is a downside in itself, especially given that sharing GOMODCACHE isn't crucial. This reverts commit c5fd45a58807701e2c6ea10b9f7499d6a5e31d2e. Note that we keep the env.txt test in place, just flipping the expectation that GOMODCACHE does not contain ${WORK}. --- cmd/testscript/testdata/simple.txt | 3 --- cmd/txtar-addmod/addmod.go | 19 +++++-------------- gotooltest/setup.go | 5 +---- gotooltest/testdata/env.txt | 6 +++--- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/cmd/testscript/testdata/simple.txt b/cmd/testscript/testdata/simple.txt index 4dfedec3..c8a1f4ad 100644 --- a/cmd/testscript/testdata/simple.txt +++ b/cmd/testscript/testdata/simple.txt @@ -1,7 +1,5 @@ # With .gomodproxy supporting files, any GOPROXY from the # environment should be overridden by the test proxy. -# Note that we want an empty GOMODCACHE, to ensure we have to download modules -# from our proxy. env GOPROXY=0.1.2.3 unquote file.txt @@ -10,7 +8,6 @@ stdout 'example.com/mod' ! stderr .+ -- file.txt -- ->env GOMODCACHE=$WORK >go get -d fruit.com >go list -m >stdout 'example.com/mod' diff --git a/cmd/txtar-addmod/addmod.go b/cmd/txtar-addmod/addmod.go index 67a4a52e..663f1ab0 100644 --- a/cmd/txtar-addmod/addmod.go +++ b/cmd/txtar-addmod/addmod.go @@ -20,7 +20,6 @@ package main import ( "bytes" - "encoding/json" "flag" "fmt" "io/ioutil" @@ -101,17 +100,9 @@ func main1() int { return string(out) } - var goEnv struct { - GOPATH string - GOMODCACHE string - } - if err := json.Unmarshal([]byte(run("go", "env", "-json", "GOPATH", "GOMODCACHE")), &goEnv); err != nil { - fatalf("cannot unmarshal 'go env -json': %v", err) - } - modCache := goEnv.GOMODCACHE - if modCache == "" { - // For Go 1.14 and older. - modCache = filepath.Join(goEnv.GOPATH, "pkg", "mod") + gopath := strings.TrimSpace(run("go", "env", "GOPATH")) + if gopath == "" { + fatalf("cannot find GOPATH") } exitCode := 0 @@ -140,13 +131,13 @@ func main1() int { } path = encpath - mod, err := ioutil.ReadFile(filepath.Join(modCache, "cache", "download", path, "@v", vers+".mod")) + mod, err := ioutil.ReadFile(filepath.Join(gopath, "pkg/mod/cache/download", path, "@v", vers+".mod")) if err != nil { log.Printf("%s: %v", arg, err) exitCode = 1 continue } - info, err := ioutil.ReadFile(filepath.Join(modCache, "cache", "download", path, "@v", vers+".info")) + info, err := ioutil.ReadFile(filepath.Join(gopath, "pkg/mod/cache/download", path, "@v", vers+".info")) if err != nil { log.Printf("%s: %v", arg, err) exitCode = 1 diff --git a/gotooltest/setup.go b/gotooltest/setup.go index eee59faa..84705cfa 100644 --- a/gotooltest/setup.go +++ b/gotooltest/setup.go @@ -27,7 +27,6 @@ var ( goEnv struct { GOROOT string GOCACHE string - GOMODCACHE string GOPROXY string goversion string releaseTags []string @@ -60,14 +59,13 @@ func initGoEnv() error { eout, stderr, err := run("go", "env", "-json", "GOROOT", "GOCACHE", - "GOMODCACHE", "GOPROXY", ) if err != nil { return fmt.Errorf("failed to determine environment from go command: %v\n%v", err, stderr) } if err := json.Unmarshal(eout.Bytes(), &goEnv); err != nil { - return fmt.Errorf("failed to unmarshal 'go env -json' output: %v\n%v", err, eout) + return fmt.Errorf("failed to unmarshal GOROOT and GOCACHE tags from go command out: %v\n%v", err, eout) } version := goEnv.releaseTags[len(goEnv.releaseTags)-1] @@ -140,7 +138,6 @@ func goEnviron(env0 []string) []string { "GOOS=" + runtime.GOOS, "GOROOT=" + goEnv.GOROOT, "GOCACHE=" + goEnv.GOCACHE, - "GOMODCACHE=" + goEnv.GOMODCACHE, "GOPROXY=" + goEnv.GOPROXY, "goversion=" + goEnv.goversion, }...) diff --git a/gotooltest/testdata/env.txt b/gotooltest/testdata/env.txt index 73436307..057aa3c4 100644 --- a/gotooltest/testdata/env.txt +++ b/gotooltest/testdata/env.txt @@ -1,6 +1,6 @@ -# GOPATH should point inside the temporary work directory, but GOCACHE and -# GOMODCACHE should not, as they should reuse the host's. +# GOPATH and GOMODCACHE are not shared with the host, +# but GOCACHE is. go env stdout GOPATH=.*${WORK@R} +stdout GOMODCACHE=.*${WORK@R} ! stdout GOCACHE=.*${WORK@R} -! stdout GOMODCACHE=.*${WORK@R}