Commit 67e47124 authored by Dmitri Shuralyov's avatar Dmitri Shuralyov Committed by Dmitri Shuralyov

go/build: return partial information on Import error, for local import paths

Documentation of build.Import says:

	// If the path is a local import path naming a package that can be imported
	// using a standard import path, the returned package will set p.ImportPath
	// to that path.
	// ...
	// If an error occurs, Import returns a non-nil error and a non-nil
	// *Package containing partial information.

That behavior was previously untested, and broken by change in CL 33158.

Fix that by avoiding returning early on error for local import paths.
First, gather partial information, and only then check that the p.Dir
directory exists.

Add tests for this behavior.

Fixes #19769.
Fixes #20175 (duplicate of #19769).
Updates #17863.

Change-Id: I169cb35291099d05e02aaa3cb23a7403d1cc3657
Reviewed-on: https://go-review.googlesource.com/42350Reviewed-by: default avatarRuss Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 9e83c11f
......@@ -529,10 +529,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
if !ctxt.isAbsPath(path) {
p.Dir = ctxt.joinPath(srcDir, path)
}
if !ctxt.isDir(p.Dir) {
// package was not found
return p, fmt.Errorf("cannot find package %q in:\n\t%s", path, p.Dir)
}
// p.Dir directory may or may not exist. Gather partial information first, check if it exists later.
// Determine canonical import path, if any.
// Exclude results where the import path would include /testdata/.
inTestdata := func(sub string) bool {
......@@ -687,6 +684,16 @@ Found:
}
}
// If it's a local import path, by the time we get here, we still haven't checked
// that p.Dir directory exists. This is the right time to do that check.
// We can't do it earlier, because we want to gather partial information for the
// non-nil *Package returned when an error occurs.
// We need to do this before we return early on FindOnly flag.
if IsLocalImport(path) && !ctxt.isDir(p.Dir) {
// package was not found
return p, fmt.Errorf("cannot find package %q in:\n\t%s", path, p.Dir)
}
if mode&FindOnly != 0 {
return p, pkgerr
}
......
......@@ -303,6 +303,7 @@ func TestShellSafety(t *testing.T) {
}
// Want to get a "cannot find package" error when directory for package does not exist.
// There should be valid partial information in the returned non-nil *Package.
func TestImportDirNotExist(t *testing.T) {
testenv.MustHaveGoBuild(t) // really must just have source
ctxt := Default
......@@ -319,10 +320,19 @@ func TestImportDirNotExist(t *testing.T) {
{"Import(local, FindOnly)", "./doesnotexist", filepath.Join(ctxt.GOROOT, "src/go/build"), FindOnly},
}
for _, test := range tests {
_, err := ctxt.Import(test.path, test.srcDir, test.mode)
p, err := ctxt.Import(test.path, test.srcDir, test.mode)
if err == nil || !strings.HasPrefix(err.Error(), "cannot find package") {
t.Errorf(`%s got error: %q, want "cannot find package" error`, test.label, err)
}
// If an error occurs, build.Import is documented to return
// a non-nil *Package containing partial information.
if p == nil {
t.Fatalf(`%s got nil p, want non-nil *Package`, test.label)
}
// Verify partial information in p.
if p.ImportPath != "go/build/doesnotexist" {
t.Errorf(`%s got p.ImportPath: %q, want "go/build/doesnotexist"`, test.label, p.ImportPath)
}
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment