From 1315bd80e8ba037fd4203373d70cc139ab2acaa3 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Wed, 16 Aug 2017 23:18:06 +0300 Subject: [PATCH 1/5] internal/gps: Add prune functions to gps Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 377 +++++++++++++++++++++++++++++++++++++ internal/gps/prune_test.go | 224 ++++++++++++++++++++++ 2 files changed, 601 insertions(+) create mode 100644 internal/gps/prune.go create mode 100644 internal/gps/prune_test.go diff --git a/internal/gps/prune.go b/internal/gps/prune.go new file mode 100644 index 0000000000..c32e66c9f0 --- /dev/null +++ b/internal/gps/prune.go @@ -0,0 +1,377 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gps + +import ( + "io/ioutil" + "log" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/golang/dep/internal/fs" + "github.com/pkg/errors" +) + +// PruneOptions represents the pruning options used to write the dependecy tree. +type PruneOptions uint8 + +const ( + // PruneNestedVendorDirs indicates if nested vendor directories should be pruned. + PruneNestedVendorDirs = 1 << iota + // PruneUnusedPackages indicates if unused Go packages should be pruned. + PruneUnusedPackages + // PruneNonGoFiles indicates if non-Go files should be pruned. + // LICENSE & COPYING files are kept for convience. + PruneNonGoFiles + // PruneGoTestFiles indicates if Go test files should be pruned. + PruneGoTestFiles +) + +var ( + // licenseFilePrefixes is a list of name prefixes for license files. + licenseFilePrefixes = []string{ + "license", + "licence", + "copying", + "unlicense", + "copyright", + "copyleft", + } + // legalFileSubstrings contains substrings that are likey part of a legal + // declaration file. + legalFileSubstrings = []string{ + "legal", + "notice", + "disclaimer", + "patent", + "third-party", + "thirdparty", + } +) + +// Prune removes excess files from the dep tree whose root is baseDir based +// on the PruneOptions passed. +// +// A Lock must be passed if PruneUnusedPackages is toggled on. +func Prune(baseDir string, options PruneOptions, l Lock, logger *log.Logger) error { + if (options & PruneNestedVendorDirs) != 0 { + if err := pruneNestedVendorDirs(baseDir); err != nil { + return err + } + } + + if (options & PruneUnusedPackages) != 0 { + if l == nil { + return errors.New("pruning unused packages requires passing a non-nil Lock") + } + if err := pruneUnusedPackages(baseDir, l, logger); err != nil { + return errors.Wrap(err, "failed to prune unused packages") + } + } + + if (options & PruneNonGoFiles) != 0 { + if err := pruneNonGoFiles(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune non-Go files") + } + } + + if (options & PruneGoTestFiles) != 0 { + if err := pruneGoTestFiles(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune Go test files") + } + } + + // Delete all empty directories. + if err := pruneEmptyDirs(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune empty dirs") + } + + return nil +} + +// pruneNestedVendorDirs deletes all nested vendor directories within baseDir. +func pruneNestedVendorDirs(baseDir string) error { + return filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + return nil + } + + // Ignore the base vendor directory. + if path == baseDir { + return nil + } + + if err := filepath.Walk(path, stripVendor); err != nil { + return err + } + + // Don't walk into directories again. + return filepath.SkipDir + }) +} + +// pruneUnusedPackages deletes unimported packages found within baseDir. +// Determining whether packages are imported or not is based on the passed Lock. +func pruneUnusedPackages(baseDir string, l Lock, logger *log.Logger) error { + unused, err := calculateUnusedPackages(baseDir, l, logger) + if err != nil { + return err + } + + for _, pkg := range unused { + pkgPath := filepath.Join(baseDir, pkg) + + files, err := ioutil.ReadDir(pkgPath) + if err != nil { + // TODO(ibrasho) Handle this error properly. + // It happens when attempting to ioutil.ReadDir a submodule. + continue + } + + // Delete *.go files in the package directory. + for _, file := range files { + // Skip directories and files that don't have a .go suffix. + if file.IsDir() || !strings.HasSuffix(file.Name(), ".go") { + continue + } + + if err := os.Remove(filepath.Join(pkgPath, file.Name())); err != nil { + return err + } + } + } + + return nil +} + +// calculateUnusedPackages generates a list of unused packages existing within +// baseDir depending on the imported packages found in the passed Lock. +func calculateUnusedPackages(baseDir string, l Lock, logger *log.Logger) ([]string, error) { + imported := calculateImportedPackages(l) + sort.Strings(imported) + + var unused []string + + if logger != nil { + logger.Println("Calculating unused packages to prune.") + logger.Println("Checking the following packages:") + } + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore baseDir and anything that's not a directory. + if path == baseDir || !info.IsDir() { + return nil + } + + pkg := strings.TrimPrefix(path, baseDir+string(filepath.Separator)) + if logger != nil { + logger.Printf(" %s", pkg) + } + + // If pkg is not a parent of an imported package, add it to the + // unused list. + i := sort.Search(len(imported), func(i int) bool { + return pkg <= imported[i] + }) + if i >= len(imported) || !strings.HasPrefix(imported[i], pkg) { + unused = append(unused, path) + } + + return nil + }) + + return unused, err +} + +// calculateImportedPackages generates a list of imported packages from +// the passed Lock. +func calculateImportedPackages(l Lock) []string { + var imported []string + + for _, project := range l.Projects() { + projectRoot := string(project.Ident().ProjectRoot) + for _, pkg := range project.Packages() { + imported = append(imported, filepath.Join(projectRoot, pkg)) + } + } + return imported +} + +// pruneNonGoFiles delete all non-Go files existing within baseDir. +// Files with names that are prefixed by any entry in preservedNonGoFiles +// are not deleted. +func pruneNonGoFiles(baseDir string, logger *log.Logger) error { + files, err := calculateNonGoFiles(baseDir) + if err != nil { + return errors.Wrap(err, "could not prune non-Go files") + } + + if err := deleteFiles(files); err != nil { + return err + } + + return nil +} + +// calculateNonGoFiles returns a list of all non-Go files within baseDir. +// Files with names that are prefixed by any entry in preservedNonGoFiles +// are not deleted. +func calculateNonGoFiles(baseDir string) ([]string, error) { + var files []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore directories. + if info.IsDir() { + return nil + } + + // Ignore all Go files. + if strings.HasSuffix(info.Name(), ".go") { + return nil + } + + if !isPreservedNonGoFile(info.Name()) { + files = append(files, path) + } + + return nil + }) + + return files, err +} + +// isPreservedNonGoFile checks if the file name idicates that the file should be +// preserved. It assumes the file is not a Go file (doesn't have a .go suffix). +func isPreservedNonGoFile(name string) bool { + name = strings.ToLower(name) + + for _, prefix := range licenseFilePrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + + for _, substring := range legalFileSubstrings { + if strings.Contains(name, substring) { + return true + } + } + + return false +} + +// pruneGoTestFiles deletes all Go test files (*_test.go) within baseDir. +func pruneGoTestFiles(baseDir string, logger *log.Logger) error { + files, err := calculateGoTestFiles(baseDir) + if err != nil { + return errors.Wrap(err, "could not prune Go test files") + } + + if err := deleteFiles(files); err != nil { + return err + } + + return nil +} + +// calculateGoTestFiles walks over baseDir and returns a list of all +// Go test files (any file that has the name *_test.go). +func calculateGoTestFiles(baseDir string) ([]string, error) { + var files []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore directories. + if info.IsDir() { + return nil + } + + // Ignore any files that is not a Go test file. + if !strings.HasSuffix(info.Name(), "_test.go") { + return nil + } + + files = append(files, path) + + return nil + }) + + return files, err +} + +// pruneEmptyDirs delete all empty directories within baseDir. +func pruneEmptyDirs(baseDir string, logger *log.Logger) error { + empty, err := calculateEmptyDirs(baseDir) + if err != nil { + return err + } + + if logger != nil { + logger.Println("Deleting empty directories:") + } + + for _, dir := range empty { + if logger != nil { + logger.Printf(" %s\n", strings.TrimPrefix(dir, baseDir+string(os.PathSeparator))) + } + if err := os.Remove(dir); err != nil { + return err + } + } + + return nil +} + +// calculateEmptyDirs walks over baseDir and returns a slice of empty directory paths. +func calculateEmptyDirs(baseDir string) ([]string, error) { + var empty []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil + } + + if baseDir == path { + return nil + } + + if !info.IsDir() { + return nil + } + + nonEmpty, err := fs.IsNonEmptyDir(path) + if err != nil { + return err + } else if !nonEmpty { + empty = append(empty, path) + } + + return nil + }) + + return empty, err +} + +func deleteFiles(paths []string) error { + for _, path := range paths { + if err := os.Remove(path); err != nil { + return err + } + } + return nil +} diff --git a/internal/gps/prune_test.go b/internal/gps/prune_test.go new file mode 100644 index 0000000000..fbb9092627 --- /dev/null +++ b/internal/gps/prune_test.go @@ -0,0 +1,224 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gps + +import ( + "io/ioutil" + "log" + "testing" + + "github.com/golang/dep/internal/fs" + "github.com/golang/dep/internal/test" +) + +func TestPruneEmptyDirs(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + h.TempDir(".") + + testcases := []struct { + name string + fs fsTestCase + err bool + }{ + { + "empty-dir", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + }, + after: filesystemState{}, + }, + false, + }, + { + "non-empty-dir", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + }, + false, + }, + { + "nested-empty-dirs", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dirs"}, + {"dirs", "dir1"}, + {"dirs", "dir2"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dirs"}, + }, + }, + }, + false, + }, + { + "mixed-dirs", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir1"}, + {"dir2"}, + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + }, + false, + }, + } + + logger := log.New(ioutil.Discard, "", 0) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + h.TempDir(tc.name) + baseDir := h.Path(tc.name) + tc.fs.before.root = baseDir + tc.fs.after.root = baseDir + + tc.fs.before.setup(t) + + err := pruneEmptyDirs(baseDir, logger) + if tc.err && err == nil { + t.Errorf("expected an error, got nil") + } else if !tc.err && err != nil { + t.Errorf("unexpected error: %s", err) + } + + tc.fs.after.assert(t) + }) + } +} + +func TestCalculateEmptyDirs(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + h.TempDir(".") + + testcases := []struct { + name string + fs filesystemState + emptyDirs int + err bool + }{ + { + "empty-dir", + filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + }, + 1, + false, + }, + { + "non-empty-dir", + filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + 0, + false, + }, + { + "nested-empty-dirs", + filesystemState{ + dirs: []fsPath{ + {"dirs"}, + {"dirs", "dir1"}, + {"dirs", "dir2"}, + }, + }, + 2, + false, + }, + { + "mixed-dirs", + filesystemState{ + dirs: []fsPath{ + {"dir1"}, + {"dir2"}, + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + 2, + false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + h.TempDir(tc.name) + baseDir := h.Path(tc.name) + + tc.fs.root = baseDir + tc.fs.setup(t) + + emptyDirs, err := calculateEmptyDirs(baseDir) + if tc.err && err == nil { + t.Errorf("expected an error, got nil") + } else if !tc.err && err != nil { + t.Errorf("unexpected error: %s", err) + } + if len(emptyDirs) != tc.emptyDirs { + t.Fatalf("expected %d paths, got %d", tc.emptyDirs, len(emptyDirs)) + } + for _, dir := range emptyDirs { + if nonEmpty, err := fs.IsNonEmptyDir(dir); err != nil { + t.Fatalf("unexpected error: %s", err) + } else if nonEmpty { + t.Fatalf("expected %s to be empty, but it wasn't", dir) + } + } + }) + } +} From b14d646c4b0ee5f7e8138ef595be58b05703ae3e Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Thu, 17 Aug 2017 01:57:26 +0300 Subject: [PATCH 2/5] internal/gps: update stripVendor functions Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 12 +---- internal/gps/strip_vendor.go | 34 +++++++------- internal/gps/strip_vendor_windows.go | 66 +++++++++++++++------------- 3 files changed, 53 insertions(+), 59 deletions(-) diff --git a/internal/gps/prune.go b/internal/gps/prune.go index c32e66c9f0..984e030135 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -214,11 +214,7 @@ func pruneNonGoFiles(baseDir string, logger *log.Logger) error { return errors.Wrap(err, "could not prune non-Go files") } - if err := deleteFiles(files); err != nil { - return err - } - - return nil + return deleteFiles(files) } // calculateNonGoFiles returns a list of all non-Go files within baseDir. @@ -279,11 +275,7 @@ func pruneGoTestFiles(baseDir string, logger *log.Logger) error { return errors.Wrap(err, "could not prune Go test files") } - if err := deleteFiles(files); err != nil { - return err - } - - return nil + return deleteFiles(files) } // calculateGoTestFiles walks over baseDir and returns a list of all diff --git a/internal/gps/strip_vendor.go b/internal/gps/strip_vendor.go index 184a0d2d2e..1b224d0e6a 100644 --- a/internal/gps/strip_vendor.go +++ b/internal/gps/strip_vendor.go @@ -16,29 +16,27 @@ func stripVendor(path string, info os.FileInfo, err error) error { return err } - if info.Name() == "vendor" { - if _, err := os.Lstat(path); err != nil { + // Skip anything not named vendor + if info.Name() != "vendor" { + return nil + } + + // If the file is a symlink to a directory, delete the symlink. + if (info.Mode() & os.ModeSymlink) != 0 { + realInfo, err := os.Stat(path) + if err != nil { return err } - - if (info.Mode() & os.ModeSymlink) != 0 { - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { - return os.Remove(path) - } + if realInfo.IsDir() { + return os.Remove(path) } + } - if info.IsDir() { - if err := os.RemoveAll(path); err != nil { - return err - } - return filepath.SkipDir + if info.IsDir() { + if err := os.RemoveAll(path); err != nil { + return err } - - return nil + return filepath.SkipDir } return nil diff --git a/internal/gps/strip_vendor_windows.go b/internal/gps/strip_vendor_windows.go index 7d6acc4a3c..5e51513590 100644 --- a/internal/gps/strip_vendor_windows.go +++ b/internal/gps/strip_vendor_windows.go @@ -14,38 +14,42 @@ func stripVendor(path string, info os.FileInfo, err error) error { return err } - if info.Name() == "vendor" { - if _, err := os.Lstat(path); err == nil { - symlink := (info.Mode() & os.ModeSymlink) != 0 - dir := info.IsDir() - - switch { - case symlink && dir: - // This could be a windows junction directory. Support for these in the - // standard library is spotty, and we could easily delete an important - // folder if we called os.Remove or os.RemoveAll. Just skip these. - // - // TODO: If we could distinguish between junctions and Windows symlinks, - // we might be able to safely delete symlinks, even though junctions are - // dangerous. - return filepath.SkipDir - - case symlink: - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { - return os.Remove(path) - } - - case dir: - if err := os.RemoveAll(path); err != nil { - return err - } - return filepath.SkipDir - } + if info.Name() != "vendor" { + return nil + } + + if _, err := os.Lstat(path); err != nil { + return nil + } + + symlink := (info.Mode() & os.ModeSymlink) != 0 + dir := info.IsDir() + + switch { + case symlink && dir: + // This could be a windows junction directory. Support for these in the + // standard library is spotty, and we could easily delete an important + // folder if we called os.Remove or os.RemoveAll. Just skip these. + // + // TODO: If we could distinguish between junctions and Windows symlinks, + // we might be able to safely delete symlinks, even though junctions are + // dangerous. + return filepath.SkipDir + + case symlink: + realInfo, err := os.Stat(path) + if err != nil { + return err + } + if realInfo.IsDir() { + return os.Remove(path) + } + + case dir: + if err := os.RemoveAll(path); err != nil { + return err } + return filepath.SkipDir } return nil From feac5a42208a64708018355252ef7a00e013da0c Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Sat, 2 Sep 2017 10:37:40 +0300 Subject: [PATCH 3/5] internal/gps: add pruneProject func Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 286 ++++++++++++++++--------------------- internal/gps/prune_test.go | 262 ++++++++++++++++++++++----------- 2 files changed, 302 insertions(+), 246 deletions(-) diff --git a/internal/gps/prune.go b/internal/gps/prune.go index 984e030135..e74a9d484e 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -5,14 +5,12 @@ package gps import ( - "io/ioutil" + "fmt" "log" "os" "path/filepath" - "sort" "strings" - "github.com/golang/dep/internal/fs" "github.com/pkg/errors" ) @@ -58,131 +56,110 @@ var ( // // A Lock must be passed if PruneUnusedPackages is toggled on. func Prune(baseDir string, options PruneOptions, l Lock, logger *log.Logger) error { + // TODO(ibrasho) allow passing specific options per project + for _, lp := range l.Projects() { + projectDir := filepath.Join(baseDir, string(lp.Ident().ProjectRoot)) + err := pruneProject(projectDir, lp, options, logger) + if err != nil { + return err + } + } + + return nil +} + +// pruneProject remove excess files according to the options passed, from +// the lp directory in baseDir. +func pruneProject(baseDir string, lp LockedProject, options PruneOptions, logger *log.Logger) error { + projectDir := filepath.Join(baseDir, string(lp.Ident().ProjectRoot)) + if (options & PruneNestedVendorDirs) != 0 { - if err := pruneNestedVendorDirs(baseDir); err != nil { + if err := pruneNestedVendorDirs(projectDir); err != nil { return err } } if (options & PruneUnusedPackages) != 0 { - if l == nil { - return errors.New("pruning unused packages requires passing a non-nil Lock") - } - if err := pruneUnusedPackages(baseDir, l, logger); err != nil { + if err := pruneUnusedPackages(lp, projectDir, logger); err != nil { return errors.Wrap(err, "failed to prune unused packages") } } if (options & PruneNonGoFiles) != 0 { - if err := pruneNonGoFiles(baseDir, logger); err != nil { + if err := pruneNonGoFiles(projectDir, logger); err != nil { return errors.Wrap(err, "failed to prune non-Go files") } } if (options & PruneGoTestFiles) != 0 { - if err := pruneGoTestFiles(baseDir, logger); err != nil { + if err := pruneGoTestFiles(projectDir, logger); err != nil { return errors.Wrap(err, "failed to prune Go test files") } } - // Delete all empty directories. - if err := pruneEmptyDirs(baseDir, logger); err != nil { - return errors.Wrap(err, "failed to prune empty dirs") - } - return nil } // pruneNestedVendorDirs deletes all nested vendor directories within baseDir. func pruneNestedVendorDirs(baseDir string) error { - return filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { - if !info.IsDir() { - return nil - } - - // Ignore the base vendor directory. - if path == baseDir { - return nil - } - - if err := filepath.Walk(path, stripVendor); err != nil { - return err - } - - // Don't walk into directories again. - return filepath.SkipDir - }) + return filepath.Walk(baseDir, stripVendor) } // pruneUnusedPackages deletes unimported packages found within baseDir. -// Determining whether packages are imported or not is based on the passed Lock. -func pruneUnusedPackages(baseDir string, l Lock, logger *log.Logger) error { - unused, err := calculateUnusedPackages(baseDir, l, logger) +// Determining whether packages are imported or not is based on the passed LockedProject. +func pruneUnusedPackages(lp LockedProject, projectDir string, logger *log.Logger) error { + pr := string(lp.Ident().ProjectRoot) + logger.Printf("Calculating unused packages in %s to prune.\n", pr) + + unusedPackages, err := calculateUnusedPackages(lp, projectDir) if err != nil { - return err + return errors.Wrapf(err, "could not calculate unused packages in %s", pr) } - for _, pkg := range unused { - pkgPath := filepath.Join(baseDir, pkg) - - files, err := ioutil.ReadDir(pkgPath) - if err != nil { - // TODO(ibrasho) Handle this error properly. - // It happens when attempting to ioutil.ReadDir a submodule. - continue - } + logger.Printf("Found the following unused packages in %s:\n", pr) + for pkg := range unusedPackages { + logger.Printf(" * %s\n", filepath.Join(pr, pkg)) + } - // Delete *.go files in the package directory. - for _, file := range files { - // Skip directories and files that don't have a .go suffix. - if file.IsDir() || !strings.HasSuffix(file.Name(), ".go") { - continue - } + unusedPackagesFiles, err := collectUnusedPackagesFiles(projectDir, unusedPackages) + if err != nil { + return errors.Wrapf(err, "could not collect unused packages' files in %s", pr) + } - if err := os.Remove(filepath.Join(pkgPath, file.Name())); err != nil { - return err - } - } + if err := deleteFiles(unusedPackagesFiles); err != nil { + return errors.Wrapf(err, "") } return nil } -// calculateUnusedPackages generates a list of unused packages existing within -// baseDir depending on the imported packages found in the passed Lock. -func calculateUnusedPackages(baseDir string, l Lock, logger *log.Logger) ([]string, error) { - imported := calculateImportedPackages(l) - sort.Strings(imported) - - var unused []string - - if logger != nil { - logger.Println("Calculating unused packages to prune.") - logger.Println("Checking the following packages:") +// calculateUnusedPackages generates a list of unused packages in lp. +func calculateUnusedPackages(lp LockedProject, projectDir string) (map[string]struct{}, error) { + // TODO(ibrasho): optimize this... + unused := make(map[string]struct{}) + imported := make(map[string]struct{}) + for _, pkg := range lp.Packages() { + imported[pkg] = struct{}{} } - err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(projectDir, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - // Ignore baseDir and anything that's not a directory. - if path == baseDir || !info.IsDir() { + // Ignore anything that's not a directory. + if !info.IsDir() { return nil } - pkg := strings.TrimPrefix(path, baseDir+string(filepath.Separator)) - if logger != nil { - logger.Printf(" %s", pkg) + pkg, err := filepath.Rel(projectDir, path) + if err != nil { + return errors.Wrap(err, "unexpected error while calculating unused packages") } + fmt.Println(pkg) - // If pkg is not a parent of an imported package, add it to the - // unused list. - i := sort.Search(len(imported), func(i int) bool { - return pkg <= imported[i] - }) - if i >= len(imported) || !strings.HasPrefix(imported[i], pkg) { - unused = append(unused, path) + if _, ok := imported[pkg]; !ok { + unused[pkg] = struct{}{} } return nil @@ -191,37 +168,62 @@ func calculateUnusedPackages(baseDir string, l Lock, logger *log.Logger) ([]stri return unused, err } -// calculateImportedPackages generates a list of imported packages from -// the passed Lock. -func calculateImportedPackages(l Lock) []string { - var imported []string +// collectUnusedPackagesFiles returns a slice of all files in the unused packages in projectDir. +func collectUnusedPackagesFiles(projectDir string, unusedPackages map[string]struct{}) ([]string, error) { + // TODO(ibrasho): is this useful? + files := make([]string, 0, len(unusedPackages)) + fmt.Println(unusedPackages) - for _, project := range l.Projects() { - projectRoot := string(project.Ident().ProjectRoot) - for _, pkg := range project.Packages() { - imported = append(imported, filepath.Join(projectRoot, pkg)) + err := filepath.Walk(projectDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err } - } - return imported + + // Ignore directories. + if info.IsDir() { + return nil + } + + // Ignore perserved files. + if isPreservedFile(info.Name()) { + return nil + } + + pkg, err := filepath.Rel(projectDir, filepath.Dir(path)) + if err != nil { + return errors.Wrap(err, "unexpected error while calculating unused packages") + } + + if _, ok := unusedPackages[pkg]; ok { + files = append(files, path) + } + + return nil + }) + + return files, err } // pruneNonGoFiles delete all non-Go files existing within baseDir. // Files with names that are prefixed by any entry in preservedNonGoFiles // are not deleted. func pruneNonGoFiles(baseDir string, logger *log.Logger) error { - files, err := calculateNonGoFiles(baseDir) + files, err := collectNonGoFiles(baseDir, logger) if err != nil { - return errors.Wrap(err, "could not prune non-Go files") + return errors.Wrap(err, "could not collect non-Go files") } - return deleteFiles(files) + if err := deleteFiles(files); err != nil { + return errors.Wrap(err, "could not prune Go test files") + } + + return nil } -// calculateNonGoFiles returns a list of all non-Go files within baseDir. -// Files with names that are prefixed by any entry in preservedNonGoFiles -// are not deleted. -func calculateNonGoFiles(baseDir string) ([]string, error) { - var files []string +// collectNonGoFiles returns a slice containing all non-Go files in baseDir. +// Files meeting the checks in isPreservedFile are not returned. +func collectNonGoFiles(baseDir string, logger *log.Logger) ([]string, error) { + files := make([]string, 0) err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -238,19 +240,24 @@ func calculateNonGoFiles(baseDir string) ([]string, error) { return nil } - if !isPreservedNonGoFile(info.Name()) { - files = append(files, path) + // Ignore perserved files. + if isPreservedFile(info.Name()) { + return nil } + files = append(files, path) + return nil }) return files, err } -// isPreservedNonGoFile checks if the file name idicates that the file should be -// preserved. It assumes the file is not a Go file (doesn't have a .go suffix). -func isPreservedNonGoFile(name string) bool { +// isPreservedFile checks if the file name idicates that the file should be +// preserved. +// isPreservedFile checks if the file name contains one of the prefixes in +// licenseFilePrefixes or contains one of the legalFileSubstrings entries. +func isPreservedFile(name string) bool { name = strings.ToLower(name) for _, prefix := range licenseFilePrefixes { @@ -270,18 +277,22 @@ func isPreservedNonGoFile(name string) bool { // pruneGoTestFiles deletes all Go test files (*_test.go) within baseDir. func pruneGoTestFiles(baseDir string, logger *log.Logger) error { - files, err := calculateGoTestFiles(baseDir) + files, err := collectGoTestsFile(baseDir) if err != nil { + return errors.Wrap(err, "could not collect Go test files") + } + + if err := deleteFiles(files); err != nil { return errors.Wrap(err, "could not prune Go test files") } - return deleteFiles(files) + return nil } -// calculateGoTestFiles walks over baseDir and returns a list of all -// Go test files (any file that has the name *_test.go). -func calculateGoTestFiles(baseDir string) ([]string, error) { - var files []string +// collectGoTestsFile returns a slice contains all Go test files (any files +// prefixed with _test.go) in baseDir. +func collectGoTestsFile(baseDir string) ([]string, error) { + files := make([]string, 0) err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -294,71 +305,16 @@ func calculateGoTestFiles(baseDir string) ([]string, error) { } // Ignore any files that is not a Go test file. - if !strings.HasSuffix(info.Name(), "_test.go") { - return nil + if strings.HasSuffix(info.Name(), "_test.go") { + files = append(files, path) } - files = append(files, path) - return nil }) return files, err } -// pruneEmptyDirs delete all empty directories within baseDir. -func pruneEmptyDirs(baseDir string, logger *log.Logger) error { - empty, err := calculateEmptyDirs(baseDir) - if err != nil { - return err - } - - if logger != nil { - logger.Println("Deleting empty directories:") - } - - for _, dir := range empty { - if logger != nil { - logger.Printf(" %s\n", strings.TrimPrefix(dir, baseDir+string(os.PathSeparator))) - } - if err := os.Remove(dir); err != nil { - return err - } - } - - return nil -} - -// calculateEmptyDirs walks over baseDir and returns a slice of empty directory paths. -func calculateEmptyDirs(baseDir string) ([]string, error) { - var empty []string - - err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return nil - } - - if baseDir == path { - return nil - } - - if !info.IsDir() { - return nil - } - - nonEmpty, err := fs.IsNonEmptyDir(path) - if err != nil { - return err - } else if !nonEmpty { - empty = append(empty, path) - } - - return nil - }) - - return empty, err -} - func deleteFiles(paths []string) error { for _, path := range paths { if err := os.Remove(path); err != nil { diff --git a/internal/gps/prune_test.go b/internal/gps/prune_test.go index fbb9092627..de7791cd77 100644 --- a/internal/gps/prune_test.go +++ b/internal/gps/prune_test.go @@ -9,96 +9,196 @@ import ( "log" "testing" - "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/test" ) -func TestPruneEmptyDirs(t *testing.T) { +func TestPruneUnusedPackages(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() h.TempDir(".") + pr := "github.com/test/project" + pi := ProjectIdentifier{ProjectRoot: ProjectRoot(pr)} + testcases := []struct { name string + lp LockedProject fs fsTestCase err bool }{ { - "empty-dir", + "one-package", + LockedProject{ + pi: pi, + pkgs: []string{"."}, + }, fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"dir"}, + files: []fsPath{ + {"main.go"}, + }, + }, + after: filesystemState{ + files: []fsPath{ + {"main.go"}, }, }, - after: filesystemState{}, }, false, }, { - "non-empty-dir", + "nested-package", + LockedProject{ + pi: pi, + pkgs: []string{"pkg"}, + }, fsTestCase{ before: filesystemState{ dirs: []fsPath{ - {"dir"}, + {"pkg"}, }, files: []fsPath{ - {"dir", "file"}, + {"main.go"}, + {"pkg", "main.go"}, }, }, after: filesystemState{ dirs: []fsPath{ - {"dir"}, + {"pkg"}, }, files: []fsPath{ - {"dir", "file"}, + {"pkg", "main.go"}, }, }, }, false, }, { - "nested-empty-dirs", + "complex-project", + LockedProject{ + pi: pi, + pkgs: []string{"pkg", "pkg/nestedpkg/otherpkg"}, + }, fsTestCase{ before: filesystemState{ dirs: []fsPath{ - {"dirs"}, - {"dirs", "dir1"}, - {"dirs", "dir2"}, + {"pkg"}, + {"pkg", "nestedpkg"}, + {"pkg", "nestedpkg", "otherpkg"}, + }, + files: []fsPath{ + {"main.go"}, + {"COPYING"}, + {"pkg", "main.go"}, + {"pkg", "nestedpkg", "main.go"}, + {"pkg", "nestedpkg", "PATENT.md"}, + {"pkg", "nestedpkg", "otherpkg", "main.go"}, }, }, after: filesystemState{ dirs: []fsPath{ - {"dirs"}, + {"pkg"}, + {"pkg", "nestedpkg"}, + {"pkg", "nestedpkg", "otherpkg"}, + }, + files: []fsPath{ + {"COPYING"}, + {"pkg", "main.go"}, + {"pkg", "nestedpkg", "PATENT.md"}, + {"pkg", "nestedpkg", "otherpkg", "main.go"}, + }, + }, + }, + false, + }, + } + + logger := log.New(ioutil.Discard, "", 0) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + h.TempDir(pr) + projectDir := h.Path(pr) + tc.fs.before.root = projectDir + tc.fs.after.root = projectDir + + tc.fs.before.setup(t) + + err := pruneUnusedPackages(tc.lp, projectDir, logger) + if tc.err && err == nil { + t.Errorf("expected an error, got nil") + } else if !tc.err && err != nil { + t.Errorf("unexpected error: %s", err) + } + + tc.fs.after.assert(t) + }) + } +} + +func TestPruneNonGoFiles(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + h.TempDir(".") + + testcases := []struct { + name string + fs fsTestCase + err bool + }{ + { + "one-file", + fsTestCase{ + before: filesystemState{ + files: []fsPath{ + {"README.md"}, + }, + }, + after: filesystemState{}, + }, + false, + }, + { + "multiple-files", + fsTestCase{ + before: filesystemState{ + files: []fsPath{ + {"main.go"}, + {"main_test.go"}, + {"README"}, + }, + }, + after: filesystemState{ + files: []fsPath{ + {"main.go"}, + {"main_test.go"}, }, }, }, false, }, { - "mixed-dirs", + "mixed-files", fsTestCase{ before: filesystemState{ dirs: []fsPath{ - {"dir1"}, - {"dir2"}, - {"dir3"}, - {"dir4"}, + {"dir"}, }, files: []fsPath{ - {"dir3", "file"}, - {"dir4", "file"}, + {"dir", "main.go"}, + {"dir", "main_test.go"}, + {"dir", "db.sqlite"}, }, }, after: filesystemState{ dirs: []fsPath{ - {"dir3"}, - {"dir4"}, + {"dir"}, }, files: []fsPath{ - {"dir3", "file"}, - {"dir4", "file"}, + {"dir", "main.go"}, + {"dir", "main_test.go"}, }, }, }, @@ -117,7 +217,7 @@ func TestPruneEmptyDirs(t *testing.T) { tc.fs.before.setup(t) - err := pruneEmptyDirs(baseDir, logger) + err := pruneNonGoFiles(baseDir, logger) if tc.err && err == nil { t.Errorf("expected an error, got nil") } else if !tc.err && err != nil { @@ -129,96 +229,96 @@ func TestPruneEmptyDirs(t *testing.T) { } } -func TestCalculateEmptyDirs(t *testing.T) { +func TestPruneGoTestFiles(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() h.TempDir(".") testcases := []struct { - name string - fs filesystemState - emptyDirs int - err bool + name string + fs fsTestCase + err bool }{ { - "empty-dir", - filesystemState{ - dirs: []fsPath{ - {"dir"}, + "one-test-file", + fsTestCase{ + before: filesystemState{ + files: []fsPath{ + {"main_test.go"}, + }, }, + after: filesystemState{}, }, - 1, false, }, { - "non-empty-dir", - filesystemState{ - dirs: []fsPath{ - {"dir"}, + "multiple-files", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "main_test.go"}, + {"dir", "main2_test.go"}, + }, }, - files: []fsPath{ - {"dir", "file"}, + after: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, }, }, - 0, false, }, { - "nested-empty-dirs", - filesystemState{ - dirs: []fsPath{ - {"dirs"}, - {"dirs", "dir1"}, - {"dirs", "dir2"}, + "mixed-files", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "main.go"}, + {"dir", "main2.go"}, + {"dir", "main_test.go"}, + {"dir", "main2_test.go"}, + }, }, - }, - 2, - false, - }, - { - "mixed-dirs", - filesystemState{ - dirs: []fsPath{ - {"dir1"}, - {"dir2"}, - {"dir3"}, - {"dir4"}, - }, - files: []fsPath{ - {"dir3", "file"}, - {"dir4", "file"}, + after: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "main.go"}, + {"dir", "main2.go"}, + }, }, }, - 2, false, }, } + logger := log.New(ioutil.Discard, "", 0) + for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { h.TempDir(tc.name) baseDir := h.Path(tc.name) + tc.fs.before.root = baseDir + tc.fs.after.root = baseDir - tc.fs.root = baseDir - tc.fs.setup(t) + tc.fs.before.setup(t) - emptyDirs, err := calculateEmptyDirs(baseDir) + err := pruneGoTestFiles(baseDir, logger) if tc.err && err == nil { t.Errorf("expected an error, got nil") } else if !tc.err && err != nil { t.Errorf("unexpected error: %s", err) } - if len(emptyDirs) != tc.emptyDirs { - t.Fatalf("expected %d paths, got %d", tc.emptyDirs, len(emptyDirs)) - } - for _, dir := range emptyDirs { - if nonEmpty, err := fs.IsNonEmptyDir(dir); err != nil { - t.Fatalf("unexpected error: %s", err) - } else if nonEmpty { - t.Fatalf("expected %s to be empty, but it wasn't", dir) - } - } + + tc.fs.after.assert(t) }) } } From 98dd01efa9bdf344896bfb269f14bb1892ebe99c Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Fri, 8 Sep 2017 18:45:35 +0300 Subject: [PATCH 4/5] Add AUTHORS and CONTRIBUTERS to legal files list Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gps/prune.go b/internal/gps/prune.go index e74a9d484e..7bfc7d8084 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -42,6 +42,8 @@ var ( // legalFileSubstrings contains substrings that are likey part of a legal // declaration file. legalFileSubstrings = []string{ + "authors", + "contributors", "legal", "notice", "disclaimer", From 9b6892cfce954789ab4737f675c1b7c71791f54d Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Sat, 9 Sep 2017 23:21:04 +0300 Subject: [PATCH 5/5] internal/gps: export PruneProject and fix minor nits Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/internal/gps/prune.go b/internal/gps/prune.go index 7bfc7d8084..b67e01b1ff 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -5,7 +5,6 @@ package gps import ( - "fmt" "log" "os" "path/filepath" @@ -23,7 +22,8 @@ const ( // PruneUnusedPackages indicates if unused Go packages should be pruned. PruneUnusedPackages // PruneNonGoFiles indicates if non-Go files should be pruned. - // LICENSE & COPYING files are kept for convience. + // Files matching licenseFilePrefixes and legalFileSubstrings are kept in + // an attempt to comply with legal requirements. PruneNonGoFiles // PruneGoTestFiles indicates if Go test files should be pruned. PruneGoTestFiles @@ -61,7 +61,7 @@ func Prune(baseDir string, options PruneOptions, l Lock, logger *log.Logger) err // TODO(ibrasho) allow passing specific options per project for _, lp := range l.Projects() { projectDir := filepath.Join(baseDir, string(lp.Ident().ProjectRoot)) - err := pruneProject(projectDir, lp, options, logger) + err := PruneProject(projectDir, lp, options, logger) if err != nil { return err } @@ -70,9 +70,9 @@ func Prune(baseDir string, options PruneOptions, l Lock, logger *log.Logger) err return nil } -// pruneProject remove excess files according to the options passed, from +// PruneProject remove excess files according to the options passed, from // the lp directory in baseDir. -func pruneProject(baseDir string, lp LockedProject, options PruneOptions, logger *log.Logger) error { +func PruneProject(baseDir string, lp LockedProject, options PruneOptions, logger *log.Logger) error { projectDir := filepath.Join(baseDir, string(lp.Ident().ProjectRoot)) if (options & PruneNestedVendorDirs) != 0 { @@ -158,8 +158,8 @@ func calculateUnusedPackages(lp LockedProject, projectDir string) (map[string]st if err != nil { return errors.Wrap(err, "unexpected error while calculating unused packages") } - fmt.Println(pkg) + pkg = filepath.ToSlash(pkg) if _, ok := imported[pkg]; !ok { unused[pkg] = struct{}{} } @@ -174,7 +174,6 @@ func calculateUnusedPackages(lp LockedProject, projectDir string) (map[string]st func collectUnusedPackagesFiles(projectDir string, unusedPackages map[string]struct{}) ([]string, error) { // TODO(ibrasho): is this useful? files := make([]string, 0, len(unusedPackages)) - fmt.Println(unusedPackages) err := filepath.Walk(projectDir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -196,6 +195,7 @@ func collectUnusedPackagesFiles(projectDir string, unusedPackages map[string]str return errors.Wrap(err, "unexpected error while calculating unused packages") } + pkg = filepath.ToSlash(pkg) if _, ok := unusedPackages[pkg]; ok { files = append(files, path) } @@ -255,10 +255,8 @@ func collectNonGoFiles(baseDir string, logger *log.Logger) ([]string, error) { return files, err } -// isPreservedFile checks if the file name idicates that the file should be -// preserved. -// isPreservedFile checks if the file name contains one of the prefixes in -// licenseFilePrefixes or contains one of the legalFileSubstrings entries. +// isPreservedFile checks if the file name indicates that the file should be +// preserved based on licenseFilePrefixes or legalFileSubstrings. func isPreservedFile(name string) bool { name = strings.ToLower(name) @@ -279,7 +277,7 @@ func isPreservedFile(name string) bool { // pruneGoTestFiles deletes all Go test files (*_test.go) within baseDir. func pruneGoTestFiles(baseDir string, logger *log.Logger) error { - files, err := collectGoTestsFile(baseDir) + files, err := collectGoTestFiles(baseDir) if err != nil { return errors.Wrap(err, "could not collect Go test files") } @@ -291,9 +289,9 @@ func pruneGoTestFiles(baseDir string, logger *log.Logger) error { return nil } -// collectGoTestsFile returns a slice contains all Go test files (any files +// collectGoTestFiles returns a slice contains all Go test files (any files // prefixed with _test.go) in baseDir. -func collectGoTestsFile(baseDir string) ([]string, error) { +func collectGoTestFiles(baseDir string) ([]string, error) { files := make([]string, 0) err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error {