go/packages: prefer import map to identity imports
For any identity imports to be added to the import map of a package,
make sure these are not already in there. If the package imports "C",
be sure not to overwrite the import map entries for "runtime/cgo", "unsafe",
and "syscall".
Test the import map when test variants are loaded. This tests the fix
in 'go list' made by https://golang.org/cl/128836
Fixes golang/go#26847
Change-Id: I05dff4d3a75fab03f333f6d88128de6a6bf169e6
Reviewed-on: https://go-review.googlesource.com/128876
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/go/packages/golist.go b/go/packages/golist.go
index 3e9d1e0..26d6277 100644
--- a/go/packages/golist.go
+++ b/go/packages/golist.go
@@ -230,13 +230,7 @@
delete(ids, id)
}
for id := range ids {
- // Go issue 26136: go list omits imports in cgo-generated files.
if id == "C" {
- imports["unsafe"] = &Package{ID: "unsafe"}
- imports["syscall"] = &Package{ID: "syscall"}
- if pkgpath != "runtime/cgo" {
- imports["runtime/cgo"] = &Package{ID: "runtime/cgo"}
- }
continue
}
diff --git a/go/packages/golist_fallback.go b/go/packages/golist_fallback.go
index 79400de..331bb655 100644
--- a/go/packages/golist_fallback.go
+++ b/go/packages/golist_fallback.go
@@ -54,10 +54,10 @@
for _, id := range importlist {
if id == "C" {
- importMap["unsafe"] = &Package{ID: "unsafe"}
- importMap["syscall"] = &Package{ID: "syscall"}
- if pkgpath != "runtime/cgo" {
- importMap["runtime/cgo"] = &Package{ID: "runtime/cgo"}
+ for _, path := range []string{"unsafe", "syscall", "runtime/cgo"} {
+ if pkgpath != path && importMap[path] == nil {
+ importMap[path] = &Package{ID: path}
+ }
}
continue
}
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 93e0e8a..d826b93 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -16,6 +16,7 @@
"os"
"path/filepath"
"reflect"
+ "runtime"
"sort"
"strings"
"sync"
@@ -279,6 +280,98 @@
}
}
+func TestLoadImportsTestVariants(t *testing.T) {
+ if usesOldGolist {
+ t.Skip("not yet supported in pre-Go 1.10.4 golist fallback implementation")
+ }
+
+ tmp, cleanup := makeTree(t, map[string]string{
+ "src/a/a.go": `package a; import _ "b"`,
+ "src/b/b.go": `package b`,
+ "src/b/b_test.go": `package b`,
+ "src/b/bx_test.go": `package b_test; import _ "a"`,
+ })
+ defer cleanup()
+
+ cfg := &packages.Config{
+ Mode: packages.LoadImports,
+ Env: append(os.Environ(), "GOPATH="+tmp),
+ Tests: true,
+ }
+ initial, err := packages.Load(cfg, "a", "b")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Check graph topology.
+ graph, _ := importGraph(initial)
+ wantGraph := `
+* a
+ a [b.test]
+* b
+* b [b.test]
+* b.test
+* b_test [b.test]
+ a -> b
+ a [b.test] -> b [b.test]
+ b.test -> b [b.test]
+ b.test -> b_test [b.test]
+ b.test -> os (pruned)
+ b.test -> testing (pruned)
+ b.test -> testing/internal/testdeps (pruned)
+ b_test [b.test] -> a [b.test]
+`[1:]
+
+ if graph != wantGraph {
+ t.Errorf("wrong import graph: got <<%s>>, want <<%s>>", graph, wantGraph)
+ }
+}
+
+func TestLoadImportsC(t *testing.T) {
+ // This test checks that when a package depends on the
+ // test variant of "syscall", "unsafe", or "runtime/cgo", that dependency
+ // is not removed when those packages are added when it imports "C".
+ //
+ // For this test to work, the external test of syscall must have a dependency
+ // on net, and net must import "syscall" and "C".
+ if runtime.GOOS == "windows" {
+ t.Skipf("skipping on windows; packages on windows do not satisfy conditions for test.")
+ }
+
+ if usesOldGolist {
+ t.Skip("not yet supported in pre-Go 1.10.4 golist fallback implementation")
+ }
+
+ cfg := &packages.Config{
+ Mode: packages.LoadImports,
+ Tests: true,
+ }
+ initial, err := packages.Load(cfg, "syscall", "net")
+ if err != nil {
+ t.Fatalf("failed to load imports: %v", err)
+ }
+
+ _, all := importGraph(initial)
+
+ for _, test := range []struct {
+ pattern string
+ wantImport string // an import to check for
+ }{
+ {"net", "syscall:syscall"},
+ {"net [syscall.test]", "syscall:syscall [syscall.test]"},
+ {"syscall_test [syscall.test]", "net:net [syscall.test]"},
+ } {
+ // Test the import paths.
+ pkg := all[test.pattern]
+ if pkg == nil {
+ t.Errorf("package %q not loaded", test.pattern)
+ }
+ if imports := strings.Join(imports(pkg), " "); !strings.Contains(imports, test.wantImport) {
+ t.Errorf("package %q: got \n%s, \nwant to have %s", test.pattern, imports, test.wantImport)
+ }
+ }
+}
+
func TestVendorImports(t *testing.T) {
tmp, cleanup := makeTree(t, map[string]string{
"src/a/a.go": `package a; import _ "b"; import _ "c";`,
