From fce851e91e1a4651a31cdf5e62ffc93d3fd95974 Mon Sep 17 00:00:00 2001 From: "Seungyoung \"Steve\" Kim" Date: Wed, 20 Sep 2017 17:41:44 -0700 Subject: [PATCH 1/2] Skip broken vendor symlink rather than returning an error. --- internal/gps/strip_vendor.go | 6 +---- internal/gps/strip_vendor_nonwindows_test.go | 25 ++++++++++++++++++++ internal/gps/strip_vendor_windows.go | 6 +---- internal/gps/strip_vendor_windows_test.go | 25 ++++++++++++++++++++ 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/internal/gps/strip_vendor.go b/internal/gps/strip_vendor.go index 184a0d2d2e..a5cef83ac2 100644 --- a/internal/gps/strip_vendor.go +++ b/internal/gps/strip_vendor.go @@ -22,11 +22,7 @@ func stripVendor(path string, info os.FileInfo, err error) error { } if (info.Mode() & os.ModeSymlink) != 0 { - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { + if realInfo, err := os.Stat(path); err == nil && realInfo.IsDir() { return os.Remove(path) } } diff --git a/internal/gps/strip_vendor_nonwindows_test.go b/internal/gps/strip_vendor_nonwindows_test.go index a8eb9ae928..8ff55155b0 100644 --- a/internal/gps/strip_vendor_nonwindows_test.go +++ b/internal/gps/strip_vendor_nonwindows_test.go @@ -82,6 +82,31 @@ func TestStripVendorSymlinks(t *testing.T) { }, })) + 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{ diff --git a/internal/gps/strip_vendor_windows.go b/internal/gps/strip_vendor_windows.go index 7d6acc4a3c..d86e534534 100644 --- a/internal/gps/strip_vendor_windows.go +++ b/internal/gps/strip_vendor_windows.go @@ -31,11 +31,7 @@ func stripVendor(path string, info os.FileInfo, err error) error { return filepath.SkipDir case symlink: - realInfo, err := os.Stat(path) - if err != nil { - return err - } - if realInfo.IsDir() { + if realInfo, err := os.Stat(path); err == nil && realInfo.IsDir() { return os.Remove(path) } diff --git a/internal/gps/strip_vendor_windows_test.go b/internal/gps/strip_vendor_windows_test.go index 7569a48741..67ff900299 100644 --- a/internal/gps/strip_vendor_windows_test.go +++ b/internal/gps/strip_vendor_windows_test.go @@ -90,6 +90,31 @@ func TestStripVendorSymlinks(t *testing.T) { }, })) + 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{ // Curiously, if a symlink on windows points to *another* symlink which // eventually points at a directory, we'll correctly remove that first From 82b5519797b88eed0855d12e4c0244a2e2bb7ad1 Mon Sep 17 00:00:00 2001 From: Steve Kim Date: Wed, 4 Oct 2017 10:52:23 -0700 Subject: [PATCH 2/2] Merge the latest strip codes fromt the master --- internal/gps/strip_vendor.go | 4 +-- internal/gps/strip_vendor_windows.go | 53 +++++++++++++++------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/internal/gps/strip_vendor.go b/internal/gps/strip_vendor.go index 7a95457e77..aaaf9bcd22 100644 --- a/internal/gps/strip_vendor.go +++ b/internal/gps/strip_vendor.go @@ -21,8 +21,8 @@ func stripVendor(path string, info os.FileInfo, err error) error { return nil } - // If the file is a symlink to a directory, delete the symlink. - if (info.Mode() & os.ModeSymlink) != 0 { + // 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) } diff --git a/internal/gps/strip_vendor_windows.go b/internal/gps/strip_vendor_windows.go index ab659cbe7e..c6b0a13346 100644 --- a/internal/gps/strip_vendor_windows.go +++ b/internal/gps/strip_vendor_windows.go @@ -14,34 +14,37 @@ func stripVendor(path string, info os.FileInfo, err error) error { return err } - if info.Name() != "vendor" { + if info.Name() != "vendor" { return nil } - 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: - 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 - } + 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 }