From 5804fef4c0556d3e5e7d2440740a3d7aced7d58b Mon Sep 17 00:00:00 2001
From: Dmitri Shuralyov <shurcooL@gmail.com>
Date: Fri, 8 Jan 2016 19:44:58 -0800
Subject: [PATCH] x/tools/go/vcs: fix FromDir returning bad root on Windows

Import path is a '/'-separated path. FromDir 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 root value returned from FromDir, it was
previously not tested.

Additionally, remove a dubious statement from the documentation
"(thus root is a prefix of importPath)". There is no variable
importPath that is being referred to. It's also redundant and
confusing. 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

Fixes golang/go#7723.

Change-Id: If9f5f55b5751e01a7f88b79d9b039402af3e9312
Reviewed-on: https://go-review.googlesource.com/18461
Reviewed-by: Chris Manghane <cmang@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
---
 go/vcs/vcs.go      |  5 ++---
 go/vcs/vcs_test.go | 28 +++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/go/vcs/vcs.go b/go/vcs/vcs.go
index a7024966..8b45cd88 100644
--- a/go/vcs/vcs.go
+++ b/go/vcs/vcs.go
@@ -335,8 +335,7 @@ type vcsPath struct {
 // FromDir 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).
+// corresponding to the root of the repository.
 func FromDir(dir, srcRoot string) (vcs *Cmd, root string, err error) {
 	// Clean and double-check that dir is in (a subdirectory of) srcRoot.
 	dir = filepath.Clean(dir)
@@ -348,7 +347,7 @@ func FromDir(dir, srcRoot string) (vcs *Cmd, 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
 			}
 		}
 
diff --git a/go/vcs/vcs_test.go b/go/vcs/vcs_test.go
index d77f20b0..1ff519a1 100644
--- a/go/vcs/vcs_test.go
+++ b/go/vcs/vcs_test.go
@@ -7,6 +7,7 @@ package vcs
 import (
 	"io/ioutil"
 	"os"
+	pathpkg "path"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -61,11 +62,11 @@ func TestRepoRootForImportPath(t *testing.T) {
 	}
 }
 
-// Test that FromDir correctly inspects a given directory and returns the right VCS.
+// Test that FromDir correctly inspects a given directory and returns the right VCS and root.
 func TestFromDir(t *testing.T) {
 	type testStruct struct {
 		path string
-		want *Cmd
+		want *RepoRoot
 	}
 
 	tests := make([]testStruct, len(vcsList))
@@ -77,16 +78,29 @@ func TestFromDir(t *testing.T) {
 
 	for i, vcs := range vcsList {
 		tests[i] = testStruct{
-			filepath.Join(tempDir, vcs.Name, "."+vcs.Cmd),
-			vcs,
+			path: filepath.Join(tempDir, "example.com", vcs.Name, "."+vcs.Cmd),
+			want: &RepoRoot{
+				VCS:  vcs,
+				Root: pathpkg.Join("example.com", vcs.Name),
+			},
 		}
 	}
 
 	for _, test := range tests {
 		os.MkdirAll(test.path, 0755)
-		got, _, _ := FromDir(test.path, tempDir)
-		if got.Name != test.want.Name {
-			t.Errorf("FromDir(%q, %q) = %s, want %s", test.path, tempDir, got, test.want)
+		var (
+			got = new(RepoRoot)
+			err error
+		)
+		got.VCS, got.Root, err = FromDir(test.path, tempDir)
+		if err != nil {
+			t.Errorf("FromDir(%q, %q): %v", test.path, tempDir, err)
+			os.RemoveAll(test.path)
+			continue
+		}
+		want := test.want
+		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)", test.path, tempDir, got.VCS, got.Root, want.VCS, want.Root)
 		}
 		os.RemoveAll(test.path)
 	}