@ 260812.1

241025.1 · aa71a4ca319d67550c8db59f9158165af96c3a28

fastwalk: don't clean the path argument to Walk on Windows

On Windows don't attempt to clean the path argument to Walk since the existing clean logic (cleanRootPath) transforms paths like "C:\" => "C:" which are not equivalent.

This logic only existed to make the joining of paths simpler and should probably be removed since we shouldn't be modifying user provided paths.

TODO: Investigate if anything relies on the current clean logic and remove it if nothing does.

Fixes: https://github.com/charlievieth/fastwalk/issues/37

reading declarations

dirent_export_test.go

@@ -4,6 +4,7 @@ import ( 	"fmt" 	"io/fs" 	"os"+	"runtime" 	"testing" 	"time" )@@ -29,26 +30,44 @@ func FormatFileInfo(fi fs.FileInfo) string { 	}) } +// NB: this test lives here and not in fastwalk_test.go since we need to+// access the internal cleanRootPath function. func TestCleanRootPath(t *testing.T) {-	tests := map[string]string{-		"":      "",-		"/":     "/",-		"//":    "/",-		"/foo":  "/foo",-		"/foo/": "/foo",-		"a":     "a",-		`C:/`:   `C:`,-	}-	if os.PathSeparator != '/' {-		const sep = string(os.PathSeparator)-		tests["C:"+sep] = `C:`-		tests["C:"+sep+sep] = `C:`-		tests[sep+sep] = sep-	}-	for in, want := range tests {-		got := cleanRootPath(in)-		if got != want {-			t.Errorf("cleanRootPath(%q) = %q; want: %q", in, got, want)+	test := func(t *testing.T, tests map[string]string) {+		t.Helper()+		for in, want := range tests {+			got := cleanRootPath(in)+			if got != want {+				t.Errorf("cleanRootPath(%q) = %q; want: %q", in, got, want)+			} 		} 	}+	// NB: The name here isn't exactly correct since we run this for+	// any non-Windows OS.+	t.Run("Unix", func(t *testing.T) {+		if runtime.GOOS == "windows" {+			t.Skip("test not supported on Windows")+		}+		test(t, map[string]string{+			"":      "",+			".":     ".",+			"/":     "/",+			"//":    "/",+			"/foo":  "/foo",+			"/foo/": "/foo",+			"a":     "a",+		})+	})+	// Test that cleanRootPath is a no-op on Windows+	t.Run("Windows", func(t *testing.T) {+		if runtime.GOOS != "windows" {+			t.Skip("test only supported on Windows")+		}+		test(t, map[string]string{+			`C:/`:              `C:/`,+			`C://`:             `C://`,+			`\\?\GLOBALROOT`:   `\\?\GLOBALROOT`,+			`\\?\GLOBALROOT\\`: `\\?\GLOBALROOT\\`,+		})+	}) }

fastwalk.go

@@ -563,12 +563,14 @@ func (w *walker) joinPaths(dir, base string) string { 	// Handle the case where the root path argument to Walk is "/" 	// without this the returned path is prefixed with "//". 	if os.PathSeparator == '/' {-		if dir == "/" {+		if len(dir) != 0 && dir[len(dir)-1] == '/' { 			return dir + base 		} 		return dir + "/" + base 	}-	// TODO: handle the above case of the argument to Walk being "/"+	if len(dir) != 0 && os.IsPathSeparator(dir[len(dir)-1]) {+		return dir + base+	} 	if w.toSlash { 		return dir + "/" + base 	}@@ -625,7 +627,17 @@ func (w *walker) walk(root string, info DirEntry, runUserCallback bool) error { 	return nil } +// cleanRootPath returns the root path trimmed of extraneous trailing slashes.+// This is a no-op on Windows. func cleanRootPath(root string) string {+	if runtime.GOOS == "windows" || len(filepath.VolumeName(root)) != 0 {+		// Windows paths or any path with a volume name (which AFAIK should+		// only be Windows) are a bit too complicated to clean.+		return root+	}+	if len(filepath.VolumeName(root)) != 0 {+		return root+	} 	for i := len(root) - 1; i >= 0; i-- { 		if !os.IsPathSeparator(root[i]) { 			return root[:i+1]

fastwalk_test.go

@@ -12,6 +12,7 @@ import ( 	"os" 	"path/filepath" 	"reflect"+	"regexp" 	"runtime" 	"sort" 	"strings"@@ -388,6 +389,207 @@ func TestFastWalk_LongPath(t *testing.T) { 	} } +func TestFastWalk_WindowsRootPaths(t *testing.T) {+	if runtime.GOOS != "windows" {+		t.Skip("test only supported on Windows")+	}++	sameFile := func(t *testing.T, name1, name2 string) bool {+		fi1, err := os.Stat(name1)+		if err != nil {+			t.Fatal(err)+		}+		fi2, err := os.Stat(name2)+		if err != nil {+			t.Fatal(err)+		}+		return os.SameFile(fi1, fi2)+	}++	walk := func(t *testing.T, root string) map[string]fs.DirEntry {+		var mu sync.Mutex+		seen := make(map[string]fs.DirEntry)+		errStop := errors.New("errStop")+		fn := func(path string, de fs.DirEntry, err error) error {+			if err != nil {+				return err+			}+			mu.Lock()+			seen[path] = de+			mu.Unlock()+			if path != root && de.IsDir() {+				return fs.SkipDir+			}+			return nil+		}+		err := fastwalk.Walk(nil, root, fastwalk.IgnorePermissionErrors(fn))+		if err != nil && err != errStop {+			t.Fatal(err)+		}+		if len(seen) <= 1 {+			// If we are a child of the root directory we should have visited at+			// least two entries: the root itself and a directory that leads to,+			// or is, our current working directory.+			t.Fatalf("empty directory: %s", root)+		}+		return seen+	}++	pwd, err := filepath.Abs(".")+	if err != nil {+		t.Fatal(err)+	}++	vol := filepath.VolumeName(pwd)+	if !regexp.MustCompile(`^[A-Za-z]:$`).MatchString(vol) {+		// Ignore UNC names and other weird Windows paths to keep this simple.+		t.Skipf("unsupported volume name: %s for path: %s", vol, pwd)+	}+	if !sameFile(t, pwd, vol) {+		t.Skipf("skipping %s and %s should be considered the same file", pwd, vol)+	}++	// Test that walking the disk root ("C:\") actually walks the disk root.+	// Previously, there was a bug where the path "C:\" was transformed to "C:"+	// before walking which caused fastwalk to walk the current directory.+	//+	// https://github.com/charlievieth/fastwalk/issues/37+	t.Run("FullyQualified", func(t *testing.T) {+		root := vol + `\`+		if sameFile(t, pwd, root) {+			t.Skipf("the current working directory (%s) is the disk root: %s", pwd, root)+		}+		seen := walk(t, root)++		// Make sure we don't append an extraneous slash to the root ("C:\" => "C:\\a").+		for path := range seen {+			rest := strings.TrimPrefix(path, vol)+			if strings.Contains(rest, `\\`) {+				t.Errorf(`path contains multiple consecutive slashes after volume (%s): "%s"`,+					vol, path)+			}+			if s := filepath.Clean(path); s != path {+				t.Errorf(`filepath.Clean("%s") == "%s"`, path, s)+			}+		}++		// Make sure we didn't walk the current directory. This will happen if+		// the root argument to Walk is a drive letter ("C:\") but we strip off+		// the trailing slash ("C:\" => "C:") since this makes the path relative+		// to the current directory on drive "C".+		//+		// See: https://github.com/charlievieth/fastwalk/issues/37+		//+		// Docs: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#fully-qualified-vs-relative-paths+		for path, de := range seen {+			if path == root {+				// Ignore root since filepath.Base("C:\") == "\" and "C:\" and "\"+				// are equivalent.+				continue+			}+			fi1, err := de.Info()+			if err != nil {+				if os.IsNotExist(err) || os.IsPermission(err) {+					continue+				}+				t.Fatal(err)+			}+			name := filepath.Base(path)+			fi2, err := os.Lstat(name)+			if err != nil {+				continue+			}+			if os.SameFile(fi1, fi2) {+				t.Errorf("Walking root (%s) returned entries for the current working "++					"directory (%s): file %s is the same as %s", root, pwd, path, name)+			}+		}++		// Add file base name mappings+		for _, de := range seen {+			seen[de.Name()] = de+		}++		// Make sure we read some files from the disk root.+		des, err := os.ReadDir(root)+		if err != nil {+			t.Fatal(err)+		}+		if len(des) == 0 {+			t.Fatalf("Disk root %s contains no files!", root)+		}+		same := 0+		for _, d2 := range des {+			d1 := seen[d2.Name()]+			if d1 == nil {+				continue+			}+			fi1, err := d1.Info()+			if err != nil {+				t.Log(err)+				continue+			}+			fi2, err := d2.Info()+			if err != nil {+				t.Log(err)+				continue+			}+			if os.SameFile(fi1, fi2) {+				same+++			}+		}+		// TODO: Expect to see N% of files and use+		// a more descriptive error message+		if same == 0 {+			t.Fatalf(`Error failed to walk dist root: "%s"`, root)+		}+	})++	// Test that paths like "C:" are treated as a relative path.+	t.Run("Relative", func(t *testing.T) {+		seen := walk(t, vol)++		// Make sure we don't append an extraneous slash to the root ("C:\" => "C:\\a").+		for path := range seen {+			rest := strings.TrimPrefix(path, vol)+			if strings.Contains(rest, `\\`) {+				t.Errorf(`path contains multiple consecutive slashes after volume (%s): "%s"`,+					vol, path)+			}+			if path == vol {+				continue // Clean("C:") => "C:."+			}+			if s := filepath.Clean(path); s != path {+				t.Errorf(`filepath.Clean("%s") == "%s"`, path, s)+			}+		}++		// Make sure we walk the current directory.+		for path, de := range seen {+			if path == vol {+				// Ignore the volume since filepath.Base("C:") == "\" and "C:" and "\"+				// are not equivalent.+				continue+			}+			fi1, err := de.Info()+			if err != nil {+				t.Fatal(err)+			}+			name := filepath.Base(path)+			fi2, err := os.Lstat(name)+			if err != nil {+				// NB: This test will fail if this file is removed while it's+				// running. There are workarounds for this, but for now it's+				// simpler to just error if that happens.+				t.Fatal(err)+			}+			if !os.SameFile(fi1, fi2) {+				t.Errorf("Expected files (%s) and (%s) to be the same", path, name)+			}+		}+	})+}+ func TestFastWalk_Symlink(t *testing.T) { 	testFastWalk(t, map[string]string{ 		"foo/foo.go":       "one",