Correctly normalize Windows package paths.

[jrick]

When the name passed to util.NormalizeName contains backslashes from a
Windows filepath, the backslashes must be first converted to forward
slashes (used by all package names, including on Windows). This fixes
the strings.TrimPrefix call used to strip the repo root from the
subpackage.

As a result of this, subpackages in lockfiles are written identically
on Windows as other Unix platforms and no duplicate subpackages are
introduced.

Fixes #389.

[mattfarina]

What do we get from this? The first thing GetRootFromPackage does is run filepath.ToSlash. GetRootFromPackage is sometimes called outside NormalizeName.

Is there a problem in the latest code that needs to be addressed?

[jrick]

The issue is that name contains backslashes. The root is normalized, but the full name is not, so the prefix trim doesn't work as intended.

With github.com\foo\bar\baz as input:

// name == "github.com\foo\bar\baz"
root := GetRootFromPackage(name) // root == "github.com/foo/bar"
extra := strings.TrimPrefix(name, root) // extra == "github.com\foo\bar\baz"

[mattfarina]

I see the bug I introduced now.

[jrick]

Oops, intended to add a testcase for this and it fails because it just concats the root + extra, which no longer matches the initial input. Fixing shortly.

[mattfarina]

So, this is entirely a micro-optimization but...

Since the root name is already being normalized then the extra is the only part that needs to have it done. Can that happen here instead?

[jrick]

No. That will convert the backslashes to forward slashes, but the returned subpackage will still contain the full root since the TrimPrefix just above it did not remove the root.

[jrick]

I'm not sure why it's failing on Travis now. Passes locally.

[jrick]

Oh.. I see.

From filepath/path.go:

func ToSlash(path string) string {
    if Separator == '/' {
        return path
    }
    return strings.Replace(path, string(Separator), "/", -1)
}

Separator is os.PathSeparator so this has different behavior on Windows vs Unix. Specifically, on Unix, it won't replace backslashes with forward slashes since backslashes are not a separator character on those platforms, causing the new test to fail there.

What do you want to do in this case? Should glide replace uses of filepath.ToSlash with its own version that works consistently on all platforms?

[mattfarina]

Maybe we need to tweak the use of normalizeSlash.

[jrick]

Looks like we could just use that instead of filepath.ToSlash.

Suggestions on how to use it though? it's unexported in another package. Copy it into util?

[mattfarina]

Is there an operating system with the path separator other than / or \ that Go supports? Or that's in wide use? If so that's a separate issue because other things will fail.

[jrick]

Not to my knowledge. Should this only replace backslashes then?

[mattfarina]

I think using v = strings.Replace(v, "\\", "/", -1) should be enough.

[mattfarina]

@jrick thanks for the contribution!