diff --git a/glob.go b/glob.go index 4553f69..4f7e978 100644 --- a/glob.go +++ b/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 } diff --git a/pathtools/fs.go b/pathtools/fs.go index 6cb240b..6b0d503 100644 --- a/pathtools/fs.go +++ b/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(name string) (dirs []string, err error) + // ListDirsRecursive returns a list of all the directories in a path, following symlinks if requested. + 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) { - return startGlob(fs, pattern, excludes) +func (fs osFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) { + 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) { - return listDirsRecursive(OsFs, name) +func (osFs) ListDirsRecursive(name string, follow ShouldFollowSymlinks) (dirs []string, err error) { + 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) { - return startGlob(m, pattern, excludes) +func (m *mockFs) Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, dirs []string, err error) { + 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) { - return listDirsRecursive(m, name) +func (m *mockFs) ListDirsRecursive(name string, follow ShouldFollowSymlinks) ([]string, error) { + 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 } diff --git a/pathtools/fs_test.go b/pathtools/fs_test.go index b2c82c1..03fbcd4 100644 --- a/pathtools/fs_test.go +++ b/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) diff --git a/pathtools/glob.go b/pathtools/glob.go index 3b0f96b..67394d2 100644 --- a/pathtools/glob.go +++ b/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) { - return startGlob(OsFs, pattern, excludes) +func Glob(pattern string, excludes []string, follow ShouldFollowSymlinks) (matches, deps []string, err error) { + 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) - if os.IsNotExist(err) { - if isSymlink, _ := fs.IsSymlink(match); isSymlink { - return nil, nil, fmt.Errorf("%s: dangling symlink", match) - } - } + isSymlink, err := fs.IsSymlink(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 { - matches[i] = match + "/" + if isDir { + 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 } diff --git a/pathtools/glob_test.go b/pathtools/glob_test.go index f91e7dc..0265df6 100644 --- a/pathtools/glob_test.go +++ b/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)