From 3b01418823bb795631902079dc72a92f22685aff Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Sat, 2 Sep 2017 17:42:09 +0300 Subject: [PATCH] [WIP]: internal/fs: add EquivalentPaths function (#940) * internal/fs: add EquivalentPaths * internal/fs: fix isCaseSensitiveFilesystem bug isCaseSensitiveFilesystem returns true when os.Stat fails on the dir passed. This caused HasFilepathPrefix to always treat *nix systems as case-sensitive, since it passed a relative path (missing the root) to isCaseSensitiveFilesystem. This commit updates isCaseSensitiveFilesystem to return an error in addtion to the boolean check. Also, HasFilepathPrefix now passes absolute paths to isCaseSensitiveFilesystem. Signed-off-by: Ibrahim AshShohail --- cmd/dep/gopath_scanner.go | 2 +- context.go | 16 ++++-- context_test.go | 40 ++++++--------- internal/fs/fs.go | 100 +++++++++++++++++++++++++++++--------- internal/fs/fs_test.go | 65 +++++++++++++++++++++++-- 5 files changed, 167 insertions(+), 56 deletions(-) diff --git a/cmd/dep/gopath_scanner.go b/cmd/dep/gopath_scanner.go index 99dbb1f6ed..48d3189ac9 100644 --- a/cmd/dep/gopath_scanner.go +++ b/cmd/dep/gopath_scanner.go @@ -134,7 +134,7 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) { } func trimPathPrefix(p1, p2 string) string { - if fs.HasFilepathPrefix(p1, p2) { + if isPrefix, _ := fs.HasFilepathPrefix(p1, p2); isPrefix { return p1[len(p2):] } return p1 diff --git a/context.go b/context.go index feacfddab9..6a33809df1 100644 --- a/context.go +++ b/context.go @@ -182,7 +182,7 @@ func (c *Ctx) DetectProjectGOPATH(p *Project) (string, error) { pGOPATH, perr := c.detectGOPATH(p.AbsRoot) // If p.AbsRoot is a not symlink, attempt to detect GOPATH for p.AbsRoot only. - if p.AbsRoot == p.ResolvedAbsRoot { + if equal, _ := fs.EquivalentPaths(p.AbsRoot, p.ResolvedAbsRoot); equal { return pGOPATH, perr } @@ -194,7 +194,7 @@ func (c *Ctx) DetectProjectGOPATH(p *Project) (string, error) { } // If pGOPATH equals rGOPATH, then both are within the same GOPATH. - if pGOPATH == rGOPATH { + if equal, _ := fs.EquivalentPaths(pGOPATH, rGOPATH); equal { return "", errors.Errorf("both %s and %s are in the same GOPATH %s", p.AbsRoot, p.ResolvedAbsRoot, pGOPATH) } @@ -213,7 +213,11 @@ func (c *Ctx) DetectProjectGOPATH(p *Project) (string, error) { // detectGOPATH detects the GOPATH for a given path from ctx.GOPATHs. func (c *Ctx) detectGOPATH(path string) (string, error) { for _, gp := range c.GOPATHs { - if fs.HasFilepathPrefix(path, gp) { + isPrefix, err := fs.HasFilepathPrefix(path, gp) + if err != nil { + return "", errors.Wrap(err, "failed to detect GOPATH") + } + if isPrefix { return gp, nil } } @@ -224,7 +228,11 @@ func (c *Ctx) detectGOPATH(path string) (string, error) { // `$GOPATH/src/` prefix. Returns an error for paths equal to, or without this prefix. func (c *Ctx) ImportForAbs(path string) (string, error) { srcprefix := filepath.Join(c.GOPATH, "src") + string(filepath.Separator) - if fs.HasFilepathPrefix(path, srcprefix) { + isPrefix, err := fs.HasFilepathPrefix(path, srcprefix) + if err != nil { + return "", errors.Wrap(err, "failed to find import path") + } + if isPrefix { if len(path) <= len(srcprefix) { return "", errors.New("dep does not currently support using GOPATH/src as the project root") } diff --git a/context_test.go b/context_test.go index e522d25b4d..503ed1beaf 100644 --- a/context_test.go +++ b/context_test.go @@ -314,23 +314,16 @@ func TestDetectProjectGOPATH(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() - h.TempDir("go") - h.TempDir("go-two") + h.TempDir(filepath.Join("sym", "symlink")) + h.TempDir(filepath.Join("go", "src", "sym", "path")) + h.TempDir(filepath.Join("go", "src", "real", "path")) + h.TempDir(filepath.Join("go-two", "src", "real", "path")) + h.TempDir(filepath.Join("go-two", "src", "sym")) ctx := &Ctx{ GOPATHs: []string{h.Path("go"), h.Path("go-two")}, } - h.TempDir("go/src/real/path") - - // Another directory used as a GOPATH - h.TempDir("go-two/src/sym") - - h.TempDir(filepath.Join(".", "sym/symlink")) // Directory for symlinks - h.TempDir(filepath.Join("go", "src", "sym", "path")) - h.TempDir(filepath.Join("go", "src", " real", "path")) - h.TempDir(filepath.Join("go-two", "src", "real", "path")) - testcases := []struct { name string root string @@ -383,7 +376,7 @@ func TestDetectProjectGOPATH(t *testing.T) { { name: "AbsRoot-is-a-symlink-to-ResolvedAbsRoot", root: filepath.Join(h.Path("."), "sym", "symlink"), - resolvedRoot: filepath.Join(ctx.GOPATHs[0], "src", " real", "path"), + resolvedRoot: filepath.Join(ctx.GOPATHs[0], "src", "real", "path"), GOPATH: ctx.GOPATHs[0], }, } @@ -412,36 +405,33 @@ func TestDetectGOPATH(t *testing.T) { th := test.NewHelper(t) defer th.Cleanup() - th.TempDir("go") - th.TempDir("gotwo") + th.TempDir(filepath.Join("code", "src", "github.com", "username", "package")) + th.TempDir(filepath.Join("go", "src", "github.com", "username", "package")) + th.TempDir(filepath.Join("gotwo", "src", "github.com", "username", "package")) ctx := &Ctx{GOPATHs: []string{ th.Path("go"), th.Path("gotwo"), }} - th.TempDir(filepath.Join("code", "src", "github.com", "username", "package")) - th.TempDir(filepath.Join("go", "src", "github.com", "username", "package")) - th.TempDir(filepath.Join("gotwo", "src", "github.com", "username", "package")) - testcases := []struct { GOPATH string path string err bool }{ - {th.Path("go"), filepath.Join(th.Path("go"), "src/github.com/username/package"), false}, - {th.Path("go"), filepath.Join(th.Path("go"), "src/github.com/username/package"), false}, - {th.Path("gotwo"), filepath.Join(th.Path("gotwo"), "src/github.com/username/package"), false}, - {"", filepath.Join(th.Path("."), "code/src/github.com/username/package"), true}, + {th.Path("go"), th.Path(filepath.Join("go", "src", "github.com", "username", "package")), false}, + {th.Path("go"), th.Path(filepath.Join("go", "src", "github.com", "username", "package")), false}, + {th.Path("gotwo"), th.Path(filepath.Join("gotwo", "src", "github.com", "username", "package")), false}, + {"", th.Path(filepath.Join("code", "src", "github.com", "username", "package")), true}, } for _, tc := range testcases { GOPATH, err := ctx.detectGOPATH(tc.path) if tc.err && err == nil { - t.Error("Expected error but got none") + t.Error("expected error but got none") } if GOPATH != tc.GOPATH { - t.Errorf("Expected GOPATH to be %s, got %s", GOPATH, tc.GOPATH) + t.Errorf("expected GOPATH to be %s, got %s", GOPATH, tc.GOPATH) } } } diff --git a/internal/fs/fs.go b/internal/fs/fs.go index abcebee6fe..0fb84a8e39 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -30,7 +30,7 @@ import ( // same file. In that situation HasFilepathPrefix("/Foo/Bar", "/foo") // will return true. The implementation is *not* OS-specific, so a FAT32 // filesystem mounted on Linux will be handled correctly. -func HasFilepathPrefix(path, prefix string) bool { +func HasFilepathPrefix(path, prefix string) (bool, error) { // this function is more convoluted then ideal due to need for special // handling of volume name/drive letter on Windows. vnPath and vnPrefix // are first compared, and then used to initialize initial values of p and @@ -42,21 +42,19 @@ func HasFilepathPrefix(path, prefix string) bool { vnPath := strings.ToLower(filepath.VolumeName(path)) vnPrefix := strings.ToLower(filepath.VolumeName(prefix)) if vnPath != vnPrefix { - return false + return false, nil } - // because filepath.Join("c:","dir") returns "c:dir", we have to manually add path separator to drive letters - if strings.HasSuffix(vnPath, ":") { - vnPath += string(os.PathSeparator) - } - if strings.HasSuffix(vnPrefix, ":") { - vnPrefix += string(os.PathSeparator) - } + // Because filepath.Join("c:","dir") returns "c:dir", we have to manually + // add path separator to drive letters. Also, we need to set the path root + // on *nix systems, since filepath.Join("", "dir") returns a relative path. + vnPath += string(os.PathSeparator) + vnPrefix += string(os.PathSeparator) var dn string if isDir, err := IsDir(path); err != nil { - return false + return false, errors.Wrap(err, "failed to check filepath prefix") } else if isDir { dn = path } else { @@ -71,10 +69,10 @@ func HasFilepathPrefix(path, prefix string) bool { prefixes := strings.Split(prefix, string(os.PathSeparator))[1:] if len(prefixes) > len(dirs) { - return false + return false, nil } - // d,p are initialized with "" on *nix and volume name on Windows + // d,p are initialized with "/" on *nix and volume name on Windows d := vnPath p := vnPrefix @@ -84,7 +82,11 @@ func HasFilepathPrefix(path, prefix string) bool { // something like ext4 filesystem mounted on FAT // mountpoint, mounted on ext4 filesystem, i.e. the // problematic filesystem is not the last one. - if isCaseSensitiveFilesystem(filepath.Join(d, dirs[i])) { + caseSensitive, err := isCaseSensitiveFilesystem(filepath.Join(d, dirs[i])) + if err != nil { + return false, errors.Wrap(err, "failed to check filepath prefix") + } + if caseSensitive { d = filepath.Join(d, dirs[i]) p = filepath.Join(p, prefixes[i]) } else { @@ -93,11 +95,62 @@ func HasFilepathPrefix(path, prefix string) bool { } if p != d { - return false + return false, nil + } + } + + return true, nil +} + +// EquivalentPaths compares the paths passed to check if they are equivalent. +// It respects the case-sensitivity of the underlying filesysyems. +func EquivalentPaths(p1, p2 string) (bool, error) { + p1 = filepath.Clean(p1) + p2 = filepath.Clean(p2) + + fi1, err := os.Stat(p1) + if err != nil { + return false, errors.Wrapf(err, "could not check for path equivalence") + } + fi2, err := os.Stat(p2) + if err != nil { + return false, errors.Wrapf(err, "could not check for path equivalence") + } + + p1Filename, p2Filename := "", "" + + if !fi1.IsDir() { + p1, p1Filename = filepath.Split(p1) + } + if !fi2.IsDir() { + p2, p2Filename = filepath.Split(p2) + } + + if isPrefix1, err := HasFilepathPrefix(p1, p2); err != nil { + return false, errors.Wrap(err, "failed to check for path equivalence") + } else if isPrefix2, err := HasFilepathPrefix(p2, p1); err != nil { + return false, errors.Wrap(err, "failed to check for path equivalence") + } else if !isPrefix1 || !isPrefix2 { + return false, nil + } + + if p1Filename != "" || p2Filename != "" { + caseSensitive, err := isCaseSensitiveFilesystem(filepath.Join(p1, p1Filename)) + if err != nil { + return false, errors.Wrap(err, "could not check for filesystem case-sensitivity") + } + if caseSensitive { + if p1Filename != p2Filename { + return false, nil + } + } else { + if strings.ToLower(p1Filename) != strings.ToLower(p2Filename) { + return false, nil + } } } - return true + return true, nil } // RenameWithFallback attempts to rename a file or directory, but falls back to @@ -159,21 +212,25 @@ func renameByCopy(src, dst string) error { // If the input directory is such that the last component is composed // exclusively of case-less codepoints (e.g. numbers), this function will // return false. -func isCaseSensitiveFilesystem(dir string) bool { - alt := filepath.Join(filepath.Dir(dir), - genTestFilename(filepath.Base(dir))) +func isCaseSensitiveFilesystem(dir string) (bool, error) { + alt := filepath.Join(filepath.Dir(dir), genTestFilename(filepath.Base(dir))) dInfo, err := os.Stat(dir) if err != nil { - return true + return false, errors.Wrap(err, "could not determine the case-sensitivity of the filesystem") } aInfo, err := os.Stat(alt) if err != nil { - return true + // If the file doesn't exists, assume we are on a case-sensitive filesystem. + if os.IsNotExist(err) { + return true, nil + } + + return false, errors.Wrap(err, "could not determine the case-sensitivity of the filesystem") } - return !os.SameFile(dInfo, aInfo) + return !os.SameFile(dInfo, aInfo), nil } // genTestFilename returns a string with at most one rune case-flipped. @@ -343,7 +400,6 @@ func cloneSymlink(sl, dst string) error { // IsDir determines is the path given is a directory or not. func IsDir(name string) (bool, error) { - // TODO: lstat? fi, err := os.Stat(name) if err != nil { return false, err diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index e16f772c9c..920d925bb1 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -75,7 +75,11 @@ func TestHasFilepathPrefix(t *testing.T) { t.Fatal(err) } - if got := HasFilepathPrefix(c.path, c.prefix); c.want != got { + got, err := HasFilepathPrefix(c.path, c.prefix) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if c.want != got { t.Fatalf("dir: %q, prefix: %q, expected: %v, got: %v", c.path, c.prefix, c.want, got) } } @@ -122,18 +126,71 @@ func TestHasFilepathPrefix_Files(t *testing.T) { path string prefix string want bool + err bool }{ - {existingFile, filepath.Join(dir2), true}, - {nonExistingFile, filepath.Join(dir2), false}, + {existingFile, filepath.Join(dir2), true, false}, + {nonExistingFile, filepath.Join(dir2), false, true}, } for _, c := range cases { - if got := HasFilepathPrefix(c.path, c.prefix); c.want != got { + got, err := HasFilepathPrefix(c.path, c.prefix) + if err != nil && !c.err { + t.Fatalf("unexpected error: %s", err) + } + if c.want != got { t.Fatalf("dir: %q, prefix: %q, expected: %v, got: %v", c.path, c.prefix, c.want, got) } } } +func TestEquivalentPaths(t *testing.T) { + h := test.NewHelper(t) + h.TempDir("dir") + h.TempDir("dir2") + + h.TempFile("file", "") + h.TempFile("file2", "") + + h.TempDir("DIR") + h.TempFile("FILE", "") + + testcases := []struct { + p1, p2 string + caseSensitiveEquivalent bool + caseInensitiveEquivalent bool + err bool + }{ + {h.Path("dir"), h.Path("dir"), true, true, false}, + {h.Path("file"), h.Path("file"), true, true, false}, + {h.Path("dir"), h.Path("dir2"), false, false, false}, + {h.Path("file"), h.Path("file2"), false, false, false}, + {h.Path("dir"), h.Path("file"), false, false, false}, + {h.Path("dir"), h.Path("DIR"), false, true, false}, + {strings.ToLower(h.Path("dir")), strings.ToUpper(h.Path("dir")), false, true, true}, + } + + caseSensitive, err := isCaseSensitiveFilesystem(h.Path("dir")) + if err != nil { + t.Fatal("unexpcted error:", err) + } + + for _, tc := range testcases { + got, err := EquivalentPaths(tc.p1, tc.p2) + if err != nil && !tc.err { + t.Error("unexpected error:", err) + } + if caseSensitive { + if tc.caseSensitiveEquivalent != got { + t.Errorf("expected EquivalentPaths(%q, %q) to be %t on case-sensitive filesystem, got %t", tc.p1, tc.p2, tc.caseSensitiveEquivalent, got) + } + } else { + if tc.caseInensitiveEquivalent != got { + t.Errorf("expected EquivalentPaths(%q, %q) to be %t on case-insensitive filesystem, got %t", tc.p1, tc.p2, tc.caseInensitiveEquivalent, got) + } + } + } +} + func TestRenameWithFallback(t *testing.T) { dir, err := ioutil.TempDir("", "dep") if err != nil {