Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "gotooltest: proxy the new GOMODCACHE env var too" #136

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

mvdan
Copy link
Collaborator

@mvdan mvdan commented Mar 12, 2021

(see commit message)

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}.
@myitcv
Copy link
Collaborator

myitcv commented Mar 12, 2021

Merging per @mvdan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants