Skip to content

Commit

Permalink
Revert "gotooltest: proxy the new GOMODCACHE env var too" (#136)
Browse files Browse the repository at this point in the history
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 golang/go#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 c5fd45a.

Note that we keep the env.txt test in place, just flipping the
expectation that GOMODCACHE does not contain ${WORK}.
  • Loading branch information
mvdan authored Mar 12, 2021
1 parent 50426be commit 8ef1273
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 24 deletions.
3 changes: 0 additions & 3 deletions cmd/testscript/testdata/simple.txt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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'
Expand Down
19 changes: 5 additions & 14 deletions cmd/txtar-addmod/addmod.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package main

import (
"bytes"
"encoding/json"
"flag"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions gotooltest/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ var (
goEnv struct {
GOROOT string
GOCACHE string
GOMODCACHE string
GOPROXY string
goversion string
releaseTags []string
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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,
}...)
Expand Down
6 changes: 3 additions & 3 deletions gotooltest/testdata/env.txt
Original file line number Diff line number Diff line change
@@ -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}

0 comments on commit 8ef1273

Please sign in to comment.