cmd/go: fix vcsFromDir returning bad root on Windows

Apply golang/tools@5804fef.

In the context of cmd/go build tool, import path is a '/'-separated path.
This can be inferred from `go help importpath` and `go help packages`.
vcsFromDir documentation says on return, root is the import path
corresponding to the root of the repository. On Windows and other
OSes where os.PathSeparator is not '/', that wasn't true since root
would contain characters other than '/', and therefore it wasn't a
valid import path corresponding to the root of the repository.
Fix that by using filepath.ToSlash.

Add test coverage for vcsFromDir, it was previously not tested.
It's taken from golang.org/x/tools/go/vcs tests, and modified to
improve style.

Additionally, remove an unneccessary statement from the documentation
"(thus root is a prefix of importPath)". There is no variable
importPath that is being referred to (it's possible p.ImportPath
was being referred to). Without it, the description of root value
matches the documentation of repoRoot.root struct field:

	// root is the import path corresponding to the root of the
	// repository
	root string

Rename and change signature of vcsForDir(p *Package) to
vcsFromDir(dir, srcRoot string). This is more in sync with the x/tools
version. It's also simpler, since vcsFromDir only needs those two
values from Package, and nothing more. Change "for" to "from" in name
because it's more consistent and clear.

Update usage of vcsFromDir to match the new signature, and respect
that returned root is a '/'-separated path rather than a os.PathSeparator
separated path.

Fixes #15040.
Updates #7723.
Helps #11490.

Change-Id: Idf51b9239f57248739daaa200aa1c6e633cb5f7f
Reviewed-on: https://go-review.googlesource.com/21345
Reviewed-by: Alex Brainman <[email protected]>
Run-TryBot: Alex Brainman <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

src/cmd/go/get.go

@@ -45,7 +45,7 @@ missing packages but does not use it to look for updates to existing packages.
 
 Get also accepts build flags to control the installation. See 'go help build'.
 
-When checking out a new package, get creates the target directory 
+When checking out a new package, get creates the target directory
 GOPATH/src/<import-path>. If the GOPATH contains multiple entries,
 get uses the first one. See 'go help gopath'.
 
@@ -346,15 +346,15 @@ func downloadPackage(p *Package) error {
 
 	if p.build.SrcRoot != "" {
 		// Directory exists. Look for checkout along path to src.
-		vcs, rootPath, err = vcsForDir(p)
+		vcs, rootPath, err = vcsFromDir(p.Dir, p.build.SrcRoot)
 		if err != nil {
 			return err
 		}
 		repo = "<local>" // should be unused; make distinctive
 
 		// Double-check where it came from.
 		if *getU && vcs.remoteRepo != nil {
-			dir := filepath.Join(p.build.SrcRoot, rootPath)
+			dir := filepath.Join(p.build.SrcRoot, filepath.FromSlash(rootPath))
 			remote, err := vcs.remoteRepo(vcs, dir)
 			if err != nil {
 				return err
@@ -401,7 +401,7 @@ func downloadPackage(p *Package) error {
 		p.build.SrcRoot = filepath.Join(list[0], "src")
 		p.build.PkgRoot = filepath.Join(list[0], "pkg")
 	}
-	root := filepath.Join(p.build.SrcRoot, rootPath)
+	root := filepath.Join(p.build.SrcRoot, filepath.FromSlash(rootPath))
 	// If we've considered this repository already, don't do it again.
 	if downloadRootCache[root] {
 		return nil

src/cmd/go/vcs.go

@@ -479,15 +479,14 @@ type vcsPath struct {
 	regexp *regexp.Regexp // cached compiled form of re
 }
 
-// vcsForDir inspects dir and its parents to determine the
+// vcsFromDir inspects dir and its parents to determine the
 // version control system and code repository to use.
 // On return, root is the import path
-// corresponding to the root of the repository
-// (thus root is a prefix of importPath).
-func vcsForDir(p *Package) (vcs *vcsCmd, root string, err error) {
+// corresponding to the root of the repository.
+func vcsFromDir(dir, srcRoot string) (vcs *vcsCmd, root string, err error) {
 	// Clean and double-check that dir is in (a subdirectory of) srcRoot.
-	dir := filepath.Clean(p.Dir)
-	srcRoot := filepath.Clean(p.build.SrcRoot)
+	dir = filepath.Clean(dir)
+	srcRoot = filepath.Clean(srcRoot)
 	if len(dir) <= len(srcRoot) || dir[len(srcRoot)] != filepath.Separator {
 		return nil, "", fmt.Errorf("directory %q is outside source root %q", dir, srcRoot)
 	}
@@ -496,7 +495,7 @@ func vcsForDir(p *Package) (vcs *vcsCmd, root string, err error) {
 	for len(dir) > len(srcRoot) {
 		for _, vcs := range vcsList {
 			if fi, err := os.Stat(filepath.Join(dir, "."+vcs.cmd)); err == nil && fi.IsDir() {
-				return vcs, dir[len(srcRoot)+1:], nil
+				return vcs, filepath.ToSlash(dir[len(srcRoot)+1:]), nil
 			}
 		}
 

src/cmd/go/vcs_test.go

@@ -6,6 +6,10 @@ package main
 
 import (
 	"internal/testenv"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
 	"testing"
 )
 
@@ -128,6 +132,37 @@ func TestRepoRootForImportPath(t *testing.T) {
 	}
 }
 
+// Test that vcsFromDir correctly inspects a given directory and returns the right VCS and root.
+func TestFromDir(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "vcstest")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	for _, vcs := range vcsList {
+		dir := filepath.Join(tempDir, "example.com", vcs.name, "."+vcs.cmd)
+		err := os.MkdirAll(dir, 0755)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		want := repoRoot{
+			vcs:  vcs,
+			root: path.Join("example.com", vcs.name),
+		}
+		var got repoRoot
+		got.vcs, got.root, err = vcsFromDir(dir, tempDir)
+		if err != nil {
+			t.Errorf("FromDir(%q, %q): %v", dir, tempDir, err)
+			continue
+		}
+		if got.vcs.name != want.vcs.name || got.root != want.root {
+			t.Errorf("FromDir(%q, %q) = VCS(%s) Root(%s), want VCS(%s) Root(%s)", dir, tempDir, got.vcs, got.root, want.vcs, want.root)
+		}
+	}
+}
+
 func TestIsSecure(t *testing.T) {
 	tests := []struct {
 		vcs    *vcsCmd