Merge pull request #704 from mikijov/issue675-old

Fixes #675. Changes to HasFilepathPrefix to handle Windows better.

Author: sdboyer

internal/fs/fs.go

@@ -29,10 +29,28 @@ import (
 // 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 {
-	if filepath.VolumeName(path) != filepath.VolumeName(prefix) {
+	// 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
+	// d which will be appended to for incremental checks using
+	// isCaseSensitiveFilesystem and then equality.
+
+	// no need to check isCaseSensitiveFilesystem because VolumeName return
+	// empty string on all non-Windows machines
+	vnPath := strings.ToLower(filepath.VolumeName(path))
+	vnPrefix := strings.ToLower(filepath.VolumeName(prefix))
+	if vnPath != vnPrefix {
 		return false
 	}
 
+	// 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)
+	}
+
 	var dn string
 
 	if isDir, err := IsDir(path); err != nil {
@@ -46,14 +64,17 @@ func HasFilepathPrefix(path, prefix string) bool {
 	dn = strings.TrimSuffix(dn, string(os.PathSeparator))
 	prefix = strings.TrimSuffix(prefix, string(os.PathSeparator))
 
+	// [1:] in the lines below eliminates empty string on *nix and volume name on Windows
 	dirs := strings.Split(dn, string(os.PathSeparator))[1:]
 	prefixes := strings.Split(prefix, string(os.PathSeparator))[1:]
 
 	if len(prefixes) > len(dirs) {
 		return false
 	}
 
-	var d, p string
+	// d,p are initialized with "" on *nix and volume name on Windows
+	d := vnPath
+	p := vnPrefix
 
 	for i := range prefixes {
 		// need to test each component of the path for

internal/fs/fs_test.go

@@ -9,37 +9,61 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/golang/dep/internal/test"
 )
 
+// This function tests HadFilepathPrefix. It should test it on both case
+// sensitive and insensitive situations. However, the only reliable way to test
+// case-insensitive behaviour is if using case-insensitive filesystem.  This
+// cannot be guaranteed in an automated test. Therefore, the behaviour of the
+// tests is not to test case sensitivity on *nix and to assume that Windows is
+// case-insensitive. Please see link below for some background.
+//
+// https://superuser.com/questions/266110/how-do-you-make-windows-7-fully-case-sensitive-with-respect-to-the-filesystem
+//
+// NOTE: NTFS can be made case-sensitive. However many Windows programs,
+// including Windows Explorer do not handle gracefully multiple files that
+// differ only in capitalization. It is possible that this can cause these tests
+// to fail on some setups.
 func TestHasFilepathPrefix(t *testing.T) {
 	dir, err := ioutil.TempDir("", "dep")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(dir)
 
+	// dir2 is the same as dir but with different capitalization on Windows to
+	// test case insensitivity
+	var dir2 string
+	if runtime.GOOS == "windows" {
+		dir = strings.ToLower(dir)
+		dir2 = strings.ToUpper(dir)
+	} else {
+		dir2 = dir
+	}
+
 	cases := []struct {
 		path   string
 		prefix string
 		want   bool
 	}{
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir), true},
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a"), true},
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "b"), true},
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir, "c"), false},
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "d", "b"), false},
-		{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "b2"), false},
-		{filepath.Join(dir), filepath.Join(dir, "a", "b"), false},
-		{filepath.Join(dir, "ab"), filepath.Join(dir, "a", "b"), false},
-		{filepath.Join(dir, "ab"), filepath.Join(dir, "a"), false},
-		{filepath.Join(dir, "123"), filepath.Join(dir, "123"), true},
-		{filepath.Join(dir, "123"), filepath.Join(dir, "1"), false},
-		{filepath.Join(dir, "⌘"), filepath.Join(dir, "⌘"), true},
-		{filepath.Join(dir, "a"), filepath.Join(dir, "⌘"), false},
-		{filepath.Join(dir, "⌘"), filepath.Join(dir, "a"), false},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2), true},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2, "a"), true},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2, "a", "b"), true},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2, "c"), false},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2, "a", "d", "b"), false},
+		{filepath.Join(dir, "a", "b"), filepath.Join(dir2, "a", "b2"), false},
+		{filepath.Join(dir), filepath.Join(dir2, "a", "b"), false},
+		{filepath.Join(dir, "ab"), filepath.Join(dir2, "a", "b"), false},
+		{filepath.Join(dir, "ab"), filepath.Join(dir2, "a"), false},
+		{filepath.Join(dir, "123"), filepath.Join(dir2, "123"), true},
+		{filepath.Join(dir, "123"), filepath.Join(dir2, "1"), false},
+		{filepath.Join(dir, "⌘"), filepath.Join(dir2, "⌘"), true},
+		{filepath.Join(dir, "a"), filepath.Join(dir2, "⌘"), false},
+		{filepath.Join(dir, "⌘"), filepath.Join(dir2, "a"), false},
 	}
 
 	for _, c := range cases {
@@ -57,13 +81,36 @@ func TestHasFilepathPrefix(t *testing.T) {
 	}
 }
 
+// This function tests HadFilepathPrefix. It should test it on both case
+// sensitive and insensitive situations. However, the only reliable way to test
+// case-insensitive behaviour is if using case-insensitive filesystem.  This
+// cannot be guaranteed in an automated test. Therefore, the behaviour of the
+// tests is not to test case sensitivity on *nix and to assume that Windows is
+// case-insensitive. Please see link below for some background.
+//
+// https://superuser.com/questions/266110/how-do-you-make-windows-7-fully-case-sensitive-with-respect-to-the-filesystem
+//
+// NOTE: NTFS can be made case-sensitive. However many Windows programs,
+// including Windows Explorer do not handle gracefully multiple files that
+// differ only in capitalization. It is possible that this can cause these tests
+// to fail on some setups.
 func TestHasFilepathPrefix_Files(t *testing.T) {
 	dir, err := ioutil.TempDir("", "dep")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(dir)
 
+	// dir2 is the same as dir but with different capitalization on Windows to
+	// test case insensitivity
+	var dir2 string
+	if runtime.GOOS == "windows" {
+		dir = strings.ToLower(dir)
+		dir2 = strings.ToUpper(dir)
+	} else {
+		dir2 = dir
+	}
+
 	existingFile := filepath.Join(dir, "exists")
 	if _, err := os.Create(existingFile); err != nil {
 		t.Fatal(err)
@@ -76,8 +123,8 @@ func TestHasFilepathPrefix_Files(t *testing.T) {
 		prefix string
 		want   bool
 	}{
-		{existingFile, filepath.Join(dir), false},
-		{nonExistingFile, filepath.Join(dir), true},
+		{existingFile, filepath.Join(dir2), false},
+		{nonExistingFile, filepath.Join(dir2), true},
 	}
 
 	for _, c := range cases {