From bfe75b3cd1cfd9376c3aa5a39c4230d4b5526a66 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 8 Aug 2017 02:05:31 +0300 Subject: [PATCH 1/5] cmd/dep: remove dep prune command Signed-off-by: Ibrahim AshShohail --- cmd/dep/main.go | 1 - cmd/dep/prune.go | 212 ------------------ cmd/dep/prune_test.go | 46 ---- .../prune/without_lock/final/Gopkg.toml | 3 - .../prune/without_lock/initial/Gopkg.toml | 3 - .../prune/without_lock/testcase.json | 6 - 6 files changed, 271 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 5b8db13d3c..d308adc3a0 100644 --- a/cmd/dep/main.go +++ b/cmd/dep/main.go @@ -61,7 +61,6 @@ func (c *Config) Run() (exitCode int) { &statusCommand{}, &ensureCommand{}, &hashinCommand{}, - &pruneCommand{}, } examples := [][2]string{ diff --git a/cmd/dep/prune.go b/cmd/dep/prune.go deleted file mode 100644 index 94f057028e..0000000000 --- a/cmd/dep/prune.go +++ /dev/null @@ -1,212 +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/internal/fs" - "github.com/golang/dep/internal/gps" - "github.com/golang/dep/internal/gps/pkgtree" - "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.") - } - - var pruneLogger *log.Logger - if ctx.Verbose { - pruneLogger = ctx.Err - } - 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 logger != nil { - 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) { - if logger != nil { - 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)) - if logger != nil { - 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 a59d443c11..0000000000 --- a/cmd/dep/prune_test.go +++ /dev/null @@ -1,46 +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 ( - "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"), - } - - got, err := calculatePrune(h.Path(vendorDir), toKeep, nil) - 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 b02c2cf6d8af93737f749115e406cf93597e63d1 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 8 Aug 2017 02:14:55 +0300 Subject: [PATCH 2/5] dep: add NewManifest and add prune options This change updates the manifest to accept a new table: prune. This table defines pruning options accepted by dep. By default, dep will always prune nested vendor directories. Three additional options can be toggled in the manifest: * non-go: Prune non-Go files (will keep LICENSE & COPYING files) * go-tests: Prune Go test files * unused-packages: Prune unused Go packages from dependencies. The implementation of these flags is not part of this commit. A constructor for dep.Manifest was also added. Signed-off-by: Ibrahim AshShohail --- Gopkg.toml | 5 +++ cmd/dep/ensure.go | 5 +-- cmd/dep/glide_importer.go | 4 +- cmd/dep/glide_importer_test.go | 24 ++++++----- cmd/dep/godep_importer.go | 4 +- cmd/dep/gopath_scanner.go | 7 ++- cmd/dep/gopath_scanner_test.go | 21 ++++----- cmd/dep/root_analyzer.go | 8 ++-- cmd/dep/vndr_importer.go | 8 ++-- cmd/dep/vndr_importer_test.go | 17 +++----- internal/gps/prune.go | 27 ++++++++++++ manifest.go | 78 +++++++++++++++++++++++++++------- manifest_test.go | 45 +++++++++----------- project_test.go | 5 ++- 14 files changed, 159 insertions(+), 99 deletions(-) create mode 100644 internal/gps/prune.go diff --git a/Gopkg.toml b/Gopkg.toml index fcd285c732..4ae7bd467f 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -18,3 +18,8 @@ [[constraint]] name = "github.com/pkg/errors" version = "0.8.0" + +[prune] + non-go = true + go-tests = true + unused-packages = true diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index d646fd361b..360abfbad0 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -603,9 +603,8 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm // Prep post-actions and feedback from adds. var reqlist []string - appender := &dep.Manifest{ - Constraints: make(gps.ProjectConstraints), - } + appender := dep.NewManifest() + for pr, instr := range addInstructions { for path := range instr.ephReq { reqlist = append(reqlist, path) diff --git a/cmd/dep/glide_importer.go b/cmd/dep/glide_importer.go index b343e0c794..17503cfec3 100644 --- a/cmd/dep/glide_importer.go +++ b/cmd/dep/glide_importer.go @@ -141,9 +141,7 @@ func (g *glideImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e task.WriteString("...") g.logger.Println(task) - manifest := &dep.Manifest{ - Constraints: make(gps.ProjectConstraints), - } + manifest := dep.NewManifest() for _, pkg := range append(g.yaml.Imports, g.yaml.TestImports...) { pc, err := g.buildProjectConstraint(pkg) diff --git a/cmd/dep/glide_importer_test.go b/cmd/dep/glide_importer_test.go index af9904b51c..6aa0a4be65 100644 --- a/cmd/dep/glide_importer_test.go +++ b/cmd/dep/glide_importer_test.go @@ -66,13 +66,14 @@ func TestGlideConfig_Convert(t *testing.T) { }, }, }, - projectRoot: "github.com/sdboyer/deptest", - wantSourceRepo: "https://github.com/sdboyer/deptest.git", - matchPairedVersion: true, - wantConstraint: "^1.0.0", - wantLockCount: 1, - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", + projectRoot: "github.com/sdboyer/deptest", + wantSourceRepo: "https://github.com/sdboyer/deptest.git", + matchPairedVersion: true, + wantConstraint: "^1.0.0", + wantLockCount: 1, + wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), + wantVersion: "v1.0.0", + wantIgnoredPackages: []string{}, }, "test project": { yaml: glideYaml{ @@ -91,10 +92,11 @@ func TestGlideConfig_Convert(t *testing.T) { }, }, }, - projectRoot: "github.com/sdboyer/deptest", - wantLockCount: 1, - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", + projectRoot: "github.com/sdboyer/deptest", + wantLockCount: 1, + wantConstraint: "^1.0.0", + wantVersion: "v1.0.0", + wantIgnoredPackages: []string{}, }, "with ignored package": { yaml: glideYaml{ diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go index 937fd2c0da..1baa8aef04 100644 --- a/cmd/dep/godep_importer.go +++ b/cmd/dep/godep_importer.go @@ -88,9 +88,7 @@ func (g *godepImporter) load(projectDir string) error { func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { g.logger.Println("Converting from Godeps.json ...") - manifest := &dep.Manifest{ - Constraints: make(gps.ProjectConstraints), - } + manifest := dep.NewManifest() lock := &dep.Lock{} for _, pkg := range g.json.Imports { diff --git a/cmd/dep/gopath_scanner.go b/cmd/dep/gopath_scanner.go index b04e368e2a..99dbb1f6ed 100644 --- a/cmd/dep/gopath_scanner.go +++ b/cmd/dep/gopath_scanner.go @@ -52,10 +52,9 @@ func (g *gopathScanner) InitializeRootManifestAndLock(rootM *dep.Manifest, rootL return err } - g.origM = &dep.Manifest{ - Constraints: g.pd.constraints, - Ovr: make(gps.ProjectConstraints), - } + g.origM = dep.NewManifest() + g.origM.Constraints = g.pd.constraints + g.origL = &dep.Lock{ P: make([]gps.LockedProject, 0, len(g.pd.ondisk)), } diff --git a/cmd/dep/gopath_scanner_test.go b/cmd/dep/gopath_scanner_test.go index d40e35b310..87591646de 100644 --- a/cmd/dep/gopath_scanner_test.go +++ b/cmd/dep/gopath_scanner_test.go @@ -27,19 +27,14 @@ func TestGopathScanner_OverlayManifestConstraints(t *testing.T) { v1 := gps.NewVersion("v1.0.0") v2 := gps.NewVersion("v2.0.0") v3 := gps.NewVersion("v3.0.0") - rootM := &dep.Manifest{ - Constraints: gps.ProjectConstraints{ - pi1.ProjectRoot: gps.ProjectProperties{Constraint: v1}, - }, - } + rootM := dep.NewManifest() + rootM.Constraints[pi1.ProjectRoot] = gps.ProjectProperties{Constraint: v1} rootL := &dep.Lock{} + origM := dep.NewManifest() + origM.Constraints[pi1.ProjectRoot] = gps.ProjectProperties{Constraint: v2} + origM.Constraints[pi2.ProjectRoot] = gps.ProjectProperties{Constraint: v3} gs := gopathScanner{ - origM: &dep.Manifest{ - Constraints: gps.ProjectConstraints{ - pi1.ProjectRoot: gps.ProjectProperties{Constraint: v2}, - pi2.ProjectRoot: gps.ProjectProperties{Constraint: v3}, - }, - }, + origM: origM, origL: &dep.Lock{}, ctx: ctx, pd: projectData{ @@ -79,7 +74,7 @@ func TestGopathScanner_OverlayLockProjects(t *testing.T) { h := test.NewHelper(t) ctx := newTestContext(h) - rootM := &dep.Manifest{} + rootM := dep.NewManifest() pi1 := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(testProject1)} pi2 := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(testProject2)} v1 := gps.NewVersion("v1.0.0") @@ -89,7 +84,7 @@ func TestGopathScanner_OverlayLockProjects(t *testing.T) { P: []gps.LockedProject{gps.NewLockedProject(pi1, v1, []string{})}, } gs := gopathScanner{ - origM: &dep.Manifest{Constraints: make(gps.ProjectConstraints)}, + origM: dep.NewManifest(), origL: &dep.Lock{ P: []gps.LockedProject{ gps.NewLockedProject(pi1, v2, []string{}), // ignored, already exists in lock diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index 76a4d6d817..75b010c070 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.go @@ -52,10 +52,7 @@ func (a *rootAnalyzer) InitializeRootManifestAndLock(dir string, pr gps.ProjectR } if rootM == nil { - rootM = &dep.Manifest{ - Constraints: make(gps.ProjectConstraints), - Ovr: make(gps.ProjectConstraints), - } + rootM = dep.NewManifest() } if rootL == nil { rootL = &dep.Lock{} @@ -88,7 +85,8 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup } } - var emptyManifest = &dep.Manifest{Constraints: make(gps.ProjectConstraints), Ovr: make(gps.ProjectConstraints)} + var emptyManifest = dep.NewManifest() + return emptyManifest, nil, nil } diff --git a/cmd/dep/vndr_importer.go b/cmd/dep/vndr_importer.go index db936401c1..5ca889c4a4 100644 --- a/cmd/dep/vndr_importer.go +++ b/cmd/dep/vndr_importer.go @@ -86,11 +86,9 @@ func (v *vndrImporter) loadVndrFile(dir string) error { func (v *vndrImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { var ( - manifest = &dep.Manifest{ - Constraints: make(gps.ProjectConstraints), - } - lock = &dep.Lock{} - err error + manifest = dep.NewManifest() + lock = &dep.Lock{} + err error ) for _, pkg := range v.packages { diff --git a/cmd/dep/vndr_importer_test.go b/cmd/dep/vndr_importer_test.go index 2d7fb1a045..d67541fa6c 100644 --- a/cmd/dep/vndr_importer_test.go +++ b/cmd/dep/vndr_importer_test.go @@ -175,16 +175,13 @@ func TestVndrConfig_Import(t *testing.T) { constraint, err := gps.NewSemverConstraint("^2.0.0") h.Must(err) - wantM := &dep.Manifest{ - Constraints: gps.ProjectConstraints{ - "github.com/sdboyer/deptest": gps.ProjectProperties{ - Source: "https://github.com/sdboyer/deptest.git", - Constraint: gps.Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f"), - }, - "github.com/sdboyer/deptestdos": gps.ProjectProperties{ - Constraint: constraint, - }, - }, + wantM := dep.NewManifest() + wantM.Constraints["github.com/sdboyer/deptest"] = gps.ProjectProperties{ + Source: "https://github.com/sdboyer/deptest.git", + Constraint: gps.Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f"), + } + wantM.Constraints["github.com/sdboyer/deptestdos"] = gps.ProjectProperties{ + Constraint: constraint, } if !reflect.DeepEqual(wantM, m) { t.Errorf("unexpected manifest\nhave=%+v\nwant=%+v", m, wantM) diff --git a/internal/gps/prune.go b/internal/gps/prune.go new file mode 100644 index 0000000000..dd17dff9ec --- /dev/null +++ b/internal/gps/prune.go @@ -0,0 +1,27 @@ +// 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 + +// PruneOptions represents the pruning options used to write the dependecy tree. +type PruneOptions uint8 + +const ( + // PruneNestedVendorDirs indicates if nested vendor directories should be pruned. + PruneNestedVendorDirs = 1 << iota + // PruneUnusedPackages indicates if unused Go packages should be pruned. + PruneUnusedPackages + // PruneNonGoFiles indicates if non-Go files should be pruned. + // LICENSE & COPYING files are kept for convience. + PruneNonGoFiles + // PruneGoTestFiles indicates if Go test files should be pruned. + PruneGoTestFiles +) + +var ( + preservedNonGoFiles = []string{ + "LICENSE", + "COPYING", + } +) diff --git a/manifest.go b/manifest.go index f1708dbea3..551bf30b8f 100644 --- a/manifest.go +++ b/manifest.go @@ -26,21 +26,24 @@ var ( errInvalidOverride = errors.New("\"override\" must be a TOML array of tables") errInvalidRequired = errors.New("\"required\" must be a TOML list of strings") errInvalidIgnored = errors.New("\"ignored\" must be a TOML list of strings") + errInvalidPrune = errors.New("\"prune\" must be a TOML table of boolean values") ) // Manifest holds manifest file data and implements gps.RootManifest. type Manifest struct { - Constraints gps.ProjectConstraints - Ovr gps.ProjectConstraints - Ignored []string - Required []string + Constraints gps.ProjectConstraints + Ovr gps.ProjectConstraints + Ignored []string + Required []string + PruneOptions gps.PruneOptions } type rawManifest struct { - Constraints []rawProject `toml:"constraint,omitempty"` - Overrides []rawProject `toml:"override,omitempty"` - Ignored []string `toml:"ignored,omitempty"` - Required []string `toml:"required,omitempty"` + Constraints []rawProject `toml:"constraint,omitempty"` + Overrides []rawProject `toml:"override,omitempty"` + Ignored []string `toml:"ignored,omitempty"` + Required []string `toml:"required,omitempty"` + PruneOptions rawPruneOptions `toml:"prune,omitempty"` } type rawProject struct { @@ -51,6 +54,23 @@ type rawProject struct { Source string `toml:"source,omitempty"` } +type rawPruneOptions struct { + UnusedPackages bool `toml:"unused-packages,omitempty"` + NonGoFiles bool `toml:"non-go,omitempty"` + GoTests bool `toml:"go-tests,omitempty"` +} + +// NewManifest instantites a new manifest. +func NewManifest() *Manifest { + return &Manifest{ + Constraints: make(gps.ProjectConstraints), + Ovr: make(gps.ProjectConstraints), + Ignored: make([]string, 0), + Required: make([]string, 0), + PruneOptions: gps.PruneNestedVendorDirs, + } +} + func validateManifest(s string) ([]error, error) { var warns []error // Load the TomlTree from string @@ -103,7 +123,7 @@ func validateManifest(s string) ([]error, error) { } default: // unknown/invalid key - warns = append(warns, fmt.Errorf("Invalid key %q in %q", key, prop)) + warns = append(warns, fmt.Errorf("invalid key %q in %q", key, prop)) } } } @@ -140,8 +160,24 @@ func validateManifest(s string) ([]error, error) { return warns, errInvalidRequired } } + case "prune": + // Check if prune is of Map type + if reflect.TypeOf(val).Kind() != reflect.Map { + warns = append(warns, errors.New("prune should be a TOML table")) + } + for key, value := range val.(map[string]interface{}) { + switch key { + case "non-go", "go-tests", "unused-packages": + if _, ok := value.(bool); !ok { + warns = append(warns, errors.Errorf("unexpected type for prune.%s: %T", key, value)) + return warns, errInvalidPrune + } + default: + warns = append(warns, errors.Errorf("unknown field: prune.%s", key)) + } + } default: - warns = append(warns, fmt.Errorf("Unknown field in manifest: %v", prop)) + warns = append(warns, fmt.Errorf("unknown field in manifest: %v", prop)) } } @@ -172,12 +208,12 @@ func readManifest(r io.Reader) (*Manifest, []error, error) { } func fromRawManifest(raw rawManifest) (*Manifest, error) { - m := &Manifest{ - Constraints: make(gps.ProjectConstraints, len(raw.Constraints)), - Ovr: make(gps.ProjectConstraints, len(raw.Overrides)), - Ignored: raw.Ignored, - Required: raw.Required, - } + m := NewManifest() + + m.Constraints = make(gps.ProjectConstraints, len(raw.Constraints)) + m.Ovr = make(gps.ProjectConstraints, len(raw.Overrides)) + m.Ignored = raw.Ignored + m.Required = raw.Required for i := 0; i < len(raw.Constraints); i++ { name, prj, err := toProject(raw.Constraints[i]) @@ -201,6 +237,16 @@ func fromRawManifest(raw rawManifest) (*Manifest, error) { m.Ovr[name] = prj } + if raw.PruneOptions.UnusedPackages { + m.PruneOptions |= gps.PruneUnusedPackages + } + if raw.PruneOptions.GoTests { + m.PruneOptions |= gps.PruneGoTestFiles + } + if raw.PruneOptions.NonGoFiles { + m.PruneOptions |= gps.PruneNonGoFiles + } + return m, nil } diff --git a/manifest_test.go b/manifest_test.go index a747b4f8b4..ba0a0f0d32 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -62,23 +62,18 @@ func TestWriteManifest(t *testing.T) { golden := "manifest/golden.toml" want := h.GetTestFileString(golden) c, _ := gps.NewSemverConstraint("^0.12.0") - m := &Manifest{ - Constraints: map[gps.ProjectRoot]gps.ProjectProperties{ - gps.ProjectRoot("github.com/golang/dep/internal/gps"): { - Constraint: c, - }, - gps.ProjectRoot("github.com/babble/brook"): { - Constraint: gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb"), - }, - }, - Ovr: map[gps.ProjectRoot]gps.ProjectProperties{ - gps.ProjectRoot("github.com/golang/dep/internal/gps"): { - Source: "https://github.com/golang/dep/internal/gps", - Constraint: gps.NewBranch("master"), - }, - }, - Ignored: []string{"github.com/foo/bar"}, + m := NewManifest() + m.Constraints[gps.ProjectRoot("github.com/golang/dep/internal/gps")] = gps.ProjectProperties{ + Constraint: c, + } + m.Constraints[gps.ProjectRoot("github.com/babble/brook")] = gps.ProjectProperties{ + Constraint: gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb"), + } + m.Ovr[gps.ProjectRoot("github.com/golang/dep/internal/gps")] = gps.ProjectProperties{ + Source: "https://github.com/golang/dep/internal/gps", + Constraint: gps.NewBranch("master"), } + m.Ignored = []string{"github.com/foo/bar"} got, err := m.MarshalTOML() if err != nil { @@ -222,9 +217,9 @@ func TestValidateManifest(t *testing.T) { version = "" `, wantWarn: []error{ - errors.New("Unknown field in manifest: foo"), - errors.New("Unknown field in manifest: bar"), - errors.New("Unknown field in manifest: version"), + errors.New("unknown field in manifest: foo"), + errors.New("unknown field in manifest: bar"), + errors.New("unknown field in manifest: version"), }, wantError: nil, }, @@ -308,9 +303,9 @@ func TestValidateManifest(t *testing.T) { nick = "foo" `, wantWarn: []error{ - errors.New("Invalid key \"location\" in \"constraint\""), - errors.New("Invalid key \"link\" in \"constraint\""), - errors.New("Invalid key \"nick\" in \"override\""), + errors.New("invalid key \"location\" in \"constraint\""), + errors.New("invalid key \"link\" in \"constraint\""), + errors.New("invalid key \"nick\" in \"override\""), errors.New("metadata in \"constraint\" should be a TOML table"), }, wantError: nil, @@ -361,18 +356,18 @@ func TestValidateManifest(t *testing.T) { // compare validation errors if err != c.wantError { - t.Fatalf("Manifest errors are not as expected: \n\t(GOT) %v \n\t(WNT) %v", err, c.wantError) + t.Fatalf("manifest errors are not as expected: \n\t(GOT) %v \n\t(WNT) %v", err, c.wantError) } // compare length of error slice if len(errs) != len(c.wantWarn) { - t.Fatalf("Number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.wantWarn), c.wantWarn) + t.Fatalf("number of manifest errors are not as expected: \n\t(GOT) %v errors(%v)\n\t(WNT) %v errors(%v).", len(errs), errs, len(c.wantWarn), c.wantWarn) } // check if the expected errors exist in actual errors slice for _, er := range errs { if !contains(c.wantWarn, er) { - t.Fatalf("Manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.wantWarn) + t.Fatalf("manifest errors are not as expected: \n\t(MISSING) %v\n\t(FROM) %v", er, c.wantWarn) } } } diff --git a/project_test.go b/project_test.go index 2629a279d9..d2dedece7d 100644 --- a/project_test.go +++ b/project_test.go @@ -62,10 +62,13 @@ func TestFindRoot(t *testing.T) { } func TestProjectMakeParams(t *testing.T) { + m := NewManifest() + m.Ignored = []string{"ignoring this"} + p := Project{ AbsRoot: "someroot", ImportRoot: gps.ProjectRoot("Some project root"), - Manifest: &Manifest{Ignored: []string{"ignoring this"}}, + Manifest: m, Lock: &Lock{}, } From 40838f52fbbb6b8c0dc1edacb955e49a1b41bb96 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 8 Aug 2017 21:41:35 +0300 Subject: [PATCH 3/5] internal/gps: implement Prune and the related functions Signed-off-by: Ibrahim AshShohail --- internal/gps/prune.go | 346 ++++++++++++++++++++++++++- internal/gps/prune_test.go | 124 ++++++++++ internal/gps/strip_vendor.go | 52 ++-- internal/gps/strip_vendor_test.go | 2 +- internal/gps/strip_vendor_windows.go | 72 +++--- 5 files changed, 545 insertions(+), 51 deletions(-) create mode 100644 internal/gps/prune_test.go diff --git a/internal/gps/prune.go b/internal/gps/prune.go index dd17dff9ec..be008fda69 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -4,6 +4,18 @@ package gps +import ( + "io/ioutil" + "log" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/golang/dep/internal/fs" + "github.com/pkg/errors" +) + // PruneOptions represents the pruning options used to write the dependecy tree. type PruneOptions uint8 @@ -20,8 +32,336 @@ const ( ) var ( - preservedNonGoFiles = []string{ - "LICENSE", - "COPYING", + // licenseFilePrefixes is a list of name prefixes for licesnse files. + licenseFilePrefixes = []string{ + "license", + "licence", + "copying", + "unlicense", + "copyright", + "copyleft", + } + // legalFileSubstrings contains substrings that are likey part of a legal + // declaration file. + legalFileSubstrings = []string{ + "legal", + "notice", + "disclaimer", + "patent", + "third-party", + "thirdparty", } ) + +// Prune removes excess files from the dep tree whose root is baseDir based +// on the PruneOptions passed. +// +// A Lock must be passed if PruneUnusedPackages is toggled on. +func Prune(baseDir string, options PruneOptions, l Lock, logger *log.Logger) error { + if (options & PruneNestedVendorDirs) != 0 { + if err := pruneNestedVendorDirs(baseDir); err != nil { + return err + } + } + + if err := pruneEmptyDirs(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune empty dirs") + } + + if (options & PruneUnusedPackages) != 0 { + if l == nil { + return errors.New("pruning unused packages requires passing a non-nil Lock") + } + if err := pruneUnusedPackages(baseDir, l, logger); err != nil { + return errors.Wrap(err, "failed to prune unused packages") + } + } + + if (options & PruneNonGoFiles) != 0 { + if err := pruneNonGoFiles(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune non-Go files") + } + } + + if (options & PruneGoTestFiles) != 0 { + if err := pruneGoTestFiles(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune Go test files") + } + } + + // Delete all empty directories. + if err := pruneEmptyDirs(baseDir, logger); err != nil { + return errors.Wrap(err, "failed to prune empty dirs") + } + + return nil +} + +// pruneNestedVendorDirs deletes all nested vendor directories within baseDir. +func pruneNestedVendorDirs(baseDir string) error { + return filepath.Walk(baseDir, stripNestedVendorDirs(baseDir)) +} + +// pruneUnusedPackages deletes unimported packages found within baseDir. +// Determining whether packages are imported or not is based on the passed Lock. +func pruneUnusedPackages(baseDir string, l Lock, logger *log.Logger) error { + unused, err := calculateUnusedPackages(baseDir, l, logger) + if err != nil { + return err + } + + for _, pkg := range unused { + pkgPath := filepath.Join(baseDir, pkg) + + files, err := ioutil.ReadDir(pkgPath) + if err != nil { + // TODO(ibrasho) Handle this error properly. + // It happens when attempting to ioutil.ReadDir a submodule. + continue + } + + // Delete *.go files in the package directory. + for _, file := range files { + // Skip directories and files that don't have a .go suffix. + if file.IsDir() || !strings.HasSuffix(file.Name(), ".go") { + continue + } + + if err := os.Remove(filepath.Join(pkgPath, file.Name())); err != nil { + return err + } + } + } + + return nil +} + +// calculateUnusedPackages generates a list of unused packages existing within +// baseDir depending on the imported packages found in the passed Lock. +func calculateUnusedPackages(baseDir string, l Lock, logger *log.Logger) ([]string, error) { + imported := calculateImportedPackages(l) + sort.Strings(imported) + + var unused []string + + if logger != nil { + logger.Println("Calculating unused packages to prune.") + logger.Println("Checking the following packages:") + } + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore baseDir and anything that's not a directory. + if path == baseDir || !info.IsDir() { + return nil + } + + pkg := strings.TrimPrefix(path, baseDir+string(filepath.Separator)) + if logger != nil { + logger.Printf(" %s", pkg) + } + + // If pkg is not a parent of an imported package, add it to the + // unused list. + i := sort.Search(len(imported), func(i int) bool { + return pkg <= imported[i] + }) + if i >= len(imported) || !strings.HasPrefix(imported[i], pkg) { + unused = append(unused, path) + } + + return nil + }) + + return unused, err +} + +// calculateImportedPackages generates a list of imported packages from +// the passed Lock. +func calculateImportedPackages(l Lock) []string { + var imported []string + + for _, project := range l.Projects() { + projectRoot := string(project.Ident().ProjectRoot) + for _, pkg := range project.Packages() { + imported = append(imported, filepath.Join(projectRoot, pkg)) + } + } + return imported +} + +// pruneNonGoFiles delete all non-Go files existing within baseDir. +// Files with names that are prefixed by any entry in preservedNonGoFiles +// are not deleted. +func pruneNonGoFiles(baseDir string, logger *log.Logger) error { + files, err := calculateNonGoFiles(baseDir) + if err != nil { + return errors.Wrap(err, "could not prune non-Go files") + } + + if err := deleteFiles(files); err != nil { + return err + } + + return nil +} + +// calculateNonGoFiles returns a list of all non-Go files within baseDir. +// Files with names that are prefixed by any entry in preservedNonGoFiles +// are not deleted. +func calculateNonGoFiles(baseDir string) ([]string, error) { + var files []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore directories. + if info.IsDir() { + return nil + } + + // Ignore all Go files. + if strings.HasSuffix(info.Name(), ".go") { + return nil + } + + if !isPreservedNonGoFile(info.Name()) { + files = append(files, path) + } + + return nil + }) + + return files, err +} + +// isPreservedNonGoFile checks if the file name idicates that the file should be +// preserved. It assumes the file is not a Go file (doesn't have a .go suffix). +func isPreservedNonGoFile(name string) bool { + name = strings.ToLower(name) + + for _, prefix := range licenseFilePrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + + for _, substring := range legalFileSubstrings { + if strings.Contains(name, substring) { + return true + } + } + + return false +} + +// pruneGoTestFiles deletes all Go test files (*_test.go) within baseDirr. +func pruneGoTestFiles(baseDir string, logger *log.Logger) error { + files, err := calculateGoTestFiles(baseDir) + if err != nil { + return errors.Wrap(err, "could not prune Go test files") + } + + if err := deleteFiles(files); err != nil { + return err + } + + return nil +} + +// calculateGoTestFiles walks over baseDir and returns a list of all +// Go test files (any file that has the name *_test.go). +func calculateGoTestFiles(baseDir string) ([]string, error) { + var files []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Ignore directories. + if info.IsDir() { + return nil + } + + // Ignore any files that is not a Go test file. + if !strings.HasSuffix(info.Name(), "_test.go") { + return nil + } + + files = append(files, path) + + return nil + }) + + return files, err +} + +// pruneEmptyDirs delete all empty directories within baseDir. +func pruneEmptyDirs(baseDir string, logger *log.Logger) error { + empty, err := calculateEmptyDirs(baseDir) + if err != nil { + return err + } + + if logger != nil { + logger.Println("Deleting empty directories:") + } + + for _, dir := range empty { + if logger != nil { + logger.Printf(" %s\n", strings.TrimPrefix(dir, baseDir+string(os.PathSeparator))) + } + } + for _, dir := range empty { + if err := os.Remove(dir); err != nil { + return err + } + } + + return nil +} + +// calculateEmptyDirs walks over baseDir and returns a slice of empty directory paths. +func calculateEmptyDirs(baseDir string) ([]string, error) { + var empty []string + + err := filepath.Walk(baseDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil + } + + if baseDir == path { + return nil + } + + if !info.IsDir() { + return nil + } + + nonEmpty, err := fs.IsNonEmptyDir(path) + if err != nil { + return err + } else if !nonEmpty { + empty = append(empty, path) + } + + return nil + }) + + return empty, err +} + +func deleteFiles(paths []string) error { + for _, path := range paths { + if err := os.Remove(path); err != nil { + return err + } + } + return nil +} diff --git a/internal/gps/prune_test.go b/internal/gps/prune_test.go new file mode 100644 index 0000000000..b1347296d3 --- /dev/null +++ b/internal/gps/prune_test.go @@ -0,0 +1,124 @@ +package gps + +import ( + "io/ioutil" + "log" + "testing" + + "github.com/golang/dep/internal/test" +) + +func TestPruneEmptyDirs(t *testing.T) { + h := test.NewHelper(t) + h.TempDir("baseDir1/empty-dir") + + h.TempDir("baseDir2/non-empty-dir") + h.TempFile("baseDir2/non-empty-dir/file", "") + + h.TempDir("baseDir3/nested-empty-dirs") + h.TempDir("baseDir3/nested-empty-dirs/empty-dir1") + h.TempDir("baseDir3/nested-empty-dirs/empty-dir2") + + h.TempDir("baseDir4") + h.TempDir("baseDir4/empty-dir1") + h.TempDir("baseDir4/empty-dir2") + h.TempDir("baseDir4/non-empty-dir1") + h.TempFile("baseDir4/non-empty-dir1/file", "") + h.TempDir("baseDir4/non-empty-dir2") + h.TempFile("baseDir4/non-empty-dir2/file", "") + + testcases := []struct { + name string + baseDir string + emptyDirs []string + err bool + }{ + {"1 empty dir", h.Path("baseDir1"), []string{ + h.Path("baseDir1/empty-dir"), + }, false}, + {"1 non-empty dir", h.Path("baseDir2"), []string{}, false}, + {"nested empty dirs", h.Path("baseDir3"), []string{ + h.Path("baseDir3/nested-empty-dirs/empty-dir1"), + h.Path("baseDir3/nested-empty-dirs/empty-dir2"), + }, false}, + {"mixed dirs", h.Path("baseDir4"), []string{ + h.Path("baseDir4/empty-dir1"), + }, false}, + } + + logger := log.New(ioutil.Discard, "", 0) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := pruneEmptyDirs(tc.baseDir, logger) + if tc.err && err == nil { + t.Errorf("expected an error, got nil") + } else if !tc.err && err != nil { + t.Errorf("unexpected error: %s", err) + } + for _, path := range tc.emptyDirs { + h.ShouldNotExist(path) + } + }) + } +} + +func TestCalculateEmptyDirs(t *testing.T) { + h := test.NewHelper(t) + + h.TempDir("baseDir1/empty-dir") + + h.TempDir("baseDir2/non-empty-dir") + h.TempFile("baseDir2/non-empty-dir/file", "") + + h.TempDir("baseDir3/nested-empty-dirs") + h.TempDir("baseDir3/nested-empty-dirs/empty-dir1") + h.TempDir("baseDir3/nested-empty-dirs/empty-dir2") + + h.TempDir("baseDir4") + h.TempDir("baseDir4/empty-dir1") + h.TempDir("baseDir4/empty-dir2") + h.TempDir("baseDir4/non-empty-dir1") + h.TempFile("baseDir4/non-empty-dir1/file", "") + h.TempDir("baseDir4/non-empty-dir2") + h.TempFile("baseDir4/non-empty-dir2/file", "") + + testcases := []struct { + name string + baseDir string + emptyDirs []string + err bool + }{ + {"1 empty dir", h.Path("baseDir1"), []string{ + h.Path("baseDir1/empty-dir"), + }, false}, + {"1 non-empty dir", h.Path("baseDir2"), []string{}, false}, + {"nested empty dirs", h.Path("baseDir3"), []string{ + h.Path("baseDir3/nested-empty-dirs/empty-dir1"), + h.Path("baseDir3/nested-empty-dirs/empty-dir2"), + }, false}, + {"mixed dirs", h.Path("baseDir4"), []string{ + h.Path("baseDir4/empty-dir1"), + h.Path("baseDir4/empty-dir2"), + }, false}, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + emptyDirs, err := calculateEmptyDirs(tc.baseDir) + if len(emptyDirs) != len(tc.emptyDirs) { + t.Fatalf("expected %d paths, got %d", len(tc.emptyDirs), len(emptyDirs)) + } + for i := range tc.emptyDirs { + if tc.emptyDirs[i] != emptyDirs[i] { + t.Fatalf("expected %s to exists in the list, got %s", tc.emptyDirs[i], emptyDirs) + } + } + if tc.err && err == nil { + t.Errorf("expected an error, got nil") + } else if !tc.err && err != nil { + t.Errorf("unexpected error: %s", err) + } + }) + } +} diff --git a/internal/gps/strip_vendor.go b/internal/gps/strip_vendor.go index fec4ee13fb..3d70a5ff16 100644 --- a/internal/gps/strip_vendor.go +++ b/internal/gps/strip_vendor.go @@ -6,25 +6,43 @@ package gps -import "os" - -func stripVendor(path string, info os.FileInfo, err error) error { - if info.Name() == "vendor" { - if _, err := os.Lstat(path); err == nil { - if (info.Mode() & os.ModeSymlink) != 0 { - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { - return os.Remove(path) - } +import ( + "os" + "path/filepath" +) + +func stripNestedVendorDirs(baseDir string) filepath.WalkFunc { + return func(path string, info os.FileInfo, err error) error { + // Ignore anything that's not named "vendor". + if info.Name() != "vendor" { + return nil + } + + // Ignore the base vendor directory. + if path == baseDir { + return nil + } + + // If it's a directory, delete it along with its content. + if info.IsDir() { + return removeAll(path) + } + + if _, err := os.Lstat(path); err != nil { + return nil + } + + // If it is a symlink, check if the target is a directory and delete that instead. + if (info.Mode() & os.ModeSymlink) != 0 { + realInfo, err := os.Stat(path) + if err != nil { + return err } - if info.IsDir() { - return removeAll(path) + if realInfo.IsDir() { + return os.Remove(path) } } - } - return nil + return nil + } } diff --git a/internal/gps/strip_vendor_test.go b/internal/gps/strip_vendor_test.go index 18a722cd9c..e15812b8bd 100644 --- a/internal/gps/strip_vendor_test.go +++ b/internal/gps/strip_vendor_test.go @@ -27,7 +27,7 @@ func stripVendorTestCase(tc fsTestCase) func(*testing.T) { tc.before.setup(t) - if err := filepath.Walk(tempDir, stripVendor); err != nil { + if err := filepath.Walk(tempDir, stripNestedVendorDirs(tempDir)); err != nil { t.Errorf("filepath.Walk err=%q", err) } diff --git a/internal/gps/strip_vendor_windows.go b/internal/gps/strip_vendor_windows.go index 7286934cf3..64a5362fcd 100644 --- a/internal/gps/strip_vendor_windows.go +++ b/internal/gps/strip_vendor_windows.go @@ -9,37 +9,49 @@ import ( "path/filepath" ) -func stripVendor(path string, info os.FileInfo, err error) error { - if info.Name() == "vendor" { - if _, err := os.Lstat(path); err == nil { - symlink := (info.Mode() & os.ModeSymlink) != 0 - dir := info.IsDir() - - switch { - case symlink && dir: - // This could be a windows junction directory. Support for these in the - // standard library is spotty, and we could easily delete an important - // folder if we called os.Remove or os.RemoveAll. Just skip these. - // - // TODO: If we could distinguish between junctions and Windows symlinks, - // we might be able to safely delete symlinks, even though junctions are - // dangerous. - return filepath.SkipDir - - case symlink: - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { - return os.Remove(path) - } - - case dir: - return removeAll(path) +func stripNestedVendorDirs(baseDir string) filepath.WalkFunc { + return func(path string, info os.FileInfo, err error) error { + // Ignore anything that's not a directory or not named "vendor". + if !info.IsDir() || info.Name() != "vendor" { + return nil + } + + // Ignore the base vendor directory. + if path == baseDir { + return nil + } + + if _, err := os.Lstat(path); err != nil { + return nil + } + + symlink := (info.Mode() & os.ModeSymlink) != 0 + dir := info.IsDir() + + switch { + case symlink && dir: + // This could be a windows junction directory. Support for these in the + // standard library is spotty, and we could easily delete an important + // folder if we called os.Remove or os.RemoveAll. Just skip these. + // + // TODO: If we could distinguish between junctions and Windows symlinks, + // we might be able to safely delete symlinks, even though junctions are + // dangerous. + return filepath.SkipDir + + case symlink: + realInfo, err := os.Stat(path) + if err != nil { + return err + } + if realInfo.IsDir() { + return os.Remove(path) } + + case dir: + return removeAll(path) } - } - return nil + return nil + } } From 7a92654524390239b945d83fcde1185690a23b61 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Tue, 8 Aug 2017 21:42:08 +0300 Subject: [PATCH 4/5] dep, internal/gps: update SafeWriter.Write and gps.WriteDepTree Signed-off-by: Ibrahim AshShohail --- cmd/dep/ensure.go | 20 ++++++++--- cmd/dep/init.go | 3 +- internal/gps/result.go | 71 ++++++++++++++++++++++++++----------- internal/gps/result_test.go | 16 ++++----- txn_writer.go | 9 +++-- txn_writer_test.go | 28 +++++++-------- 6 files changed, 95 insertions(+), 52 deletions(-) diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 360abfbad0..1e0b30241c 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -252,7 +252,9 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - return errors.WithMessage(sw.Write(p.AbsRoot, sm, true, logger), "grouped write of manifest, lock and vendor") + + err = sw.Write(p.AbsRoot, sm, true, p.Manifest.PruneOptions, logger) + return errors.WithMessage(err, "grouped write of manifest, lock and vendor") } solution, err := solver.Solve() @@ -273,7 +275,9 @@ func (cmd *ensureCommand) runDefault(ctx *dep.Ctx, args []string, p *dep.Project if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - return errors.Wrap(sw.Write(p.AbsRoot, sm, false, logger), "grouped write of manifest, lock and vendor") + + err = sw.Write(p.AbsRoot, sm, false, p.Manifest.PruneOptions, logger) + return errors.Wrap(err, "grouped write of manifest, lock and vendor") } func (cmd *ensureCommand) runVendorOnly(ctx *dep.Ctx, args []string, p *dep.Project, sm gps.SourceManager, params gps.SolveParameters) error { @@ -306,7 +310,9 @@ func (cmd *ensureCommand) runVendorOnly(ctx *dep.Ctx, args []string, p *dep.Proj if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - return errors.WithMessage(sw.Write(p.AbsRoot, sm, true, logger), "grouped write of manifest, lock and vendor") + + err = sw.Write(p.AbsRoot, sm, true, p.Manifest.PruneOptions, logger) + return errors.WithMessage(err, "grouped write of manifest, lock and vendor") } func (cmd *ensureCommand) runUpdate(ctx *dep.Ctx, args []string, p *dep.Project, sm gps.SourceManager, params gps.SolveParameters) error { @@ -401,7 +407,9 @@ func (cmd *ensureCommand) runUpdate(ctx *dep.Ctx, args []string, p *dep.Project, if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - return errors.Wrap(sw.Write(p.AbsRoot, sm, false, logger), "grouped write of manifest, lock and vendor") + + err = sw.Write(p.AbsRoot, sm, false, p.Manifest.PruneOptions, logger) + return errors.Wrap(err, "grouped write of manifest, lock and vendor") } func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm gps.SourceManager, params gps.SolveParameters) error { @@ -654,7 +662,9 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - if err := errors.Wrap(sw.Write(p.AbsRoot, sm, true, logger), "grouped write of manifest, lock and vendor"); err != nil { + + err = sw.Write(p.AbsRoot, sm, true, p.Manifest.PruneOptions, logger) + if err := errors.Wrap(err, "grouped write of manifest, lock and vendor"); err != nil { return err } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index b0a4745b14..c4b4e3ab46 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -213,7 +213,8 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { if !ctx.Verbose { logger = log.New(ioutil.Discard, "", 0) } - if err := sw.Write(root, sm, !cmd.noExamples, logger); err != nil { + + if err := sw.Write(root, sm, !cmd.noExamples, p.Manifest.PruneOptions, logger); err != nil { return errors.Wrap(err, "safe write of manifest and lock") } diff --git a/internal/gps/result.go b/internal/gps/result.go index 3e3f982947..7479ba106b 100644 --- a/internal/gps/result.go +++ b/internal/gps/result.go @@ -5,10 +5,12 @@ package gps import ( - "fmt" "log" "os" "path/filepath" + "sync" + + "github.com/pkg/errors" ) // A Solution is returned by a solver run. It is mostly just a Lock, with some @@ -43,38 +45,65 @@ type solution struct { solv Solver } -// 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 and a Lock, 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 +// 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, and takes a PruneOptions +// indicating the pruning options required for the exported dependencies. +func WriteDepTree(baseDir string, l Lock, sm SourceManager, prune PruneOptions, logger *log.Logger) error { + if baseDir == "" { + return errors.New("must provide a non-empty baseDir") + } if l == nil { - return fmt.Errorf("must provide non-nil Lock to WriteDepTree") + return errors.New("must provide a non-nil Lock to WriteDepTree") + } + if sm == nil { + return errors.New("must provide a non-nil SourceManager to WriteDepTree") } - err := os.MkdirAll(basedir, 0777) - if err != nil { + if err := os.MkdirAll(baseDir, 0777); err != nil { return err } - // TODO(sdboyer) parallelize + var wg sync.WaitGroup + errCh := make(chan error, len(l.Projects())) + for _, p := range l.Projects() { - to := filepath.FromSlash(filepath.Join(basedir, string(p.Ident().ProjectRoot))) + wg.Add(1) + go func(p LockedProject) { + to := filepath.FromSlash(filepath.Join(baseDir, string(p.Ident().ProjectRoot))) - logger.Printf("Writing out %s@%s", p.Ident().errString(), p.Version()) - err = sm.ExportProject(p.Ident(), p.Version(), to) - if err != nil { - removeAll(basedir) - return fmt.Errorf("error while exporting %s@%s: %s", p.Ident().errString(), p.Version(), err) - } - if sv { - filepath.Walk(to, stripVendor) + logger.Printf("Writing out %s@%s", p.Ident().errString(), p.Version()) + + if err := sm.ExportProject(p.Ident(), p.Version(), to); err != nil { + removeAll(to) + errCh <- errors.Wrapf(err, "failed to export %s", p.Ident().ProjectRoot) + } + + wg.Done() + }(p) + } + + wg.Wait() + close(errCh) + + if len(errCh) > 0 { + logger.Println("Failed to write dep tree. The following errors occurred:") + for err := range errCh { + logger.Println(" * ", err) } + removeAll(baseDir) + // TODO(ibrasho) handle multiple errors + return <-errCh + } + + if err := Prune(baseDir, prune, l, logger); err != nil { + logger.Println("Failed to prune dep tree.") + removeAll(baseDir) + return err } return nil diff --git a/internal/gps/result_test.go b/internal/gps/result_test.go index 0385d48270..90705c2de2 100644 --- a/internal/gps/result_test.go +++ b/internal/gps/result_test.go @@ -13,9 +13,10 @@ import ( "testing" ) -var discardLogger = log.New(ioutil.Discard, "", 0) - -var basicResult solution +var ( + basicResult solution + discardLogger = log.New(ioutil.Discard, "", 0) +) func pi(n string) ProjectIdentifier { return ProjectIdentifier{ @@ -93,13 +94,12 @@ func testWriteDepTree(t *testing.T) { } // nil lock/result should err immediately - err = WriteDepTree(tmp, nil, sm, true, discardLogger) - if err == nil { + + if err = WriteDepTree(tmp, nil, sm, PruneNestedVendorDirs, discardLogger); err == nil { t.Errorf("Should error if nil lock is passed to WriteDepTree") } - err = WriteDepTree(tmp, r, sm, true, discardLogger) - if err != nil { + if err = WriteDepTree(tmp, r, sm, PruneNestedVendorDirs, discardLogger); err != nil { t.Errorf("Unexpected error while creating vendor tree: %s", err) } @@ -146,7 +146,7 @@ func BenchmarkCreateVendorTree(b *testing.B) { // ease manual inspection os.RemoveAll(exp) b.StartTimer() - err = WriteDepTree(exp, r, sm, true, discardLogger) + err = WriteDepTree(exp, r, sm, PruneNestedVendorDirs, discardLogger) b.StopTimer() if err != nil { b.Errorf("unexpected error after %v iterations: %s", i, err) diff --git a/txn_writer.go b/txn_writer.go index 8327c9d7db..e75d9cf7ed 100644 --- a/txn_writer.go +++ b/txn_writer.go @@ -253,7 +253,7 @@ func (sw SafeWriter) validate(root string, sm gps.SourceManager) error { return nil } -// Write saves some combination of config yaml, lock, and a vendor tree. +// Write saves some combination of a manifest, a lock, and a vendor tree. // root is the absolute path of root dir in which to write. // sm is only required if vendor is being written. // @@ -261,7 +261,7 @@ func (sw SafeWriter) validate(root string, sm gps.SourceManager) error { // operations succeeded. It also does its best to roll back if any moves fail. // This mostly guarantees that dep cannot exit with a partial write that would // leave an undefined state on disk. -func (sw *SafeWriter) Write(root string, sm gps.SourceManager, examples bool, logger *log.Logger) error { +func (sw *SafeWriter) Write(root string, sm gps.SourceManager, examples bool, prune gps.PruneOptions, logger *log.Logger) error { err := sw.validate(root, sm) if err != nil { return err @@ -313,7 +313,10 @@ 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) + // Ensure that gps.PruneNestedVendorDirs is toggled on. + prune |= gps.PruneNestedVendorDirs + + err = gps.WriteDepTree(filepath.Join(td, "vendor"), sw.lock, sm, prune, 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 2178954704..32117d317d 100644 --- a/txn_writer_test.go +++ b/txn_writer_test.go @@ -26,7 +26,7 @@ func TestSafeWriter_BadInput_MissingRoot(t *testing.T) { defer pc.Release() sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) - err := sw.Write("", pc.SourceManager, true, discardLogger) + err := sw.Write("", pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) if err == nil { t.Fatal("should have errored without a root path, but did not") @@ -44,7 +44,7 @@ func TestSafeWriter_BadInput_MissingSourceManager(t *testing.T) { pc.Load() sw, _ := NewSafeWriter(nil, nil, pc.Project.Lock, VendorAlways) - err := sw.Write(pc.Project.AbsRoot, nil, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, nil, true, PruneNestedVendorDirs, discardLogger) if err == nil { t.Fatal("should have errored without a source manager when forceVendor is true, but did not") @@ -92,7 +92,7 @@ func TestSafeWriter_BadInput_NonexistentRoot(t *testing.T) { sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) missingroot := filepath.Join(pc.Project.AbsRoot, "nonexistent") - err := sw.Write(missingroot, pc.SourceManager, true, discardLogger) + err := sw.Write(missingroot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) if err == nil { t.Fatal("should have errored with nonexistent dir for root path, but did not") @@ -110,7 +110,7 @@ func TestSafeWriter_BadInput_RootIsFile(t *testing.T) { sw, _ := NewSafeWriter(nil, nil, nil, VendorOnChanged) fileroot := pc.CopyFile("fileroot", "txn_writer/badinput_fileroot") - err := sw.Write(fileroot, pc.SourceManager, true, discardLogger) + err := sw.Write(fileroot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) if err == nil { t.Fatal("should have errored when root path is a file, but did not") @@ -145,7 +145,7 @@ func TestSafeWriter_Manifest(t *testing.T) { } // Write changes - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -190,7 +190,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLock(t *testing.T) { } // Write changes - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -235,7 +235,7 @@ func TestSafeWriter_ManifestAndUnmodifiedLockWithForceVendor(t *testing.T) { } // Write changes - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -285,7 +285,7 @@ func TestSafeWriter_ModifiedLock(t *testing.T) { } // Write changes - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -335,7 +335,7 @@ func TestSafeWriter_ModifiedLockSkipVendor(t *testing.T) { } // Write changes - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -363,7 +363,7 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) { pc.Load() sw, _ := NewSafeWriter(nil, pc.Project.Lock, pc.Project.Lock, VendorAlways) - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify prepared actions @@ -381,7 +381,7 @@ func TestSafeWriter_ForceVendorWhenVendorAlreadyExists(t *testing.T) { t.Fatal("Expected the payload to contain the vendor directory ") } - err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -431,7 +431,7 @@ func TestSafeWriter_NewLock(t *testing.T) { } // Write changes - err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -478,7 +478,7 @@ func TestSafeWriter_NewLockSkipVendor(t *testing.T) { } // Write changes - err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err = sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes @@ -571,7 +571,7 @@ func TestSafeWriter_VendorDotGitPreservedWithForceVendor(t *testing.T) { t.Fatal("Expected the payload to contain the vendor directory") } - err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, discardLogger) + err := sw.Write(pc.Project.AbsRoot, pc.SourceManager, true, PruneNestedVendorDirs, discardLogger) h.Must(errors.Wrap(err, "SafeWriter.Write failed")) // Verify file system changes From b306f42a30bed31d8e9c20ba74865968f465dbb8 Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Sun, 13 Aug 2017 19:53:49 +0300 Subject: [PATCH 5/5] internal/gps: update prune tests Signed-off-by: Ibrahim AshShohail --- internal/gps/prune_test.go | 242 ++++++++++++++++++++++++++----------- 1 file changed, 169 insertions(+), 73 deletions(-) diff --git a/internal/gps/prune_test.go b/internal/gps/prune_test.go index b1347296d3..8bebcba59e 100644 --- a/internal/gps/prune_test.go +++ b/internal/gps/prune_test.go @@ -5,120 +5,216 @@ import ( "log" "testing" + "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/test" ) func TestPruneEmptyDirs(t *testing.T) { h := test.NewHelper(t) - h.TempDir("baseDir1/empty-dir") + defer h.Cleanup() - h.TempDir("baseDir2/non-empty-dir") - h.TempFile("baseDir2/non-empty-dir/file", "") - - h.TempDir("baseDir3/nested-empty-dirs") - h.TempDir("baseDir3/nested-empty-dirs/empty-dir1") - h.TempDir("baseDir3/nested-empty-dirs/empty-dir2") - - h.TempDir("baseDir4") - h.TempDir("baseDir4/empty-dir1") - h.TempDir("baseDir4/empty-dir2") - h.TempDir("baseDir4/non-empty-dir1") - h.TempFile("baseDir4/non-empty-dir1/file", "") - h.TempDir("baseDir4/non-empty-dir2") - h.TempFile("baseDir4/non-empty-dir2/file", "") + h.TempDir(".") testcases := []struct { - name string - baseDir string - emptyDirs []string - err bool + name string + fs fsTestCase + err bool }{ - {"1 empty dir", h.Path("baseDir1"), []string{ - h.Path("baseDir1/empty-dir"), - }, false}, - {"1 non-empty dir", h.Path("baseDir2"), []string{}, false}, - {"nested empty dirs", h.Path("baseDir3"), []string{ - h.Path("baseDir3/nested-empty-dirs/empty-dir1"), - h.Path("baseDir3/nested-empty-dirs/empty-dir2"), - }, false}, - {"mixed dirs", h.Path("baseDir4"), []string{ - h.Path("baseDir4/empty-dir1"), - }, false}, + { + "empty-dir", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + }, + after: filesystemState{}, + }, + false, + }, + { + "non-empty-dir", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + }, + false, + }, + { + "nested-empty-dirs", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dirs"}, + {"dirs", "dir1"}, + {"dirs", "dir2"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dirs"}, + }, + }, + }, + false, + }, + { + "mixed-dirs", + fsTestCase{ + before: filesystemState{ + dirs: []fsPath{ + {"dir1"}, + {"dir2"}, + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + after: filesystemState{ + dirs: []fsPath{ + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + }, + false, + }, } logger := log.New(ioutil.Discard, "", 0) for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - err := pruneEmptyDirs(tc.baseDir, logger) + h.TempDir(tc.name) + baseDir := h.Path(tc.name) + tc.fs.before.root = baseDir + tc.fs.after.root = baseDir + + tc.fs.before.setup(t) + + err := pruneEmptyDirs(baseDir, logger) if tc.err && err == nil { t.Errorf("expected an error, got nil") } else if !tc.err && err != nil { t.Errorf("unexpected error: %s", err) } - for _, path := range tc.emptyDirs { - h.ShouldNotExist(path) - } + + tc.fs.after.assert(t) }) } } func TestCalculateEmptyDirs(t *testing.T) { h := test.NewHelper(t) + defer h.Cleanup() - h.TempDir("baseDir1/empty-dir") - - h.TempDir("baseDir2/non-empty-dir") - h.TempFile("baseDir2/non-empty-dir/file", "") - - h.TempDir("baseDir3/nested-empty-dirs") - h.TempDir("baseDir3/nested-empty-dirs/empty-dir1") - h.TempDir("baseDir3/nested-empty-dirs/empty-dir2") - - h.TempDir("baseDir4") - h.TempDir("baseDir4/empty-dir1") - h.TempDir("baseDir4/empty-dir2") - h.TempDir("baseDir4/non-empty-dir1") - h.TempFile("baseDir4/non-empty-dir1/file", "") - h.TempDir("baseDir4/non-empty-dir2") - h.TempFile("baseDir4/non-empty-dir2/file", "") + h.TempDir(".") testcases := []struct { name string - baseDir string - emptyDirs []string + fs filesystemState + emptyDirs int err bool }{ - {"1 empty dir", h.Path("baseDir1"), []string{ - h.Path("baseDir1/empty-dir"), - }, false}, - {"1 non-empty dir", h.Path("baseDir2"), []string{}, false}, - {"nested empty dirs", h.Path("baseDir3"), []string{ - h.Path("baseDir3/nested-empty-dirs/empty-dir1"), - h.Path("baseDir3/nested-empty-dirs/empty-dir2"), - }, false}, - {"mixed dirs", h.Path("baseDir4"), []string{ - h.Path("baseDir4/empty-dir1"), - h.Path("baseDir4/empty-dir2"), - }, false}, + { + "empty-dir", + filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + }, + 1, + false, + }, + { + "non-empty-dir", + filesystemState{ + dirs: []fsPath{ + {"dir"}, + }, + files: []fsPath{ + {"dir", "file"}, + }, + }, + 0, + false, + }, + { + "nested-empty-dirs", + filesystemState{ + dirs: []fsPath{ + {"dirs"}, + {"dirs", "dir1"}, + {"dirs", "dir2"}, + }, + }, + 2, + false, + }, + { + "mixed-dirs", + filesystemState{ + dirs: []fsPath{ + {"dir1"}, + {"dir2"}, + {"dir3"}, + {"dir4"}, + }, + files: []fsPath{ + {"dir3", "file"}, + {"dir4", "file"}, + }, + }, + 2, + false, + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - emptyDirs, err := calculateEmptyDirs(tc.baseDir) - if len(emptyDirs) != len(tc.emptyDirs) { - t.Fatalf("expected %d paths, got %d", len(tc.emptyDirs), len(emptyDirs)) - } - for i := range tc.emptyDirs { - if tc.emptyDirs[i] != emptyDirs[i] { - t.Fatalf("expected %s to exists in the list, got %s", tc.emptyDirs[i], emptyDirs) - } - } + h.TempDir(tc.name) + baseDir := h.Path(tc.name) + + tc.fs.root = baseDir + tc.fs.setup(t) + + emptyDirs, err := calculateEmptyDirs(baseDir) if tc.err && err == nil { t.Errorf("expected an error, got nil") } else if !tc.err && err != nil { t.Errorf("unexpected error: %s", err) } + if len(emptyDirs) != tc.emptyDirs { + t.Fatalf("expected %d paths, got %d", tc.emptyDirs, len(emptyDirs)) + } + for _, dir := range emptyDirs { + if nonEmpty, err := fs.IsNonEmptyDir(dir); err != nil { + t.Fatalf("unexpected error: %s", err) + } else if nonEmpty { + t.Fatalf("expected %s to be empty, but it wasn't", dir) + } + } }) } }