cmd/go: set Root and target fields for packages in GOPATH

This change replicates the behavior filed in issue #37015 for packages
imported from the module index. That behavior is that packages that
happen to exist in a GOPATH src directory have p.Root and p.Target set
even when the packages are loaded from modules. This is likely
unintentional behavior because in module mode, packages shouldn't behave
differently depending on whether their directories exist in GOPATH. But
for uniformity, (and because two of our tests depend on this behavior),
this CL will implement this behavior. We can remove it from the module
index when we remove it from the go/build logic.

Change-Id: I3f501c92fbb76eaf86b6b9275539f2129b67f884
Reviewed-on: https://go-review.googlesource.com/c/go/+/410822
Reviewed-by: Michael Matloob <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>

Author: matloob

src/cmd/go/internal/modindex/build.go

@@ -177,6 +177,37 @@ func hasSubdir(root, dir string) (rel string, ok bool) {
 	return filepath.ToSlash(dir[len(root):]), true
 }
 
+// gopath returns the list of Go path directories.
+func (ctxt *Context) gopath() []string {
+	var all []string
+	for _, p := range ctxt.splitPathList(ctxt.GOPATH) {
+		if p == "" || p == ctxt.GOROOT {
+			// Empty paths are uninteresting.
+			// If the path is the GOROOT, ignore it.
+			// People sometimes set GOPATH=$GOROOT.
+			// Do not get confused by this common mistake.
+			continue
+		}
+		if strings.HasPrefix(p, "~") {
+			// Path segments starting with ~ on Unix are almost always
+			// users who have incorrectly quoted ~ while setting GOPATH,
+			// preventing it from expanding to $HOME.
+			// The situation is made more confusing by the fact that
+			// bash allows quoted ~ in $PATH (most shells do not).
+			// Do not get confused by this, and do not try to use the path.
+			// It does not exist, and printing errors about it confuses
+			// those users even more, because they think "sure ~ exists!".
+			// The go command diagnoses this situation and prints a
+			// useful error.
+			// On Windows, ~ is used in short names, such as c:\progra~1
+			// for c:\program files.
+			continue
+		}
+		all = append(all, p)
+	}
+	return all
+}
+
 var defaultToolTags, defaultReleaseTags []string
 
 // A Package describes the Go package found in a directory.

src/cmd/go/internal/modindex/read.go

@@ -242,31 +242,47 @@ func (mi *ModuleIndex) Import(bctxt build.Context, relpath string, mode build.Im
 		return p, fmt.Errorf("import %q: import of unknown directory", p.Dir)
 	}
 
-	// goroot
+	// goroot and gopath
 	inTestdata := func(sub string) bool {
 		return strings.Contains(sub, "/testdata/") || strings.HasSuffix(sub, "/testdata") || str.HasPathPrefix(sub, "testdata")
 	}
-	if ctxt.GOROOT != "" && str.HasFilePathPrefix(mi.modroot, cfg.GOROOTsrc) && !inTestdata(relpath) {
-		modprefix := str.TrimFilePathPrefix(mi.modroot, cfg.GOROOTsrc)
-		p.Goroot = true
-		p.ImportPath = relpath
-		if modprefix != "" {
-			p.ImportPath = filepath.Join(modprefix, p.ImportPath)
-		}
+	if !inTestdata(relpath) {
 		// In build.go, p.Root should only be set in the non-local-import case, or in
 		// GOROOT or GOPATH. Since module mode only calls Import with path set to "."
 		// and the module index doesn't apply outside modules, the GOROOT case is
 		// the only case where GOROOT needs to be set.
-		// TODO(#37015): p.Root actually might be set in the local-import case outside
-		// GOROOT, if the directory is contained in GOPATH/src, even in module
-		// mode, but that's a bug.
-		p.Root = ctxt.GOROOT
+		// But: p.Root is actually set in the local-import case outside GOROOT, if
+		// the directory is contained in GOPATH/src
+		// TODO(#37015): fix that behavior in go/build and remove the gopath case
+		// below.
+		if ctxt.GOROOT != "" && str.HasFilePathPrefix(p.Dir, cfg.GOROOTsrc) && p.Dir != cfg.GOROOTsrc {
+			p.Root = ctxt.GOROOT
+			p.Goroot = true
+			modprefix := str.TrimFilePathPrefix(mi.modroot, cfg.GOROOTsrc)
+			p.ImportPath = relpath
+			if modprefix != "" {
+				p.ImportPath = filepath.Join(modprefix, p.ImportPath)
+			}
+		}
+		for _, root := range ctxt.gopath() {
+			// TODO(matloob): do we need to reimplement the conflictdir logic?
 
-		// Set GOROOT-specific fields
+			// TODO(matloob): ctxt.hasSubdir evaluates symlinks, so it
+			// can be slower than we'd like. Find out if we can drop this
+			// logic before the release.
+			if sub, ok := ctxt.hasSubdir(filepath.Join(root, "src"), p.Dir); ok {
+				p.ImportPath = sub
+				p.Root = root
+			}
+		}
+	}
+	if p.Root != "" {
+		// Set GOROOT-specific fields (sometimes for modules in a GOPATH directory).
 		// The fields set below (SrcRoot, PkgRoot, BinDir, PkgTargetRoot, and PkgObj)
 		// are only set in build.Import if p.Root != "". As noted in the comment
 		// on setting p.Root above, p.Root should only be set in the GOROOT case for the
-		// set of packages we care about.
+		// set of packages we care about, but is also set for modules in a GOPATH src
+		// directory.
 		var pkgtargetroot string
 		var pkga string
 		suffix := ""