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

Suggestion: Check case of configuration filenames to avoid cross-platform incompatibilities #1035

Closed
lpar opened this issue Aug 18, 2017 · 4 comments · Fixed by #1114
Closed

Comments

@lpar
Copy link

lpar commented Aug 18, 2017

Following discussion on Reddit, I think it might be a good idea if dep checked the case of the names of its configuration files such as Gopkg.toml, at least on Mac.

This would prevent the problem of Mac users checking in a project with gopkg.toml in lower case, which works on the Mac (because it's case-insensitive) but fails on Linux.

@sudo-suhas
Copy link
Contributor

@sdboyer Can I work on this? If yes, can you point me in the right direction wrt where I would have to make the changes?

@sdboyer
Copy link
Member

sdboyer commented Aug 27, 2017

@sudo-suhas yes, absolutely! 🎉 🎉

the key place to look will be in Ctx.LoadProject(). that's where the check will need to happen.

as for doing the check itself, you'll probably want to take inspiration from fs.isCaseSensitiveFilesystem(), though i'd prefer we do a bit of copying rather than exporting it directly, at least for now.

that should be enough to get you started - please let me know what other pointers would be helpful!

@sudo-suhas
Copy link
Contributor

@sdboyer I took a look at the functions you pointed to. However, I think the problem here is slightly different. What I need to do is check if the Gopkg.toml exists with an incorrect case. Which can mean any of gopkg.toml, gopkg.TOML, GoPkg.toml etc. The simple way would be, ask for os.stat(ManifestName) and cross check the contents of stat.Name(). However, this simply returns the name we supplied for getting stats and not the actual file name:

package main

import (
	"fmt"
	"os"
)

func main() {
	fileStat, err := os.Stat("./test.txt")
	if err != nil {
		panic(err)
	}
	fmt.Println(fileStat.Name())
}
$ ls
main.go  Test.txt

$ go run main.go
test.txt

The only way to print the exact case of the file that I have been able to figure out is to use File#Readdir:

package main

import (
	"fmt"
	"os"
	"path/filepath"
)

func main() {
	dirname := "." + string(filepath.Separator)

	d, err := os.Open(dirname)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	defer d.Close()

	files, err := d.Readdir(-1)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}

	fmt.Println("Reading " + dirname)

	for _, file := range files {
		if file.Mode().IsRegular() {
			if filepath.Ext(file.Name()) == ".txt" {
				fmt.Println(file.Name())
			}
		}
	}
}
$ go run main.go
Reading .\
Test.txt

However this is quite wasteful. Is there any better way to do this?

@sdboyer
Copy link
Member

sdboyer commented Aug 29, 2017

However this is quite wasteful. Is there any better way to do this?

there is not, AFAIK. that's why i was loathe to recommend it, and just suggested the crappier approach of trying the one case variation. you're right, of course, that we have to try all the combinations if we actually want to truly enforce this as a rule.

the best i can offer is that you can at least avoid having to os.Stat() everything by doing case-insensitive comparison of each of the files in the list using strings.FoldEqual()

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants