From 1c71899d35ca07586495ab5b33711a8b592faad7 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Wed, 13 Nov 2019 10:39:05 -0500 Subject: [PATCH] cmd/godoc: don't execute go mod download when GOMOD is /dev/null When the GOMOD value is the operating system's null device, there isn't a main module and thus there cannot be module requirements. Don't try to fill module cache, since running 'go mod download' would cause a "no modules specified (see 'go help mod download')" error to be printed, which is confusing. Fixes golang/go#35476 Change-Id: Ia1f5ee50797a4dc3e36c3b9773e19da781bd9d39 Reviewed-on: https://go-review.googlesource.com/c/tools/+/206886 Run-TryBot: Dmitri Shuralyov TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills --- cmd/godoc/godoc_test.go | 34 +++++++++++++++++++++++++++++++++- cmd/godoc/main.go | 9 +++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/cmd/godoc/godoc_test.go b/cmd/godoc/godoc_test.go index 3654e37efd..3748388fd8 100644 --- a/cmd/godoc/godoc_test.go +++ b/cmd/godoc/godoc_test.go @@ -182,7 +182,7 @@ func TestURL(t *testing.T) { cmd.Stderr = stderr cmd.Args[0] = "godoc" - // Set GOPATH variable to non-existing absolute path + // Set GOPATH variable to a non-existing absolute path // and GOPROXY=off to disable module fetches. // We cannot just unset GOPATH variable because godoc would default it to ~/go. // (We don't want the indexer looking at the local workspace during tests.) @@ -444,6 +444,38 @@ package a; import _ "godoc.test/repo2/a"; const Name = "repo1a"`, } } +// Test for golang.org/issue/35476. +func TestNoMainModule(t *testing.T) { + if testing.Short() { + t.Skip("skipping test in -short mode") + } + if runtime.GOOS == "plan9" { + t.Skip("skipping on plan9; for consistency with other tests that build godoc binary") + } + bin, cleanup := buildGodoc(t) + defer cleanup() + tempDir, err := ioutil.TempDir("", "godoc-test-") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + // Run godoc in an empty directory with module mode explicitly on, + // so that 'go env GOMOD' reports os.DevNull. + cmd := exec.Command(bin, "-url=/") + cmd.Dir = tempDir + cmd.Env = append(os.Environ(), "GO111MODULE=on") + var stderr bytes.Buffer + cmd.Stderr = &stderr + err = cmd.Run() + if err != nil { + t.Fatal(err) + } + if strings.Contains(stderr.String(), "go mod download") { + t.Errorf("stderr contains 'go mod download', is that intentional?\nstderr=%q", stderr.String()) + } +} + // Basic integration test for godoc -analysis=type (via HTTP interface). func TestTypeAnalysis(t *testing.T) { bin, cleanup := buildGodoc(t) diff --git a/cmd/godoc/main.go b/cmd/godoc/main.go index 082fda85d7..1d402ba0bd 100644 --- a/cmd/godoc/main.go +++ b/cmd/godoc/main.go @@ -222,7 +222,7 @@ func main() { // This may fail if module downloading is disallowed (GOPROXY=off) or due to // limited connectivity, in which case we print errors to stderr and show // documentation only for packages that are available. - fillModuleCache(os.Stderr) + fillModuleCache(os.Stderr, goModFile) // Determine modules in the build list. mods, err := buildList() @@ -414,7 +414,12 @@ func goMod() (string, error) { // It should only be used when operating in module mode. // // See https://golang.org/cmd/go/#hdr-Download_modules_to_local_cache. -func fillModuleCache(w io.Writer) { +func fillModuleCache(w io.Writer, goMod string) { + if goMod == os.DevNull { + // No module requirements, nothing to do. + return + } + cmd := exec.Command("go", "mod", "download", "-json") var out bytes.Buffer cmd.Stdout = &out