cmd/internal/pkgsite: improve godoc mode search in GOROOT

If a user starts go doc -http inside of GOROOT, we create three module
getters when we only need one: We create a StdlibZip module getter which
we're not going to use because we're using a local GOROOT, we create a
gopackages module getter for the module determined as corresponding to
the current directory, which is a stdlib module, and finally we create
the NewGoPackagesStdlibModuleGetter, which is the main module getter for
the standard library in that case.

The StdlibZip module getter is not used when we're using a local GOROOT,
so don't add it in that case. And if we detect that the module for a
specified directory is std or cmd for the specified local GOROOT, skip
adding it because it will be redundant with the module getter we create
using NewGoPackagesStdlibModuleGetter specifically for the standard
libarry.

Removing the redundant module getter will prevent redundant search
results returned by each of the gopackages module getters corresponding
to the standard library.

While we're here, if there's a VERSION file in GOROOT, set the version
on the module metadata we return for the standard library to the
stdlib.VersionForTag of that version, so we can report the version in
the search results.

Fixes golang/go#74459

Change-Id: If250f715b052f57f8668b02a57b4c96725be515a
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/687918
Reviewed-by: Jonathan Amsterdam <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
kokoro-CI: kokoro <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Auto-Submit: Michael Matloob <[email protected]>

Author: matloob

cmd/internal/pkgsite/server.go

@@ -69,7 +69,7 @@ func BuildServer(ctx context.Context, serverCfg ServerConfig) (*frontend.Server,
 		}
 	} else {
 		var err error
-		cfg.dirs, err = getModuleDirs(ctx, serverCfg.Paths, serverCfg.GoDocMode)
+		cfg.dirs, err = getModuleDirs(ctx, serverCfg.Paths, serverCfg.GoRepoPath, serverCfg.GoDocMode)
 		if err != nil {
 			return nil, fmt.Errorf("searching modules: %v", err)
 		}
@@ -122,7 +122,7 @@ func BuildServer(ctx context.Context, serverCfg ServerConfig) (*frontend.Server,
 //
 // An error is returned if any operations failed unexpectedly, or if no
 // requested directories contain any valid modules.
-func getModuleDirs(ctx context.Context, dirs []string, allowNoModules bool) (map[string][]frontend.LocalModule, error) {
+func getModuleDirs(ctx context.Context, dirs []string, goRepoPath string, allowNoModules bool) (map[string][]frontend.LocalModule, error) {
 	dirModules := make(map[string][]frontend.LocalModule)
 	for _, dir := range dirs {
 		output, err := runGo(dir, "list", "-m", "-json")
@@ -136,6 +136,12 @@ func getModuleDirs(ctx context.Context, dirs []string, allowNoModules bool) (map
 			if err := decoder.Decode(&m); err != nil {
 				return nil, err
 			}
+			if allowNoModules && m.ModulePath == "std" && (m.Dir == filepath.Join(goRepoPath, "src") || m.Dir == filepath.Join(goRepoPath, "src", "cmd")) {
+				// This module is in the stdlib in the GOROOT specified by goRepoPath. We will
+				// already create a GoPackagesStdlibModuleGetter for that GOROOT, so don't
+				// add a redundant module directory here.
+				continue
+			}
 			if m.ModulePath != "command-line-arguments" {
 				modules = append(modules, m)
 			}
@@ -240,6 +246,7 @@ func buildGetters(ctx context.Context, cfg getterConfig) ([]fetch.ModuleGetter,
 		getters = append(getters, g)
 	}
 
+	var usingLocalStdlib bool
 	if cfg.useLocalStdlib {
 		goRepo := cfg.goRepoPath
 		if goRepo == "" {
@@ -250,6 +257,7 @@ func buildGetters(ctx context.Context, cfg getterConfig) ([]fetch.ModuleGetter,
 			if err != nil {
 				log.Errorf(ctx, "loading packages from stdlib: %v", err)
 			} else {
+				usingLocalStdlib = true
 				getters = append(getters, mg)
 			}
 		}
@@ -260,7 +268,9 @@ func buildGetters(ctx context.Context, cfg getterConfig) ([]fetch.ModuleGetter,
 		getters = append(getters, fetch.NewProxyModuleGetter(cfg.proxy, source.NewClient(&http.Client{Timeout: time.Second})))
 	}
 
-	getters = append(getters, fetch.NewStdlibZipModuleGetter())
+	if !usingLocalStdlib {
+		getters = append(getters, fetch.NewStdlibZipModuleGetter())
+	}
 
 	return getters, nil
 }

internal/fetch/getters.go

@@ -16,6 +16,7 @@ import (
 	"go/doc"
 	"go/parser"
 	"go/token"
+	goversion "go/version"
 	"io"
 	"io/fs"
 	"os"
@@ -304,8 +305,10 @@ func NewGoPackagesStdlibModuleGetter(ctx context.Context, dir string) (*goPackag
 		return nil, err
 	}
 
-	stdmod := &packages.Module{Path: "std",
-		Dir: filepath.Join(abs, "src"),
+	stdmod := &packages.Module{
+		Path:    "std",
+		Dir:     filepath.Join(abs, "src"),
+		Version: goVersion(abs),
 	}
 	modules := []*packages.Module{stdmod}
 	for _, p := range pkgs {
@@ -320,6 +323,25 @@ func NewGoPackagesStdlibModuleGetter(ctx context.Context, dir string) (*goPackag
 	}, nil
 }
 
+// goVersion returns the go version of the go distribution at GOROOT, converted
+// into a semantic version that can be parsed by stdlib.TagForVersion; or LocalVersion,
+// if the version could not be determined. goVersion determines the version using
+// the VERSION file. Alternatively, it could run "go version" to get the version
+// in cases the VERSION file doesn't exist, but those versions likely wouldn't
+// be parsable by VersionForTag anyway.
+func goVersion(goroot string) string {
+	b, err := os.ReadFile(filepath.Join(goroot, "VERSION"))
+	if err != nil {
+		return LocalVersion
+	}
+	b, _, _ = bytes.Cut(b, []byte("\n"))
+	gov := strings.TrimSpace(string(b))
+	if !goversion.IsValid(gov) {
+		return LocalVersion
+	}
+	return stdlib.VersionForTag(gov)
+}
+
 // findModule searches known modules for a module matching the provided path.
 func (g *goPackagesModuleGetter) findModule(path string) (*packages.Module, error) {
 	i := sort.Search(len(g.modules), func(i int) bool {