Skip to content

Commit d0050e2

Browse files
committed
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 <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
1 parent 1c241d2 commit d0050e2

File tree

2 files changed

+54
-19
lines changed

2 files changed

+54
-19
lines changed

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

+48-11
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,67 @@
22

33
# go/build's Import should find modules by invoking the go command
44

5-
go build -o $WORK/testimport.exe ./testimport
5+
go build -o $WORK ./testimport ./testfindonly
66

77
# GO111MODULE=off
88
env GO111MODULE=off
9-
! exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
9+
! exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
1010

1111
# GO111MODULE=auto in GOPATH/src
1212
env GO111MODULE=auto
13-
exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
13+
exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
1414

1515
# GO111MODULE=auto outside GOPATH/src
1616
cd $GOPATH/other
1717
env GO111MODULE=auto
18-
exec $WORK/testimport.exe other/x/y/z/w .
18+
exec $WORK/testimport$GOEXE other/x/y/z/w .
1919
stdout w2.go
2020

21-
! exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
21+
! exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
2222
stderr 'cannot find module providing package gobuild.example.com/x/y/z/w'
2323

2424
cd z
25-
exec $WORK/testimport.exe other/x/y/z/w .
25+
exec $WORK/testimport$GOEXE other/x/y/z/w .
2626
stdout w2.go
2727

2828
# GO111MODULE=on outside GOPATH/src
2929
env GO111MODULE=
30-
exec $WORK/testimport.exe other/x/y/z/w .
30+
exec $WORK/testimport$GOEXE other/x/y/z/w .
3131
stdout w2.go
3232
env GO111MODULE=on
33-
exec $WORK/testimport.exe other/x/y/z/w .
33+
exec $WORK/testimport$GOEXE other/x/y/z/w .
3434
stdout w2.go
3535

3636
# GO111MODULE=on in GOPATH/src
3737
cd $GOPATH/src
3838
env GO111MODULE=
39-
exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
39+
exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
4040
stdout w1.go
4141
env GO111MODULE=on
42-
exec $WORK/testimport.exe gobuild.example.com/x/y/z/w .
42+
exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w .
4343
stdout w1.go
4444
cd w
45-
exec $WORK/testimport.exe gobuild.example.com/x/y/z/w ..
45+
exec $WORK/testimport$GOEXE gobuild.example.com/x/y/z/w ..
4646
stdout w1.go
4747

48+
# go/build's Import in FindOnly mode should find directories by invoking the go command
49+
#
50+
# Calling build.Import in build.FindOnly mode on an import path of a Go package
51+
# that produces errors when loading (e.g., due to build constraints not matching
52+
# the current build context) should return the package directory and nil error.
53+
54+
# Issue 31603: Import with non-empty srcDir should work.
55+
env GO111MODULE=on
56+
exec $WORK/testfindonly$GOEXE gobuild.example.com/x/y/z/i $WORK
57+
! stdout 'build constraints'
58+
stdout '^dir=\$WORK.+i err=<nil>$'
59+
60+
# Issue 37153: Import with empty srcDir should work.
61+
env GO111MODULE=on
62+
exec $WORK/testfindonly$GOEXE gobuild.example.com/x/y/z/i ''
63+
! stdout 'build constraints'
64+
stdout '^dir=\$WORK.+i err=<nil>$'
65+
4866
-- go.mod --
4967
module gobuild.example.com/x/y/z
5068

@@ -54,6 +72,11 @@ package z
5472
-- w/w1.go --
5573
package w
5674

75+
-- i/i.go --
76+
// +build i
77+
78+
package i
79+
5780
-- testimport/x.go --
5881
package main
5982

@@ -89,6 +112,20 @@ func main() {
89112
fmt.Printf("%s\n%s\n", p1.Dir, strings.Join(p1.GoFiles, " "))
90113
}
91114

115+
-- testfindonly/x.go --
116+
package main
117+
118+
import (
119+
"fmt"
120+
"go/build"
121+
"os"
122+
)
123+
124+
func main() {
125+
p, err := build.Import(os.Args[1], os.Args[2], build.FindOnly)
126+
fmt.Printf("dir=%s err=%v\n", p.Dir, err)
127+
}
128+
92129
-- $GOPATH/other/go.mod --
93130
module other/x/y
94131

src/go/build/build.go

+6-8
Original file line numberDiff line numberDiff line change
@@ -1015,8 +1015,6 @@ var errNoModules = errors.New("not using modules")
10151015
// Then we reinvoke it for every dependency. But this is still better than not working at all.
10161016
// See golang.org/issue/26504.
10171017
func (ctxt *Context) importGo(p *Package, path, srcDir string, mode ImportMode) error {
1018-
const debugImportGo = false
1019-
10201018
// To invoke the go command,
10211019
// we must not being doing special things like AllowBinary or IgnoreVendor,
10221020
// 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)
11351133
}
11361134
dir := f[0]
11371135
errStr := strings.TrimSpace(f[4])
1138-
if errStr != "" && p.Dir == "" {
1139-
// If 'go list' could not locate the package, return the same error that
1140-
// 'go list' reported.
1141-
// If 'go list' did locate the package (p.Dir is not empty), ignore the
1142-
// error. It was probably related to loading source files, and we'll
1143-
// encounter it ourselves shortly.
1136+
if errStr != "" && dir == "" {
1137+
// If 'go list' could not locate the package (dir is empty),
1138+
// return the same error that 'go list' reported.
11441139
return errors.New(errStr)
11451140
}
11461141

1142+
// If 'go list' did locate the package, ignore the error.
1143+
// It was probably related to loading source files, and we'll
1144+
// encounter it ourselves shortly if the FindOnly flag isn't set.
11471145
p.Dir = dir
11481146
p.ImportPath = f[1]
11491147
p.Root = f[2]

0 commit comments

Comments
 (0)