Add ShouldFollowSymlinks argument to pathtools.Glob

Allow the caller to specify whether symlinks should be followed
(the old behavior) or not.

Test: glob_test.go
Test: fs_test.go
Change-Id: I550dc91b8e6370fb32a9a1cbdcb2edade48bda46

glob.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+
+	"github.com/google/blueprint/pathtools"
 )
 
 type GlobPath struct {
@@ -59,7 +61,7 @@ func (c *Context) glob(pattern string, excludes []string) ([]string, error) {
 	}
 
 	// Get a globbed file list
-	files, deps, err := c.fs.Glob(pattern, excludes)
+	files, deps, err := c.fs.Glob(pattern, excludes, pathtools.FollowSymlinks)
 	if err != nil {
 		return nil, err
 	}

pathtools/fs.go

@@ -29,6 +29,13 @@ import (
 
 // Based on Andrew Gerrand's "10 things you (probably) dont' know about Go"
 
+type ShouldFollowSymlinks bool
+
+const (
+	FollowSymlinks     = ShouldFollowSymlinks(true)
+	DontFollowSymlinks = ShouldFollowSymlinks(false)
+)
+
 var OsFs FileSystem = osFs{}
 
 func MockFs(files map[string][]byte) FileSystem {
@@ -78,7 +85,7 @@ type FileSystem interface {
 	// Exists returns whether the file exists and whether it is a directory.  Follows symlinks.
 	Exists(name string) (bool, bool, error)
 
-	Glob(pattern string, excludes []string) (matches, dirs []string, err error)
+	Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error)
 	glob(pattern string) (matches []string, err error)
 
 	// IsDir returns true if the path points to a directory, false it it points to a file.  Follows symlinks.
@@ -92,8 +99,8 @@ type FileSystem interface {
 	// Lstat returns info on a file without following symlinks.
 	Lstat(name string) (os.FileInfo, error)
 
-	// ListDirsRecursive returns a list of all the directories in a path, following symlinks.
+	// ListDirsRecursive returns a list of all the directories in a path, following symlinks if requested.
-	ListDirsRecursive(name string) (dirs []string, err error)
+	ListDirsRecursive(name string, follow ShouldFollowSymlinks) (dirs []string, err error)
 
 	// ReadDirNames returns a list of everything in a directory.
 	ReadDirNames(name string) ([]string, error)
@@ -130,8 +137,8 @@ func (osFs) IsSymlink(name string) (bool, error) {
 	}
 }
 
-func (fs osFs) Glob(pattern string, excludes []string) (matches, dirs []string, err error) {
+func (fs osFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) {
-	return startGlob(fs, pattern, excludes)
+	return startGlob(fs, pattern, excludes, follow)
 }
 
 func (osFs) glob(pattern string) ([]string, error) {
@@ -143,8 +150,8 @@ func (osFs) Lstat(path string) (stats os.FileInfo, err error) {
 }
 
 // Returns a list of all directories under dir
-func (osFs) ListDirsRecursive(name string) (dirs []string, err error) {
+func (osFs) ListDirsRecursive(name string, follow ShouldFollowSymlinks) (dirs []string, err error) {
-	return listDirsRecursive(OsFs, name)
+	return listDirsRecursive(OsFs, name, follow)
 }
 
 func (osFs) ReadDirNames(name string) ([]string, error) {
@@ -272,8 +279,8 @@ func (m *mockFs) IsSymlink(name string) (bool, error) {
 	return false, os.ErrNotExist
 }
 
-func (m *mockFs) Glob(pattern string, excludes []string) (matches, dirs []string, err error) {
+func (m *mockFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) {
-	return startGlob(m, pattern, excludes)
+	return startGlob(m, pattern, excludes, follow)
 }
 
 func unescapeGlob(s string) string {
@@ -376,11 +383,11 @@ func (m *mockFs) ReadDirNames(name string) ([]string, error) {
 	return ret, nil
 }
 
-func (m *mockFs) ListDirsRecursive(name string) ([]string, error) {
+func (m *mockFs) ListDirsRecursive(name string, follow ShouldFollowSymlinks) ([]string, error) {
-	return listDirsRecursive(m, name)
+	return listDirsRecursive(m, name, follow)
 }
 
-func listDirsRecursive(fs FileSystem, name string) ([]string, error) {
+func listDirsRecursive(fs FileSystem, name string, follow ShouldFollowSymlinks) ([]string, error) {
 	name = filepath.Clean(name)
 
 	isDir, err := fs.IsDir(name)
@@ -394,7 +401,7 @@ func listDirsRecursive(fs FileSystem, name string) ([]string, error) {
 
 	dirs := []string{name}
 
-	subDirs, err := listDirsRecursiveRelative(fs, name, 0)
+	subDirs, err := listDirsRecursiveRelative(fs, name, follow, 0)
 	if err != nil {
 		return nil, err
 	}
@@ -406,7 +413,7 @@ func listDirsRecursive(fs FileSystem, name string) ([]string, error) {
 	return dirs, nil
 }
 
-func listDirsRecursiveRelative(fs FileSystem, name string, depth int) ([]string, error) {
+func listDirsRecursiveRelative(fs FileSystem, name string, follow ShouldFollowSymlinks, depth int) ([]string, error) {
 	depth++
 	if depth > 255 {
 		return nil, fmt.Errorf("too many symlinks")
@@ -422,9 +429,12 @@ func listDirsRecursiveRelative(fs FileSystem, name string, depth int) ([]string,
 			continue
 		}
 		f = filepath.Join(name, f)
+		if isSymlink, _ := fs.IsSymlink(f); isSymlink && follow == DontFollowSymlinks {
+			continue
+		}
 		if isDir, _ := fs.IsDir(f); isDir {
 			dirs = append(dirs, f)
-			subDirs, err := listDirsRecursiveRelative(fs, f, depth)
+			subDirs, err := listDirsRecursiveRelative(fs, f, follow, depth)
 			if err != nil {
 				return nil, err
 			}

pathtools/fs_test.go

@@ -170,7 +170,7 @@ func TestFs_IsDir(t *testing.T) {
 	}
 }
 
-func TestFs_ListDirsRecursive(t *testing.T) {
+func TestFs_ListDirsRecursiveFollowSymlinks(t *testing.T) {
 	testCases := []struct {
 		name string
 		dirs []string
@@ -211,7 +211,59 @@ func TestFs_ListDirsRecursive(t *testing.T) {
 
 			for _, test := range testCases {
 				t.Run(test.name, func(t *testing.T) {
-					got, err := fs.ListDirsRecursive(test.name)
+					got, err := fs.ListDirsRecursive(test.name, FollowSymlinks)
+					checkErr(t, test.err, err)
+					if !reflect.DeepEqual(got, test.dirs) {
+						t.Errorf("want: %v, got %v", test.dirs, got)
+					}
+				})
+			}
+		})
+	}
+}
+
+func TestFs_ListDirsRecursiveDontFollowSymlinks(t *testing.T) {
+	testCases := []struct {
+		name string
+		dirs []string
+		err  error
+	}{
+		{".", []string{".", "a", "a/a"}, nil},
+
+		{"a", []string{"a", "a/a"}, nil},
+		{"a/a", []string{"a/a"}, nil},
+		{"a/a/a", nil, nil},
+
+		{"b", []string{"b", "b/a"}, nil},
+		{"b/a", []string{"b/a"}, nil},
+		{"b/a/a", nil, nil},
+
+		{"c", []string{"c"}, nil},
+		{"c/a", nil, nil},
+
+		{"d", []string{"d"}, nil},
+		{"d/a", nil, nil},
+
+		{"e", nil, nil},
+
+		{"dangling", nil, os.ErrNotExist},
+
+		{"missing", nil, os.ErrNotExist},
+	}
+
+	mock := symlinkMockFs()
+	fsList := []FileSystem{mock, OsFs}
+	names := []string{"mock", "os"}
+
+	os.Chdir("testdata/dangling")
+	defer os.Chdir("../..")
+
+	for i, fs := range fsList {
+		t.Run(names[i], func(t *testing.T) {
+
+			for _, test := range testCases {
+				t.Run(test.name, func(t *testing.T) {
+					got, err := fs.ListDirsRecursive(test.name, DontFollowSymlinks)
 					checkErr(t, test.err, err)
 					if !reflect.DeepEqual(got, test.dirs) {
 						t.Errorf("want: %v, got %v", test.dirs, got)

pathtools/glob.go

@@ -39,15 +39,17 @@ var GlobLastRecursiveErr = errors.New("pattern ** as last path element")
 // In general ModuleContext.GlobWithDeps or SingletonContext.GlobWithDeps
 // should be used instead, as they will automatically set up dependencies
 // to rerun the primary builder when the list of matching files changes.
-func Glob(pattern string, excludes []string) (matches, deps []string, err error) {
+func Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, deps []string, err error) {
-	return startGlob(OsFs, pattern, excludes)
+	return startGlob(OsFs, pattern, excludes, follow)
 }
 
-func startGlob(fs FileSystem, pattern string, excludes []string) (matches, deps []string, err error) {
+func startGlob(fs FileSystem, pattern string, excludes []string,
+	follow ShouldFollowSymlinks) (matches, deps []string, err error) {
+
 	if filepath.Base(pattern) == "**" {
 		return nil, nil, GlobLastRecursiveErr
 	} else {
-		matches, deps, err = glob(fs, pattern, false)
+		matches, deps, err = glob(fs, pattern, false, follow)
 	}
 
 	if err != nil {
@@ -73,18 +75,24 @@ func startGlob(fs FileSystem, pattern string, excludes []string) (matches, deps
 	}
 
 	for i, match := range matches {
-		isDir, err := fs.IsDir(match)
+		isSymlink, err := fs.IsSymlink(match)
-		if os.IsNotExist(err) {
-			if isSymlink, _ := fs.IsSymlink(match); isSymlink {
-				return nil, nil, fmt.Errorf("%s: dangling symlink", match)
-			}
-		}
 		if err != nil {
-			return nil, nil, fmt.Errorf("%s: %s", match, err.Error())
+			return nil, nil, err
 		}
+		if !(isSymlink && follow == DontFollowSymlinks) {
+			isDir, err := fs.IsDir(match)
+			if os.IsNotExist(err) {
+				if isSymlink {
+					return nil, nil, fmt.Errorf("%s: dangling symlink", match)
+				}
+			}
+			if err != nil {
+				return nil, nil, fmt.Errorf("%s: %s", match, err.Error())
+			}
 
-		if isDir {
+			if isDir {
-			matches[i] = match + "/"
+				matches[i] = match + "/"
+			}
 		}
 	}
 
@@ -93,7 +101,9 @@ func startGlob(fs FileSystem, pattern string, excludes []string) (matches, deps
 
 // glob is a recursive helper function to handle globbing each level of the pattern individually,
 // allowing searched directories to be tracked.  Also handles the recursive glob pattern, **.
-func glob(fs FileSystem, pattern string, hasRecursive bool) (matches, dirs []string, err error) {
+func glob(fs FileSystem, pattern string, hasRecursive bool,
+	follow ShouldFollowSymlinks) (matches, dirs []string, err error) {
+
 	if !isWild(pattern) {
 		// If there are no wilds in the pattern, check whether the file exists or not.
 		// Uses filepath.Glob instead of manually statting to get consistent results.
@@ -128,7 +138,7 @@ func glob(fs FileSystem, pattern string, hasRecursive bool) (matches, dirs []str
 		hasRecursive = true
 	}
 
-	dirMatches, dirs, err := glob(fs, dir, hasRecursive)
+	dirMatches, dirs, err := glob(fs, dir, hasRecursive, follow)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -146,7 +156,7 @@ func glob(fs FileSystem, pattern string, hasRecursive bool) (matches, dirs []str
 
 		if isDir {
 			if file == "**" {
-				recurseDirs, err := fs.ListDirsRecursive(m)
+				recurseDirs, err := fs.ListDirsRecursive(m, follow)
 				if err != nil {
 					return nil, nil, err
 				}
@@ -333,7 +343,7 @@ func GlobPatternList(patterns []string, prefix string) (globedList []string, dep
 
 	for _, pattern := range patterns {
 		if isWild(pattern) {
-			matches, deps, err = Glob(filepath.Join(prefix, pattern), nil)
+			matches, deps, err = Glob(filepath.Join(prefix, pattern), nil, FollowSymlinks)
 			if err != nil {
 				return nil, nil, err
 			}
@@ -377,7 +387,7 @@ func HasGlob(in []string) bool {
 // should be used instead, as they will automatically set up dependencies
 // to rerun the primary builder when the list of matching files changes.
 func GlobWithDepFile(glob, fileListFile, depFile string, excludes []string) (files []string, err error) {
-	files, deps, err := Glob(glob, excludes)
+	files, deps, err := Glob(glob, excludes, FollowSymlinks)
 	if err != nil {
 		return nil, err
 	}

pathtools/glob_test.go

@@ -489,7 +489,7 @@ func TestMockGlob(t *testing.T) {
 
 	for _, testCase := range globTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, mock, testCase)
+			testGlob(t, mock, testCase, FollowSymlinks)
 		})
 	}
 }
@@ -499,7 +499,7 @@ func TestGlob(t *testing.T) {
 	defer os.Chdir("../..")
 	for _, testCase := range globTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, OsFs, testCase)
+			testGlob(t, OsFs, testCase, FollowSymlinks)
 		})
 	}
 }
@@ -548,7 +548,7 @@ func TestMockGlobEscapes(t *testing.T) {
 
 	for _, testCase := range globEscapeTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, mock, testCase)
+			testGlob(t, mock, testCase, FollowSymlinks)
 		})
 	}
 
@@ -559,7 +559,7 @@ func TestGlobEscapes(t *testing.T) {
 	defer os.Chdir("../..")
 	for _, testCase := range globEscapeTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, OsFs, testCase)
+			testGlob(t, OsFs, testCase, FollowSymlinks)
 		})
 	}
 
@@ -571,6 +571,11 @@ var globSymlinkTestCases = []globTestCase{
 		matches: []string{"a/", "b/", "c/", "d/", "e", "a/a/", "a/a/a", "b/a/", "b/a/a", "c/a", "d/a"},
 		deps:    []string{".", "a", "a/a", "b", "b/a", "c", "d"},
 	},
+	{
+		pattern: `b/**/*`,
+		matches: []string{"b/a/", "b/a/a"},
+		deps:    []string{"b", "b/a"},
+	},
 }
 
 func TestMockGlobSymlinks(t *testing.T) {
@@ -592,7 +597,7 @@ func TestMockGlobSymlinks(t *testing.T) {
 
 	for _, testCase := range globSymlinkTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, mock, testCase)
+			testGlob(t, mock, testCase, FollowSymlinks)
 		})
 	}
 }
@@ -603,14 +608,113 @@ func TestGlobSymlinks(t *testing.T) {
 
 	for _, testCase := range globSymlinkTestCases {
 		t.Run(testCase.pattern, func(t *testing.T) {
-			testGlob(t, OsFs, testCase)
+			testGlob(t, OsFs, testCase, FollowSymlinks)
 		})
 	}
 }
 
-func testGlob(t *testing.T, fs FileSystem, testCase globTestCase) {
+var globDontFollowSymlinkTestCases = []globTestCase{
+	{
+		pattern: `**/*`,
+		matches: []string{"a/", "b", "c", "d", "e", "a/a/", "a/a/a"},
+		deps:    []string{".", "a", "a/a"},
+	},
+	{
+		pattern: `b/**/*`,
+		matches: []string{"b/a/", "b/a/a"},
+		deps:    []string{"b", "b/a"},
+	},
+}
+
+func TestMockGlobDontFollowSymlinks(t *testing.T) {
+	files := []string{
+		"a/a/a",
+		"b -> a",
+		"c -> a/a",
+		"d -> c",
+		"e -> a/a/a",
+	}
+
+	mockFiles := make(map[string][]byte)
+
+	for _, f := range files {
+		mockFiles[f] = nil
+	}
+
+	mock := MockFs(mockFiles)
+
+	for _, testCase := range globDontFollowSymlinkTestCases {
+		t.Run(testCase.pattern, func(t *testing.T) {
+			testGlob(t, mock, testCase, DontFollowSymlinks)
+		})
+	}
+}
+
+func TestGlobDontFollowSymlinks(t *testing.T) {
+	os.Chdir("testdata/symlinks")
+	defer os.Chdir("../..")
+
+	for _, testCase := range globDontFollowSymlinkTestCases {
+		t.Run(testCase.pattern, func(t *testing.T) {
+			testGlob(t, OsFs, testCase, DontFollowSymlinks)
+		})
+	}
+}
+
+var globDontFollowDanglingSymlinkTestCases = []globTestCase{
+	{
+		pattern: `**/*`,
+		matches: []string{"a/", "b", "c", "d", "dangling", "e", "f", "a/a/", "a/a/a", "a/a/f"},
+		deps:    []string{".", "a", "a/a"},
+	},
+	{
+		pattern: `dangling`,
+		matches: []string{"dangling"},
+		deps:    []string{"dangling"},
+	},
+}
+
+func TestMockGlobDontFollowDanglingSymlinks(t *testing.T) {
+	files := []string{
+		"a/a/a",
+		"a/a/f -> ../../f",
+		"b -> a",
+		"c -> a/a",
+		"d -> c",
+		"e -> a/a/a",
+		"f",
+		"dangling -> missing",
+	}
+
+	mockFiles := make(map[string][]byte)
+
+	for _, f := range files {
+		mockFiles[f] = nil
+	}
+
+	mock := MockFs(mockFiles)
+
+	for _, testCase := range globDontFollowDanglingSymlinkTestCases {
+		t.Run(testCase.pattern, func(t *testing.T) {
+			testGlob(t, mock, testCase, DontFollowSymlinks)
+		})
+	}
+}
+
+func TestGlobDontFollowDanglingSymlinks(t *testing.T) {
+	os.Chdir("testdata/dangling")
+	defer os.Chdir("../..")
+
+	for _, testCase := range globDontFollowDanglingSymlinkTestCases {
+		t.Run(testCase.pattern, func(t *testing.T) {
+			testGlob(t, OsFs, testCase, DontFollowSymlinks)
+		})
+	}
+}
+
+func testGlob(t *testing.T, fs FileSystem, testCase globTestCase, follow ShouldFollowSymlinks) {
 	t.Helper()
-	matches, deps, err := fs.Glob(testCase.pattern, testCase.excludes)
+	matches, deps, err := fs.Glob(testCase.pattern, testCase.excludes, follow)
 	if err != testCase.err {
 		if err == nil {
 			t.Fatalf("missing error: %s", testCase.err)