cmd/go: resolve non-standard imports from within GOROOT/src using vendor directories

Updates #30228
Fixes #26924

Change-Id: Ie625c64721559c7633396342320536396cd1fcf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/164621
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

src/cmd/go/internal/cfg/cfg.go

@@ -38,7 +38,7 @@ var (
 	BuildWork              bool // -work flag
 	BuildX                 bool // -x flag
 
-	CmdName string // "build", "install", "list", etc.
+	CmdName string // "build", "install", "list", "mod tidy", etc.
 
 	DebugActiongraph string // -debug-actiongraph flag (undocumented, unstable)
 )

src/cmd/go/internal/load/pkg.go

@@ -32,14 +32,14 @@ var (
 	ModInit func()
 
 	// module hooks; nil if module use is disabled
-	ModBinDir            func() string                                       // return effective bin directory
-	ModLookup            func(path string) (dir, realPath string, err error) // lookup effective meaning of import
-	ModPackageModuleInfo func(path string) *modinfo.ModulePublic             // return module info for Package struct
-	ModImportPaths       func(args []string) []*search.Match                 // expand import paths
-	ModPackageBuildInfo  func(main string, deps []string) string             // return module info to embed in binary
-	ModInfoProg          func(info string) []byte                            // wrap module info in .go code for binary
-	ModImportFromFiles   func([]string)                                      // update go.mod to add modules for imports in these files
-	ModDirImportPath     func(string) string                                 // return effective import path for directory
+	ModBinDir            func() string                                                                            // return effective bin directory
+	ModLookup            func(parentPath string, parentIsStd bool, path string) (dir, realPath string, err error) // lookup effective meaning of import
+	ModPackageModuleInfo func(path string) *modinfo.ModulePublic                                                  // return module info for Package struct
+	ModImportPaths       func(args []string) []*search.Match                                                      // expand import paths
+	ModPackageBuildInfo  func(main string, deps []string) string                                                  // return module info to embed in binary
+	ModInfoProg          func(info string) []byte                                                                 // wrap module info in .go code for binary
+	ModImportFromFiles   func([]string)                                                                           // update go.mod to add modules for imports in these files
+	ModDirImportPath     func(string) string                                                                      // return effective import path for directory
 )
 
 var IgnoreImports bool // control whether we ignore imports in packages
@@ -483,8 +483,10 @@ func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPo
 	}
 
 	parentPath := ""
+	parentIsStd := false
 	if parent != nil {
 		parentPath = parent.ImportPath
+		parentIsStd = parent.Standard
 	}
 
 	// Determine canonical identifier for this package.
@@ -501,7 +503,7 @@ func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPo
 		importPath = dirToImportPath(filepath.Join(srcDir, path))
 	} else if cfg.ModulesEnabled {
 		var p string
-		modDir, p, modErr = ModLookup(path)
+		modDir, p, modErr = ModLookup(parentPath, parentIsStd, path)
 		if modErr == nil {
 			importPath = p
 		}
@@ -641,7 +643,13 @@ func isDir(path string) bool {
 // Go 1.11 module legacy conversion (golang.org/issue/25069).
 func ResolveImportPath(parent *Package, path string) (found string) {
 	if cfg.ModulesEnabled {
-		if _, p, e := ModLookup(path); e == nil {
+		parentPath := ""
+		parentIsStd := false
+		if parent != nil {
+			parentPath = parent.ImportPath
+			parentIsStd = parent.Standard
+		}
+		if _, p, e := ModLookup(parentPath, parentIsStd, path); e == nil {
 			return p
 		}
 		return path
@@ -1401,16 +1409,6 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) {
 			continue
 		}
 		p1 := LoadImport(path, p.Dir, p, stk, p.Internal.Build.ImportPos[path], ResolveImport)
-		if p.Standard && p.Error == nil && !p1.Standard && p1.Error == nil {
-			p.Error = &PackageError{
-				ImportStack: stk.Copy(),
-				Err:         fmt.Sprintf("non-standard import %q in standard package %q", path, p.ImportPath),
-			}
-			pos := p.Internal.Build.ImportPos[path]
-			if len(pos) > 0 {
-				p.Error.Pos = pos[0].String()
-			}
-		}
 
 		path = p1.ImportPath
 		importPaths[i] = path

src/cmd/go/internal/modload/build.go

@@ -38,9 +38,6 @@ func findStandardImportPath(path string) string {
 		if goroot.IsStandardPackage(cfg.GOROOT, cfg.BuildContext.Compiler, path) {
 			return filepath.Join(cfg.GOROOT, "src", path)
 		}
-		if goroot.IsStandardPackage(cfg.GOROOT, cfg.BuildContext.Compiler, "vendor/"+path) {
-			return filepath.Join(cfg.GOROOT, "src/vendor", path)
-		}
 	}
 	return ""
 }

src/cmd/go/internal/modload/import.go

@@ -64,6 +64,19 @@ func Import(path string) (m module.Version, dir string, err error) {
 	if search.IsStandardImportPath(path) {
 		if goroot.IsStandardPackage(cfg.GOROOT, cfg.BuildContext.Compiler, path) {
 			dir := filepath.Join(cfg.GOROOT, "src", path)
+
+			// If the main module is in the standard library, attribute its packages
+			// to that module.
+			switch Target.Path {
+			case "cmd":
+				if strings.HasPrefix(path, "cmd") {
+					return Target, dir, nil
+				}
+			case "std":
+				if !strings.HasPrefix(path, "cmd") {
+					return Target, dir, nil
+				}
+			}
 			return module.Version{}, dir, nil
 		}
 	}

src/cmd/go/internal/modload/init.go

@@ -18,6 +18,7 @@ import (
 	"cmd/go/internal/mvs"
 	"cmd/go/internal/renameio"
 	"cmd/go/internal/search"
+	"cmd/go/internal/str"
 	"encoding/json"
 	"fmt"
 	"go/build"
@@ -380,6 +381,11 @@ func InitMod() {
 // modFileToBuildList initializes buildList from the modFile.
 func modFileToBuildList() {
 	Target = modFile.Module.Mod
+	if (str.HasPathPrefix(Target.Path, "std") || str.HasPathPrefix(Target.Path, "cmd")) &&
+		search.InDir(cwd, cfg.GOROOTsrc) == "" {
+		base.Fatalf("go: reserved module path %s not allow outside of GOROOT/src", Target.Path)
+	}
+
 	list := []module.Version{Target}
 	for _, r := range modFile.Require {
 		list = append(list, r.Mod)

src/cmd/go/internal/modload/load.go

@@ -12,6 +12,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	pathpkg "path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -90,7 +91,9 @@ func ImportPaths(patterns []string) []*search.Match {
 				// the exact version of a particular module increases during
 				// the loader iterations.
 				m.Pkgs = str.StringList(fsDirs[i])
-				for j, pkg := range m.Pkgs {
+				pkgs := m.Pkgs
+				m.Pkgs = m.Pkgs[:0]
+				for _, pkg := range pkgs {
 					dir := pkg
 					if !filepath.IsAbs(dir) {
 						dir = filepath.Join(cwd, pkg)
@@ -108,6 +111,16 @@ func ImportPaths(patterns []string) []*search.Match {
 						if strings.HasPrefix(suffix, "/vendor/") {
 							// TODO getmode vendor check
 							pkg = strings.TrimPrefix(suffix, "/vendor/")
+						} else if Target.Path == "std" {
+							// Don't add the prefix "std/" to packages in the "std" module.
+							// It's the one module path that isn't a prefix of its packages.
+							pkg = strings.TrimPrefix(suffix, "/")
+							if pkg == "builtin" {
+								// "builtin" is a pseudo-package with a real source file.
+								// It's not included in "std", so it shouldn't be included in
+								// "./..." within module "std" either.
+								continue
+							}
 						} else {
 							pkg = Target.Path + suffix
 						}
@@ -129,10 +142,10 @@ func ImportPaths(patterns []string) []*search.Match {
 						// After loader is done iterating, we still need to return the
 						// path, so that "go list -e" produces valid output.
 						if iterating {
-							pkg = ""
+							continue
 						}
 					}
-					m.Pkgs[j] = pkg
+					m.Pkgs = append(m.Pkgs, pkg)
 				}
 
 			case strings.Contains(m.Pattern, "..."):
@@ -163,9 +176,7 @@ func ImportPaths(patterns []string) []*search.Match {
 		updateMatches(true)
 		for _, m := range matches {
 			for _, pkg := range m.Pkgs {
-				if pkg != "" {
-					roots = append(roots, pkg)
-				}
+				roots = append(roots, pkg)
 			}
 		}
 		return roots
@@ -394,13 +405,17 @@ func ModuleUsedDirectly(path string) bool {
 }
 
 // Lookup returns the source directory, import path, and any loading error for
-// the package at path.
+// the package at path as imported from the package in parentDir.
 // Lookup requires that one of the Load functions in this package has already
 // been called.
-func Lookup(path string) (dir, realPath string, err error) {
+func Lookup(parentPath string, parentIsStd bool, path string) (dir, realPath string, err error) {
 	if path == "" {
 		panic("Lookup called with empty package path")
 	}
+
+	if parentIsStd {
+		path = loaded.stdVendor(parentPath, path)
+	}
 	pkg, ok := loaded.pkgCache.Get(path).(*loadPkg)
 	if !ok {
 		// The loader should have found all the relevant paths.
@@ -434,10 +449,11 @@ func Lookup(path string) (dir, realPath string, err error) {
 // TODO(rsc): It might be nice to make the loader take and return
 // a buildList rather than hard-coding use of the global.
 type loader struct {
-	tags      map[string]bool // tags for scanDir
-	testRoots bool            // include tests for roots
-	isALL     bool            // created with LoadALL
-	testAll   bool            // include tests for all packages
+	tags           map[string]bool // tags for scanDir
+	testRoots      bool            // include tests for roots
+	isALL          bool            // created with LoadALL
+	testAll        bool            // include tests for all packages
+	forceStdVendor bool            // if true, load standard-library dependencies from the vendor subtree
 
 	// reset on each iteration
 	roots    []*loadPkg
@@ -457,6 +473,17 @@ func newLoader() *loader {
 	ld := new(loader)
 	ld.tags = imports.Tags()
 	ld.testRoots = LoadTests
+
+	switch Target.Path {
+	case "std", "cmd":
+		// Inside the "std" and "cmd" modules, we prefer to use the vendor directory
+		// unless the command explicitly changes the module graph.
+		// TODO(golang.org/issue/30240): Remove this special case.
+		if cfg.CmdName != "get" && !strings.HasPrefix(cfg.CmdName, "mod ") {
+			ld.forceStdVendor = true
+		}
+	}
+
 	return ld
 }
 
@@ -631,7 +658,11 @@ func (ld *loader) doPkg(item interface{}) {
 		}
 	}
 
+	inStd := (search.IsStandardImportPath(pkg.path) && search.InDir(pkg.dir, cfg.GOROOTsrc) != "")
 	for _, path := range imports {
+		if inStd {
+			path = ld.stdVendor(pkg.path, path)
+		}
 		pkg.imports = append(pkg.imports, ld.pkg(path, false))
 	}
 
@@ -642,6 +673,30 @@ func (ld *loader) doPkg(item interface{}) {
 	}
 }
 
+// stdVendor returns the canonical import path for the package with the given
+// path when imported from the standard-library package at parentPath.
+func (ld *loader) stdVendor(parentPath, path string) string {
+	if search.IsStandardImportPath(path) {
+		return path
+	}
+
+	if str.HasPathPrefix(parentPath, "cmd") && (Target.Path != "cmd" || ld.forceStdVendor) {
+		vendorPath := pathpkg.Join("cmd", "vendor", path)
+		if _, err := os.Stat(filepath.Join(cfg.GOROOTsrc, filepath.FromSlash(vendorPath))); err == nil {
+			return vendorPath
+		}
+	}
+	if Target.Path != "std" || ld.forceStdVendor {
+		vendorPath := pathpkg.Join("vendor", path)
+		if _, err := os.Stat(filepath.Join(cfg.GOROOTsrc, filepath.FromSlash(vendorPath))); err == nil {
+			return vendorPath
+		}
+	}
+
+	// Not vendored: resolve from modules.
+	return path
+}
+
 // computePatternAll returns the list of packages matching pattern "all",
 // starting with a list of the import paths for the packages in the main module.
 func (ld *loader) computePatternAll(paths []string) []string {
@@ -932,6 +987,20 @@ func (r *mvsReqs) required(mod module.Version) ([]module.Version, error) {
 		return vendorList, nil
 	}
 
+	switch Target.Path {
+	case "std", "cmd":
+		// When inside "std" or "cmd", only fetch and read go.mod files if we're
+		// explicitly running a command that can change the module graph. If we have
+		// to resolve a new dependency, we might pick the wrong version, but 'go mod
+		// tidy' will fix it — and new standard-library dependencies should be rare
+		// anyway.
+		//
+		// TODO(golang.org/issue/30240): Drop this special-case.
+		if cfg.CmdName != "get" && !strings.HasPrefix(cfg.CmdName, "mod ") {
+			return nil, nil
+		}
+	}
+
 	origPath := mod.Path
 	if repl := Replacement(mod); repl.Path != "" {
 		if repl.Version == "" {

src/cmd/go/internal/modload/query.go

@@ -9,6 +9,7 @@ import (
 	"cmd/go/internal/modfetch/codehost"
 	"cmd/go/internal/module"
 	"cmd/go/internal/semver"
+	"cmd/go/internal/str"
 	"fmt"
 	pathpkg "path"
 	"strings"
@@ -131,6 +132,10 @@ func Query(path, query string, allowed func(module.Version) bool) (*modfetch.Rev
 		return &modfetch.RevInfo{Version: Target.Version}, nil
 	}
 
+	if str.HasPathPrefix(path, "std") || str.HasPathPrefix(path, "cmd") {
+		return nil, fmt.Errorf("explicit requirement on standard-library module %s not allowed", path)
+	}
+
 	// Load versions and execute query.
 	repo, err := modfetch.Lookup(path)
 	if err != nil {

src/cmd/go/internal/modload/search.go

@@ -35,12 +35,8 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 	}
 	var pkgs []string
 
-	walkPkgs := func(root, importPathRoot string) {
+	walkPkgs := func(root, importPathRoot string, includeVendor bool) {
 		root = filepath.Clean(root)
-		var cmd string
-		if root == cfg.GOROOTsrc {
-			cmd = filepath.Join(root, "cmd")
-		}
 		filepath.Walk(root, func(path string, fi os.FileInfo, err error) error {
 			if err != nil {
 				return nil
@@ -51,14 +47,6 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 				return nil
 			}
 
-			// GOROOT/src/cmd makes use of GOROOT/src/cmd/vendor,
-			// which module mode can't deal with. Eventually we'll stop using
-			// that vendor directory, and then we can remove this exclusion.
-			// golang.org/issue/26924.
-			if path == cmd {
-				return filepath.SkipDir
-			}
-
 			want := true
 			// Avoid .foo, _foo, and testdata directory trees.
 			_, elem := filepath.Split(path)
@@ -86,6 +74,7 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 			if !want {
 				return filepath.SkipDir
 			}
+			// Stop at module boundaries.
 			if path != root {
 				if _, err := os.Stat(filepath.Join(path, "go.mod")); err == nil {
 					return filepath.SkipDir
@@ -101,7 +90,7 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 				}
 			}
 
-			if elem == "vendor" {
+			if elem == "vendor" && !includeVendor {
 				return filepath.SkipDir
 			}
 			return nil
@@ -109,11 +98,14 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 	}
 
 	if useStd {
-		walkPkgs(cfg.GOROOTsrc, "")
+		walkPkgs(cfg.GOROOTsrc, "", true)
+		if treeCanMatch("cmd") {
+			walkPkgs(filepath.Join(cfg.GOROOTsrc, "cmd"), "cmd", true)
+		}
 	}
 
 	if cfg.BuildMod == "vendor" {
-		walkPkgs(filepath.Join(ModRoot(), "vendor"), "")
+		walkPkgs(filepath.Join(ModRoot(), "vendor"), "", false)
 		return pkgs
 	}
 
@@ -135,7 +127,11 @@ func matchPackages(pattern string, tags map[string]bool, useStd bool, modules []
 				continue
 			}
 		}
-		walkPkgs(root, mod.Path)
+		modPrefix := mod.Path
+		if mod.Path == "std" {
+			modPrefix = ""
+		}
+		walkPkgs(root, modPrefix, false)
 	}
 
 	return pkgs

src/cmd/go/testdata/script/gopath_std_vendor.txt

@@ -2,6 +2,9 @@ env GO111MODULE=off
 
 [!gc] skip
 
+go list -f '{{.Dir}}' vendor/golang.org/x/net/http2/hpack
+stdout $GOPATH[/\\]src[/\\]vendor
+
 # A package importing 'net/http' should resolve its dependencies
 # to the package 'vendor/golang.org/x/net/http2/hpack' within GOROOT.
 cd importnethttp

src/cmd/go/testdata/script/mod_list_std.txt

+env GO111MODULE=on
+env GOPROXY=off
+
+[!gc] skip
+
+# Outside of GOROOT, our vendored packages should be reported as part of the standard library.
+go list -f '{{if .Standard}}{{.ImportPath}}{{end}}' std cmd
+stdout ^internal/x/net/http2/hpack
+stdout ^cmd/vendor/golang\.org/x/arch/x86/x86asm
+
+# cmd/... should match the same packages it used to match in GOPATH mode.
+go list cmd/...
+stdout ^cmd/compile
+! stdout ^cmd/vendor/golang\.org/x/arch/x86/x86asm
+
+
+# Within the std module, listing ./... should omit the 'std' prefix:
+# the package paths should be the same via ./... or the 'std' meta-pattern.
+# TODO(golang.org/issue/30241): Make that work.
+# Today, they are listed in 'std' but not './...'.
+cd $GOROOT/src
+go list ./...
+stdout ^internal/x
+
+cp stdout $WORK/listdot.txt
+go list std
+stdout ^internal/x            # TODO
+# TODO: cmp stdout $WORK/listdot.txt
+
+go list all
+! stdout ^internal/x            # TODO: this will exist when src/go.mod is added
+! stdout ^std/
+
+
+# Within the std module, the vendored dependencies of std should appear
+# to come from the actual modules.
+# TODO(golang.org/issue/30241): Make that work.
+# Today, they still have the vendor/ prefix.
+go list std
+stdout ^internal/x/net/http2/hpack  # TODO
+! stdout ^golang.org/x/net/http2/hpack       # TODO
+
+go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' std
+# ! stdout ^internal/x/net/http2/hpack  # TODO
+! stdout ^golang.org/x/net/http2/hpack         # TODO
+
+
+# Within std, the vendored dependencies of cmd should still appear to be part of cmd.
+go list -f '{{if .Standard}}{{.ImportPath}}{{end}}' cmd
+stdout ^cmd/vendor/golang\.org/x/arch/x86/x86asm
+
+go list -f '{{if not .Standard}}{{.ImportPath}}{{end}}' cmd
+! stdout .
+
+go list cmd/...
+stdout ^cmd/compile
+! stdout ^cmd/vendor/golang\.org/x/arch/x86/x86asm

src/cmd/go/testdata/script/mod_patterns.txt

@@ -7,7 +7,6 @@ cd m
 # library or active modules.
 #
 # 'go list ...' should list packages in all active modules and the standard library.
-# But not cmd/* - see golang.org/issue/26924.
 #
 # 'go list example.com/m/...' should list packages in all modules that begin with 'example.com/m/'.
 #
@@ -26,7 +25,7 @@ stdout 'example.com/m/nested/useencoding: \[\.\.\. example.com/m/...\]' # but NO
 stdout '^unicode: \[all \.\.\.\]'
 stdout '^unsafe: \[all \.\.\.\]'
 stdout 'index/suffixarray: \[\.\.\.\]'
-! stdout cmd/pprof # golang.org/issue/26924
+stdout 'cmd/pprof: \[\.\.\.\]'
 
 stderr -count=1 '^go: warning: "./xyz..." matched no packages$'
 

src/cmd/go/testdata/script/mod_std_vendor.txt

 env GO111MODULE=on
+env GOPROXY=off
 
 go list -f '{{.TestImports}}'
 stdout net/http # from .TestImports
 
-go list -test -f '{{.Deps}}'
+# 'go list' should find standard-vendored packages.
+go list -f '{{.Dir}}' internal/x/net/http2/hpack
+stdout $GOROOT[/\\]src[/\\]internal
+
+# 'go list -test' should report vendored transitive dependencies of _test.go
+# imports in the Deps field.
+go list -test -f '{{range .Deps}}{{.}}{{"\n"}}{{end}}'
 stdout internal/x/crypto # dep of .TestImports
 
+
+# Modules outside the standard library should not use the packages vendored there...
+cd broken
+! go build -mod=readonly
+stderr 'updates to go.mod needed'
+! go build -mod=vendor
+stderr 'cannot find package'
+stderr 'hpack'
+
+# ...even if they explicitly use the "cmd/vendor/" or "vendor/" prefix.
+cd ../importcmd
+! go build .
+stderr 'use of vendored package'
+
+cd ../importstd
+! go build .
+stderr 'use of internal package'
+
+
+# When run within the 'std' module, 'go list -test' should report vendored
+# transitive dependencies at their original module paths.
+# TODO(golang.org/issue/30241): Make that work.
+# Today, they're standard packages as long as they exist.
+cd $GOROOT/src
+go list -test -f '{{range .Deps}}{{.}}{{"\n"}}{{end}}' net/http
+! stdout ^vendor/golang.org/x/net/http2/hpack  # TODO: this will exist later
+stdout ^internal/x/net/http2/hpack
+
 -- go.mod --
 module m
 
@@ -17,3 +52,26 @@ package x
 import "testing"
 import _ "net/http"
 func Test(t *testing.T) {}
+
+-- broken/go.mod --
+module broken
+-- broken/http.go --
+package broken
+
+import (
+	_ "net/http"
+	_ "golang.org/x/net/http2/hpack"
+)
+
+-- importcmd/go.mod --
+module importcmd
+-- importcmd/x.go --
+package importcmd
+
+import _ "cmd/vendor/golang.org/x/tools/go/analysis"
+-- importstd/go.mod --
+module importvendor
+-- importstd/x.go --
+package importstd
+
+import _ "internal/x/net/http2/hpack"

src/cmd/go/testdata/script/std_vendor.txt

@@ -6,11 +6,20 @@ env GO111MODULE=off
 go list -f '{{.TestImports}}'
 stdout net/http # from .TestImports
 
+# 'go list' should report standard-vendored packages by path.
+go list -f '{{.Dir}}' internal/x/net/http2/hpack
+stdout $GOROOT[/\\]src[/\\]internal
+
 # 'go list -test' should report vendored transitive dependencies of _test.go
 # imports in the Deps field, with a 'vendor' prefix on their import paths.
 go list -test -f '{{.Deps}}'
 stdout internal/x/crypto # dep of .TestImports
 
+# Packages outside the standard library should not use its copy of vendored packages.
+cd broken
+! go build
+stderr 'cannot find package'
+
 -- go.mod --
 module m
 
@@ -22,3 +31,13 @@ package x
 import "testing"
 import _ "net/http"
 func Test(t *testing.T) {}
+
+-- broken/go.mod --
+module broken
+-- broken/http.go --
+package broken
+
+import (
+	_ "net/http"
+	_ "golang.org/x/net/http/httpproxy"
+)