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", - }, - }, - }, - })) -}