]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go/internal/str: fix PathPrefix functions for root directories
authorBryan C. Mills <bcmills@google.com>
Fri, 20 Jan 2023 19:40:09 +0000 (14:40 -0500)
committerGopher Robot <gobot@golang.org>
Wed, 25 Jan 2023 16:49:13 +0000 (16:49 +0000)
For #51506.
For #50807.

Change-Id: I4c0ae85a2103ac4f07351a4f01ce24fa02f03104
Reviewed-on: https://go-review.googlesource.com/c/go/+/463176
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
src/cmd/go/internal/search/search.go
src/cmd/go/internal/str/path.go
src/cmd/go/internal/str/str_test.go

index 60953fdee33619128f20f6df7ab6c69b02f23b28..7ea6493d4aaf872dabdf892c5a7d5ab8dd51684a 100644 (file)
@@ -8,6 +8,7 @@ import (
        "cmd/go/internal/base"
        "cmd/go/internal/cfg"
        "cmd/go/internal/fsys"
+       "cmd/go/internal/str"
        "cmd/internal/pkgpattern"
        "fmt"
        "go/build"
@@ -255,7 +256,7 @@ func (m *Match) MatchDirs(modRoots []string) {
                }
                var found bool
                for _, modRoot := range modRoots {
-                       if modRoot != "" && hasFilepathPrefix(abs, modRoot) {
+                       if modRoot != "" && str.HasFilePathPrefix(abs, modRoot) {
                                found = true
                        }
                }
@@ -428,22 +429,6 @@ func CleanPatterns(patterns []string) []string {
        return out
 }
 
-// hasFilepathPrefix reports whether the path s begins with the
-// elements in prefix.
-func hasFilepathPrefix(s, prefix string) bool {
-       switch {
-       default:
-               return false
-       case len(s) == len(prefix):
-               return s == prefix
-       case len(s) > len(prefix):
-               if prefix != "" && prefix[len(prefix)-1] == filepath.Separator {
-                       return strings.HasPrefix(s, prefix)
-               }
-               return s[len(prefix)] == filepath.Separator && s[:len(prefix)] == prefix
-       }
-}
-
 // IsStandardImportPath reports whether $GOROOT/src/path should be considered
 // part of the standard distribution. For historical reasons we allow people to add
 // their own code to $GOROOT instead of using $GOPATH, but we assume that
@@ -475,67 +460,44 @@ func IsRelativePath(pattern string) bool {
 // If not, InDir returns an empty string.
 // InDir makes some effort to succeed even in the presence of symbolic links.
 func InDir(path, dir string) string {
-       if rel := inDirLex(path, dir); rel != "" {
+       // inDirLex reports whether path is lexically in dir,
+       // without considering symbolic or hard links.
+       inDirLex := func(path, dir string) (string, bool) {
+               if dir == "" {
+                       return path, true
+               }
+               rel := str.TrimFilePathPrefix(path, dir)
+               if rel == path {
+                       return "", false
+               }
+               if rel == "" {
+                       return ".", true
+               }
+               return rel, true
+       }
+
+       if rel, ok := inDirLex(path, dir); ok {
                return rel
        }
        xpath, err := filepath.EvalSymlinks(path)
        if err != nil || xpath == path {
                xpath = ""
        } else {
-               if rel := inDirLex(xpath, dir); rel != "" {
+               if rel, ok := inDirLex(xpath, dir); ok {
                        return rel
                }
        }
 
        xdir, err := filepath.EvalSymlinks(dir)
        if err == nil && xdir != dir {
-               if rel := inDirLex(path, xdir); rel != "" {
+               if rel, ok := inDirLex(path, xdir); ok {
                        return rel
                }
                if xpath != "" {
-                       if rel := inDirLex(xpath, xdir); rel != "" {
+                       if rel, ok := inDirLex(xpath, xdir); ok {
                                return rel
                        }
                }
        }
        return ""
 }
-
-// inDirLex is like inDir but only checks the lexical form of the file names.
-// It does not consider symbolic links.
-// TODO(rsc): This is a copy of str.HasFilePathPrefix, modified to
-// return the suffix. Most uses of str.HasFilePathPrefix should probably
-// be calling InDir instead.
-func inDirLex(path, dir string) string {
-       pv := strings.ToUpper(filepath.VolumeName(path))
-       dv := strings.ToUpper(filepath.VolumeName(dir))
-       path = path[len(pv):]
-       dir = dir[len(dv):]
-       switch {
-       default:
-               return ""
-       case pv != dv:
-               return ""
-       case len(path) == len(dir):
-               if path == dir {
-                       return "."
-               }
-               return ""
-       case dir == "":
-               return path
-       case len(path) > len(dir):
-               if dir[len(dir)-1] == filepath.Separator {
-                       if path[:len(dir)] == dir {
-                               return path[len(dir):]
-                       }
-                       return ""
-               }
-               if path[len(dir)] == filepath.Separator && path[:len(dir)] == dir {
-                       if len(path) == len(dir)+1 {
-                               return "."
-                       }
-                       return path[len(dir)+1:]
-               }
-               return ""
-       }
-}
index c165b91785d63d6a7b3b29cd4ce57f47d24c5ce6..0c8f47988ece5b8699d32c83bdb4812999be00f6 100644 (file)
@@ -5,7 +5,9 @@
 package str
 
 import (
+       "os"
        "path/filepath"
+       "runtime"
        "strings"
 )
 
@@ -28,11 +30,32 @@ func HasPathPrefix(s, prefix string) bool {
 
 // HasFilePathPrefix reports whether the filesystem path s
 // begins with the elements in prefix.
+//
+// HasFilePathPrefix is case-sensitive (except for volume names) even if the
+// filesystem is not, does not apply Unicode normalization even if the
+// filesystem does, and assumes that all path separators are canonicalized to
+// filepath.Separator (as returned by filepath.Clean).
 func HasFilePathPrefix(s, prefix string) bool {
-       sv := strings.ToUpper(filepath.VolumeName(s))
-       pv := strings.ToUpper(filepath.VolumeName(prefix))
+       sv := filepath.VolumeName(s)
+       pv := filepath.VolumeName(prefix)
+
+       // Strip the volume from both paths before canonicalizing sv and pv:
+       // it's unlikely that strings.ToUpper will change the length of the string,
+       // but doesn't seem impossible.
        s = s[len(sv):]
        prefix = prefix[len(pv):]
+
+       // Always treat Windows volume names as case-insensitive, even though
+       // we don't treat the rest of the path as such.
+       //
+       // TODO(bcmills): Why do we care about case only for the volume name? It's
+       // been this way since https://go.dev/cl/11316, but I don't understand why
+       // that problem doesn't apply to case differences in the entire path.
+       if sv != pv {
+               sv = strings.ToUpper(sv)
+               pv = strings.ToUpper(pv)
+       }
+
        switch {
        default:
                return false
@@ -50,21 +73,36 @@ func HasFilePathPrefix(s, prefix string) bool {
        }
 }
 
-// TrimFilePathPrefix returns s without the leading path elements in prefix.
+// TrimFilePathPrefix returns s without the leading path elements in prefix,
+// such that joining the string to prefix produces s.
+//
 // If s does not start with prefix (HasFilePathPrefix with the same arguments
 // returns false), TrimFilePathPrefix returns s. If s equals prefix,
 // TrimFilePathPrefix returns "".
 func TrimFilePathPrefix(s, prefix string) string {
+       if prefix == "" {
+               // Trimming the empty string from a path should join to produce that path.
+               // (Trim("/tmp/foo", "") should give "/tmp/foo", not "tmp/foo".)
+               return s
+       }
        if !HasFilePathPrefix(s, prefix) {
                return s
        }
+
        trimmed := s[len(prefix):]
-       if len(trimmed) == 0 || trimmed[0] != filepath.Separator {
-               // Prefix either is equal to s, or ends with a separator
-               // (for example, if it is exactly "/").
-               return trimmed
+       if len(trimmed) > 0 && os.IsPathSeparator(trimmed[0]) {
+               if runtime.GOOS == "windows" && prefix == filepath.VolumeName(prefix) && len(prefix) == 2 && prefix[1] == ':' {
+                       // Joining a relative path to a bare Windows drive letter produces a path
+                       // relative to the working directory on that drive, but the original path
+                       // was absolute, not relative. Keep the leading path separator so that it
+                       // remains absolute when joined to prefix.
+               } else {
+                       // Prefix ends in a regular path element, so strip the path separator that
+                       // follows it.
+                       trimmed = trimmed[1:]
+               }
        }
-       return trimmed[1:]
+       return trimmed
 }
 
 // QuoteGlob returns s with all Glob metacharacters quoted.
index 158fe65dc16edfc7e8f9136fcdb9b279bcc1ab74..7c1987766635c8b291eda6c3344ff1eecb000156 100644 (file)
@@ -6,7 +6,9 @@ package str
 
 import (
        "os"
+       "path/filepath"
        "runtime"
+       "strings"
        "testing"
 )
 
@@ -30,29 +32,75 @@ func TestFoldDup(t *testing.T) {
        }
 }
 
-type trimFilePathPrefixTest struct {
-       s, prefix, want string
+func TestHasPathPrefix(t *testing.T) {
+       type testCase struct {
+               s, prefix string
+               want      bool
+       }
+       for _, tt := range []testCase{
+               {"", "", true},
+               {"", "/", false},
+               {"foo", "", true},
+               {"foo", "/", false},
+               {"foo", "foo", true},
+               {"foo", "foo/", false},
+               {"foo", "/foo", false},
+               {"foo/bar", "", true},
+               {"foo/bar", "foo", true},
+               {"foo/bar", "foo/", true},
+               {"foo/bar", "/foo", false},
+               {"foo/bar", "foo/bar", true},
+               {"foo/bar", "foo/bar/", false},
+               {"foo/bar", "/foo/bar", false},
+       } {
+               got := HasPathPrefix(tt.s, tt.prefix)
+               if got != tt.want {
+                       t.Errorf("HasPathPrefix(%q, %q) = %v; want %v", tt.s, tt.prefix, got, tt.want)
+               }
+       }
 }
 
 func TestTrimFilePathPrefixSlash(t *testing.T) {
        if os.PathSeparator != '/' {
                t.Skipf("test requires slash-separated file paths")
        }
-       tests := []trimFilePathPrefixTest{
-               {"/foo", "", "foo"},
+
+       type testCase struct {
+               s, prefix, want string
+       }
+       for _, tt := range []testCase{
+               {"/", "", "/"},
+               {"/", "/", ""},
+               {"/foo", "", "/foo"},
                {"/foo", "/", "foo"},
                {"/foo", "/foo", ""},
                {"/foo/bar", "/foo", "bar"},
                {"/foo/bar", "/foo/", "bar"},
+               {"/foo/", "/", "foo/"},
+               {"/foo/", "/foo", ""},
+               {"/foo/", "/foo/", ""},
+
                // if prefix is not s's prefix, return s
+               {"", "/", ""},
                {"/foo", "/bar", "/foo"},
                {"/foo", "/foo/bar", "/foo"},
-       }
-
-       for _, tt := range tests {
-               if got := TrimFilePathPrefix(tt.s, tt.prefix); got != tt.want {
+               {"foo", "/foo", "foo"},
+               {"/foo", "foo", "/foo"},
+               {"/foo", "/foo/", "/foo"},
+       } {
+               got := TrimFilePathPrefix(tt.s, tt.prefix)
+               if got == tt.want {
+                       t.Logf("TrimFilePathPrefix(%q, %q) = %q", tt.s, tt.prefix, got)
+               } else {
                        t.Errorf("TrimFilePathPrefix(%q, %q) = %q, want %q", tt.s, tt.prefix, got, tt.want)
                }
+
+               if HasFilePathPrefix(tt.s, tt.prefix) {
+                       joined := filepath.Join(tt.prefix, got)
+                       if clean := filepath.Clean(tt.s); joined != clean {
+                               t.Errorf("filepath.Join(%q, %q) = %q, want %q", tt.prefix, got, joined, clean)
+                       }
+               }
        }
 }
 
@@ -60,16 +108,29 @@ func TestTrimFilePathPrefixWindows(t *testing.T) {
        if runtime.GOOS != "windows" {
                t.Skipf("test requires Windows file paths")
        }
-       tests := []trimFilePathPrefixTest{
-               {`C:\foo`, `C:`, `foo`},
+       type testCase struct {
+               s, prefix, want string
+       }
+       for _, tt := range []testCase{
+               {`\`, ``, `\`},
+               {`\`, `\`, ``},
+               {`C:`, `C:`, ``},
+               {`C:\`, `C:`, `\`},
+               {`C:\`, `C:\`, ``},
+               {`C:\foo`, ``, `C:\foo`},
+               {`C:\foo`, `C:`, `\foo`},
                {`C:\foo`, `C:\`, `foo`},
                {`C:\foo`, `C:\foo`, ``},
+               {`C:\foo\`, `C:\foo`, ``},
                {`C:\foo\bar`, `C:\foo`, `bar`},
                {`C:\foo\bar`, `C:\foo\`, `bar`},
                // if prefix is not s's prefix, return s
                {`C:\foo`, `C:\bar`, `C:\foo`},
                {`C:\foo`, `C:\foo\bar`, `C:\foo`},
+               {`C:`, `C:\`, `C:`},
                // if volumes are different, return s
+               {`C:`, ``, `C:`},
+               {`C:\`, ``, `C:\`},
                {`C:\foo`, ``, `C:\foo`},
                {`C:\foo`, `\foo`, `C:\foo`},
                {`C:\foo`, `D:\foo`, `C:\foo`},
@@ -90,11 +151,35 @@ func TestTrimFilePathPrefixWindows(t *testing.T) {
                {`\\host\share\foo`, `\\other\share\`, `\\host\share\foo`},
                {`\\host\share\foo`, `\\host\`, `\\host\share\foo`},
                {`\\host\share\foo`, `\share\`, `\\host\share\foo`},
-       }
 
-       for _, tt := range tests {
-               if got := TrimFilePathPrefix(tt.s, tt.prefix); got != tt.want {
-                       t.Errorf("TrimFilePathPrefix(%q, %q) = %q, want %q", tt.s, tt.prefix, got, tt.want)
+               // only volume names are case-insensitive
+               {`C:\foo`, `c:`, `\foo`},
+               {`C:\foo`, `c:\foo`, ``},
+               {`c:\foo`, `C:`, `\foo`},
+               {`c:\foo`, `C:\foo`, ``},
+               {`C:\foo`, `C:\Foo`, `C:\foo`},
+               {`\\Host\Share\foo`, `\\host\share`, `foo`},
+               {`\\Host\Share\foo`, `\\host\share\foo`, ``},
+               {`\\host\share\foo`, `\\Host\Share`, `foo`},
+               {`\\host\share\foo`, `\\Host\Share\foo`, ``},
+               {`\\Host\Share\foo`, `\\Host\Share\Foo`, `\\Host\Share\foo`},
+       } {
+               got := TrimFilePathPrefix(tt.s, tt.prefix)
+               if got == tt.want {
+                       t.Logf("TrimFilePathPrefix(%#q, %#q) = %#q", tt.s, tt.prefix, got)
+               } else {
+                       t.Errorf("TrimFilePathPrefix(%#q, %#q) = %#q, want %#q", tt.s, tt.prefix, got, tt.want)
+               }
+
+               if HasFilePathPrefix(tt.s, tt.prefix) {
+                       // Although TrimFilePathPrefix is only case-insensitive in the volume name,
+                       // what we care about in testing Join is that absolute paths remain
+                       // absolute and relative paths remaining relative — there is no harm in
+                       // over-normalizing letters in the comparison, so we use EqualFold.
+                       joined := filepath.Join(tt.prefix, got)
+                       if clean := filepath.Clean(tt.s); !strings.EqualFold(joined, clean) {
+                               t.Errorf("filepath.Join(%#q, %#q) = %#q, want %#q", tt.prefix, got, joined, clean)
+                       }
                }
        }
 }