From a9866431adbeea12b9d01b6f97d1cc4197ca4184 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 4 Jun 2015 18:38:33 -0400 Subject: [PATCH] refactor/importgraph: don't ignore imports in packages with errors. And in gomvpkg, don't stop just because some packages had errors. This is inevitable in a large GOPATH tree. Fixes issue golang/go#10907 Change-Id: I9a60b070228d06d44880202eeef54394e914f5d5 Reviewed-on: https://go-review.googlesource.com/10715 Reviewed-by: Sameer Ajmani --- refactor/importgraph/graph.go | 33 +++++++++++++++++++-------------- refactor/rename/mvpkg.go | 3 ++- refactor/rename/rename.go | 2 ++ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/refactor/importgraph/graph.go b/refactor/importgraph/graph.go index df73e237..8ad8014c 100644 --- a/refactor/importgraph/graph.go +++ b/refactor/importgraph/graph.go @@ -54,7 +54,8 @@ func (g Graph) Search(roots ...string) map[string]bool { // Build scans the specified Go workspace and builds the forward and // reverse import dependency graphs for all its packages. // It also returns a mapping from import paths to errors for packages -// that could not be loaded. +// whose loading was not entirely successful. +// A package may appear in the graph and in the errors mapping. func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error) { type importEdge struct { from, to string @@ -75,22 +76,26 @@ func Build(ctxt *build.Context) (forward, reverse Graph, errors map[string]error ch <- pathError{path, err} return } + bp, err := ctxt.Import(path, "", 0) - if _, ok := err.(*build.NoGoError); ok { - return // empty directory is not an error - } if err != nil { - ch <- pathError{path, err} - return + if _, ok := err.(*build.NoGoError); ok { + // empty directory is not an error + } else { + ch <- pathError{path, err} + } + // Even in error cases, Import usually returns a package. } - for _, imp := range bp.Imports { - ch <- importEdge{path, imp} - } - for _, imp := range bp.TestImports { - ch <- importEdge{path, imp} - } - for _, imp := range bp.XTestImports { - ch <- importEdge{path, imp} + if bp != nil { + for _, imp := range bp.Imports { + ch <- importEdge{path, imp} + } + for _, imp := range bp.TestImports { + ch <- importEdge{path, imp} + } + for _, imp := range bp.XTestImports { + ch <- importEdge{path, imp} + } } }() }) diff --git a/refactor/rename/mvpkg.go b/refactor/rename/mvpkg.go index e31166da..9cc743d6 100644 --- a/refactor/rename/mvpkg.go +++ b/refactor/rename/mvpkg.go @@ -61,11 +61,12 @@ func Move(ctxt *build.Context, from, to, moveTmpl string) error { // Build the import graph and figure out which packages to update. fwd, rev, errors := importgraph.Build(ctxt) if len(errors) > 0 { + // With a large GOPATH tree, errors are inevitable. + // Report them but proceed. fmt.Fprintf(os.Stderr, "While scanning Go workspace:\n") for path, err := range errors { fmt.Fprintf(os.Stderr, "Package %q: %s.\n", path, err) } - return fmt.Errorf("failed to construct import graph") } // Determine the affected packages---the set of packages whose import diff --git a/refactor/rename/rename.go b/refactor/rename/rename.go index c115012f..a028c21b 100644 --- a/refactor/rename/rename.go +++ b/refactor/rename/rename.go @@ -254,6 +254,8 @@ func Main(ctxt *build.Context, offsetFlag, fromFlag, to string) error { // Scan the workspace and build the import graph. _, rev, errors := importgraph.Build(ctxt) if len(errors) > 0 { + // With a large GOPATH tree, errors are inevitable. + // Report them but proceed. fmt.Fprintf(os.Stderr, "While scanning Go workspace:\n") for path, err := range errors { fmt.Fprintf(os.Stderr, "Package %q: %s.\n", path, err)