From 4e4925d3825045e0252abcf49cc403b8f35b7e29 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Wed, 29 Nov 2017 09:35:27 +0400 Subject: [PATCH 01/11] cmd/dep: remove dep prune command Signed-off-by: Ibrahim AshShohail --- cmd/dep/main.go | 1 - cmd/dep/prune.go | 206 ------------------ cmd/dep/prune_test.go | 50 ----- .../prune/without_lock/final/Gopkg.toml | 3 - .../prune/without_lock/initial/Gopkg.toml | 3 - .../prune/without_lock/testcase.json | 6 - 6 files changed, 269 deletions(-) delete mode 100644 cmd/dep/prune.go delete mode 100644 cmd/dep/prune_test.go delete mode 100644 cmd/dep/testdata/harness_tests/prune/without_lock/final/Gopkg.toml delete mode 100644 cmd/dep/testdata/harness_tests/prune/without_lock/initial/Gopkg.toml delete mode 100644 cmd/dep/testdata/harness_tests/prune/without_lock/testcase.json diff --git a/cmd/dep/main.go b/cmd/dep/main.go index aa29e0a589..7aba6af43a 100644 --- a/cmd/dep/main.go +++ b/cmd/dep/main.go @@ -67,7 +67,6 @@ func (c *Config) Run() int { &statusCommand{}, &ensureCommand{}, &hashinCommand{}, - &pruneCommand{}, &versionCommand{}, } diff --git a/cmd/dep/prune.go b/cmd/dep/prune.go deleted file mode 100644 index 202a0980a6..0000000000 --- a/cmd/dep/prune.go +++ /dev/null @@ -1,206 +0,0 @@ -// Copyright 2016 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 main - -import ( - "bytes" - "flag" - "io/ioutil" - "log" - "os" - "path/filepath" - "sort" - "strings" - - "github.com/golang/dep" - "github.com/golang/dep/gps" - "github.com/golang/dep/gps/pkgtree" - "github.com/golang/dep/internal/fs" - "github.com/pkg/errors" -) - -const pruneShortHelp = `Prune the vendor tree of unused packages` -const pruneLongHelp = ` -Prune is used to remove unused packages from your vendor tree. - -STABILITY NOTICE: this command creates problems for vendor/ verification. As -such, it may be removed and/or moved out into a separate project later on. -` - -type pruneCommand struct { -} - -func (cmd *pruneCommand) Name() string { return "prune" } -func (cmd *pruneCommand) Args() string { return "" } -func (cmd *pruneCommand) ShortHelp() string { return pruneShortHelp } -func (cmd *pruneCommand) LongHelp() string { return pruneLongHelp } -func (cmd *pruneCommand) Hidden() bool { return false } - -func (cmd *pruneCommand) Register(fs *flag.FlagSet) { -} - -func (cmd *pruneCommand) Run(ctx *dep.Ctx, args []string) error { - p, err := ctx.LoadProject() - if err != nil { - return err - } - - sm, err := ctx.SourceManager() - if err != nil { - return err - } - sm.UseDefaultSignalHandling() - defer sm.Release() - - // While the network churns on ListVersions() requests, statically analyze - // code from the current project. - ptree, err := pkgtree.ListPackages(p.ResolvedAbsRoot, string(p.ImportRoot)) - if err != nil { - return errors.Wrap(err, "analysis of local packages failed: %v") - } - - // Set up a solver in order to check the InputHash. - params := p.MakeParams() - params.RootPackageTree = ptree - - if ctx.Verbose { - params.TraceLogger = ctx.Err - } - - s, err := gps.Prepare(params, sm) - if err != nil { - return errors.Wrap(err, "could not set up solver for input hashing") - } - - if p.Lock == nil { - return errors.Errorf("Gopkg.lock must exist for prune to know what files are safe to remove.") - } - - if !bytes.Equal(s.HashInputs(), p.Lock.SolveMeta.InputsDigest) { - return errors.Errorf("Gopkg.lock is out of sync; run dep ensure before pruning.") - } - - pruneLogger := ctx.Err - if !ctx.Verbose { - pruneLogger = log.New(ioutil.Discard, "", 0) - } - return pruneProject(p, sm, pruneLogger) -} - -// pruneProject removes unused packages from a project. -func pruneProject(p *dep.Project, sm gps.SourceManager, logger *log.Logger) error { - td, err := ioutil.TempDir(os.TempDir(), "dep") - if err != nil { - return errors.Wrap(err, "error while creating temp dir for writing manifest/lock/vendor") - } - defer os.RemoveAll(td) - - if err := gps.WriteDepTree(td, p.Lock, sm, true, logger); err != nil { - return err - } - - var toKeep []string - for _, project := range p.Lock.Projects() { - projectRoot := string(project.Ident().ProjectRoot) - for _, pkg := range project.Packages() { - toKeep = append(toKeep, filepath.Join(projectRoot, pkg)) - } - } - - toDelete, err := calculatePrune(td, toKeep, logger) - if err != nil { - return err - } - - if len(toDelete) > 0 { - logger.Println("Calculated the following directories to prune:") - for _, d := range toDelete { - logger.Printf(" %s\n", d) - } - } else { - logger.Println("No directories found to prune") - } - - if err := deleteDirs(toDelete); err != nil { - return err - } - - vpath := filepath.Join(p.AbsRoot, "vendor") - vendorbak := vpath + ".orig" - var failerr error - if _, err := os.Stat(vpath); err == nil { - // Move out the old vendor dir. just do it into an adjacent dir, to - // try to mitigate the possibility of a pointless cross-filesystem - // move with a temp directory. - if _, err := os.Stat(vendorbak); err == nil { - // If the adjacent dir already exists, bite the bullet and move - // to a proper tempdir. - vendorbak = filepath.Join(td, "vendor.orig") - } - failerr = fs.RenameWithFallback(vpath, vendorbak) - if failerr != nil { - goto fail - } - } - - // Move in the new one. - failerr = fs.RenameWithFallback(td, vpath) - if failerr != nil { - goto fail - } - - os.RemoveAll(vendorbak) - - return nil - -fail: - fs.RenameWithFallback(vendorbak, vpath) - return failerr -} - -func calculatePrune(vendorDir string, keep []string, logger *log.Logger) ([]string, error) { - logger.Println("Calculating prune. Checking the following packages:") - sort.Strings(keep) - toDelete := []string{} - err := filepath.Walk(vendorDir, func(path string, info os.FileInfo, err error) error { - if _, err := os.Lstat(path); err != nil { - return nil - } - if !info.IsDir() { - return nil - } - if path == vendorDir { - return nil - } - - name := strings.TrimPrefix(path, vendorDir+string(filepath.Separator)) - logger.Printf(" %s", name) - i := sort.Search(len(keep), func(i int) bool { - return name <= keep[i] - }) - if i >= len(keep) || !strings.HasPrefix(keep[i], name) { - toDelete = append(toDelete, path) - } - return nil - }) - return toDelete, err -} - -func deleteDirs(toDelete []string) error { - // sort by length so we delete sub dirs first - sort.Sort(byLen(toDelete)) - for _, path := range toDelete { - if err := os.RemoveAll(path); err != nil { - return err - } - } - return nil -} - -type byLen []string - -func (a byLen) Len() int { return len(a) } -func (a byLen) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a byLen) Less(i, j int) bool { return len(a[i]) > len(a[j]) } diff --git a/cmd/dep/prune_test.go b/cmd/dep/prune_test.go deleted file mode 100644 index 8a9c1d1d96..0000000000 --- a/cmd/dep/prune_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// 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 main - -import ( - "io/ioutil" - "log" - "path/filepath" - "reflect" - "sort" - "testing" - - "github.com/golang/dep/internal/test" -) - -func TestCalculatePrune(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - vendorDir := "vendor" - h.TempDir(vendorDir) - h.TempDir(filepath.Join(vendorDir, "github.com/keep/pkg/sub")) - h.TempDir(filepath.Join(vendorDir, "github.com/prune/pkg/sub")) - - toKeep := []string{ - filepath.FromSlash("github.com/keep/pkg"), - filepath.FromSlash("github.com/keep/pkg/sub"), - } - - discardLogger := log.New(ioutil.Discard, "", 0) - - got, err := calculatePrune(h.Path(vendorDir), toKeep, discardLogger) - if err != nil { - t.Fatal(err) - } - - sort.Sort(byLen(got)) - - want := []string{ - h.Path(filepath.Join(vendorDir, "github.com/prune/pkg/sub")), - h.Path(filepath.Join(vendorDir, "github.com/prune/pkg")), - h.Path(filepath.Join(vendorDir, "github.com/prune")), - } - - if !reflect.DeepEqual(want, got) { - t.Fatalf("calculated prune paths are not as expected.\n(WNT) %s\n(GOT) %s", want, got) - } -} diff --git a/cmd/dep/testdata/harness_tests/prune/without_lock/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/prune/without_lock/final/Gopkg.toml deleted file mode 100644 index 94deb714a4..0000000000 --- a/cmd/dep/testdata/harness_tests/prune/without_lock/final/Gopkg.toml +++ /dev/null @@ -1,3 +0,0 @@ -[[constraint]] - name = "github.com/sdboyer/deptest" - version = "^0.8.0" diff --git a/cmd/dep/testdata/harness_tests/prune/without_lock/initial/Gopkg.toml b/cmd/dep/testdata/harness_tests/prune/without_lock/initial/Gopkg.toml deleted file mode 100644 index 94deb714a4..0000000000 --- a/cmd/dep/testdata/harness_tests/prune/without_lock/initial/Gopkg.toml +++ /dev/null @@ -1,3 +0,0 @@ -[[constraint]] - name = "github.com/sdboyer/deptest" - version = "^0.8.0" diff --git a/cmd/dep/testdata/harness_tests/prune/without_lock/testcase.json b/cmd/dep/testdata/harness_tests/prune/without_lock/testcase.json deleted file mode 100644 index da6463ed6a..0000000000 --- a/cmd/dep/testdata/harness_tests/prune/without_lock/testcase.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "commands": [ - ["prune"] - ], - "error-expected": "Gopkg.lock must exist" -} From 869f2d711ab3322d2fa849ef2a536166dc9a9d95 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Wed, 29 Nov 2017 09:37:45 +0400 Subject: [PATCH 02/11] gps: refactor the filesystemState struct Signed-off-by: Ibrahim AshShohail --- gps/filesystem.go | 77 ++++++++++++++++++++++++++++++++++++++ gps/filesystem_test.go | 85 +++++++++++++++--------------------------- 2 files changed, 107 insertions(+), 55 deletions(-) create mode 100644 gps/filesystem.go diff --git a/gps/filesystem.go b/gps/filesystem.go new file mode 100644 index 0000000000..c3cbfa772a --- /dev/null +++ b/gps/filesystem.go @@ -0,0 +1,77 @@ +// 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 ( + "os" + "path/filepath" +) + +// filesystemState represents the state of a file system. +type filesystemState struct { + root string + dirs []string + files []string + links []fsLink +} + +// fsLink represents a symbolic link. +type fsLink struct { + path string + to string +} + +// deriveFilesystemState returns a filesystemState based on the state of +// the filesystem on root. +func deriveFilesystemState(root string) (filesystemState, error) { + fs := filesystemState{ + root: root, + } + + err := filepath.Walk(fs.root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if path == fs.root { + return nil + } + + relPath, err := filepath.Rel(fs.root, path) + if err != nil { + return err + } + + if (info.Mode() & os.ModeSymlink) != 0 { + eval, err := filepath.EvalSymlinks(path) + if err != nil { + return err + } + + fs.links = append(fs.links, fsLink{ + path: relPath, + to: eval, + }) + + return nil + } + + if info.IsDir() { + fs.dirs = append(fs.dirs, relPath) + + return nil + } + + fs.files = append(fs.files, relPath) + + return nil + }) + + if err != nil { + return filesystemState{}, err + } + + return fs, nil +} diff --git a/gps/filesystem_test.go b/gps/filesystem_test.go index faa4dd5f5c..78c28cdac1 100644 --- a/gps/filesystem_test.go +++ b/gps/filesystem_test.go @@ -12,54 +12,34 @@ import ( // This file contains utilities for running tests around file system state. -// fspath represents a file system path in an OS-agnostic way. -type fsPath []string - -func (f fsPath) String() string { return filepath.Join(f...) } - -func (f fsPath) prepend(prefix string) fsPath { - p := fsPath{filepath.FromSlash(prefix)} - return append(p, f...) -} - type fsTestCase struct { before, after filesystemState } -// filesystemState represents the state of a file system. It has a setup method -// which inflates its state to the actual host file system, and an assert -// method which checks that the actual file system matches the described state. -type filesystemState struct { - root string - dirs []fsPath - files []fsPath - links []fsLink -} - -// assert makes sure that the fs state matches the state of the actual host -// file system -func (fs filesystemState) assert(t *testing.T) { +// assert makes sure that the tc.after state matches the state of the actual host +// file system at tc.after.root. +func (tc fsTestCase) assert(t *testing.T) { dirMap := make(map[string]bool) fileMap := make(map[string]bool) linkMap := make(map[string]bool) - for _, d := range fs.dirs { - dirMap[d.prepend(fs.root).String()] = true + for _, d := range tc.after.dirs { + dirMap[filepath.Join(tc.after.root, d)] = true } - for _, f := range fs.files { - fileMap[f.prepend(fs.root).String()] = true + for _, f := range tc.after.files { + fileMap[filepath.Join(tc.after.root, f)] = true } - for _, l := range fs.links { - linkMap[l.path.prepend(fs.root).String()] = true + for _, l := range tc.after.links { + linkMap[filepath.Join(tc.after.root, l.path)] = true } - err := filepath.Walk(fs.root, func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(tc.after.root, func(path string, info os.FileInfo, err error) error { if err != nil { t.Errorf("filepath.Walk path=%q err=%q", path, err) return err } - if path == fs.root { + if path == tc.after.root { return nil } @@ -106,32 +86,27 @@ func (fs filesystemState) assert(t *testing.T) { } } -// fsLink represents a symbolic link. -type fsLink struct { - path fsPath - to string -} - -// setup inflates fs onto the actual host file system -func (fs filesystemState) setup(t *testing.T) { - fs.setupDirs(t) - fs.setupFiles(t) - fs.setupLinks(t) +// setup inflates fs onto the actual host file system at tc.before.root. +// It doesn't delete existing files and should be used on empty roots only. +func (tc fsTestCase) setup(t *testing.T) { + tc.setupDirs(t) + tc.setupFiles(t) + tc.setupLinks(t) } -func (fs filesystemState) setupDirs(t *testing.T) { - for _, dir := range fs.dirs { - p := dir.prepend(fs.root) - if err := os.MkdirAll(p.String(), 0777); err != nil { +func (tc fsTestCase) setupDirs(t *testing.T) { + for _, dir := range tc.before.dirs { + p := filepath.Join(tc.before.root, dir) + if err := os.MkdirAll(p, 0777); err != nil { t.Fatalf("os.MkdirAll(%q, 0777) err=%q", p, err) } } } -func (fs filesystemState) setupFiles(t *testing.T) { - for _, file := range fs.files { - p := file.prepend(fs.root) - f, err := os.Create(p.String()) +func (tc fsTestCase) setupFiles(t *testing.T) { + for _, file := range tc.before.files { + p := filepath.Join(tc.before.root, file) + f, err := os.Create(p) if err != nil { t.Fatalf("os.Create(%q) err=%q", p, err) } @@ -141,17 +116,17 @@ func (fs filesystemState) setupFiles(t *testing.T) { } } -func (fs filesystemState) setupLinks(t *testing.T) { - for _, link := range fs.links { - p := link.path.prepend(fs.root) +func (tc fsTestCase) setupLinks(t *testing.T) { + for _, link := range tc.before.links { + p := filepath.Join(tc.before.root, link.path) // On Windows, relative symlinks confuse filepath.Walk. This is golang/go // issue 17540. So, we'll just sigh and do absolute links, assuming they are // relative to the directory of link.path. - dir := filepath.Dir(p.String()) + dir := filepath.Dir(p) to := filepath.Join(dir, link.to) - if err := os.Symlink(to, p.String()); err != nil { + if err := os.Symlink(to, p); err != nil { t.Fatalf("os.Symlink(%q, %q) err=%q", to, p, err) } } From 2a1be0fc0649db189730851d60c14588e2f27f48 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Wed, 29 Nov 2017 09:41:56 +0400 Subject: [PATCH 03/11] gps: update prune functions to use filesystemState Signed-off-by: Ibrahim AshShohail --- gps/prune.go | 308 +++++++++--------- gps/prune_test.go | 273 ++++++++++------ gps/prune_vendor.go | 29 ++ gps/prune_vendor_test.go | 76 +++++ gps/prune_vendor_windows.go | 38 +++ ...s_test.go => prune_vendor_windows_test.go} | 98 +++--- gps/strip_vendor.go | 39 --- gps/strip_vendor_nonwindows_test.go | 171 ---------- gps/strip_vendor_test.go | 71 ---- gps/strip_vendor_windows.go | 52 --- 10 files changed, 511 insertions(+), 644 deletions(-) create mode 100644 gps/prune_vendor.go create mode 100644 gps/prune_vendor_test.go create mode 100644 gps/prune_vendor_windows.go rename gps/{strip_vendor_windows_test.go => prune_vendor_windows_test.go} (57%) delete mode 100644 gps/strip_vendor.go delete mode 100644 gps/strip_vendor_nonwindows_test.go delete mode 100644 gps/strip_vendor_test.go delete mode 100644 gps/strip_vendor_windows.go diff --git a/gps/prune.go b/gps/prune.go index 84ac74eb6c..357dae3570 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" + "github.com/golang/dep/internal/fs" "github.com/pkg/errors" ) @@ -56,204 +57,171 @@ var ( } ) -// 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 { - // 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)) + fs, err := deriveFilesystemState(baseDir) + if err != nil { + return errors.Wrap(err, "could not derive filesystem state") + } if (options & PruneNestedVendorDirs) != 0 { - if err := pruneNestedVendorDirs(projectDir); err != nil { + if err := pruneVendorDirs(fs); err != nil { return errors.Wrapf(err, "failed to prune nested vendor directories") } } if (options & PruneUnusedPackages) != 0 { - if err := pruneUnusedPackages(lp, projectDir, logger); err != nil { + if _, err := pruneUnusedPackages(lp, fs); err != nil { return errors.Wrap(err, "failed to prune unused packages") } } if (options & PruneNonGoFiles) != 0 { - if err := pruneNonGoFiles(projectDir, logger); err != nil { + if err := pruneNonGoFiles(fs); err != nil { return errors.Wrap(err, "failed to prune non-Go files") } } if (options & PruneGoTestFiles) != 0 { - if err := pruneGoTestFiles(projectDir, logger); err != nil { + if err := pruneGoTestFiles(fs); err != nil { return errors.Wrap(err, "failed to prune Go test files") } } - return nil -} + if err := deleteEmptyDirs(fs); err != nil { + return errors.Wrap(err, "could not delete empty dirs") + } -// pruneNestedVendorDirs deletes all nested vendor directories within baseDir. -func pruneNestedVendorDirs(baseDir string) error { - return filepath.Walk(baseDir, stripVendor) + return nil } -// pruneUnusedPackages deletes unimported packages found within baseDir. -// 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) +// pruneVendorDirs deletes all nested vendor directories within baseDir. +func pruneVendorDirs(fs filesystemState) error { + toDelete := collectNestedVendorDirs(fs) - unusedPackages, err := calculateUnusedPackages(lp, projectDir) - if err != nil { - return errors.Wrapf(err, "could not calculate unused packages in %s", pr) + for _, path := range toDelete { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } } - logger.Printf("Found the following unused packages in %s:\n", pr) - for pkg := range unusedPackages { - logger.Printf(" * %s\n", filepath.Join(pr, pkg)) - } + return nil +} - unusedPackagesFiles, err := collectUnusedPackagesFiles(projectDir, unusedPackages) - if err != nil { - return errors.Wrapf(err, "could not collect unused packages' files in %s", pr) - } +// pruneUnusedPackages deletes unimported packages found in fsState. +// Determining whether packages are imported or not is based on the passed LockedProject. +func pruneUnusedPackages(lp LockedProject, fsState filesystemState) (map[string]interface{}, error) { + unusedPackages := calculateUnusedPackages(lp, fsState) + toDelete := collectUnusedPackagesFiles(fsState, unusedPackages) - if err := deleteFiles(unusedPackagesFiles); err != nil { - return errors.Wrapf(err, "") + for _, path := range toDelete { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return nil, err + } } - return nil + return unusedPackages, nil } // calculateUnusedPackages generates a list of unused packages in lp. -func calculateUnusedPackages(lp LockedProject, projectDir string) (map[string]struct{}, error) { - unused := make(map[string]struct{}) - imported := make(map[string]struct{}) +func calculateUnusedPackages(lp LockedProject, fsState filesystemState) map[string]interface{} { + unused := make(map[string]interface{}) + imported := make(map[string]interface{}) + for _, pkg := range lp.Packages() { - imported[pkg] = struct{}{} + imported[pkg] = nil } - err := filepath.Walk(projectDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // Ignore anything that's not a directory. - if !info.IsDir() { - return nil - } + // Add the root package if it's not imported. + if _, ok := imported["."]; !ok { + unused["."] = nil + } - pkg, err := filepath.Rel(projectDir, path) - if err != nil { - return errors.Wrap(err, "unexpected error while calculating unused packages") - } + for _, dirPath := range fsState.dirs { + pkg := filepath.ToSlash(dirPath) - pkg = filepath.ToSlash(pkg) if _, ok := imported[pkg]; !ok { - unused[pkg] = struct{}{} + unused[pkg] = nil } + } - return nil - }) - - return unused, err + return unused } -// collectUnusedPackagesFiles returns a slice of all files in the unused packages in projectDir. -func collectUnusedPackagesFiles(projectDir string, unusedPackages map[string]struct{}) ([]string, error) { +// collectUnusedPackagesFiles returns a slice of all files in the unused +// packages based on fsState. +func collectUnusedPackagesFiles(fsState filesystemState, unusedPackages map[string]interface{}) []string { + // TODO(ibrasho): is this useful? files := make([]string, 0, len(unusedPackages)) - err := filepath.Walk(projectDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err + for _, path := range fsState.files { + // Keep perserved files. + if isPreservedFile(filepath.Base(path)) { + continue } - // Ignore directories. - if info.IsDir() { - return nil - } + pkg := filepath.ToSlash(filepath.Dir(path)) - // Ignore preserved 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") - } - - pkg = filepath.ToSlash(pkg) if _, ok := unusedPackages[pkg]; ok { - files = append(files, path) + files = append(files, filepath.Join(fsState.root, 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 := collectNonGoFiles(baseDir, logger) - if err != nil { - return errors.Wrap(err, "could not collect non-Go files") - } - - if err := deleteFiles(files); err != nil { - return errors.Wrap(err, "could not prune Go test files") } - return nil + return files } -// 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 { - return err +// pruneNonGoFiles delete all non-Go files existing in fsState. +// +// Files matching licenseFilePrefixes and legalFileSubstrings are not pruned. +func pruneNonGoFiles(fsState filesystemState) error { + // TODO(ibrasho) detemine a sane capacity + toDelete := make([]string, 0, len(fsState.files)/4) + + for _, path := range fsState.files { + ext := fileExt(path) + + // Refer to: https://sourcegraph.com/github.com/golang/go/-/blob/src/go/build/build.go#L750 + switch ext { + case ".go": + continue + case ".c": + continue + case ".cc", ".cpp", ".cxx": + continue + case ".m": + continue + case ".h", ".hh", ".hpp", ".hxx": + continue + case ".f", ".F", ".for", ".f90": + continue + case ".s": + continue + case ".S": + continue + case ".swig": + continue + case ".swigcxx": + continue + case ".syso": + continue } - // Ignore directories. - if info.IsDir() { - return nil + // Ignore perserved files. + if isPreservedFile(filepath.Base(path)) { + continue } - // Ignore all Go files. - if strings.HasSuffix(info.Name(), ".go") { - return nil - } + toDelete = append(toDelete, filepath.Join(fsState.root, path)) + } - // Ignore preserved files. - if isPreservedFile(info.Name()) { - return nil + for _, path := range toDelete { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err } + } - files = append(files, path) - - return nil - }) - - return files, err + return nil } // isPreservedFile checks if the file name indicates that the file should be @@ -276,51 +244,73 @@ func isPreservedFile(name string) bool { return false } -// pruneGoTestFiles deletes all Go test files (*_test.go) within baseDir. -func pruneGoTestFiles(baseDir string, logger *log.Logger) error { - files, err := collectGoTestFiles(baseDir) - if err != nil { - return errors.Wrap(err, "could not collect Go test files") +// pruneGoTestFiles deletes all Go test files (*_test.go) in fsState. +func pruneGoTestFiles(fsState filesystemState) error { + // TODO(ibrasho) detemine a sane capacity + toDelete := make([]string, 0, len(fsState.files)/2) + + for _, path := range fsState.files { + if strings.HasSuffix(path, "_test.go") { + toDelete = append(toDelete, filepath.Join(fsState.root, path)) + } } - if err := deleteFiles(files); err != nil { - return errors.Wrap(err, "could not prune Go test files") + for _, path := range toDelete { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } } return nil } -// collectGoTestFiles returns a slice contains all Go test files (any files -// prefixed with _test.go) in baseDir. -func collectGoTestFiles(baseDir string) ([]string, error) { - files := make([]string, 0) +func deleteEmptyDirs(fsState filesystemState) error { + for _, dir := range fsState.dirs { + path := filepath.Join(fsState.root, dir) - err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + notEmpty, err := fs.IsNonEmptyDir(path) if err != nil { return err } - // Ignore directories. - if info.IsDir() { - return nil + if !notEmpty { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } } + } - // Ignore any files that is not a Go test file. - if strings.HasSuffix(info.Name(), "_test.go") { - files = append(files, path) - } + return nil +} - return nil - }) +func deleteEmptyDirsFromSlice(deletedFiles []string) error { + dirs := make(map[string]struct{}) - return files, err -} + for _, path := range deletedFiles { + dirs[filepath.Dir(path)] = struct{}{} + } -func deleteFiles(paths []string) error { - for _, path := range paths { - if err := os.Remove(path); err != nil { + for path := range dirs { + notEmpty, err := fs.IsNonEmptyDir(path) + if err != nil { return err } + + if !notEmpty { + err := os.Remove(path) + if err != nil && !os.IsNotExist(err) { + return err + } + } } + return nil } + +func fileExt(name string) string { + i := strings.LastIndex(name, ".") + if i < 0 { + return "" + } + return name[i:] +} diff --git a/gps/prune_test.go b/gps/prune_test.go index de7791cd77..aae1ec9a75 100644 --- a/gps/prune_test.go +++ b/gps/prune_test.go @@ -12,13 +12,65 @@ import ( "github.com/golang/dep/internal/test" ) +func TestRootPruneOptions_PruneOptionsFor(t *testing.T) { + pr := ProjectRoot("github.com/golang/dep") + + o := RootPruneOptions{ + PruneOptions: PruneNestedVendorDirs, + ProjectOptions: PruneProjectOptions{ + pr: PruneGoTestFiles, + }, + } + + if (o.PruneOptionsFor(pr) & PruneGoTestFiles) != PruneGoTestFiles { + t.Fatalf("invalid prune options.\n\t(GOT): %d\n\t(WNT): %d", o.PruneOptionsFor(pr), PruneGoTestFiles) + } +} + +func TestPruneProject(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + pr := "github.com/project/repository" + h.TempDir(pr) + + baseDir := h.Path(".") + lp := LockedProject{ + pi: ProjectIdentifier{ + ProjectRoot: ProjectRoot(pr), + }, + pkgs: []string{}, + } + + options := PruneNestedVendorDirs | PruneNonGoFiles | PruneGoTestFiles | PruneUnusedPackages + logger := log.New(ioutil.Discard, "", 0) + + err := PruneProject(baseDir, lp, options, logger) + if err != nil { + t.Fatal(err) + } +} + +// func TestPruneVendorDirs(t *testing.T) { +// h := test.NewHelper(t) +// defer h.Cleanup() + +// h.TempDir(".") +// baseDir := h.Path(".") + +// err := pruneVendorDirs(baseDir) +// if err != nil { +// t.Fatal(err) +// } +// } + func TestPruneUnusedPackages(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() h.TempDir(".") - pr := "github.com/test/project" + pr := "github.com/sample/repository" pi := ProjectIdentifier{ProjectRoot: ProjectRoot(pr)} testcases := []struct { @@ -30,18 +82,20 @@ func TestPruneUnusedPackages(t *testing.T) { { "one-package", LockedProject{ - pi: pi, - pkgs: []string{"."}, + pi: pi, + pkgs: []string{ + ".", + }, }, fsTestCase{ before: filesystemState{ - files: []fsPath{ - {"main.go"}, + files: []string{ + "main.go", }, }, after: filesystemState{ - files: []fsPath{ - {"main.go"}, + files: []string{ + "main.go", }, }, }, @@ -50,25 +104,27 @@ func TestPruneUnusedPackages(t *testing.T) { { "nested-package", LockedProject{ - pi: pi, - pkgs: []string{"pkg"}, + pi: pi, + pkgs: []string{ + "pkg", + }, }, fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"pkg"}, + dirs: []string{ + "pkg", }, - files: []fsPath{ - {"main.go"}, - {"pkg", "main.go"}, + files: []string{ + "main.go", + "pkg/main.go", }, }, after: filesystemState{ - dirs: []fsPath{ - {"pkg"}, + dirs: []string{ + "pkg", }, - files: []fsPath{ - {"pkg", "main.go"}, + files: []string{ + "pkg/main.go", }, }, }, @@ -77,36 +133,39 @@ func TestPruneUnusedPackages(t *testing.T) { { "complex-project", LockedProject{ - pi: pi, - pkgs: []string{"pkg", "pkg/nestedpkg/otherpkg"}, + pi: pi, + pkgs: []string{ + "pkg", + "pkg/nestedpkg/otherpkg", + }, }, fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"pkg"}, - {"pkg", "nestedpkg"}, - {"pkg", "nestedpkg", "otherpkg"}, + dirs: []string{ + "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"}, + files: []string{ + "main.go", + "COPYING", + "pkg/main.go", + "pkg/nestedpkg/main.go", + "pkg/nestedpkg/PATENT.md", + "pkg/nestedpkg/otherpkg/main.go", }, }, after: filesystemState{ - dirs: []fsPath{ - {"pkg"}, - {"pkg", "nestedpkg"}, - {"pkg", "nestedpkg", "otherpkg"}, + dirs: []string{ + "pkg", + "pkg/nestedpkg", + "pkg/nestedpkg/otherpkg", }, - files: []fsPath{ - {"COPYING"}, - {"pkg", "main.go"}, - {"pkg", "nestedpkg", "PATENT.md"}, - {"pkg", "nestedpkg", "otherpkg", "main.go"}, + files: []string{ + "COPYING", + "pkg/main.go", + "pkg/nestedpkg/PATENT.md", + "pkg/nestedpkg/otherpkg/main.go", }, }, }, @@ -114,25 +173,27 @@ func TestPruneUnusedPackages(t *testing.T) { }, } - 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 + baseDir := h.Path(pr) + tc.fs.before.root = baseDir + tc.fs.after.root = baseDir + tc.fs.setup(t) - tc.fs.before.setup(t) + fs, err := deriveFilesystemState(baseDir) + if err != nil { + t.Fatal(err) + } - err := pruneUnusedPackages(tc.lp, projectDir, logger) + _, err = pruneUnusedPackages(tc.lp, fs) if tc.err && err == nil { - t.Errorf("expected an error, got nil") + t.Fatalf("expected an error, got nil") } else if !tc.err && err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } - tc.fs.after.assert(t) + tc.fs.assert(t) }) } } @@ -152,8 +213,8 @@ func TestPruneNonGoFiles(t *testing.T) { "one-file", fsTestCase{ before: filesystemState{ - files: []fsPath{ - {"README.md"}, + files: []string{ + "README.md", }, }, after: filesystemState{}, @@ -164,16 +225,16 @@ func TestPruneNonGoFiles(t *testing.T) { "multiple-files", fsTestCase{ before: filesystemState{ - files: []fsPath{ - {"main.go"}, - {"main_test.go"}, - {"README"}, + files: []string{ + "main.go", + "main_test.go", + "README", }, }, after: filesystemState{ - files: []fsPath{ - {"main.go"}, - {"main_test.go"}, + files: []string{ + "main.go", + "main_test.go", }, }, }, @@ -183,22 +244,22 @@ func TestPruneNonGoFiles(t *testing.T) { "mixed-files", fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, - files: []fsPath{ - {"dir", "main.go"}, - {"dir", "main_test.go"}, - {"dir", "db.sqlite"}, + files: []string{ + "dir/main.go", + "dir/main_test.go", + "dir/db.sqlite", }, }, after: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, - files: []fsPath{ - {"dir", "main.go"}, - {"dir", "main_test.go"}, + files: []string{ + "dir/main.go", + "dir/main_test.go", }, }, }, @@ -206,8 +267,6 @@ func TestPruneNonGoFiles(t *testing.T) { }, } - logger := log.New(ioutil.Discard, "", 0) - for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { h.TempDir(tc.name) @@ -215,16 +274,21 @@ func TestPruneNonGoFiles(t *testing.T) { tc.fs.before.root = baseDir tc.fs.after.root = baseDir - tc.fs.before.setup(t) + tc.fs.setup(t) - err := pruneNonGoFiles(baseDir, logger) + fs, err := deriveFilesystemState(baseDir) + if err != nil { + t.Fatal(err) + } + + err = pruneNonGoFiles(fs) 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) + tc.fs.assert(t) }) } } @@ -244,8 +308,8 @@ func TestPruneGoTestFiles(t *testing.T) { "one-test-file", fsTestCase{ before: filesystemState{ - files: []fsPath{ - {"main_test.go"}, + files: []string{ + "main_test.go", }, }, after: filesystemState{}, @@ -256,17 +320,17 @@ func TestPruneGoTestFiles(t *testing.T) { "multiple-files", fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, - files: []fsPath{ - {"dir", "main_test.go"}, - {"dir", "main2_test.go"}, + files: []string{ + "dir/main_test.go", + "dir/main2_test.go", }, }, after: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, }, }, @@ -276,23 +340,23 @@ func TestPruneGoTestFiles(t *testing.T) { "mixed-files", fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, - files: []fsPath{ - {"dir", "main.go"}, - {"dir", "main2.go"}, - {"dir", "main_test.go"}, - {"dir", "main2_test.go"}, + files: []string{ + "dir/main.go", + "dir/main2.go", + "dir/main_test.go", + "dir/main2_test.go", }, }, after: filesystemState{ - dirs: []fsPath{ - {"dir"}, + dirs: []string{ + "dir", }, - files: []fsPath{ - {"dir", "main.go"}, - {"dir", "main2.go"}, + files: []string{ + "dir/main.go", + "dir/main2.go", }, }, }, @@ -300,8 +364,6 @@ func TestPruneGoTestFiles(t *testing.T) { }, } - logger := log.New(ioutil.Discard, "", 0) - for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { h.TempDir(tc.name) @@ -309,16 +371,21 @@ func TestPruneGoTestFiles(t *testing.T) { tc.fs.before.root = baseDir tc.fs.after.root = baseDir - tc.fs.before.setup(t) + tc.fs.setup(t) + + fs, err := deriveFilesystemState(baseDir) + if err != nil { + t.Fatal(err) + } - err := pruneGoTestFiles(baseDir, logger) + err = pruneGoTestFiles(fs) if tc.err && err == nil { - t.Errorf("expected an error, got nil") + t.Fatalf("expected an error, got nil") } else if !tc.err && err != nil { - t.Errorf("unexpected error: %s", err) + t.Fatalf("unexpected error: %s", err) } - tc.fs.after.assert(t) + tc.fs.assert(t) }) } } diff --git a/gps/prune_vendor.go b/gps/prune_vendor.go new file mode 100644 index 0000000000..c53d3e7f7b --- /dev/null +++ b/gps/prune_vendor.go @@ -0,0 +1,29 @@ +// 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. + +//+build !windows + +package gps + +import ( + "path/filepath" +) + +func collectNestedVendorDirs(fsState filesystemState) []string { + toDelete := make([]string, 0, len(fsState.dirs)/4) + + for _, dir := range fsState.dirs { + if filepath.Base(dir) == "vendor" { + toDelete = append(toDelete, filepath.Join(fsState.root, dir)) + } + } + + for _, link := range fsState.links { + if filepath.Base(link.path) == "vendor" { + toDelete = append(toDelete, filepath.Join(fsState.root, link.path)) + } + } + + return toDelete +} diff --git a/gps/prune_vendor_test.go b/gps/prune_vendor_test.go new file mode 100644 index 0000000000..79c4a3a4b4 --- /dev/null +++ b/gps/prune_vendor_test.go @@ -0,0 +1,76 @@ +// 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" + "os" + "testing" +) + +func pruneVendorDirsTestCase(tc fsTestCase) func(*testing.T) { + return func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "pruneVendorDirsTestCase") + if err != nil { + t.Fatalf("ioutil.TempDir err=%q", err) + } + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf("os.RemoveAll(%q) err=%q", tempDir, err) + } + }() + + tc.before.root = tempDir + tc.after.root = tempDir + + tc.setup(t) + + fs, err := deriveFilesystemState(tempDir) + if err != nil { + t.Fatalf("deriveFilesystemState failed: %s", err) + } + + if err := pruneVendorDirs(fs); err != nil { + t.Errorf("pruneVendorDirs err=%q", err) + } + + tc.assert(t) + } +} + +func TestPruneVendorDirs(t *testing.T) { + t.Run("vendor directory", pruneVendorDirsTestCase(fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + "package/vendor", + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + }, + })) + + t.Run("vendor file", pruneVendorDirsTestCase(fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + }, + files: []string{ + "package/vendor", + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + files: []string{ + "package/vendor", + }, + }, + })) +} diff --git a/gps/prune_vendor_windows.go b/gps/prune_vendor_windows.go new file mode 100644 index 0000000000..dd873a207b --- /dev/null +++ b/gps/prune_vendor_windows.go @@ -0,0 +1,38 @@ +// 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 ( + "os" + "path/filepath" +) + +func collectNestedVendorDirs(fsState filesystemState) []string { + toDelete := make([]string, 0, len(fsState.dirs)/4) + + for _, dir := range fsState.dirs { + if filepath.Base(dir) == "vendor" { + toDelete = append(toDelete, filepath.Join(fsState.root, dir)) + } + } + + for _, link := range fsState.links { + if filepath.Base(link.path) == "vendor" { + // 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. + if info, err := os.Stat(link.path); err == nil && info.IsDir() { + toDelete = append(toDelete, filepath.Join(fsState.root, link.path)) + } + + } + } + + return toDelete +} diff --git a/gps/strip_vendor_windows_test.go b/gps/prune_vendor_windows_test.go similarity index 57% rename from gps/strip_vendor_windows_test.go rename to gps/prune_vendor_windows_test.go index 67ff900299..aa3233092c 100644 --- a/gps/strip_vendor_windows_test.go +++ b/gps/prune_vendor_windows_test.go @@ -11,170 +11,170 @@ import "testing" func TestStripVendorSymlinks(t *testing.T) { // On windows, we skip symlinks, even if they're named 'vendor', because // they're too hard to distinguish from junctions. - t.Run("vendor symlink", stripVendorTestCase(fsTestCase{ + t.Run("vendor symlink", pruneVendorDirsTestCase(fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, + dirs: []string{ + "package", + "package/_vendor", }, links: []fsLink{ { - path: fsPath{"package", "vendor"}, + path: "package/vendor", to: "_vendor", }, }, }, after: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, + dirs: []string{ + "package", + "package/_vendor", }, links: []fsLink{ { - path: fsPath{"package", "vendor"}, + path: "package/vendor", to: "_vendor", }, }, }, })) - t.Run("nonvendor symlink", stripVendorTestCase(fsTestCase{ + t.Run("nonvendor symlink", pruneVendorDirsTestCase(fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, + dirs: []string{ + "package", + "package/_vendor", }, links: []fsLink{ { - path: fsPath{"package", "link"}, + path: "package/link", to: "_vendor", }, }, }, after: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, + dirs: []string{ + "package", + "package/_vendor", }, links: []fsLink{ { - path: fsPath{"package", "link"}, + path: "package/link", to: "_vendor", }, }, }, })) - t.Run("vendor symlink to file", stripVendorTestCase(fsTestCase{ + t.Run("vendor symlink to file", pruneVendorDirsTestCase(fsTestCase{ before: filesystemState{ - files: []fsPath{ - {"file"}, + files: []string{ + "file", }, links: []fsLink{ { - path: fsPath{"vendor"}, + path: "vendor", to: "file", }, }, }, after: filesystemState{ - files: []fsPath{ - {"file"}, + files: []string{ + "file", }, links: []fsLink{ { - path: fsPath{"vendor"}, + path: "vendor", to: "file", }, }, }, })) - t.Run("broken vendor symlink", stripVendorTestCase(fsTestCase{ + t.Run("broken vendor symlink", pruneVendorDirsTestCase(fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"package"}, + dirs: []string{ + "package", }, links: []fsLink{ { - path: fsPath{"package", "vendor"}, + path: "package/vendor", to: "nonexistence", }, }, }, after: filesystemState{ - dirs: []fsPath{ - {"package"}, + dirs: []string{ + "package", }, links: []fsLink{ { - path: fsPath{"package", "vendor"}, + path: "package/vendor", to: "nonexistence", }, }, }, })) - t.Run("chained symlinks", stripVendorTestCase(fsTestCase{ + t.Run("chained symlinks", pruneVendorDirsTestCase(fsTestCase{ // Curiously, if a symlink on windows points to *another* symlink which // eventually points at a directory, we'll correctly remove that first // symlink, because the first symlink doesn't appear to Go to be a // directory. before: filesystemState{ - dirs: []fsPath{ - {"_vendor"}, + dirs: []string{ + "_vendor", }, links: []fsLink{ { - path: fsPath{"vendor"}, + path: "vendor", to: "vendor2", }, { - path: fsPath{"vendor2"}, + path: "vendor2", to: "_vendor", }, }, }, after: filesystemState{ - dirs: []fsPath{ - {"_vendor"}, + dirs: []string{ + "_vendor", }, links: []fsLink{ { - path: fsPath{"vendor2"}, + path: "vendor2", to: "_vendor", }, }, }, })) - t.Run("circular symlinks", stripVendorTestCase(fsTestCase{ + t.Run("circular symlinks", pruneVendorDirsTestCase(fsTestCase{ before: filesystemState{ - dirs: []fsPath{ - {"package"}, + dirs: []string{ + "package", }, links: []fsLink{ { - path: fsPath{"package", "link1"}, + path: "package/link1", to: "link2", }, { - path: fsPath{"package", "link2"}, + path: "package/link2", to: "link1", }, }, }, after: filesystemState{ - dirs: []fsPath{ - {"package"}, + dirs: []string{ + "package", }, links: []fsLink{ { - path: fsPath{"package", "link1"}, + path: "package/link1", to: "link2", }, { - path: fsPath{"package", "link2"}, + path: "package/link2", to: "link1", }, }, diff --git a/gps/strip_vendor.go b/gps/strip_vendor.go deleted file mode 100644 index aaaf9bcd22..0000000000 --- a/gps/strip_vendor.go +++ /dev/null @@ -1,39 +0,0 @@ -// 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. - -//+build !windows - -package gps - -import ( - "os" - "path/filepath" -) - -func stripVendor(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - // 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 { - if realInfo, err := os.Stat(path); err == nil && realInfo.IsDir() { - return os.Remove(path) - } - } - - if info.IsDir() { - if err := os.RemoveAll(path); err != nil { - return err - } - return filepath.SkipDir - } - - return nil -} diff --git a/gps/strip_vendor_nonwindows_test.go b/gps/strip_vendor_nonwindows_test.go deleted file mode 100644 index 8ff55155b0..0000000000 --- a/gps/strip_vendor_nonwindows_test.go +++ /dev/null @@ -1,171 +0,0 @@ -// 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. - -// +build !windows - -package gps - -import "testing" - -func TestStripVendorSymlinks(t *testing.T) { - t.Run("vendor symlink", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "vendor"}, - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, - }, - }, - })) - - t.Run("nonvendor symlink", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "link"}, - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "_vendor"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "link"}, - to: "_vendor", - }, - }, - }, - })) - - t.Run("vendor symlink to file", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - files: []fsPath{ - {"file"}, - }, - links: []fsLink{ - { - path: fsPath{"vendor"}, - to: "file", - }, - }, - }, - after: filesystemState{ - files: []fsPath{ - {"file"}, - }, - links: []fsLink{ - { - path: fsPath{"vendor"}, - to: "file", - }, - }, - }, - })) - - t.Run("broken vendor symlink", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "vendor"}, - to: "nonexistence", - }, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "vendor"}, - to: "nonexistence", - }, - }, - }, - })) - - t.Run("chained symlinks", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"_vendor"}, - }, - links: []fsLink{ - { - path: fsPath{"vendor"}, - to: "vendor2", - }, - { - path: fsPath{"vendor2"}, - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"_vendor"}, - }, - links: []fsLink{ - { - path: fsPath{"vendor2"}, - to: "_vendor", - }, - }, - }, - })) - - t.Run("circular symlinks", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "link1"}, - to: "link2", - }, - { - path: fsPath{"package", "link2"}, - to: "link1", - }, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - links: []fsLink{ - { - path: fsPath{"package", "link1"}, - to: "link2", - }, - { - path: fsPath{"package", "link2"}, - to: "link1", - }, - }, - }, - })) -} diff --git a/gps/strip_vendor_test.go b/gps/strip_vendor_test.go deleted file mode 100644 index 18a722cd9c..0000000000 --- a/gps/strip_vendor_test.go +++ /dev/null @@ -1,71 +0,0 @@ -// 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" - "os" - "path/filepath" - "testing" -) - -func stripVendorTestCase(tc fsTestCase) func(*testing.T) { - return func(t *testing.T) { - tempDir, err := ioutil.TempDir("", "TestStripVendor") - if err != nil { - t.Fatalf("ioutil.TempDir err=%q", err) - } - defer func() { - if err := os.RemoveAll(tempDir); err != nil { - t.Errorf("os.RemoveAll(%q) err=%q", tempDir, err) - } - }() - tc.before.root = tempDir - tc.after.root = tempDir - - tc.before.setup(t) - - if err := filepath.Walk(tempDir, stripVendor); err != nil { - t.Errorf("filepath.Walk err=%q", err) - } - - tc.after.assert(t) - } -} - -func TestStripVendorDirectory(t *testing.T) { - t.Run("vendor directory", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - {"package", "vendor"}, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - }, - })) - - t.Run("vendor file", stripVendorTestCase(fsTestCase{ - before: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - files: []fsPath{ - {"package", "vendor"}, - }, - }, - after: filesystemState{ - dirs: []fsPath{ - {"package"}, - }, - files: []fsPath{ - {"package", "vendor"}, - }, - }, - })) -} diff --git a/gps/strip_vendor_windows.go b/gps/strip_vendor_windows.go deleted file mode 100644 index c6b0a13346..0000000000 --- a/gps/strip_vendor_windows.go +++ /dev/null @@ -1,52 +0,0 @@ -// 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 ( - "os" - "path/filepath" -) - -func stripVendor(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - 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: - if realInfo, err := os.Stat(path); err == nil && realInfo.IsDir() { - return os.Remove(path) - } - - case dir: - if err := os.RemoveAll(path); err != nil { - return err - } - return filepath.SkipDir - } - - return nil -} From a2a7418586c821bc8be8a4dee2a6c2936030ea97 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Wed, 29 Nov 2017 09:42:43 +0400 Subject: [PATCH 04/11] gps: update WriteDepTree to use PruneProject function Signed-off-by: Ibrahim AshShohail --- gps/prune.go | 27 ++++++++++++++++++++++++++ gps/solution.go | 24 +++++++++++------------ gps/solution_test.go | 6 +++--- manifest.go | 45 +++++++++++++++++--------------------------- manifest_test.go | 16 +++++++--------- txn_writer.go | 7 ++++++- 6 files changed, 71 insertions(+), 54 deletions(-) diff --git a/gps/prune.go b/gps/prune.go index 357dae3570..5d91e735a8 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -20,6 +20,13 @@ type PruneOptions uint8 // PruneProjectOptions is map of prune options per project name. type PruneProjectOptions map[ProjectRoot]PruneOptions +// RootPruneOptions represents the root prune options for the project. +// It contains the global options and a map of options per project. +type RootPruneOptions struct { + PruneOptions PruneOptions + ProjectOptions PruneProjectOptions +} + const ( // PruneNestedVendorDirs indicates if nested vendor directories should be pruned. PruneNestedVendorDirs PruneOptions = 1 << iota @@ -33,6 +40,26 @@ const ( PruneGoTestFiles ) +// DefaultRootPruneOptions instantiates a copy of the default root prune options. +func DefaultRootPruneOptions() RootPruneOptions { + return RootPruneOptions{ + PruneOptions: PruneNestedVendorDirs, + ProjectOptions: PruneProjectOptions{}, + } +} + +// PruneOptionsFor returns the prune options for the passed project root. +// +// It will return the root prune options if the project does not have specific +// options or if it does not exists in the manifest. +func (o *RootPruneOptions) PruneOptionsFor(pr ProjectRoot) PruneOptions { + if po, ok := o.ProjectOptions[pr]; ok { + return po + } + + return o.PruneOptions +} + var ( // licenseFilePrefixes is a list of name prefixes for license files. licenseFilePrefixes = []string{ diff --git a/gps/solution.go b/gps/solution.go index 2422caabe6..f737cab976 100644 --- a/gps/solution.go +++ b/gps/solution.go @@ -50,16 +50,15 @@ type solution struct { const concurrentWriters = 16 -// WriteDepTree takes a basedir and a Lock, and exports all the projects -// listed in the lock to the appropriate target location within the basedir. +// WriteDepTree takes a basedir, a Lock and a RootPruneOptions and exports all +// the projects listed in the lock to the appropriate target location within basedir. // // If the goal is to populate a vendor directory, basedir should be the absolute // path to that vendor directory, not its parent (a project root, typically). // -// It requires a SourceManager to do the work, and takes a flag indicating -// whether or not to strip vendor directories contained in the exported -// dependencies. -func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log.Logger) error { +// It requires a SourceManager to do the work. Prune options are read from the +// passed manifest. +func WriteDepTree(basedir string, l Lock, sm SourceManager, rpo RootPruneOptions, logger *log.Logger) error { if l == nil { return fmt.Errorf("must provide non-nil Lock to WriteDepTree") } @@ -96,14 +95,13 @@ func WriteDepTree(basedir string, l Lock, sm SourceManager, sv bool, logger *log return errors.Wrapf(err, "failed to export %s", projectRoot) } - if sv { - if err := ctx.Err(); err != nil { - return err - } + err := PruneProject(to, p, rpo.PruneOptionsFor(ident.ProjectRoot), logger) + if err != nil { + return errors.Wrapf(err, "failed to prune %s", projectRoot) + } - if err := filepath.Walk(to, stripVendor); err != nil { - return errors.Wrapf(err, "failed to strip vendor from %s", projectRoot) - } + if err := ctx.Err(); err != nil { + return err } return nil diff --git a/gps/solution_test.go b/gps/solution_test.go index d044f12ad3..171f1d8a6c 100644 --- a/gps/solution_test.go +++ b/gps/solution_test.go @@ -97,12 +97,12 @@ func testWriteDepTree(t *testing.T) { } // nil lock/result should err immediately - err = WriteDepTree(tmp, nil, sm, true, discardLogger()) + err = WriteDepTree(tmp, nil, sm, DefaultRootPruneOptions(), discardLogger()) if err == nil { t.Errorf("Should error if nil lock is passed to WriteDepTree") } - err = WriteDepTree(tmp, r, sm, true, discardLogger()) + err = WriteDepTree(tmp, r, sm, DefaultRootPruneOptions(), discardLogger()) if err != nil { t.Errorf("Unexpected error while creating vendor tree: %s", err) } @@ -154,7 +154,7 @@ func BenchmarkCreateVendorTree(b *testing.B) { // ease manual inspection os.RemoveAll(exp) b.StartTimer() - err = WriteDepTree(exp, r, sm, true, logger) + err = WriteDepTree(exp, r, sm, DefaultRootPruneOptions(), logger) b.StopTimer() if err != nil { b.Errorf("unexpected error after %v iterations: %s", i, err) diff --git a/manifest.go b/manifest.go index 6115a5f482..3b75f607ec 100644 --- a/manifest.go +++ b/manifest.go @@ -50,8 +50,7 @@ type Manifest struct { Ignored []string Required []string - PruneOptions gps.PruneOptions - PruneProjectOptions gps.PruneProjectOptions + PruneOptions gps.RootPruneOptions } type rawManifest struct { @@ -96,7 +95,7 @@ func NewManifest() *Manifest { return &Manifest{ Constraints: make(gps.ProjectConstraints), Ovr: make(gps.ProjectConstraints), - PruneOptions: gps.PruneNestedVendorDirs, + PruneOptions: gps.DefaultRootPruneOptions(), } } @@ -293,7 +292,7 @@ func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error { wg.Add(1) go validate(pr) } - for pr := range m.PruneProjectOptions { + for pr := range m.PruneOptions.ProjectOptions { wg.Add(1) go validate(pr) } @@ -369,41 +368,43 @@ func fromRawManifest(raw rawManifest) (*Manifest, error) { m.Ovr[name] = prj } - m.PruneOptions, m.PruneProjectOptions = fromRawPruneOptions(raw.PruneOptions) + m.PruneOptions = fromRawPruneOptions(raw.PruneOptions) return m, nil } -func fromRawPruneOptions(raw rawPruneOptions) (gps.PruneOptions, gps.PruneProjectOptions) { - rootOptions := gps.PruneNestedVendorDirs - pruneProjects := make(gps.PruneProjectOptions) +func fromRawPruneOptions(raw rawPruneOptions) gps.RootPruneOptions { + opts := gps.RootPruneOptions{ + PruneOptions: gps.PruneNestedVendorDirs, + ProjectOptions: make(gps.PruneProjectOptions), + } if raw.UnusedPackages { - rootOptions |= gps.PruneUnusedPackages + opts.PruneOptions |= gps.PruneUnusedPackages } if raw.GoTests { - rootOptions |= gps.PruneGoTestFiles + opts.PruneOptions |= gps.PruneGoTestFiles } if raw.NonGoFiles { - rootOptions |= gps.PruneNonGoFiles + opts.PruneOptions |= gps.PruneNonGoFiles } for _, p := range raw.Projects { pr := gps.ProjectRoot(p.Name) - pruneProjects[pr] = gps.PruneNestedVendorDirs + opts.ProjectOptions[pr] = gps.PruneNestedVendorDirs if raw.UnusedPackages { - pruneProjects[pr] |= gps.PruneUnusedPackages + opts.ProjectOptions[pr] |= gps.PruneUnusedPackages } if raw.GoTests { - pruneProjects[pr] |= gps.PruneGoTestFiles + opts.ProjectOptions[pr] |= gps.PruneGoTestFiles } if raw.NonGoFiles { - pruneProjects[pr] |= gps.PruneNonGoFiles + opts.ProjectOptions[pr] |= gps.PruneNonGoFiles } } - return rootOptions, pruneProjects + return opts } // toProject interprets the string representations of project information held in @@ -562,15 +563,3 @@ func (m *Manifest) RequiredPackages() map[string]bool { return mp } - -// PruneOptionsFor returns the prune options for the passed project root. -// -// It will return the root prune options if the project does not have specific -// options or if it does not exists in the manifest. -func (m *Manifest) PruneOptionsFor(pr gps.ProjectRoot) gps.PruneOptions { - if po, ok := m.PruneProjectOptions[pr]; ok { - return po - } - - return m.PruneOptions -} diff --git a/manifest_test.go b/manifest_test.go index 93eb95ab83..216e8abfe3 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -45,11 +45,13 @@ func TestReadManifest(t *testing.T) { Constraint: gps.NewBranch("master"), }, }, - Ignored: []string{"github.com/foo/bar"}, - PruneOptions: gps.PruneNestedVendorDirs | gps.PruneNonGoFiles, - PruneProjectOptions: gps.PruneProjectOptions{ - gps.ProjectRoot("github.com/golang/dep"): gps.PruneNestedVendorDirs, - gps.ProjectRoot("github.com/babble/brook"): gps.PruneNestedVendorDirs | gps.PruneGoTestFiles, + Ignored: []string{"github.com/foo/bar"}, + PruneOptions: gps.RootPruneOptions{ + PruneOptions: gps.PruneNestedVendorDirs | gps.PruneNonGoFiles, + ProjectOptions: gps.PruneProjectOptions{ + gps.ProjectRoot("github.com/golang/dep"): gps.PruneNestedVendorDirs, + gps.ProjectRoot("github.com/babble/brook"): gps.PruneNestedVendorDirs | gps.PruneGoTestFiles, + }, }, } @@ -595,10 +597,6 @@ func TestValidateProjectRoots(t *testing.T) { } } -func TestPruneOptionsFor(t *testing.T) { - -} - func containsErr(s []error, e error) bool { for _, a := range s { if a.Error() == e.Error() { diff --git a/txn_writer.go b/txn_writer.go index f7974d98e0..bd3acbf01c 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -312,7 +312,12 @@ func (sw *SafeWriter) Write(root string, sm gps.SourceManager, examples bool, lo } if sw.writeVendor { - err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.lock, sm, true, logger) + opts := gps.DefaultRootPruneOptions() + if sw.HasManifest() { + opts = sw.Manifest.PruneOptions + } + + err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.lock, sm, opts, logger) if err != nil { return errors.Wrap(err, "error while writing out vendor tree") } From 4b351b6b0e7f4d970b6d8f51e2ed27436b4bd1c8 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 5 Dec 2017 17:04:11 +0300 Subject: [PATCH 05/11] rename TestStripVendorSymlinks to TestPruneVendorSymlinks Signed-off-by: Ibrahim AshShohail --- gps/prune_vendor_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gps/prune_vendor_windows_test.go b/gps/prune_vendor_windows_test.go index aa3233092c..fb4b06bb38 100644 --- a/gps/prune_vendor_windows_test.go +++ b/gps/prune_vendor_windows_test.go @@ -8,7 +8,7 @@ package gps import "testing" -func TestStripVendorSymlinks(t *testing.T) { +func TestPruneVendorSymlinks(t *testing.T) { // On windows, we skip symlinks, even if they're named 'vendor', because // they're too hard to distinguish from junctions. t.Run("vendor symlink", pruneVendorDirsTestCase(fsTestCase{ From 148dfe560d963320663d0f17bf32c737f165524b Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 5 Dec 2017 17:52:43 +0300 Subject: [PATCH 06/11] fix missing populated prune options Signed-off-by: Ibrahim AshShohail --- cmd/dep/ensure.go | 10 +++++----- cmd/dep/init.go | 2 +- gps/prune.go | 43 ++++++++++--------------------------------- txn_writer.go | 25 +++++++++++-------------- txn_writer_test.go | 36 +++++++++++++++++++----------------- 5 files changed, 46 insertions(+), 70 deletions(-) diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 7d14778368..c338d6d9a5 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -265,7 +265,7 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project // that "verification" is supposed to look like (#121); in the meantime, // we unconditionally write out vendor/ so that `dep ensure`'s behavior // is maximally compatible with what it will eventually become. - sw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways) + sw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions) if err != nil { return err } @@ -290,7 +290,7 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project return handleAllTheFailuresOfTheWorld(err) } - sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), cmd.vendorBehavior()) + sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), cmd.vendorBehavior(), p.Manifest.PruneOptions) if err != nil { return err } @@ -315,7 +315,7 @@ func (cmd *ensureCommand) runVendorOnly(ctx *dep.Ctx, args []string, p *dep.Proj } // Pass the same lock as old and new so that the writer will observe no // difference and choose not to write it out. - sw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways) + sw, err := dep.NewSafeWriter(nil, p.Lock, p.Lock, dep.VendorAlways, p.Manifest.PruneOptions) if err != nil { return err } @@ -380,7 +380,7 @@ func (cmd *ensureCommand) runUpdate(ctx *dep.Ctx, args []string, p *dep.Project, return handleAllTheFailuresOfTheWorld(err) } - sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), cmd.vendorBehavior()) + sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), cmd.vendorBehavior(), p.Manifest.PruneOptions) if err != nil { return err } @@ -679,7 +679,7 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm } sort.Strings(reqlist) - sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), dep.VendorOnChanged) + sw, err := dep.NewSafeWriter(nil, p.Lock, dep.LockFromSolution(solution), dep.VendorOnChanged, p.Manifest.PruneOptions) if err != nil { return err } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index 205ce83f4e..7f88763849 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -211,7 +211,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { ctx.Err.Printf("Old vendor backed up to %v", vendorbak) } - sw, err := dep.NewSafeWriter(p.Manifest, nil, p.Lock, dep.VendorAlways) + sw, err := dep.NewSafeWriter(p.Manifest, nil, p.Lock, dep.VendorAlways, gps.DefaultRootPruneOptions()) if err != nil { return errors.Wrap(err, "init failed: unable to create a SafeWriter") } diff --git a/gps/prune.go b/gps/prune.go index 5d91e735a8..5fb1ee766a 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -87,36 +87,37 @@ var ( // 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 { - fs, err := deriveFilesystemState(baseDir) + fsState, err := deriveFilesystemState(baseDir) + if err != nil { return errors.Wrap(err, "could not derive filesystem state") } if (options & PruneNestedVendorDirs) != 0 { - if err := pruneVendorDirs(fs); err != nil { + if err := pruneVendorDirs(fsState); err != nil { return errors.Wrapf(err, "failed to prune nested vendor directories") } } if (options & PruneUnusedPackages) != 0 { - if _, err := pruneUnusedPackages(lp, fs); err != nil { + if _, err := pruneUnusedPackages(lp, fsState); err != nil { return errors.Wrap(err, "failed to prune unused packages") } } if (options & PruneNonGoFiles) != 0 { - if err := pruneNonGoFiles(fs); err != nil { + if err := pruneNonGoFiles(fsState); err != nil { return errors.Wrap(err, "failed to prune non-Go files") } } if (options & PruneGoTestFiles) != 0 { - if err := pruneGoTestFiles(fs); err != nil { + if err := pruneGoTestFiles(fsState); err != nil { return errors.Wrap(err, "failed to prune Go test files") } } - if err := deleteEmptyDirs(fs); err != nil { + if err := deleteEmptyDirs(fsState); err != nil { return errors.Wrap(err, "could not delete empty dirs") } @@ -124,11 +125,11 @@ func PruneProject(baseDir string, lp LockedProject, options PruneOptions, logger } // pruneVendorDirs deletes all nested vendor directories within baseDir. -func pruneVendorDirs(fs filesystemState) error { - toDelete := collectNestedVendorDirs(fs) +func pruneVendorDirs(fsState filesystemState) error { + toDelete := collectNestedVendorDirs(fsState) for _, path := range toDelete { - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + if err := os.RemoveAll(path); err != nil && !os.IsNotExist(err) { return err } } @@ -310,30 +311,6 @@ func deleteEmptyDirs(fsState filesystemState) error { return nil } -func deleteEmptyDirsFromSlice(deletedFiles []string) error { - dirs := make(map[string]struct{}) - - for _, path := range deletedFiles { - dirs[filepath.Dir(path)] = struct{}{} - } - - for path := range dirs { - notEmpty, err := fs.IsNonEmptyDir(path) - if err != nil { - return err - } - - if !notEmpty { - err := os.Remove(path) - if err != nil && !os.IsNotExist(err) { - return err - } - } - } - - return nil -} - func fileExt(name string) string { i := strings.LastIndex(name, ".") if i < 0 { diff --git a/txn_writer.go b/txn_writer.go index bd3acbf01c..4256d0175a 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -56,11 +56,12 @@ var lockFileComment = []byte(`# This file is autogenerated, do not edit; changes // It is not impervious to errors (writing to disk is hard), but it should // guard against non-arcane failure conditions. type SafeWriter struct { - Manifest *Manifest - lock *Lock - lockDiff *gps.LockDiff - writeVendor bool - writeLock bool + Manifest *Manifest + lock *Lock + lockDiff *gps.LockDiff + writeVendor bool + writeLock bool + pruneOptions gps.RootPruneOptions } // NewSafeWriter sets up a SafeWriter to write a set of manifest, lock, and @@ -78,10 +79,11 @@ type SafeWriter struct { // - If oldLock is provided without newLock, error. // // - If vendor is VendorAlways without a newLock, error. -func NewSafeWriter(manifest *Manifest, oldLock, newLock *Lock, vendor VendorBehavior) (*SafeWriter, error) { +func NewSafeWriter(manifest *Manifest, oldLock, newLock *Lock, vendor VendorBehavior, prune gps.RootPruneOptions) (*SafeWriter, error) { sw := &SafeWriter{ - Manifest: manifest, - lock: newLock, + Manifest: manifest, + lock: newLock, + pruneOptions: prune, } if oldLock != nil { @@ -312,12 +314,7 @@ func (sw *SafeWriter) Write(root string, sm gps.SourceManager, examples bool, lo } if sw.writeVendor { - opts := gps.DefaultRootPruneOptions() - if sw.HasManifest() { - opts = sw.Manifest.PruneOptions - } - - err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.lock, sm, opts, logger) + err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.lock, sm, sw.pruneOptions, logger) if err != nil { return errors.Wrap(err, "error while writing out vendor tree") } diff --git a/txn_writer_test.go b/txn_writer_test.go index 141e1f8cc1..70dde8cebf 100644 --- a/txn_writer_test.go +++ b/txn_writer_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" + "github.com/golang/dep/gps" + "github.com/golang/dep/internal/test" "github.com/pkg/errors" ) @@ -25,7 +27,7 @@ func TestSafeWriter_BadInput_MissingRoot(t *testing.T) { pc := NewTestProjectContext(h, safeWriterProject) defer pc.Release() - sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) + sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, gps.DefaultRootPruneOptions()) err := sw.Write("", pc.SourceManager, true, discardLogger()) if err == nil { @@ -43,7 +45,7 @@ func TestSafeWriter_BadInput_MissingSourceManager(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways) + sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, gps.DefaultRootPruneOptions()) err := sw.Write(pc.Project.AbsRoot, nil, true, discardLogger()) if err == nil { @@ -59,7 +61,7 @@ func TestSafeWriter_BadInput_ForceVendorMissingLock(t *testing.T) { pc := NewTestProjectContext(h, safeWriterProject) defer pc.Release() - _, err := NewSafeWriter(nil, nil, nil, VendorAlways) + _, err := NewSafeWriter(nil, nil, nil, VendorAlways, gps.DefaultRootPruneOptions()) if err == nil { t.Fatal("should have errored without a lock when forceVendor is true, but did not") } else if !strings.Contains(err.Error(), "newLock") { @@ -75,7 +77,7 @@ func TestSafeWriter_BadInput_OldLockOnly(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - _, err := NewSafeWriter(nil, pc.Project.Lock, nil, VendorAlways) + _, err := NewSafeWriter(nil, pc.Project.Lock, nil, VendorAlways, gps.DefaultRootPruneOptions()) if err == nil { t.Fatal("should have errored with only an old lock, but did not") } else if !strings.Contains(err.Error(), "oldLock") { @@ -89,7 +91,7 @@ func TestSafeWriter_BadInput_NonexistentRoot(t *testing.T) { pc := NewTestProjectContext(h, safeWriterProject) defer pc.Release() - sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) + sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, gps.DefaultRootPruneOptions()) missingroot := filepath.Join(pc.Project.AbsRoot, "nonexistent") err := sw.Write(missingroot, pc.SourceManager, true, discardLogger()) @@ -107,7 +109,7 @@ func TestSafeWriter_BadInput_RootIsFile(t *testing.T) { pc := NewTestProjectContext(h, safeWriterProject) defer pc.Release() - sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) + sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged, gps.DefaultRootPruneOptions()) fileroot := pc.CopyFile("fileroot", "txn_writer/badinput_fileroot") err := sw.Write(fileroot, pc.SourceManager, true, discardLogger()) @@ -131,7 +133,7 @@ func TestSafeWriter_Manifest(t *testing.T) { pc.CopyFile(ManifestName, safeWriterGoldenManifest) pc.Load() - sw, _ := NewSafeWriter(pc.Project.Manifest, nil, nil, VendorOnChanged) + sw, _ := NewSafeWriter(pc.Project.Manifest, nil, nil, VendorOnChanged, gps.DefaultRootPruneOptions()) // Verify prepared actions if !sw.HasManifest() { @@ -173,7 +175,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLock(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorOnChanged) + sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorOnChanged, gps.DefaultRootPruneOptions()) // Verify prepared actions if !sw.HasManifest() { @@ -218,7 +220,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways) + sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, gps.DefaultRootPruneOptions()) // Verify prepared actions if !sw.HasManifest() { @@ -268,7 +270,7 @@ func TestSafeWriter_ModifiedLock(t *testing.T) { originalLock := new(Lock) *originalLock = *pc.Project.Lock originalLock.SolveMeta.InputsDigest = []byte{} // zero out the input hash to ensure non-equivalency - sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorOnChanged) + sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorOnChanged, gps.DefaultRootPruneOptions()) // Verify prepared actions if sw.HasManifest() { @@ -318,7 +320,7 @@ func TestSafeWriter_ModifiedLockSkipVendor(t *testing.T) { originalLock := new(Lock) *originalLock = *pc.Project.Lock originalLock.SolveMeta.InputsDigest = []byte{} // zero out the input hash to ensure non-equivalency - sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorNever) + sw, _ := NewSafeWriter(nil, originalLock, pc.Project.Lock, VendorNever, gps.DefaultRootPruneOptions()) // Verify prepared actions if sw.HasManifest() { @@ -362,12 +364,12 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways) + sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways, gps.DefaultRootPruneOptions()) err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger()) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify prepared actions - sw, _ = NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways) + sw, _ = NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways, gps.DefaultRootPruneOptions()) if sw.HasManifest() { t.Fatal("Did not expect the payload to contain the manifest") } @@ -414,7 +416,7 @@ func TestSafeWriter_NewLock(t *testing.T) { defer lf.Close() newLock, err := readLock(lf) h.Must(err) - sw, _ := NewSafeWriter(nil, nil, newLock, VendorOnChanged) + sw, _ := NewSafeWriter(nil, nil, newLock, VendorOnChanged, gps.DefaultRootPruneOptions()) // Verify prepared actions if sw.HasManifest() { @@ -461,7 +463,7 @@ func TestSafeWriter_NewLockSkipVendor(t *testing.T) { defer lf.Close() newLock, err := readLock(lf) h.Must(err) - sw, _ := NewSafeWriter(nil, nil, newLock, VendorNever) + sw, _ := NewSafeWriter(nil, nil, newLock, VendorNever, gps.DefaultRootPruneOptions()) // Verify prepared actions if sw.HasManifest() { @@ -510,7 +512,7 @@ func TestSafeWriter_DiffLocks(t *testing.T) { updatedLock, err := readLock(ulf) h.Must(err) - sw, _ := NewSafeWriter(nil, pc.Project.Lock, updatedLock, VendorOnChanged) + sw, _ := NewSafeWriter(nil, pc.Project.Lock, updatedLock, VendorOnChanged, gps.DefaultRootPruneOptions()) // Verify lock diff diff := sw.lockDiff @@ -555,7 +557,7 @@ func TestSafeWriter_VendorDotGitPreservedWithForceVendor(t *testing.T) { pc.CopyFile(LockName, safeWriterGoldenLock) pc.Load() - sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways) + sw, _ := NewSafeWriter(pc.Project.Manifest, pc.Project.Lock, pc.Project.Lock, VendorAlways, gps.DefaultRootPruneOptions()) // Verify prepared actions if !sw.HasManifest() { From e113a9be23d2dee5549f41db195965f9ea68b219 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 5 Dec 2017 18:07:08 +0300 Subject: [PATCH 07/11] fix deriveFilesystemState bug on Windows junctions Signed-off-by: Ibrahim AshShohail --- gps/filesystem.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/gps/filesystem.go b/gps/filesystem.go index c3cbfa772a..903bdea13f 100644 --- a/gps/filesystem.go +++ b/gps/filesystem.go @@ -7,6 +7,7 @@ package gps import ( "os" "path/filepath" + "runtime" ) // filesystemState represents the state of a file system. @@ -44,7 +45,22 @@ func deriveFilesystemState(root string) (filesystemState, error) { return err } - if (info.Mode() & os.ModeSymlink) != 0 { + symlink := (info.Mode() & os.ModeSymlink) == os.ModeSymlink + dir := info.IsDir() + + if runtime.GOOS == "windows" && 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 nil + } + + if symlink { eval, err := filepath.EvalSymlinks(path) if err != nil { return err @@ -58,7 +74,7 @@ func deriveFilesystemState(root string) (filesystemState, error) { return nil } - if info.IsDir() { + if dir { fs.dirs = append(fs.dirs, relPath) return nil From fb9ac8c17aef648daac07b376568d14fefefab9f Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Thu, 7 Dec 2017 23:49:59 +0300 Subject: [PATCH 08/11] resolve nits Signed-off-by: Ibrahim AshShohail --- gps/filesystem.go | 4 +--- gps/prune.go | 6 ++---- txn_writer_test.go | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/gps/filesystem.go b/gps/filesystem.go index 903bdea13f..c2efa5bfbc 100644 --- a/gps/filesystem.go +++ b/gps/filesystem.go @@ -27,9 +27,7 @@ type fsLink struct { // deriveFilesystemState returns a filesystemState based on the state of // the filesystem on root. func deriveFilesystemState(root string) (filesystemState, error) { - fs := filesystemState{ - root: root, - } + fs := filesystemState{root: root} err := filepath.Walk(fs.root, func(path string, info os.FileInfo, err error) error { if err != nil { diff --git a/gps/prune.go b/gps/prune.go index 5fb1ee766a..033436de6f 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -51,7 +51,7 @@ func DefaultRootPruneOptions() RootPruneOptions { // PruneOptionsFor returns the prune options for the passed project root. // // It will return the root prune options if the project does not have specific -// options or if it does not exists in the manifest. +// options or if it does not exist in the manifest. func (o *RootPruneOptions) PruneOptionsFor(pr ProjectRoot) PruneOptions { if po, ok := o.ProjectOptions[pr]; ok { return po @@ -203,13 +203,12 @@ func collectUnusedPackagesFiles(fsState filesystemState, unusedPackages map[stri // // Files matching licenseFilePrefixes and legalFileSubstrings are not pruned. func pruneNonGoFiles(fsState filesystemState) error { - // TODO(ibrasho) detemine a sane capacity toDelete := make([]string, 0, len(fsState.files)/4) for _, path := range fsState.files { ext := fileExt(path) - // Refer to: https://sourcegraph.com/github.com/golang/go/-/blob/src/go/build/build.go#L750 + // Refer to: https://github.com/golang/go/blob/release-branch.go1.9/src/go/build/build.go#L750 switch ext { case ".go": continue @@ -274,7 +273,6 @@ func isPreservedFile(name string) bool { // pruneGoTestFiles deletes all Go test files (*_test.go) in fsState. func pruneGoTestFiles(fsState filesystemState) error { - // TODO(ibrasho) detemine a sane capacity toDelete := make([]string, 0, len(fsState.files)/2) for _, path := range fsState.files { diff --git a/txn_writer_test.go b/txn_writer_test.go index 70dde8cebf..45181f5c11 100644 --- a/txn_writer_test.go +++ b/txn_writer_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/golang/dep/gps" - "github.com/golang/dep/internal/test" "github.com/pkg/errors" ) From b4fca9bfce08e8adfe512d35fe1badaef226b4b8 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Fri, 8 Dec 2017 21:42:40 +0300 Subject: [PATCH 09/11] gps: handle symlinks properly We now delete anything that looks like a symlink if its called "vendor" while pruning. Hopefully, you didn't make a bad decision by relying on the magical properties of symlinks. Signed-off-by: Ibrahim AshShohail --- gps/filesystem.go | 99 ++++++++---- gps/filesystem_test.go | 151 ++++++++++++++---- gps/prune.go | 32 +++- gps/prune_test.go | 254 +++++++++++++++++++++++++++++-- gps/prune_vendor.go | 29 ---- gps/prune_vendor_test.go | 76 --------- gps/prune_vendor_windows.go | 38 ----- gps/prune_vendor_windows_test.go | 183 ---------------------- 8 files changed, 457 insertions(+), 405 deletions(-) delete mode 100644 gps/prune_vendor.go delete mode 100644 gps/prune_vendor_test.go delete mode 100644 gps/prune_vendor_windows.go delete mode 100644 gps/prune_vendor_windows_test.go diff --git a/gps/filesystem.go b/gps/filesystem.go index c2efa5bfbc..fd683f325c 100644 --- a/gps/filesystem.go +++ b/gps/filesystem.go @@ -7,9 +7,25 @@ package gps import ( "os" "path/filepath" - "runtime" + "strings" + + "github.com/pkg/errors" ) +// fsLink represents a symbolic link. +type fsLink struct { + path string + to string + + // circular denotes if evaluating the symlink fails with "too many links" error. + // This errors means that it's very likely that the symlink has circual refernce. + circular bool + + // broken denotes that attempting to resolve the link fails, most likely because + // the destaination doesn't exist. + broken bool +} + // filesystemState represents the state of a file system. type filesystemState struct { root string @@ -18,10 +34,51 @@ type filesystemState struct { links []fsLink } -// fsLink represents a symbolic link. -type fsLink struct { - path string - to string +func (s filesystemState) setup() error { + for _, dir := range s.dirs { + p := filepath.Join(s.root, dir) + + if err := os.MkdirAll(p, 0777); err != nil { + return errors.Errorf("os.MkdirAll(%q, 0777) err=%q", p, err) + } + } + + for _, file := range s.files { + p := filepath.Join(s.root, file) + + f, err := os.Create(p) + if err != nil { + return errors.Errorf("os.Create(%q) err=%q", p, err) + } + + if err := f.Close(); err != nil { + return errors.Errorf("file %q Close() err=%q", p, err) + } + } + + for _, link := range s.links { + p := filepath.Join(s.root, link.path) + + // On Windows, relative symlinks confuse filepath.Walk. So, we'll just sigh + // and do absolute links, assuming they are relative to the directory of + // link.path. + // + // Reference: https://github.com/golang/go/issues/17540 + // + // TODO(ibrasho): This was fixed in Go 1.9. Remove this when support for + // 1.8 is dropped. + dir := filepath.Dir(p) + to := "" + if link.to != "" { + to = filepath.Join(dir, link.to) + } + + if err := os.Symlink(to, p); err != nil { + return errors.Errorf("os.Symlink(%q, %q) err=%q", to, p, err) + } + } + + return nil } // deriveFilesystemState returns a filesystemState based on the state of @@ -43,36 +100,24 @@ func deriveFilesystemState(root string) (filesystemState, error) { return err } - symlink := (info.Mode() & os.ModeSymlink) == os.ModeSymlink - dir := info.IsDir() - - if runtime.GOOS == "windows" && 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 nil - } + if (info.Mode() & os.ModeSymlink) != 0 { + l := fsLink{path: relPath} - if symlink { - eval, err := filepath.EvalSymlinks(path) - if err != nil { + l.to, err = filepath.EvalSymlinks(path) + if err != nil && strings.HasSuffix(err.Error(), "too many links") { + l.circular = true + } else if err != nil && os.IsNotExist(err) { + l.broken = true + } else if err != nil { return err } - fs.links = append(fs.links, fsLink{ - path: relPath, - to: eval, - }) + fs.links = append(fs.links, l) return nil } - if dir { + if info.IsDir() { fs.dirs = append(fs.dirs, relPath) return nil diff --git a/gps/filesystem_test.go b/gps/filesystem_test.go index 78c28cdac1..af29846fd0 100644 --- a/gps/filesystem_test.go +++ b/gps/filesystem_test.go @@ -5,9 +5,13 @@ package gps import ( + "fmt" "os" "path/filepath" + "reflect" "testing" + + "github.com/golang/dep/internal/test" ) // This file contains utilities for running tests around file system state. @@ -89,45 +93,128 @@ func (tc fsTestCase) assert(t *testing.T) { // setup inflates fs onto the actual host file system at tc.before.root. // It doesn't delete existing files and should be used on empty roots only. func (tc fsTestCase) setup(t *testing.T) { - tc.setupDirs(t) - tc.setupFiles(t) - tc.setupLinks(t) -} - -func (tc fsTestCase) setupDirs(t *testing.T) { - for _, dir := range tc.before.dirs { - p := filepath.Join(tc.before.root, dir) - if err := os.MkdirAll(p, 0777); err != nil { - t.Fatalf("os.MkdirAll(%q, 0777) err=%q", p, err) - } + if err := tc.before.setup(); err != nil { + t.Fatal(err) } } -func (tc fsTestCase) setupFiles(t *testing.T) { - for _, file := range tc.before.files { - p := filepath.Join(tc.before.root, file) - f, err := os.Create(p) - if err != nil { - t.Fatalf("os.Create(%q) err=%q", p, err) - } - if err := f.Close(); err != nil { - t.Fatalf("file %q Close() err=%q", p, err) - } +func TestDeriveFilesystemState(t *testing.T) { + testcases := []struct { + name string + fs fsTestCase + }{ + { + name: "simple-case", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "simple-dir", + }, + files: []string{ + "simple-file", + }, + }, + after: filesystemState{ + dirs: []string{ + "simple-dir", + }, + files: []string{ + "simple-file", + }, + }, + }, + }, + { + name: "simple-symlink-case", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "simple-dir", + }, + files: []string{ + "simple-file", + }, + links: []fsLink{ + fsLink{ + path: "link", + to: "nonexisting", + broken: true, + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "simple-dir", + }, + files: []string{ + "simple-file", + }, + links: []fsLink{ + fsLink{ + path: "link", + to: "", + broken: true, + }, + }, + }, + }, + }, + { + name: "complex-symlink-case", + fs: fsTestCase{ + before: filesystemState{ + links: []fsLink{ + fsLink{ + path: "link1", + to: "link2", + circular: true, + }, + fsLink{ + path: "link2", + to: "link1", + circular: true, + }, + }, + }, + after: filesystemState{ + links: []fsLink{ + fsLink{ + path: "link1", + to: "", + circular: true, + }, + fsLink{ + path: "link2", + to: "", + circular: true, + }, + }, + }, + }, + }, } -} -func (tc fsTestCase) setupLinks(t *testing.T) { - for _, link := range tc.before.links { - p := filepath.Join(tc.before.root, link.path) + for _, tc := range testcases { + h := test.NewHelper(t) + + h.TempDir(tc.name) - // On Windows, relative symlinks confuse filepath.Walk. This is golang/go - // issue 17540. So, we'll just sigh and do absolute links, assuming they are - // relative to the directory of link.path. - dir := filepath.Dir(p) - to := filepath.Join(dir, link.to) + tc.fs.before.root = h.Path(tc.name) + tc.fs.after.root = h.Path(tc.name) - if err := os.Symlink(to, p); err != nil { - t.Fatalf("os.Symlink(%q, %q) err=%q", to, p, err) + tc.fs.setup(t) + + state, err := deriveFilesystemState(h.Path(tc.name)) + if err != nil { + t.Fatal(err) } + + if !reflect.DeepEqual(tc.fs.after, state) { + fmt.Println(tc.fs.after) + fmt.Println(state) + t.Fatal("filesystem state mismatch") + } + + h.Cleanup() } } diff --git a/gps/prune.go b/gps/prune.go index 033436de6f..5ba7f15bb1 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -8,6 +8,7 @@ import ( "log" "os" "path/filepath" + "sort" "strings" "github.com/golang/dep/internal/fs" @@ -126,11 +127,21 @@ func PruneProject(baseDir string, lp LockedProject, options PruneOptions, logger // pruneVendorDirs deletes all nested vendor directories within baseDir. func pruneVendorDirs(fsState filesystemState) error { - toDelete := collectNestedVendorDirs(fsState) + for _, dir := range fsState.dirs { + if filepath.Base(dir) == "vendor" { + err := os.RemoveAll(filepath.Join(fsState.root, dir)) + if err != nil && !os.IsNotExist(err) { + return err + } + } + } - for _, path := range toDelete { - if err := os.RemoveAll(path); err != nil && !os.IsNotExist(err) { - return err + for _, link := range fsState.links { + if filepath.Base(link.path) == "vendor" { + err := os.Remove(filepath.Join(fsState.root, link.path)) + if err != nil && !os.IsNotExist(err) { + return err + } } } @@ -291,6 +302,8 @@ func pruneGoTestFiles(fsState filesystemState) error { } func deleteEmptyDirs(fsState filesystemState) error { + toDelete := make(sort.StringSlice, 0) + for _, dir := range fsState.dirs { path := filepath.Join(fsState.root, dir) @@ -300,9 +313,14 @@ func deleteEmptyDirs(fsState filesystemState) error { } if !notEmpty { - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - return err - } + toDelete = append(toDelete, path) + } + } + + sort.Sort(sort.Reverse(sort.StringSlice(toDelete))) + for _, path := range toDelete { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err } } diff --git a/gps/prune_test.go b/gps/prune_test.go index aae1ec9a75..417d88892f 100644 --- a/gps/prune_test.go +++ b/gps/prune_test.go @@ -7,6 +7,7 @@ package gps import ( "io/ioutil" "log" + "os" "testing" "github.com/golang/dep/internal/test" @@ -51,19 +52,6 @@ func TestPruneProject(t *testing.T) { } } -// func TestPruneVendorDirs(t *testing.T) { -// h := test.NewHelper(t) -// defer h.Cleanup() - -// h.TempDir(".") -// baseDir := h.Path(".") - -// err := pruneVendorDirs(baseDir) -// if err != nil { -// t.Fatal(err) -// } -// } - func TestPruneUnusedPackages(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() @@ -389,3 +377,243 @@ func TestPruneGoTestFiles(t *testing.T) { }) } } + +func TestPruneVendorDirs(t *testing.T) { + tests := []struct { + name string + test fsTestCase + }{ + { + name: "vendor directory", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + "package/vendor", + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + }, + }, + }, + { + name: "vendor file", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + }, + files: []string{ + "package/vendor", + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + files: []string{ + "package/vendor", + }, + }, + }, + }, + { + name: "vendor symlink", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + "package/_vendor", + }, + links: []fsLink{ + { + path: "package/vendor", + to: "_vendor", + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + "package/_vendor", + }, + }, + }, + }, + { + name: "nonvendor symlink", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + "package/_vendor", + }, + links: []fsLink{ + { + path: "package/link", + to: "_vendor", + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + "package/_vendor", + }, + links: []fsLink{ + { + path: "package/link", + to: "_vendor", + }, + }, + }, + }, + }, + { + name: "vendor symlink to file", + test: fsTestCase{ + before: filesystemState{ + files: []string{ + "file", + }, + links: []fsLink{ + { + path: "vendor", + to: "file", + }, + }, + }, + after: filesystemState{ + files: []string{ + "file", + }, + }, + }, + }, + { + name: "broken vendor symlink", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + }, + links: []fsLink{ + { + path: "package/vendor", + to: "nonexistence", + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + links: []fsLink{}, + }, + }, + }, + { + name: "chained symlinks", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "_vendor", + }, + links: []fsLink{ + { + path: "vendor", + to: "vendor2", + }, + { + path: "vendor2", + to: "_vendor", + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "_vendor", + }, + links: []fsLink{ + { + path: "vendor2", + to: "_vendor", + }, + }, + }, + }, + }, + { + name: "circular symlinks", + test: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "package", + }, + links: []fsLink{ + { + path: "package/link1", + to: "link2", + }, + { + path: "package/link2", + to: "link1", + }, + }, + }, + after: filesystemState{ + dirs: []string{ + "package", + }, + links: []fsLink{ + { + path: "package/link1", + to: "link2", + }, + { + path: "package/link2", + to: "link1", + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, pruneVendorDirsTestCase(test.test)) + } +} + +func pruneVendorDirsTestCase(tc fsTestCase) func(*testing.T) { + return func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "pruneVendorDirsTestCase") + if err != nil { + t.Fatalf("ioutil.TempDir err=%q", err) + } + defer func() { + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf("os.RemoveAll(%q) err=%q", tempDir, err) + } + }() + + tc.before.root = tempDir + tc.after.root = tempDir + + tc.setup(t) + + fs, err := deriveFilesystemState(tempDir) + if err != nil { + t.Fatalf("deriveFilesystemState failed: %s", err) + } + + if err := pruneVendorDirs(fs); err != nil { + t.Errorf("pruneVendorDirs err=%q", err) + } + + tc.assert(t) + } +} diff --git a/gps/prune_vendor.go b/gps/prune_vendor.go deleted file mode 100644 index c53d3e7f7b..0000000000 --- a/gps/prune_vendor.go +++ /dev/null @@ -1,29 +0,0 @@ -// 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. - -//+build !windows - -package gps - -import ( - "path/filepath" -) - -func collectNestedVendorDirs(fsState filesystemState) []string { - toDelete := make([]string, 0, len(fsState.dirs)/4) - - for _, dir := range fsState.dirs { - if filepath.Base(dir) == "vendor" { - toDelete = append(toDelete, filepath.Join(fsState.root, dir)) - } - } - - for _, link := range fsState.links { - if filepath.Base(link.path) == "vendor" { - toDelete = append(toDelete, filepath.Join(fsState.root, link.path)) - } - } - - return toDelete -} diff --git a/gps/prune_vendor_test.go b/gps/prune_vendor_test.go deleted file mode 100644 index 79c4a3a4b4..0000000000 --- a/gps/prune_vendor_test.go +++ /dev/null @@ -1,76 +0,0 @@ -// 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" - "os" - "testing" -) - -func pruneVendorDirsTestCase(tc fsTestCase) func(*testing.T) { - return func(t *testing.T) { - tempDir, err := ioutil.TempDir("", "pruneVendorDirsTestCase") - if err != nil { - t.Fatalf("ioutil.TempDir err=%q", err) - } - defer func() { - if err := os.RemoveAll(tempDir); err != nil { - t.Errorf("os.RemoveAll(%q) err=%q", tempDir, err) - } - }() - - tc.before.root = tempDir - tc.after.root = tempDir - - tc.setup(t) - - fs, err := deriveFilesystemState(tempDir) - if err != nil { - t.Fatalf("deriveFilesystemState failed: %s", err) - } - - if err := pruneVendorDirs(fs); err != nil { - t.Errorf("pruneVendorDirs err=%q", err) - } - - tc.assert(t) - } -} - -func TestPruneVendorDirs(t *testing.T) { - t.Run("vendor directory", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - "package/vendor", - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - }, - }, - })) - - t.Run("vendor file", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - }, - files: []string{ - "package/vendor", - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - }, - files: []string{ - "package/vendor", - }, - }, - })) -} diff --git a/gps/prune_vendor_windows.go b/gps/prune_vendor_windows.go deleted file mode 100644 index dd873a207b..0000000000 --- a/gps/prune_vendor_windows.go +++ /dev/null @@ -1,38 +0,0 @@ -// 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 ( - "os" - "path/filepath" -) - -func collectNestedVendorDirs(fsState filesystemState) []string { - toDelete := make([]string, 0, len(fsState.dirs)/4) - - for _, dir := range fsState.dirs { - if filepath.Base(dir) == "vendor" { - toDelete = append(toDelete, filepath.Join(fsState.root, dir)) - } - } - - for _, link := range fsState.links { - if filepath.Base(link.path) == "vendor" { - // 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. - if info, err := os.Stat(link.path); err == nil && info.IsDir() { - toDelete = append(toDelete, filepath.Join(fsState.root, link.path)) - } - - } - } - - return toDelete -} diff --git a/gps/prune_vendor_windows_test.go b/gps/prune_vendor_windows_test.go deleted file mode 100644 index fb4b06bb38..0000000000 --- a/gps/prune_vendor_windows_test.go +++ /dev/null @@ -1,183 +0,0 @@ -// 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. - -// +build windows - -package gps - -import "testing" - -func TestPruneVendorSymlinks(t *testing.T) { - // On windows, we skip symlinks, even if they're named 'vendor', because - // they're too hard to distinguish from junctions. - t.Run("vendor symlink", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - "package/_vendor", - }, - links: []fsLink{ - { - path: "package/vendor", - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - "package/_vendor", - }, - links: []fsLink{ - { - path: "package/vendor", - to: "_vendor", - }, - }, - }, - })) - - t.Run("nonvendor symlink", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - "package/_vendor", - }, - links: []fsLink{ - { - path: "package/link", - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - "package/_vendor", - }, - links: []fsLink{ - { - path: "package/link", - to: "_vendor", - }, - }, - }, - })) - - t.Run("vendor symlink to file", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - files: []string{ - "file", - }, - links: []fsLink{ - { - path: "vendor", - to: "file", - }, - }, - }, - after: filesystemState{ - files: []string{ - "file", - }, - links: []fsLink{ - { - path: "vendor", - to: "file", - }, - }, - }, - })) - - t.Run("broken vendor symlink", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - }, - links: []fsLink{ - { - path: "package/vendor", - to: "nonexistence", - }, - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - }, - links: []fsLink{ - { - path: "package/vendor", - to: "nonexistence", - }, - }, - }, - })) - - t.Run("chained symlinks", pruneVendorDirsTestCase(fsTestCase{ - // Curiously, if a symlink on windows points to *another* symlink which - // eventually points at a directory, we'll correctly remove that first - // symlink, because the first symlink doesn't appear to Go to be a - // directory. - before: filesystemState{ - dirs: []string{ - "_vendor", - }, - links: []fsLink{ - { - path: "vendor", - to: "vendor2", - }, - { - path: "vendor2", - to: "_vendor", - }, - }, - }, - after: filesystemState{ - dirs: []string{ - "_vendor", - }, - links: []fsLink{ - { - path: "vendor2", - to: "_vendor", - }, - }, - }, - })) - - t.Run("circular symlinks", pruneVendorDirsTestCase(fsTestCase{ - before: filesystemState{ - dirs: []string{ - "package", - }, - links: []fsLink{ - { - path: "package/link1", - to: "link2", - }, - { - path: "package/link2", - to: "link1", - }, - }, - }, - after: filesystemState{ - dirs: []string{ - "package", - }, - links: []fsLink{ - { - path: "package/link1", - to: "link2", - }, - { - path: "package/link2", - to: "link1", - }, - }, - }, - })) -} From c7c3fd83f65b2b0589fcf75b9d6d9759a27cd4bb Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 12 Dec 2017 11:09:19 +0300 Subject: [PATCH 10/11] gps: fix a bug that kept empty dirs after pruning Signed-off-by: Ibrahim AshShohail --- gps/prune.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/gps/prune.go b/gps/prune.go index 5ba7f15bb1..40d645460e 100644 --- a/gps/prune.go +++ b/gps/prune.go @@ -302,7 +302,7 @@ func pruneGoTestFiles(fsState filesystemState) error { } func deleteEmptyDirs(fsState filesystemState) error { - toDelete := make(sort.StringSlice, 0) + sort.Sort(sort.Reverse(sort.StringSlice(fsState.dirs))) for _, dir := range fsState.dirs { path := filepath.Join(fsState.root, dir) @@ -313,14 +313,9 @@ func deleteEmptyDirs(fsState filesystemState) error { } if !notEmpty { - toDelete = append(toDelete, path) - } - } - - sort.Sort(sort.Reverse(sort.StringSlice(toDelete))) - for _, path := range toDelete { - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - return err + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } } } From cef261f120a75804a8f640ada94343c7c23d6cf3 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Thu, 14 Dec 2017 09:36:51 +0300 Subject: [PATCH 11/11] gps: add tests for deleteEmptyDirs Signed-off-by: Ibrahim AshShohail --- gps/prune_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/gps/prune_test.go b/gps/prune_test.go index 417d88892f..d4c7371237 100644 --- a/gps/prune_test.go +++ b/gps/prune_test.go @@ -617,3 +617,98 @@ func pruneVendorDirsTestCase(tc fsTestCase) func(*testing.T) { tc.assert(t) } } + +func TestDeleteEmptyDirs(t *testing.T) { + testcases := []struct { + name string + fs fsTestCase + }{ + { + name: "empty-dir", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "pkg1", + }, + }, + after: filesystemState{}, + }, + }, + { + name: "nested-empty-dirs", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "pkg1", + "pkg1/pkg2", + }, + }, + after: filesystemState{}, + }, + }, + { + name: "non-empty-dir", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "pkg1", + }, + files: []string{ + "pkg1/file1", + }, + }, + after: filesystemState{ + dirs: []string{ + "pkg1", + }, + files: []string{ + "pkg1/file1", + }, + }, + }, + }, + { + name: "mixed-dirs", + fs: fsTestCase{ + before: filesystemState{ + dirs: []string{ + "pkg1", + "pkg1/pkg2", + }, + files: []string{ + "pkg1/file1", + }, + }, + after: filesystemState{ + dirs: []string{ + "pkg1", + }, + files: []string{ + "pkg1/file1", + }, + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + h := test.NewHelper(t) + h.Cleanup() + h.TempDir(".") + + tc.fs.before.root = h.Path(".") + tc.fs.after.root = h.Path(".") + + if err := tc.fs.before.setup(); err != nil { + t.Fatal("unexpected error in fs setup: ", err) + } + + if err := deleteEmptyDirs(tc.fs.before); err != nil { + t.Fatal("unexpected error in deleteEmptyDirs: ", err) + } + + tc.fs.assert(t) + }) + } +}