cmd/go: refactor load.LoadPackage into other functions

LoadPackage was used to load a *load.Package for a command line
argument, after pattern expansion. It provided two special cases on
top of LoadImport. First, it ensured that "cmd/" packages in GOROOT
were installed in "$GOROOT/bin" or "$GOROOT/pkg/tool". Second, it
translated absolute paths to packages in GOROOT and GOPATH into
regular import paths.

With this change, LoadImport now ensures "cmd/" packages have the
right Target (without the need for a special case) and
search.ImportPaths translates absolute paths.

LoadPackage no longer handles these special cases and has been renamed
to LoadImportWithFlags, since it's still useful for loading implicit
dependencies.

Updates #29758

Change-Id: I9d54036f90c3ccd9b3a0fe0eaddaa7749593cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/167748
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>

src/cmd/go/internal/get/get.go

@@ -177,12 +177,6 @@ func runGet(cmd *base.Command, args []string) {
 	// everything.
 	load.ClearPackageCache()
 
-	// In order to rebuild packages information completely,
-	// we need to clear commands cache. Command packages are
-	// referring to evicted packages from the package cache.
-	// This leads to duplicated loads of the standard packages.
-	load.ClearCmdCache()
-
 	pkgs := load.PackagesForBuild(args)
 
 	// Phase 3. Install.
@@ -240,7 +234,8 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int)
 	}
 	load1 := func(path string, mode int) *load.Package {
 		if parent == nil {
-			return load.LoadPackageNoFlags(path, stk)
+			mode := 0 // don't do module or vendor resolution
+			return load.LoadImport(path, base.Cwd, nil, stk, nil, mode)
 		}
 		return load.LoadImport(path, parent.Dir, parent, stk, nil, mode|load.ResolveModule)
 	}

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

@@ -377,7 +377,7 @@ func ClearPackageCachePartial(args []string) {
 	}
 }
 
-// ReloadPackageNoFlags is like LoadPackageNoFlags but makes sure
+// ReloadPackageNoFlags is like LoadImport but makes sure
 // not to use the package cache.
 // It is only for use by GOPATH-based "go get".
 // TODO(rsc): When GOPATH-based "go get" is removed, delete this function.
@@ -387,7 +387,7 @@ func ReloadPackageNoFlags(arg string, stk *ImportStack) *Package {
 		delete(packageCache, p.Dir)
 		delete(packageCache, p.ImportPath)
 	}
-	return LoadPackageNoFlags(arg, stk)
+	return LoadImport(arg, base.Cwd, nil, stk, nil, 0)
 }
 
 // dirToImportPath returns the pseudo-import path we use for a package
@@ -552,7 +552,7 @@ func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPo
 		bp.ImportPath = importPath
 		if cfg.GOBIN != "" {
 			bp.BinDir = cfg.GOBIN
-		} else if cfg.ModulesEnabled {
+		} else if cfg.ModulesEnabled && !bp.Goroot {
 			bp.BinDir = ModBinDir()
 		}
 		if modDir == "" && err == nil && !isLocal && bp.ImportComment != "" && bp.ImportComment != path &&
@@ -1716,99 +1716,17 @@ func TestPackageList(roots []*Package) []*Package {
 	return all
 }
 
-var cmdCache = map[string]*Package{}
-
-func ClearCmdCache() {
-	for name := range cmdCache {
-		delete(cmdCache, name)
-	}
-}
-
-// LoadPackage loads the package named by arg.
-func LoadPackage(arg string, stk *ImportStack) *Package {
-	p := loadPackage(arg, stk)
+// LoadImportWithFlags loads the package with the given import path and
+// sets tool flags on that package. This function is useful loading implicit
+// dependencies (like sync/atomic for coverage).
+// TODO(jayconrod): delete this function and set flags automatically
+// in LoadImport instead.
+func LoadImportWithFlags(path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
+	p := LoadImport(path, srcDir, parent, stk, importPos, mode)
 	setToolFlags(p)
 	return p
 }
 
-// LoadPackageNoFlags is like LoadPackage
-// but does not guarantee that the build tool flags are set in the result.
-// It is only for use by GOPATH-based "go get"
-// and is only appropriate for preliminary loading of packages.
-// A real load using LoadPackage or (more likely)
-// Packages, PackageAndErrors, or PackagesForBuild
-// must be done before passing the package to any build
-// steps, so that the tool flags can be set properly.
-// TODO(rsc): When GOPATH-based "go get" is removed, delete this function.
-func LoadPackageNoFlags(arg string, stk *ImportStack) *Package {
-	return loadPackage(arg, stk)
-}
-
-// loadPackage is like loadImport but is used for command-line arguments,
-// not for paths found in import statements. In addition to ordinary import paths,
-// loadPackage accepts pseudo-paths beginning with cmd/ to denote commands
-// in the Go command directory, as well as paths to those directories.
-func loadPackage(arg string, stk *ImportStack) *Package {
-	if arg == "" {
-		panic("loadPackage called with empty package path")
-	}
-	if build.IsLocalImport(arg) {
-		dir := arg
-		if !filepath.IsAbs(dir) {
-			if abs, err := filepath.Abs(dir); err == nil {
-				// interpret relative to current directory
-				dir = abs
-			}
-		}
-		if sub, ok := hasSubdir(cfg.GOROOTsrc, dir); ok && strings.HasPrefix(sub, "cmd/") && !strings.Contains(sub[4:], "/") {
-			arg = sub
-		}
-	}
-	if strings.HasPrefix(arg, "cmd/") && !strings.Contains(arg[4:], "/") {
-		if p := cmdCache[arg]; p != nil {
-			return p
-		}
-		stk.Push(arg)
-		defer stk.Pop()
-
-		bp, err := cfg.BuildContext.ImportDir(filepath.Join(cfg.GOROOTsrc, arg), 0)
-		bp.ImportPath = arg
-		bp.Goroot = true
-		bp.BinDir = cfg.GOROOTbin
-		bp.Root = cfg.GOROOT
-		bp.SrcRoot = cfg.GOROOTsrc
-		p := new(Package)
-		cmdCache[arg] = p
-		p.load(stk, bp, err)
-		if p.Error == nil && p.Name != "main" {
-			p.Error = &PackageError{
-				ImportStack: stk.Copy(),
-				Err:         fmt.Sprintf("expected package main but found package %s in %s", p.Name, p.Dir),
-			}
-		}
-		return p
-	}
-
-	// Wasn't a command; must be a package.
-	// If it is a local import path but names a standard package,
-	// we treat it as if the user specified the standard package.
-	// This lets you run go test ./ioutil in package io and be
-	// referring to io/ioutil rather than a hypothetical import of
-	// "./ioutil".
-	if build.IsLocalImport(arg) || filepath.IsAbs(arg) {
-		dir := arg
-		if !filepath.IsAbs(arg) {
-			dir = filepath.Join(base.Cwd, arg)
-		}
-		bp, _ := cfg.BuildContext.ImportDir(dir, build.FindOnly)
-		if bp.ImportPath != "" && bp.ImportPath != "." {
-			arg = bp.ImportPath
-		}
-	}
-
-	return LoadImport(arg, base.Cwd, nil, stk, nil, 0)
-}
-
 // Packages returns the packages named by the
 // command line arguments 'args'. If a named package
 // cannot be loaded at all (for example, if the directory does not exist),
@@ -1850,7 +1768,7 @@ func PackagesAndErrors(patterns []string) []*Package {
 			if pkg == "" {
 				panic(fmt.Sprintf("ImportPaths returned empty package for pattern %s", m.Pattern))
 			}
-			p := loadPackage(pkg, &stk)
+			p := LoadImport(pkg, base.Cwd, nil, &stk, nil, 0)
 			p.Match = append(p.Match, m.Pattern)
 			p.Internal.CmdlinePkg = true
 			if m.Literal {

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

@@ -327,14 +327,35 @@ func ImportPathsQuiet(patterns []string) []*Match {
 			out = append(out, MatchPackages(a))
 			continue
 		}
-		if strings.Contains(a, "...") {
-			if build.IsLocalImport(a) {
-				out = append(out, MatchPackagesInFS(a))
+
+		if build.IsLocalImport(a) || filepath.IsAbs(a) {
+			var m *Match
+			if strings.Contains(a, "...") {
+				m = MatchPackagesInFS(a)
 			} else {
-				out = append(out, MatchPackages(a))
+				m = &Match{Pattern: a, Literal: true, Pkgs: []string{a}}
+			}
+
+			// Change the file import path to a regular import path if the package
+			// is in GOPATH or GOROOT. We don't report errors here; LoadImport
+			// (or something similar) will report them later.
+			for i, dir := range m.Pkgs {
+				if !filepath.IsAbs(dir) {
+					dir = filepath.Join(base.Cwd, dir)
+				}
+				if bp, _ := cfg.BuildContext.ImportDir(dir, build.FindOnly); bp.ImportPath != "" && bp.ImportPath != "." {
+					m.Pkgs[i] = bp.ImportPath
+				}
 			}
+			out = append(out, m)
 			continue
 		}
+
+		if strings.Contains(a, "...") {
+			out = append(out, MatchPackages(a))
+			continue
+		}
+
 		out = append(out, &Match{Pattern: a, Literal: true, Pkgs: []string{a}})
 	}
 	return out

src/cmd/go/internal/test/test.go

@@ -760,7 +760,7 @@ func ensureImport(p *load.Package, pkg string) {
 		}
 	}
 
-	p1 := load.LoadPackage(pkg, &load.ImportStack{})
+	p1 := load.LoadImportWithFlags(pkg, p.Dir, p, &load.ImportStack{}, nil, 0)
 	if p1.Error != nil {
 		base.Fatalf("load %s: %v", pkg, p1.Error)
 	}

src/cmd/go/internal/work/action.go

@@ -289,7 +289,7 @@ func readpkglist(shlibpath string) (pkgs []*load.Package) {
 			if strings.HasPrefix(t, "pkgpath ") {
 				t = strings.TrimPrefix(t, "pkgpath ")
 				t = strings.TrimSuffix(t, ";")
-				pkgs = append(pkgs, load.LoadPackage(t, &stk))
+				pkgs = append(pkgs, load.LoadImportWithFlags(t, base.Cwd, nil, &stk, nil, 0))
 			}
 		}
 	} else {
@@ -300,7 +300,7 @@ func readpkglist(shlibpath string) (pkgs []*load.Package) {
 		scanner := bufio.NewScanner(bytes.NewBuffer(pkglistbytes))
 		for scanner.Scan() {
 			t := scanner.Text()
-			pkgs = append(pkgs, load.LoadPackage(t, &stk))
+			pkgs = append(pkgs, load.LoadImportWithFlags(t, base.Cwd, nil, &stk, nil, 0))
 		}
 	}
 	return
@@ -405,7 +405,7 @@ func (b *Builder) vetAction(mode, depMode BuildMode, p *load.Package) *Action {
 		// vet expects to be able to import "fmt".
 		var stk load.ImportStack
 		stk.Push("vet")
-		p1 := load.LoadPackage("fmt", &stk)
+		p1 := load.LoadImportWithFlags("fmt", p.Dir, p, &stk, nil, 0)
 		stk.Pop()
 		aFmt := b.CompileAction(ModeBuild, depMode, p1)
 
@@ -705,7 +705,7 @@ func (b *Builder) linkSharedAction(mode, depMode BuildMode, shlib string, a1 *Ac
 					}
 				}
 				var stk load.ImportStack
-				p := load.LoadPackage(pkg, &stk)
+				p := load.LoadImportWithFlags(pkg, base.Cwd, nil, &stk, nil, 0)
 				if p.Error != nil {
 					base.Fatalf("load %s: %v", pkg, p.Error)
 				}

src/cmd/go/internal/work/gc.go

@@ -304,7 +304,7 @@ func (gcToolchain) symabis(b *Builder, a *Action, sfiles []string) (string, erro
 		otherPkgs = []string{"sync/atomic"}
 	}
 	for _, p2name := range otherPkgs {
-		p2 := load.LoadPackage(p2name, &load.ImportStack{})
+		p2 := load.LoadImportWithFlags(p2name, p.Dir, p, &load.ImportStack{}, nil, 0)
 		if len(p2.SFiles) == 0 {
 			continue
 		}