diff --git a/.travis.yml b/.travis.yml index 4d7c8f1..4b3823d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ go: sudo: false install: + - go get github.com/Masterminds/glide - GO15VENDOREXPERIMENT=1 go build script: diff --git a/README.md b/README.md index 363aaa0..a484f93 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ For a detailed explanation on why Glide doesn't do this see [here](http://engine ## Description -This tool will help you removing from the project vendor directories all the files not needed for building your project. By default it'll keep all the files provided by packages listed in the `glide.lock` file. +This tool will help you removing from the project vendor directories all the files not needed for building your project. By default it'll keep all the files provided by packages returned by the `glide list` command. If you want to keep only go (including tests) files you can provide the `--only-code` option. If you want to remove also the go test files you can add the `--no-tests` option. diff --git a/appveyor.yml b/appveyor.yml index fd7b236..8a35181 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,11 +12,12 @@ platform: install: - go version - go env +- go get github.com/Masterminds/glide build_script: - go build test_script: -- go test . +- PATH=$GOPATH/bin:$PATH go test . deploy: off diff --git a/gvc.go b/gvc.go index 5008a5a..3ea33aa 100644 --- a/gvc.go +++ b/gvc.go @@ -1,9 +1,11 @@ package main import ( + "encoding/json" "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" "strings" @@ -20,12 +22,21 @@ var cmd = &cobra.Command{ } type options struct { - dryrun bool - onlyCode bool - noTests bool + dryrun bool + onlyCode bool + noTests bool + noLegalFiles bool + keepPatterns []string + + // Deprecated + useLockFile bool noTestImports bool - noLegalFiles bool - keepPatterns []string +} + +type packages struct { + Installed []string `json:"installed"` + Missing []string `json:"missing"` + Gopath []string `json:"gopath"` } var ( @@ -40,10 +51,12 @@ const ( func init() { cmd.PersistentFlags().BoolVar(&opts.dryrun, "dryrun", false, "just output what will be removed") cmd.PersistentFlags().BoolVar(&opts.onlyCode, "only-code", false, "keep only go files (including go test files)") - cmd.PersistentFlags().BoolVar(&opts.noTestImports, "no-test-imports", false, "remove also testImport vendor directories") cmd.PersistentFlags().BoolVar(&opts.noTests, "no-tests", false, "remove also go test files (requires --only-code)") cmd.PersistentFlags().BoolVar(&opts.noLegalFiles, "no-legal-files", false, "remove also licenses and legal files") cmd.PersistentFlags().StringSliceVar(&opts.keepPatterns, "keep", []string{}, "A pattern to keep additional files inside needed packages. The pattern match will be relative to the deeper vendor dir. Supports double star (**) patterns. (see https://golang.org/pkg/path/filepath/#Match and https://github.com/bmatcuk/doublestar). Can be specified multiple times. For example to keep all the files with json extension use the '**/*.json' pattern.") + + cmd.PersistentFlags().BoolVar(&opts.useLockFile, "use-lock-file", false, "use glide.lock instead of glide list to determine imports") + cmd.PersistentFlags().BoolVar(&opts.noTestImports, "no-test-imports", false, "remove also testImport vendor directories") } func main() { @@ -56,53 +69,87 @@ func glidevc(cmd *cobra.Command, args []string) { os.Exit(1) } - if err := cleanup(".", opts); err != nil { + if err := cleanup("."); err != nil { fmt.Print(err) os.Exit(1) } - return } -func cleanup(path string, opts options) error { - lock, err := LoadGlideLockfile(path) +func glideLockImports(path string) ([]string, error) { + yml, err := ioutil.ReadFile(filepath.Join(path, gpath.LockFile)) if err != nil { - return fmt.Errorf("Could not load lockfile: %v", err) + return nil, err } - // The package list already have the path converted to the os specific - // path separator, needed for future comparisons. - pkgList := []string{} - repoList := []string{} - adder := func(lock cfg.Locks) { - for _, imp := range lock { - // This converts pkg separator "/" to os specific separator - repoList = append(repoList, filepath.FromSlash(imp.Name)) + lock, err := cfg.LockfileFromYaml(yml) + if err != nil { + return nil, err + } - if len(imp.Subpackages) > 0 { - for _, sp := range imp.Subpackages { - // This converts pkg separator "/" to os specific separator - pkgList = append(pkgList, filepath.Join(imp.Name, sp)) - } + var imports []string + adder := func(locks cfg.Locks) { + for _, lock := range locks { + for _, subpackage := range lock.Subpackages { + imports = append(imports, lock.Name+"/"+subpackage) } - // TODO(sgotti) we cannot skip the base import if it has subpackages - // because glide doesn't write "." as a subpackage, otherwise if some - // files in the base import are needed they will be removed. - - // This converts pkg separator "/" to os specific separator - pkgList = append(pkgList, filepath.FromSlash(imp.Name)) + imports = append(imports, lock.Name) } } + adder(lock.Imports) if !opts.noTestImports { adder(lock.DevImports) } + return imports, nil +} + +func glideListImports(path string) ([]string, error) { + cmd := exec.Command("glide", "list", "-output", "json", path) + out, err := cmd.Output() + if err != nil { + return nil, err + } + + list := &packages{} + err = json.Unmarshal(out, list) + if err != nil { + return nil, err + } + + return list.Installed, nil +} + +func cleanup(path string) error { + var ( + packages []string + err error + ) + + if opts.useLockFile { + packages, err = glideLockImports(path) + } else { + packages, err = glideListImports(path) + } + + // The package list already have the path converted to the os specific + // path separator, needed for future comparisons. + pkgList := []string{} + pkgMap := map[string]struct{}{} + for _, imp := range packages { + if _, found := pkgMap[imp]; !found { + // This converts pkg separator "/" to os specific separator + pkgList = append(pkgList, filepath.FromSlash(imp)) + pkgMap[imp] = struct{}{} + } + } + vpath, err := gpath.Vendor() if err != nil { return err } if vpath == "" { - return fmt.Errorf("cannot fine vendor dir") + return fmt.Errorf("cannot find vendor dir") } type pathData struct { @@ -124,83 +171,63 @@ func cleanup(path string, opts options) error { return nil } + // Short-circuit for test files + if opts.noTests && strings.HasSuffix(path, "_test.go") { + return nil + } + localPath := strings.TrimPrefix(path, searchPath) lastVendorPath, err := getLastVendorPath(localPath) if err != nil { return err } - if lastVendorPath == "" { - lastVendorPath = localPath - } + lastVendorPathDir := filepath.Dir(lastVendorPath) keep := false + for _, name := range pkgList { - // If the file's parent directory is a needed package, keep it. - if !info.IsDir() && filepath.Dir(lastVendorPath) == name { - if opts.onlyCode { - if opts.noTests && strings.HasSuffix(path, "_test.go") { - keep = false - continue - } - for _, suffix := range codeSuffixes { - if strings.HasSuffix(path, suffix) { - keep = true - } - } - // Match keep patterns - for _, keepPattern := range opts.keepPatterns { - ok, err := doublestar.Match(keepPattern, lastVendorPath) - // TODO(sgotti) if a bad pattern is encountered stop here. Actually there's no function to verify a pattern before using it, perhaps just a fake match at the start will work. - if err != nil { - return fmt.Errorf("bad pattern: %q", keepPattern) - } - if ok { - keep = true - } - } - } else { - keep = true - } + // If a directory is a needed package then keep it + keep = keep || info.IsDir() && name == lastVendorPath + + // The remaining tests are only for files + if info.IsDir() { + continue } - } - // Keep all the legal files inside top repo dir and required package dirs - for _, name := range append(repoList, pkgList...) { - if !info.IsDir() && filepath.Dir(lastVendorPath) == name { - if !opts.noLegalFiles { - if IsLegalFile(path) { - keep = true - } - } + // Keep legal files in directories that are the parent of a needed package + keep = keep || !opts.noLegalFiles && IsLegalFile(path) && isParentDirectory(lastVendorPathDir, name) + + // The remaining tests only apply if the file is in a needed package + if name != lastVendorPathDir { + continue } - } - // If a directory is a needed package then keep it - if keep == false && info.IsDir() { - for _, name := range pkgList { - if name == lastVendorPath { - keep = true + // Keep everything unless --only-code was specified + keep = keep || !opts.onlyCode + + // Always keep code files + for _, suffix := range codeSuffixes { + keep = keep || strings.HasSuffix(path, suffix) + } + + // Match keep patterns + for _, keepPattern := range opts.keepPatterns { + ok, err := doublestar.Match(keepPattern, lastVendorPath) + // TODO(sgotti) if a bad pattern is encountered stop here. Actually there's no function to verify a pattern before using it, perhaps just a fake match at the start will work. + if err != nil { + return fmt.Errorf("bad pattern: %q", keepPattern) } + keep = keep || ok } } if keep { - // Keep also all parents of current path - curpath := localPath - for { - curpath = filepath.Dir(curpath) - if curpath == "." { - break - } - if _, ok := markForKeep[curpath]; ok { - // Already marked for keep - break - } + // Keep all parent directories of current path + for curpath := localPath; curpath != "."; curpath = filepath.Dir(curpath) { markForKeep[curpath] = pathData{curpath, true} } - - // Mark for keep + // Fix isDir property for current path markForKeep[localPath] = pathData{localPath, info.IsDir()} } @@ -255,30 +282,22 @@ func cleanup(path string, opts options) error { } func getLastVendorPath(path string) (string, error) { - curpath := path - for { - if curpath == "." { - return "", nil - } + for curpath := path; curpath != "."; curpath = filepath.Dir(curpath) { if filepath.Base(curpath) == "vendor" { return filepath.Rel(curpath, path) } - curpath = filepath.Dir(curpath) } + return path, nil } -// LoadLockfile loads the contents of a glide.lock file. -func LoadGlideLockfile(base string) (*cfg.Lockfile, error) { - yml, err := ioutil.ReadFile(filepath.Join(base, gpath.LockFile)) - if err != nil { - return nil, err +func isParentDirectory(parent, child string) bool { + if !strings.HasSuffix(parent, "/") { + parent += "/" } - lock, err := cfg.LockfileFromYaml(yml) - if err != nil { - return nil, err + if !strings.HasSuffix(child, "/") { + child += "/" } - - return lock, nil + return strings.HasPrefix(child, parent) } // File lists and code took from https://github.com/client9/gosupplychain/blob/master/license.go @@ -316,7 +335,7 @@ func IsLegalFile(path string) bool { } } for _, substring := range LegalFileSubstring { - if strings.Index(lowerfile, substring) != -1 && !strings.HasSuffix(lowerfile, goTestSuffix) { + if strings.Contains(lowerfile, substring) && !strings.HasSuffix(lowerfile, goTestSuffix) { return true } } diff --git a/gvc_test.go b/gvc_test.go index 470366b..3e9043c 100644 --- a/gvc_test.go +++ b/gvc_test.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" ) @@ -29,6 +30,9 @@ func createVendorTree(t *testing.T, dir string, tree []FileInfo) error { if err != nil { return fmt.Errorf("failed to create file %q: %v", path, err) } + if strings.HasSuffix(path, ".go") { + fmt.Fprintf(f, "package %s", filepath.Base(filepath.Dir(path))) + } f.Close() } } @@ -70,6 +74,7 @@ func checkExpectedVendor(t *testing.T, dir string, exp []FileInfo) error { type testData struct { tree []FileInfo lockdata string + mainfile string expectedFiles []FileInfo opts options } @@ -134,13 +139,23 @@ imports: subpackages: - subpkg02 devImports: [] -devImports: [] +` + + mainfile := `package main + +import ( + _ "host01/org01/repo01" + _ "host01/org01/repo01/subpkg01" + _ "host02/org02/repo02" + _ "host02/org02/repo02/subpkg02" +) ` tests := []testData{ { tree: tree, lockdata: lockdata, + mainfile: mainfile, expectedFiles: []FileInfo{ {"host01", true}, {"host01/org01", true}, @@ -179,6 +194,7 @@ devImports: [] { tree: tree, lockdata: lockdata, + mainfile: mainfile, expectedFiles: []FileInfo{ {"host01", true}, {"host01/org01", true}, @@ -221,6 +237,7 @@ devImports: [] { tree: tree, lockdata: lockdata, + mainfile: mainfile, expectedFiles: []FileInfo{ {"host01", true}, {"host01/org01", true}, @@ -263,6 +280,7 @@ devImports: [] { tree: tree, lockdata: lockdata, + mainfile: mainfile, expectedFiles: []FileInfo{ {"host01", true}, {"host01/org01", true}, @@ -311,6 +329,7 @@ devImports: [] { tree: tree, lockdata: lockdata, + mainfile: mainfile, expectedFiles: []FileInfo{ {"host01", true}, {"host01/org01", true}, @@ -361,10 +380,13 @@ devImports: [] }, } - for i, td := range tests { - t.Logf("Test #%d", i) - if err := testCleanup(t, &td); err != nil { - t.Fatalf("#%d: unexpected error: %v", i, err) + for _, useLockFile := range []bool{false, true} { + for i, td := range tests { + t.Logf("Test #%d", i) + td.opts.useLockFile = useLockFile + if err := testCleanup(t, &td); err != nil { + t.Fatalf("#%d: unexpected error: %v", i, err) + } } } } @@ -392,11 +414,17 @@ func testCleanup(t *testing.T, td *testData) error { return fmt.Errorf("failed to create glide.lock file: %v", err) } + // Create main.go file + if err := ioutil.WriteFile(filepath.Join(tmpDir, "main.go"), []byte(td.mainfile), 0666); err != nil { + return fmt.Errorf("failed to create main.go file: %v", err) + } + if err := createVendorTree(t, tmpDir, td.tree); err != nil { return err } - if err := cleanup(tmpDir, td.opts); err != nil { + opts = td.opts + if err := cleanup(tmpDir); err != nil { return err } @@ -405,3 +433,43 @@ func testCleanup(t *testing.T, td *testData) error { } return nil } + +func TestGetLastVendorPath(t *testing.T) { + tests := map[string]string{ + "host1/org1/repo1": "host1/org1/repo1", + "host1/org1/repo1/vendor/host2/org2/repo2": "host2/org2/repo2", + "host1/org1/repo1/vendor/host2/org2/repo2/vendor/host3/org3/repo3": "host3/org3/repo3", + } + + for input, expected := range tests { + got, err := getLastVendorPath(input) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if got != expected { + t.Fatalf("got=%q, expected=%q", got, expected) + } + } +} + +func TestIsParentDirectory(t *testing.T) { + type testData2 struct { + Parent string + Child string + } + tests := map[testData2]bool{ + {"foo", "foo"}: true, + {"foo", "foo/bar"}: true, + {"foo", "foobar"}: false, + {"foo/", "foo"}: true, + {"foo", "foo/"}: true, + {"foo/", "foo/"}: true, + } + + for input, expected := range tests { + got := isParentDirectory(input.Parent, input.Child) + if got != expected { + t.Fatalf("got=%q, expected=%q", got, expected) + } + } +}