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 <[email protected]>

Author: ibrasho

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

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()
 	}
 }

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
 		}
 	}