Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
check cfg filename case on case insensitive systems
Browse files Browse the repository at this point in the history
- `fs`
  - Export `IsCaseSensitiveFilesystem`. Add and run test on windows, linux.
  - Add function `ReadActualFilenames` to read actual file names of given
    string slice. Add tests to be run on windows.
- `project`
  - Add function `checkCfgFilenames` to check the filenames for manifest
    and lock have the expected case. Use `fs#IsCaseSensitiveFilesystem`
    for an early return as the check is costly. Add test to be run on windows.
- `context`
  - Call `project.go#checkCfgFilenames` after resolving project root. Add test
    for invalid manifest file name to be run on windows.
  • Loading branch information
sudo-suhas committed Sep 4, 2017
1 parent 238d8af commit 7d0e19a
Show file tree
Hide file tree
Showing 6 changed files with 335 additions and 12 deletions.
5 changes: 5 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ func (c *Ctx) LoadProject() (*Project, error) {
return nil, err
}

err = checkCfgFilenames(root)
if err != nil {
return nil, err
}

p := new(Project)

if err = p.SetRoot(root); err != nil {
Expand Down
45 changes: 45 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package dep

import (
"fmt"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -263,6 +264,50 @@ func TestLoadProjectNoSrcDir(t *testing.T) {
}
}

func TestLoadProjectCfgFileCase(t *testing.T) {
if runtime.GOOS != "windows" && runtime.GOOS == "darwin" {
t.Skip("skip this test on non-Windows, non-macOS")
}

// Here we test that a manifest filename with incorrect case
// throws an error. Similar error will also be thrown for the
// lock file as well which has been tested in
// `project_test.go#TestCheckCfgFilenames`. So not repeating here.

h := test.NewHelper(t)
defer h.Cleanup()

invalidMfName := strings.ToLower(ManifestName)

wd := filepath.Join("src", "test")
h.TempFile(filepath.Join(wd, invalidMfName), "")

ctx := &Ctx{
Out: discardLogger,
Err: discardLogger,
}

err := ctx.SetPaths(h.Path(wd), h.Path("."))
if err != nil {
t.Fatalf("%+v", err)
}

_, err = ctx.LoadProject()

if err == nil {
t.Fatal("should have returned 'Manifest Filename' error")
}

expectedErrMsg := fmt.Sprintf(
"manifest filename '%s' does not match '%s'",
invalidMfName, ManifestName,
)

if err.Error() != expectedErrMsg {
t.Fatalf("unexpected error: %+v", err)
}
}

// TestCaseInsentitive is test for Windows. This should work even though set
// difference letter cases in GOPATH.
func TestCaseInsentitiveGOPATH(t *testing.T) {
Expand Down
95 changes: 89 additions & 6 deletions internal/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func HasFilepathPrefix(path, prefix string) (bool, error) {
// 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.
// IsCaseSensitiveFilesystem and then equality.

// no need to check isCaseSensitiveFilesystem because VolumeName return
// 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))
Expand Down Expand Up @@ -82,7 +82,7 @@ func HasFilepathPrefix(path, prefix string) (bool, error) {
// something like ext4 filesystem mounted on FAT
// mountpoint, mounted on ext4 filesystem, i.e. the
// problematic filesystem is not the last one.
caseSensitive, err := 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")
}
Expand Down Expand Up @@ -135,7 +135,7 @@ func EquivalentPaths(p1, p2 string) (bool, error) {
}

if p1Filename != "" || p2Filename != "" {
caseSensitive, err := isCaseSensitiveFilesystem(filepath.Join(p1, p1Filename))
caseSensitive, err := IsCaseSensitiveFilesystem(filepath.Join(p1, p1Filename))
if err != nil {
return false, errors.Wrap(err, "could not check for filesystem case-sensitivity")
}
Expand Down Expand Up @@ -193,7 +193,7 @@ func renameByCopy(src, dst string) error {
return errors.Wrapf(os.RemoveAll(src), "cannot delete %s", src)
}

// isCaseSensitiveFilesystem determines if the filesystem where dir
// IsCaseSensitiveFilesystem determines if the filesystem where dir
// exists is case sensitive or not.
//
// CAVEAT: this function works by taking the last component of the given
Expand All @@ -212,7 +212,7 @@ 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, error) {
func IsCaseSensitiveFilesystem(dir string) (bool, error) {
alt := filepath.Join(filepath.Dir(dir), genTestFilename(filepath.Base(dir)))

dInfo, err := os.Stat(dir)
Expand Down Expand Up @@ -264,6 +264,89 @@ func genTestFilename(str string) string {
}, str)
}

var errPathNotDir = errors.New("given path is not a directory")

// ReadActualFilenames is used to determine the actual file names in given directory.
//
// On case sensitive file systems like ext4, it will check if those files exist using
// `os.Stat` and return a map with key and value as filenames which exist in the folder.
//
// Otherwise, it reads the contents of the directory
func ReadActualFilenames(dirPath string, names []string) (map[string]string, error) {
actualFilenames := make(map[string]string, len(names))
if len(names) <= 0 {
// This isn't expected to happen for current usage.
// Adding edge case handling, maybe useful in future
return actualFilenames, nil
}
// First, check that the given path is valid and it is a directory
dirStat, err := os.Stat(dirPath)
if err != nil {
return nil, errors.Wrap(err, "failed to read actual filenames")
}

if !dirStat.IsDir() {
return nil, errPathNotDir
}

// Ideally, we would use `os.Stat` for getting the actual file names
// but that returns the name we passed in as an argument and not the actual filename.
// So we are forced to list the directory contents and check
// against that. Since this check is costly, we do it only if absolutely necessary.
caseSensitive, err := IsCaseSensitiveFilesystem(dirPath)
if err != nil {
return nil, errors.Wrap(err, "failed to read actual filenames")
}
if caseSensitive {
// There will be no difference between actual filename and given filename
// So just check if those files exist.
for _, name := range names {
_, err := os.Stat(filepath.Join(dirPath, name))
if err == nil {
actualFilenames[name] = name
} else if !os.IsNotExist(err) {
// Some unexpected err, return it.
return nil, errors.Wrap(err, "failed to read actual filenames")
}
}
return actualFilenames, nil
}

dir, err := os.Open(dirPath)
if err != nil {
return nil, errors.Wrap(err, "failed to read actual filenames")
}
defer dir.Close()

// Pass -1 to read all files in directory
files, err := dir.Readdir(-1)
if err != nil {
return nil, errors.Wrap(err, "failed to read actual filenames")
}

// namesMap holds the mapping from lowercase name to search name.
// Using this, we can avoid repeatedly looping through names.
namesMap := make(map[string]string, len(names))
for _, name := range names {
namesMap[strings.ToLower(name)] = name
}

for _, file := range files {
if file.Mode().IsRegular() {
searchName, ok := namesMap[strings.ToLower(file.Name())]
if ok {
// We are interested in this file, case insensitive match successful
actualFilenames[searchName] = file.Name()
if len(actualFilenames) == len(names) {
// We found all that we were looking for
return actualFilenames, nil
}
}
}
}
return actualFilenames, nil
}

var (
errSrcNotDir = errors.New("source is not a directory")
errDstExist = errors.New("destination already exists")
Expand Down
107 changes: 101 additions & 6 deletions internal/fs/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"reflect"
"runtime"
"strings"
"testing"
Expand Down Expand Up @@ -169,7 +170,7 @@ func TestEquivalentPaths(t *testing.T) {
{strings.ToLower(h.Path("dir")), strings.ToUpper(h.Path("dir")), false, true, true},
}

caseSensitive, err := isCaseSensitiveFilesystem(h.Path("dir"))
caseSensitive, err := IsCaseSensitiveFilesystem(h.Path("dir"))
if err != nil {
t.Fatal("unexpcted error:", err)
}
Expand Down Expand Up @@ -229,6 +230,99 @@ func TestRenameWithFallback(t *testing.T) {
}
}

func TestIsCaseSensitiveFilesystem(t *testing.T) {
isLinux := runtime.GOOS == "linux"
isWindows := runtime.GOOS == "windows"
isMacOS := runtime.GOOS == "darwin"

if !isLinux && !isWindows && !isMacOS {
t.Skip("Run this test on Windows, Linux and macOS only")
}

dir, err := ioutil.TempDir("", "TestCaseSensitivity")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

var want bool
if isLinux {
want = true
} else {
want = false
}

got, err := IsCaseSensitiveFilesystem(dir)

if err != nil {
t.Fatalf("unexpected error message: \n\t(GOT) %+v", err)
}

if want != got {
t.Fatalf("unexpected value returned: \n\t(GOT) %t\n\t(WNT) %t", got, want)
}
}

func TestReadActualFilenames(t *testing.T) {
if runtime.GOOS != "windows" && runtime.GOOS == "darwin" {
t.Skip("skip this test on non-Windows, non-macOS")
}

h := test.NewHelper(t)
defer h.Cleanup()

h.TempDir("")
tmpPath := h.Path(".")

_, err := ReadActualFilenames(filepath.Join(tmpPath, "does_not_exists"), []string{""})
switch {
case err == nil:
t.Fatal("expected err for non-existing folder")
case !os.IsNotExist(err):
t.Fatalf("unexpected error: %+v", err)
}
h.TempFile("tmpFile", "")
_, err = ReadActualFilenames(h.Path("tmpFile"), []string{""})
switch {
case err == nil:
t.Fatal("expected err for passing file instead of directory")
case err != errPathNotDir:
t.Fatalf("unexpected error: %+v", err)
}

cases := []struct {
createFiles []string
names []string
want map[string]string
}{
{nil, nil, map[string]string{}}, {
[]string{"test1.txt"},
[]string{"Test1.txt"},
map[string]string{"Test1.txt": "test1.txt"},
}, {
[]string{"test2.txt", "test3.TXT"},
[]string{"test2.txt", "Test3.txt", "Test4.txt"},
map[string]string{
"test2.txt": "test2.txt",
"Test3.txt": "test3.TXT",
"Test4.txt": "",
},
},
}
for _, c := range cases {
for _, file := range c.createFiles {
h.TempFile(file, "")
}
got, err := ReadActualFilenames(tmpPath, c.names)
if err != nil {
t.Fatalf("unexpected error: %+v", err)
}
if !reflect.DeepEqual(c.want, got) {
t.Fatalf("returned value does not match expected: \n\t(GOT) %v\n\t(WNT) %v", got, c.want)
}
}
}

func TestGenTestFilename(t *testing.T) {
cases := []struct {
str string
Expand All @@ -254,11 +348,11 @@ func TestGenTestFilename(t *testing.T) {

func BenchmarkGenTestFilename(b *testing.B) {
cases := []string{
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
"αααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααα",
"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111",
"⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘",
strings.Repeat("a", 128),
strings.Repeat("A", 128),
strings.Repeat("α", 128),
strings.Repeat("1", 128),
strings.Repeat("⌘", 128),
}

for i := 0; i < b.N; i++ {
Expand Down Expand Up @@ -613,6 +707,7 @@ func TestCopyFileLongFilePath(t *testing.T) {

h := test.NewHelper(t)
h.TempDir(".")
defer h.Cleanup()

tmpPath := h.Path(".")

Expand Down
Loading

0 comments on commit 7d0e19a

Please sign in to comment.