git-codereview: work around git checkout-index bug in gofmt

If you run 'git gofmt' when files in the working directory differ from
modified files in the index and you are running the command not
from that working directory or a parent, then git checkout-index --temp
(used by git gofmt) will print mangled path names, confusing git gofmt.
The most common error this causes is:

	git-codereview: gofmt reported errors:
		stat : no such file or directory

although there are other possible ways it can fail.

I've reported the git bug, which will be fixed in Git 2.3.0.
Work around the bug by always invoking git checkout-index in the
repo root.

Fixes #9476.

Change-Id: Ifcbdfd4a966dd21518dd95ceacd81a1679048e3c
Reviewed-on: https://go-review.googlesource.com/2907
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Andrew Gerrand <adg@golang.org>

3 files changed, 80 insertions, 2 deletions

diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index 937fbd6..96b5e86 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -203,14 +203,18 @@ func runGofmt(flags int) (files []string, stderrText string) {
 			}
 			args := []string{"checkout-index", "--temp", "--"}
 			args = append(args, needTemp[:n]...)
-			for _, line := range nonBlankLines(cmdOutput("git", args...)) {
+			// Until Git 2.3.0, git checkout-index --temp is broken if not run in the repo root.
+			// Work around by running in the repo root.
+			// http://article.gmane.org/gmane.comp.version-control.git/261739
+			// https://github.com/git/git/commit/74c4de5
+			for _, line := range nonBlankLines(cmdOutputDir(repo, "git", args...)) {
 				i := strings.Index(line, "\t")
 				if i < 0 {
 					continue
 				}
 				temp, file := line[:i], line[i+1:]
 				temp = filepath.Join(repo, temp)
-				file = filepath.Join(pwd, file)
+				file = filepath.Join(repo, file)
 				tempToFile[temp] = file
 				fileToTemp[file] = temp
 			}
diff --git a/git-codereview/gofmt_test.go b/git-codereview/gofmt_test.go
index 5cc262c..d720726 100644
--- a/git-codereview/gofmt_test.go
+++ b/git-codereview/gofmt_test.go
@@ -59,6 +59,74 @@ func TestGofmt(t *testing.T) {
 	testPrintedStderr(t, "gofmt reported errors", "broken.go")
 }
 
+func TestGofmtSubdir(t *testing.T) {
+	// Check that gofmt prints relative paths for files in or below the current directory.
+	gt := newGitTest(t)
+	defer gt.done()
+
+	gt.work(t)
+
+	mkdir(t, gt.client+"/dir1")
+	mkdir(t, gt.client+"/longnamedir2")
+	write(t, gt.client+"/dir1/bad1.go", badGo)
+	write(t, gt.client+"/longnamedir2/bad2.go", badGo)
+	trun(t, gt.client, "git", "add", ".") // make files tracked
+
+	chdir(t, gt.client)
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "dir1/bad1.go", "longnamedir2/bad2.go")
+
+	chdir(t, gt.client+"/dir1")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "bad1.go", "!/bad1.go", "longnamedir2/bad2.go")
+
+	chdir(t, gt.client+"/longnamedir2")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "bad2.go", "!/bad2.go", "dir1/bad1.go")
+
+	mkdir(t, gt.client+"/z")
+	chdir(t, gt.client+"/z")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "longnamedir2/bad2.go", "dir1/bad1.go")
+}
+
+func TestGofmtSubdirIndexCheckout(t *testing.T) {
+	// Like TestGofmtSubdir but bad Go files are only in index, not working copy.
+	// Check also that prints a correct path (relative or absolute) for files outside the
+	// current directory, even when running with Git before 2.3.0 which doesn't
+	// handle those right in git checkout-index --temp.
+
+	gt := newGitTest(t)
+	defer gt.done()
+
+	gt.work(t)
+
+	mkdir(t, gt.client+"/dir1")
+	mkdir(t, gt.client+"/longnamedir2")
+	write(t, gt.client+"/dir1/bad1.go", badGo)
+	write(t, gt.client+"/longnamedir2/bad2.go", badGo)
+	trun(t, gt.client, "git", "add", ".") // put files in index
+	write(t, gt.client+"/dir1/bad1.go", goodGo)
+	write(t, gt.client+"/longnamedir2/bad2.go", goodGo)
+
+	chdir(t, gt.client)
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "dir1/bad1.go (staged)", "longnamedir2/bad2.go (staged)")
+
+	chdir(t, gt.client+"/dir1")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "bad1.go (staged)", "!/bad1.go", "longnamedir2/bad2.go (staged)")
+
+	chdir(t, gt.client+"/longnamedir2")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "bad2.go (staged)", "!/bad2.go", "dir1/bad1.go (staged)")
+
+	mkdir(t, gt.client+"/z")
+	chdir(t, gt.client+"/z")
+	testMain(t, "gofmt", "-l")
+	testPrintedStdout(t, "longnamedir2/bad2.go (staged)", "dir1/bad1.go (staged)")
+}
+
 func TestGofmtUnstaged(t *testing.T) {
 	// Test when unstaged files are different from staged ones.
 	// See TestHookPreCommitUnstaged for an explanation.
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 42be911..96716b4 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -150,6 +150,12 @@ func mkdir(t *testing.T, dir string) {
 	}
 }
 
+func chdir(t *testing.T, dir string) {
+	if err := os.Chdir(dir); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func write(t *testing.T, file, data string) {
 	if err := ioutil.WriteFile(file, []byte(data), 0666); err != nil {
 		t.Fatal(err)