From ff1e1650504e7f4475be80db5e82c01bd818adba Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 25 Jul 2016 12:12:48 -0400 Subject: [PATCH 1/2] save: use x/tools/refactor/importgraph instead of go list Also ignore all build tags, getting the maximal set of dependencies. Fixes #29. --- save.go | 118 ++++++++++++++++++++------------------------------- save_test.go | 23 ++++------ util.go | 5 ++- 3 files changed, 59 insertions(+), 87 deletions(-) diff --git a/save.go b/save.go index db2ac3c..5e7cf8d 100644 --- a/save.go +++ b/save.go @@ -2,17 +2,19 @@ package main import ( "bufio" - "bytes" "errors" "fmt" "go/build" "io" + "log" "os" "os/exec" "path" "regexp" "sort" "strings" + + "golang.org/x/tools/refactor/importgraph" ) var cmdSave = &Command{ @@ -138,52 +140,59 @@ func (p byImportPath) Swap(i, j int) { p[i], p[j] = p[j], p[i] } // getAllDeps returns a slice of package import paths for all dependencies // (including test dependencies) of the given import path (and subpackages) and commands. func getAllDeps(importPath string, cmds []string) []string { - // Filter out any cmds under the importPath (because those are handled as a side effect) - var filteredCmds []string - for _, cmd := range cmds { - if !strings.HasPrefix(cmd, importPath) { - filteredCmds = append(filteredCmds, cmd) - } + deps := map[string]struct{}{} + roots := map[string]struct{}{ + importPath: {}, } + subpackagePrefix := importPath + "/" - // Get a set of transitive dependencies (package import paths) for the - // specified package. - var pkgExprs = append([]string{path.Join(importPath, "...")}, filteredCmds...) - var output = mustRun("go", - append([]string{"list", "-f", `{{range .Deps}}{{.}}{{"\n"}}{{end}}`}, pkgExprs...)...) - var deps = filterPackages(output, nil) // filter out standard library + // Add the command packages. Note that external command packages are + // considered dependencies. + for _, pkg := range cmds { + roots[pkg] = struct{}{} - // Add the command packages. - for _, cmd := range filteredCmds { - deps[cmd] = struct{}{} + if !strings.HasPrefix(pkg, subpackagePrefix) { + deps[pkg] = struct{}{} + } } - // List dependencies of test files, which are not included in the go list .Deps - // Also, ignore any dependencies that are already covered. - var testImportOutput = mustRun("go", - append([]string{"list", "-f", `{{join .TestImports "\n"}}{{"\n"}}{{join .XTestImports "\n"}}`}, pkgExprs...)...) - var testImmediateDeps = filterPackages(testImportOutput, deps) // filter out stdlib and existing deps - for dep := range testImmediateDeps { - deps[dep] = struct{}{} - } + for _, useAllFiles := range []bool{false, true} { + buildContext := build.Default + buildContext.CgoEnabled = true + buildContext.UseAllFiles = useAllFiles + + forward, _, errs := importgraph.Build(&buildContext) + for pkg, err := range errs { + // Lots of errors because of UseAllFiles. + if !useAllFiles { + log.Printf("error loading package %s: %v", pkg, err) + } + } - // We have to get the transitive deps of the remaining test imports. - // NOTE: this will return the dependencies of the libraries imported by tests - // and not imported by main code. This output does not include the imports themselves. - if len(testImmediateDeps) > 0 { - var testDepOutput = mustRun("go", append([]string{"list", "-f", `{{range .Deps}}{{.}}{{"\n"}}{{end}}`}, setToSlice(testImmediateDeps)...)...) - var allTestDeps = filterPackages(testDepOutput, deps) // filter out standard library and existing deps - for dep := range allTestDeps { - deps[dep] = struct{}{} + // Add the subpackages. + for pkg := range forward { + if strings.HasPrefix(pkg, subpackagePrefix) { + roots[pkg] = struct{}{} + } } - } - // Return everything in deps - var result []string - for dep := range deps { - result = append(result, dep) + // Get the reflexive transitive closure for all packages of interest. + for pkg := range forward.Search(setToSlice(roots)...) { + // Exclude our roots. Note that commands are special-cased above. + if _, ok := roots[pkg]; ok { + continue + } + slash := strings.IndexByte(pkg, '/') + stdLib := slash == -1 || strings.IndexByte(pkg[:slash], '.') == -1 + // Exclude the standard library. + if stdLib { + continue + } + deps[pkg] = struct{}{} + } } - return result + + return setToSlice(deps) } func run(name string, args ...string) ([]byte, error) { @@ -194,16 +203,6 @@ func run(name string, args ...string) ([]byte, error) { return cmd.CombinedOutput() } -// mustRun is a wrapper for exec.Command(..).CombinedOutput() that provides helpful -// error message and exits on failure. -func mustRun(name string, args ...string) []byte { - var output, err = run(name, args...) - if err != nil { - perror(fmt.Errorf("%v %v\n%v\nError: %v", name, args, string(output), err)) - } - return output -} - func setToSlice(set map[string]struct{}) []string { var slice []string for k := range set { @@ -212,29 +211,6 @@ func setToSlice(set map[string]struct{}) []string { return slice } -// filterPackages accepts the output of a go list comment (one package per line) -// and returns a set of package import paths, excluding standard library. -// Additionally, any packages present in the "exclude" set will be excluded. -func filterPackages(output []byte, exclude map[string]struct{}) map[string]struct{} { - var scanner = bufio.NewScanner(bytes.NewReader(output)) - var deps = map[string]struct{}{} - for scanner.Scan() { - var ( - pkg = scanner.Text() - slash = strings.Index(pkg, "/") - stdLib = slash == -1 || strings.Index(pkg[:slash], ".") == -1 - ) - if stdLib { - continue - } - if _, ok := exclude[pkg]; ok { - continue - } - deps[pkg] = struct{}{} - } - return deps -} - // readCmds returns the list of cmds declared in the given glockfile. // They must appear at the top of the file, with the syntax: // cmd code.google.com/p/go.tools/cmd/godoc diff --git a/save_test.go b/save_test.go index bfee99f..312bf0d 100644 --- a/save_test.go +++ b/save_test.go @@ -100,8 +100,6 @@ var saveTests = []saveTest{ // the following should be included: // - package's tests' dependencies - // - // perhaps also higher degree test dependencies // - package's dependencies' tests' dependencies // - package's tests' dependencies' tests' dependencies { @@ -134,8 +132,8 @@ var saveTests = []saveTest{ []string{ "github.com/test/p2", "github.com/test/p3", - // "github.com/test/p4", // implement? - // "github.com/test/p5", + "github.com/test/p4", + "github.com/test/p5", }, }, @@ -218,7 +216,7 @@ func TestSave(t *testing.T) { func runSaveTest(t *testing.T, test saveTest) { var gopath, err = ioutil.TempDir("", "gopath") if err != nil { - panic(err) + t.Fatal(err) } t.Log(gopath) defer os.RemoveAll(gopath) @@ -228,15 +226,14 @@ func runSaveTest(t *testing.T, test saveTest) { var dir = filepath.Join(gopath, "src", pkg.importPath) err = os.MkdirAll(dir, 0777) if err != nil { - panic(err) + t.Fatal(err) } cmd := exec.Command("git", "init") cmd.Dir = strings.TrimSuffix(dir, "/subpkg") gitinit, err := cmd.CombinedOutput() if err != nil { - t.Logf("git init: %v\noutput: %v", err, string(gitinit)) - t.FailNow() + t.Fatalf("git init: %v\noutput: %v", err, string(gitinit)) } var pkgdir = filepath.Join(gopath, "src", pkg.importPath) for _, file := range pkg.files { @@ -245,13 +242,13 @@ func runSaveTest(t *testing.T, test saveTest) { if filedir != "." { err = os.MkdirAll(filepath.Join(pkgdir, filedir), 0777) if err != nil { - panic(err) + t.Fatal(err) } } var f, err = os.Create(filepath.Join(pkgdir, filedir, filename)) if err != nil { - panic(err) + t.Fatal(err) } var pkgName = pkg.importPath[strings.LastIndex(pkg.importPath, "/")+1:] if file.testPkg { @@ -270,16 +267,14 @@ import ( cmd.Dir = dir gitadd, err := cmd.CombinedOutput() if err != nil { - t.Logf("git add: %v\noutput: %v", err, string(gitadd)) - t.FailNow() + t.Fatalf("git add: %v\noutput: %v", err, string(gitadd)) } cmd = exec.Command("git", "commit", "-am", "initial") cmd.Dir = dir gitcommit, err := cmd.CombinedOutput() if err != nil { - t.Logf("git commit: %v\noutput: %v", err, string(gitcommit)) - t.FailNow() + t.Fatalf("git commit: %v\noutput: %v", err, string(gitcommit)) } } diff --git a/util.go b/util.go index bc52342..1ee6222 100644 --- a/util.go +++ b/util.go @@ -135,9 +135,10 @@ func glockfileWriter(importPath string, n bool) io.WriteCloser { return os.Stdout } - var f, err = os.Create(glockFilename(importPath)) + fileName := glockFilename(importPath) + var f, err = os.Create(fileName) if err != nil { - perror(fmt.Errorf("error creating %s: %v", glockFilename, err)) + perror(fmt.Errorf("error creating %s: %v", fileName, err)) } return f } From c39ba8a60d7eb5aaf18c433f239bf793b7a158d1 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 25 Jul 2016 13:41:50 -0400 Subject: [PATCH 2/2] Update Travis CI config --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index bbc3377..2328f4c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,10 @@ +sudo: false language: go go: + - 1.6 + - 1.5 + - 1.4 - 1.3 - - tip -install: - - go get github.com/robfig/glock +before_script: # tests need to interact with git - git config --global user.email "travis@localhost.com" - git config --global user.name "Travis" -script: - - go test github.com/robfig/glock