Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for naughty naughty directory recursion in Windows #111

Merged
merged 1 commit into from
Sep 3, 2014

Conversation

shiftkey
Copy link
Contributor

@shiftkey shiftkey commented Sep 3, 2014

Fixes #108

Obligatory warning:

After being shouted at by the go compiler a bunch, I think I've found a better way to check for the filesystem root using a gem in the filepath docs:

http://golang.org/pkg/path/filepath/#Clean

The returned path ends in a slash only if it represents a root directory, such as "/" on Unix or C:\ on Windows.

Combining that with checking the last character on the file path (not the only character) should catch this issue we're facing with finding the root of a Windows drive.

Thoughts?

On Windows the root drive will appear as C:\, so this check should
sanitize the directory path and look at the last character, as per the
filepath docs
@@ -92,7 +92,8 @@ func resolveGitDir() (string, string, error) {
}

func recursiveResolveGitDir(dir string) (string, string, error) {
if len(dir) == 1 && dir[0] == os.PathSeparator {
var cleanDir = filepath.Clean(dir)
if cleanDir[len(cleanDir)-1] == os.PathSeparator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just check that len(cleanDir) is 1 and cleanDir[0] == os.PathSeparator?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh I see now. I was thinking it was changing "C:" to just "". That's dumb though. ENDING slash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nailed it!

technoweenie added a commit that referenced this pull request Sep 3, 2014
fix for naughty naughty directory recursion in Windows
@technoweenie technoweenie merged commit 26fe77e into master Sep 3, 2014
@technoweenie technoweenie deleted the windows-root-path branch September 3, 2014 22:39
@technoweenie
Copy link
Contributor

Huzzah!

C:\Users\octocat\Documents\GitHub> git media env
LocalWorkingDir=
LocalGitDir=
LocalMediaDir=
TempDir=C:\Users\octocat\AppData\Local\Temp\git-media

@shiftkey
Copy link
Contributor Author

shiftkey commented Sep 3, 2014

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

Successfully merging this pull request may close these issues.

run "git media" in a directory which isn't a git repository -> boomtown
2 participants