go/build: populate partial package information in importGo

This is a followup to CL 199840 and CL 203820. Cumulatively, they caused
a previously known bug to trigger more often while also nearly fixing it.

This change is a small fixup to CL 199840 that resolves the known bug
and prevents it from causing an additional regression in Go 1.14.

Part 1

The intention in CL 199840 was to return the same error that 'go list'
reported when the package wasn't located, so an early return was added.
However, to determine whether the package was located or not, p.Dir was
unintentionally checked instead of dir.

p is initialized to &Package{ImportPath: path} at top of Context.Import,
and its Dir field is never set before that line in importGo is reached.
So return errors.New(errStr) was always executed whenever errStr != "".

Originally, in CL 125296, the "go list" invocation did not include an
'-e' flag, so it would return a non-zero exit code on packages where
build constraints exclude all Go files, and importGo would return an
error like "go/build: importGo import/path: unexpected output: ...".

CL 199840 added an '-e' flag to the "go list" invocation, but checking
the wrong dir variable caused partial package information to never get
populated, and thus issue #31603 continued to occur, although with a
different error message (which ironically included the location of the
package that was supposedly "not found").

Now that the right dir is checked, issue #31603 is fixed.

Part 2

importGo checks whether it can use the go command to find the directory
of a package. In Go 1.13.x and earlier, one of the conditions to use the
go command was that the source directory must be provided.

CL 203820 made a change such that knowing the source directory was
no longer required:

	 // To invoke the go command,
	-// we must know the source directory,
	 // ...

That meant build.Import invocations where srcDir is the empty string:

	build.Import(path, "", build.FindOnly)

Started using the go command to find the directory of the package, and
started to run into issue #31603 as well. That's the #37153 regression.

Since this change fixes issue #31603, it also fixes issue #37153.

Part 3

There is one more thing. Delete the debugImportGo constant, it's unused.

Updates #26504 (CL 125296)
Updates #34752 (CL 199840)
Updates #34860 (CL 203820)
Fixes #31603
Fixes #37153

Change-Id: Iaa7dcc45ba0f708a978950c75fa4c836b87006f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/218817
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

diff --git a/src/cmd/go/testdata/script/mod_gobuild_import.txt b/src/cmd/go/testdata/script/mod_gobuild_import.txt
index ae05250c5f695bfe80e914d0994cf319ad6dacc4..948496241e916911e752a12fe6a0b9f491d8dd6c 100644 (file)
--- a/src/cmd/go/testdata/script/mod_gobuild_import.txt
+++ b/src/cmd/go/testdata/script/mod_gobuild_import.txt
@@ -2,49 +2,67 @@
 
 # go/build's Import should find modules by invoking the go command
 
-go build -o $WORK/testimport.exe ./testimport
+go build -o $WORK ./testimport ./testfindonly
 
 # GO111MODULE=off
 env GO111MODULE=off
-! exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
+! exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
 
 # GO111MODULE=auto in GOPATH/src
 env GO111MODULE=auto
-exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
+exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
 
 # GO111MODULE=auto outside GOPATH/src
 cd $GOPATH/other
 env GO111MODULE=auto
-exec $WORK/testimport.exe other/x/y/z/w .
+exec $WORK/testimport$GOEXE other/x/y/z/w .
 stdout w2.go
 
-! exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
+! exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
 stderr 'cannot find module providing package gobuild.example.com/x/y/z/w'
 
 cd z
-exec $WORK/testimport.exe other/x/y/z/w .
+exec $WORK/testimport$GOEXE other/x/y/z/w .
 stdout w2.go
 
 # GO111MODULE=on outside GOPATH/src
 env GO111MODULE=
-exec $WORK/testimport.exe other/x/y/z/w .
+exec $WORK/testimport$GOEXE other/x/y/z/w .
 stdout w2.go
 env GO111MODULE=on
-exec $WORK/testimport.exe other/x/y/z/w .
+exec $WORK/testimport$GOEXE other/x/y/z/w .
 stdout w2.go
 
 # GO111MODULE=on in GOPATH/src
 cd $GOPATH/src
 env GO111MODULE=
-exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
+exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
 stdout w1.go
 env GO111MODULE=on
-exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
+exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
 stdout w1.go
 cd w
-exec $WORK/testimport.exe gobuild.example.com/x/y/z/w ..
+exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w ..
 stdout w1.go
 
+# go/build's Import in FindOnly mode should find directories by invoking the go command
+#
+# Calling build.Import in build.FindOnly mode on an import path of a Go package
+# that produces errors when loading (e.g., due to build constraints not matching
+# the current build context) should return the package directory and nil error.
+
+# Issue 31603: Import with non-empty srcDir should work.
+env GO111MODULE=on
+exec $WORK/testfindonly$GOEXE gobuild.example.com/x/y/z/i $WORK
+! stdout 'build constraints'
+stdout '^dir=\$WORK.+i err=<nil>$'
+
+# Issue 37153: Import with empty srcDir should work.
+env GO111MODULE=on
+exec $WORK/testfindonly$GOEXE gobuild.example.com/x/y/z/i ''
+! stdout 'build constraints'
+stdout '^dir=\$WORK.+i err=<nil>$'
+
 -- go.mod --
 module gobuild.example.com/x/y/z
 
@@ -54,6 +72,11 @@ package z
 -- w/w1.go --
 package w
 
+-- i/i.go --
+// +build i
+
+package i
+
 -- testimport/x.go --
 package main
 
@@ -89,6 +112,20 @@ func main() {
        fmt.Printf("%s\n%s\n", p1.Dir, strings.Join(p1.GoFiles, " "))
 }
 
+-- testfindonly/x.go --
+package main
+
+import (
+       "fmt"
+       "go/build"
+       "os"
+)
+
+func main() {
+       p, err := build.Import(os.Args[1], os.Args[2], build.FindOnly)
+       fmt.Printf("dir=%s err=%v\n", p.Dir, err)
+}
+
 -- $GOPATH/other/go.mod --
 module other/x/y
 

diff --git a/src/go/build/build.go b/src/go/build/build.go
index e89aa7708dc0dea070eeca26c152a73b2011e54e..1a122c615fe843a606ec4a88fff154ec98fd010b 100644 (file)
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -1015,8 +1015,6 @@ var errNoModules = errors.New("not using modules")
 // Then we reinvoke it for every dependency. But this is still better than not working at all.
 // See golang.org/issue/26504.
 func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode) error {
-       const debugImportGo = false
-
        // To invoke the go command,
        // we must not being doing special things like AllowBinary or IgnoreVendor,
        // and all the file system callbacks must be nil (we're meant to use the local file system).
@@ -1135,15 +1133,15 @@ func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode)
        }
        dir := f[0]
        errStr := strings.TrimSpace(f[4])
-       if errStr != "" && p.Dir == "" {
-               // If 'go list' could not locate the package, return the same error that
-               // 'go list' reported.
-               // If 'go list' did locate the package (p.Dir is not empty), ignore the
-               // error. It was probably related to loading source files, and we'll
-               // encounter it ourselves shortly.
+       if errStr != "" && dir == "" {
+               // If 'go list' could not locate the package (dir is empty),
+               // return the same error that 'go list' reported.
                return errors.New(errStr)
        }
 
+       // If 'go list' did locate the package, ignore the error.
+       // It was probably related to loading source files, and we'll
+       // encounter it ourselves shortly if the FindOnly flag isn't set.
        p.Dir = dir
        p.ImportPath = f[1]
        p.Root = f[2]