]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: stop tests from using network during -short
authorRuss Cox <rsc@golang.org>
Wed, 2 Dec 2020 15:14:53 +0000 (10:14 -0500)
committerRuss Cox <rsc@golang.org>
Wed, 2 Dec 2020 17:59:03 +0000 (17:59 +0000)
It turned out that "go get" was using the network to look up
https://github.com?go-get=1 while resolving github.com/google/go-cmp,
and that is not the fastest page to load.
Stop that lookup by adjusting the path prefixes in the vcs table.

It also turned out that "go get" was using the network to look up
https://rsc.io?go-get=1 while resolving https://rsc.io/nonexist.svn.
That's a bit more defensible maybe, since rsc.io is not a known VCS host.
But for tests we really want to avoid the network entirely, so this CL
adds a special case in repoRootFromVCSPaths that returns a hard error
for plain "rsc.io" instead of doing the web fetch.

To keep us honest in the future, I added two automatically-set env
variables TESTGONETWORK=panic and TESTGOVCS=panic.
These cause the go command to panic rather than make a network request
or invoke a VCS command.

go test -short cmd/go now passes with these checks.

This reduced the time spent in go test -short cmd/go on my
Google workstation from 154s to 30s. (Yay network firewalls.)

Change-Id: I49207fca7f901fa011765fb984dc9cec8b691f11
Reviewed-on: https://go-review.googlesource.com/c/go/+/274441
Trust: Russ Cox <rsc@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
src/cmd/go/internal/modfetch/codehost/codehost.go
src/cmd/go/internal/vcs/vcs.go
src/cmd/go/internal/web/http.go
src/cmd/go/script_test.go
src/cmd/go/testdata/script/mod_sumdb_proxy.txt

index c5fbb31b2b7f380e422391ec706fbc0f5ba7eeff..286d3f722032fdea59825a12ac3c82f1320e8cf4 100644 (file)
@@ -264,6 +264,9 @@ func RunWithStdin(dir string, stdin io.Reader, cmdline ...interface{}) ([]byte,
        }
 
        cmd := str.StringList(cmdline...)
+       if os.Getenv("TESTGOVCS") == "panic" {
+               panic(fmt.Sprintf("use of vcs: %v", cmd))
+       }
        if cfg.BuildX {
                text := new(strings.Builder)
                if dir != "" {
index e7bef9f591ac06d8d0e7594e9f73ccbc32f5b7fd..4894ecdc35655643608c4e7e63bb642d3692fc36 100644 (file)
@@ -23,6 +23,7 @@ import (
        "cmd/go/internal/base"
        "cmd/go/internal/cfg"
        "cmd/go/internal/search"
+       "cmd/go/internal/str"
        "cmd/go/internal/web"
 
        "golang.org/x/mod/module"
@@ -539,7 +540,7 @@ func (v *Cmd) TagSync(dir, tag string) error {
 // A vcsPath describes how to convert an import path into a
 // version control system and repository name.
 type vcsPath struct {
-       prefix         string                              // prefix this description applies to
+       pathPrefix     string                              // prefix this description applies to
        regexp         *lazyregexp.Regexp                  // compiled pattern for import path
        repo           string                              // repository to use (expand with match of re)
        vcs            string                              // version control system to use (expand with match of re)
@@ -826,6 +827,20 @@ var errUnknownSite = errors.New("dynamic lookup required to find mapping")
 // repoRootFromVCSPaths attempts to map importPath to a repoRoot
 // using the mappings defined in vcsPaths.
 func repoRootFromVCSPaths(importPath string, security web.SecurityMode, vcsPaths []*vcsPath) (*RepoRoot, error) {
+       if str.HasPathPrefix(importPath, "example.net") {
+               // TODO(rsc): This should not be necessary, but it's required to keep
+               // tests like ../../testdata/script/mod_get_extra.txt from using the network.
+               // That script has everything it needs in the replacement set, but it is still
+               // doing network calls.
+               return nil, fmt.Errorf("no modules on example.net")
+       }
+       if importPath == "rsc.io" {
+               // This special case allows tests like ../../testdata/script/govcs.txt
+               // to avoid making any network calls. The module lookup for a path
+               // like rsc.io/nonexist.svn/foo needs to not make a network call for
+               // a lookup on rsc.io.
+               return nil, fmt.Errorf("rsc.io is not a module")
+       }
        // A common error is to use https://packagepath because that's what
        // hg and git require. Diagnose this helpfully.
        if prefix := httpPrefix(importPath); prefix != "" {
@@ -834,20 +849,20 @@ func repoRootFromVCSPaths(importPath string, security web.SecurityMode, vcsPaths
                return nil, fmt.Errorf("%q not allowed in import path", prefix+"//")
        }
        for _, srv := range vcsPaths {
-               if !strings.HasPrefix(importPath, srv.prefix) {
+               if !str.HasPathPrefix(importPath, srv.pathPrefix) {
                        continue
                }
                m := srv.regexp.FindStringSubmatch(importPath)
                if m == nil {
-                       if srv.prefix != "" {
-                               return nil, importErrorf(importPath, "invalid %s import path %q", srv.prefix, importPath)
+                       if srv.pathPrefix != "" {
+                               return nil, importErrorf(importPath, "invalid %s import path %q", srv.pathPrefix, importPath)
                        }
                        continue
                }
 
                // Build map of named subexpression matches for expand.
                match := map[string]string{
-                       "prefix": srv.prefix,
+                       "prefix": srv.pathPrefix + "/",
                        "import": importPath,
                }
                for i, name := range srv.regexp.SubexpNames() {
@@ -1098,18 +1113,6 @@ type metaImport struct {
        Prefix, VCS, RepoRoot string
 }
 
-// pathPrefix reports whether sub is a prefix of s,
-// only considering entire path components.
-func pathPrefix(s, sub string) bool {
-       // strings.HasPrefix is necessary but not sufficient.
-       if !strings.HasPrefix(s, sub) {
-               return false
-       }
-       // The remainder after the prefix must either be empty or start with a slash.
-       rem := s[len(sub):]
-       return rem == "" || rem[0] == '/'
-}
-
 // A ImportMismatchError is returned where metaImport/s are present
 // but none match our import path.
 type ImportMismatchError struct {
@@ -1133,7 +1136,7 @@ func matchGoImport(imports []metaImport, importPath string) (metaImport, error)
 
        errImportMismatch := ImportMismatchError{importPath: importPath}
        for i, im := range imports {
-               if !pathPrefix(importPath, im.Prefix) {
+               if !str.HasPathPrefix(importPath, im.Prefix) {
                        errImportMismatch.mismatches = append(errImportMismatch.mismatches, im.Prefix)
                        continue
                }
@@ -1175,52 +1178,52 @@ func expand(match map[string]string, s string) string {
 var vcsPaths = []*vcsPath{
        // Github
        {
-               prefix: "github.com/",
-               regexp: lazyregexp.New(`^(?P<root>github\.com/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[A-Za-z0-9_.\-]+)*$`),
-               vcs:    "git",
-               repo:   "https://{root}",
-               check:  noVCSSuffix,
+               pathPrefix: "github.com",
+               regexp:     lazyregexp.New(`^(?P<root>github\.com/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[A-Za-z0-9_.\-]+)*$`),
+               vcs:        "git",
+               repo:       "https://{root}",
+               check:      noVCSSuffix,
        },
 
        // Bitbucket
        {
-               prefix: "bitbucket.org/",
-               regexp: lazyregexp.New(`^(?P<root>bitbucket\.org/(?P<bitname>[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+))(/[A-Za-z0-9_.\-]+)*$`),
-               repo:   "https://{root}",
-               check:  bitbucketVCS,
+               pathPrefix: "bitbucket.org",
+               regexp:     lazyregexp.New(`^(?P<root>bitbucket\.org/(?P<bitname>[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+))(/[A-Za-z0-9_.\-]+)*$`),
+               repo:       "https://{root}",
+               check:      bitbucketVCS,
        },
 
        // IBM DevOps Services (JazzHub)
        {
-               prefix: "hub.jazz.net/git/",
-               regexp: lazyregexp.New(`^(?P<root>hub\.jazz\.net/git/[a-z0-9]+/[A-Za-z0-9_.\-]+)(/[A-Za-z0-9_.\-]+)*$`),
-               vcs:    "git",
-               repo:   "https://{root}",
-               check:  noVCSSuffix,
+               pathPrefix: "hub.jazz.net/git",
+               regexp:     lazyregexp.New(`^(?P<root>hub\.jazz\.net/git/[a-z0-9]+/[A-Za-z0-9_.\-]+)(/[A-Za-z0-9_.\-]+)*$`),
+               vcs:        "git",
+               repo:       "https://{root}",
+               check:      noVCSSuffix,
        },
 
        // Git at Apache
        {
-               prefix: "git.apache.org/",
-               regexp: lazyregexp.New(`^(?P<root>git\.apache\.org/[a-z0-9_.\-]+\.git)(/[A-Za-z0-9_.\-]+)*$`),
-               vcs:    "git",
-               repo:   "https://{root}",
+               pathPrefix: "git.apache.org",
+               regexp:     lazyregexp.New(`^(?P<root>git\.apache\.org/[a-z0-9_.\-]+\.git)(/[A-Za-z0-9_.\-]+)*$`),
+               vcs:        "git",
+               repo:       "https://{root}",
        },
 
        // Git at OpenStack
        {
-               prefix: "git.openstack.org/",
-               regexp: lazyregexp.New(`^(?P<root>git\.openstack\.org/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(\.git)?(/[A-Za-z0-9_.\-]+)*$`),
-               vcs:    "git",
-               repo:   "https://{root}",
+               pathPrefix: "git.openstack.org",
+               regexp:     lazyregexp.New(`^(?P<root>git\.openstack\.org/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(\.git)?(/[A-Za-z0-9_.\-]+)*$`),
+               vcs:        "git",
+               repo:       "https://{root}",
        },
 
        // chiselapp.com for fossil
        {
-               prefix: "chiselapp.com/",
-               regexp: lazyregexp.New(`^(?P<root>chiselapp\.com/user/[A-Za-z0-9]+/repository/[A-Za-z0-9_.\-]+)$`),
-               vcs:    "fossil",
-               repo:   "https://{root}",
+               pathPrefix: "chiselapp.com",
+               regexp:     lazyregexp.New(`^(?P<root>chiselapp\.com/user/[A-Za-z0-9]+/repository/[A-Za-z0-9_.\-]+)$`),
+               vcs:        "fossil",
+               repo:       "https://{root}",
        },
 
        // General syntax for any server.
@@ -1238,11 +1241,11 @@ var vcsPaths = []*vcsPath{
 var vcsPathsAfterDynamic = []*vcsPath{
        // Launchpad. See golang.org/issue/11436.
        {
-               prefix: "launchpad.net/",
-               regexp: lazyregexp.New(`^(?P<root>launchpad\.net/((?P<project>[A-Za-z0-9_.\-]+)(?P<series>/[A-Za-z0-9_.\-]+)?|~[A-Za-z0-9_.\-]+/(\+junk|[A-Za-z0-9_.\-]+)/[A-Za-z0-9_.\-]+))(/[A-Za-z0-9_.\-]+)*$`),
-               vcs:    "bzr",
-               repo:   "https://{root}",
-               check:  launchpadVCS,
+               pathPrefix: "launchpad.net",
+               regexp:     lazyregexp.New(`^(?P<root>launchpad\.net/((?P<project>[A-Za-z0-9_.\-]+)(?P<series>/[A-Za-z0-9_.\-]+)?|~[A-Za-z0-9_.\-]+/(\+junk|[A-Za-z0-9_.\-]+)/[A-Za-z0-9_.\-]+))(/[A-Za-z0-9_.\-]+)*$`),
+               vcs:        "bzr",
+               repo:       "https://{root}",
+               check:      launchpadVCS,
        },
 }
 
index e0509808d67a2a8422bc5e2ec865256961113da3..72fa2b2ca6aa349241d1e7ec8a0b96c23d3ed3fc 100644 (file)
@@ -80,6 +80,13 @@ func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
                return res, nil
        }
 
+       if url.Host == "localhost.localdev" {
+               return nil, fmt.Errorf("no such host localhost.localdev")
+       }
+       if os.Getenv("TESTGONETWORK") == "panic" && !strings.HasPrefix(url.Host, "127.0.0.1") && !strings.HasPrefix(url.Host, "0.0.0.0") {
+               panic("use of network: " + url.String())
+       }
+
        fetch := func(url *urlpkg.URL) (*urlpkg.URL, *http.Response, error) {
                // Note: The -v build flag does not mean "print logging information",
                // despite its historical misuse for this in GOPATH-based go get.
index b1d1499038fd5f787106cc7e4e473cb59d9f242b..aee3742f13c23143ebde4dcdac63d5d0892d68f1 100644 (file)
@@ -142,6 +142,9 @@ func (ts *testScript) setup() {
                "goversion=" + goVersion(ts),
                ":=" + string(os.PathListSeparator),
        }
+       if !testenv.HasExternalNetwork() {
+               ts.env = append(ts.env, "TESTGONETWORK=panic", "TESTGOVCS=panic")
+       }
 
        if runtime.GOOS == "plan9" {
                ts.env = append(ts.env, "path="+testBin+string(filepath.ListSeparator)+os.Getenv("path"))
index 7bbc3f9e196222315c34093703ce54f9fc87afe5..70b8e3fc4466ffab5676cb06b2aebe8d23aedd39 100644 (file)
@@ -17,14 +17,12 @@ rm $GOPATH/pkg/mod/cache/download/sumdb
 rm go.sum
 
 # direct access fails (because localhost.localdev does not exist)
-# The text of the error message is hard to predict because some DNS servers
-# will resolve unknown domains like localhost.localdev to a real IP
-# to serve ads.
+# web.get is providing the error message - there's no actual network access.
 cp go.mod.orig go.mod
 env GOSUMDB=$sumdb
 env GOPROXY=direct
 ! go get -d rsc.io/fortune@v1.0.0
-stderr 'verifying.*localhost.localdev'
+stderr 'verifying module: rsc.io/fortune@v1.0.0: .*: no such host localhost.localdev'
 rm $GOPATH/pkg/mod/cache/download/sumdb
 rm go.sum