cmd/go/internal/str: fix PathPrefix functions for root directories
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>
diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go
index 60953fd..7ea6493 100644
--- a/src/cmd/go/internal/search/search.go
+++ b/src/cmd/go/internal/search/search.go
@@ -8,6 +8,7 @@
"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 @@
}
var found bool
for _, modRoot := range modRoots {
- if modRoot != "" && hasFilepathPrefix(abs, modRoot) {
+ if modRoot != "" && str.HasFilePathPrefix(abs, modRoot) {
found = true
}
}
@@ -428,22 +429,6 @@
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 @@
// 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 ""
- }
-}
diff --git a/src/cmd/go/internal/str/path.go b/src/cmd/go/internal/str/path.go
index c165b91..0c8f479 100644
--- a/src/cmd/go/internal/str/path.go
+++ b/src/cmd/go/internal/str/path.go
@@ -5,7 +5,9 @@
package str
import (
+ "os"
"path/filepath"
+ "runtime"
"strings"
)
@@ -28,11 +30,32 @@
// 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 @@
}
}
-// 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.
diff --git a/src/cmd/go/internal/str/str_test.go b/src/cmd/go/internal/str/str_test.go
index 158fe65..7c19877 100644
--- a/src/cmd/go/internal/str/str_test.go
+++ b/src/cmd/go/internal/str/str_test.go
@@ -6,7 +6,9 @@
import (
"os"
+ "path/filepath"
"runtime"
+ "strings"
"testing"
)
@@ -30,29 +32,75 @@
}
}
-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 @@
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 @@
{`\\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)
+ }
}
}
}
