git-codereview: make tests pass on windows
- "git gofmt" uses "git checkout-index" command that is broken if
used with many arguments - limit number of arguments passed to
"git checkout-index";
- change / into \ on windows when matching git output on windows;
- add .gitattributes to test repository to make it look more like
all real Go repositories;
- make sure no files left behind in /tmp on windows.
Fixes golang/go#9433.
Change-Id: Ifceacfffd1cd4b02190d7e941611d0d0f3c48cdd
Reviewed-on: https://go-review.googlesource.com/2089
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index 4507a2b..93da314 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -145,18 +145,68 @@
// Git stores the temporary files, named .merge_*, in the repo root.
// Unlike the Git commands above, the non-temp file names printed
// here are relative to the current directory, not the repo root.
- args := []string{"checkout-index", "--temp", "--"}
- args = append(args, needTemp...)
- for _, line := range getLines("git", args...) {
- i := strings.Index(line, "\t")
- if i < 0 {
- continue
+
+ // git checkout-index --temp is broken on windows. Running this command:
+ //
+ // git checkout-index --temp -- bad-bad-bad2.go bad-bad-broken.go bad-bad-good.go bad-bad2-bad.go bad-bad2-broken.go bad-bad2-good.go bad-broken-bad.go bad-broken-bad2.go bad-broken-good.go bad-good-bad.go bad-good-bad2.go bad-good-broken.go bad2-bad-bad2.go bad2-bad-broken.go bad2-bad-good.go bad2-bad2-bad.go bad2-bad2-broken.go bad2-bad2-good.go bad2-broken-bad.go bad2-broken-bad2.go bad2-broken-good.go bad2-good-bad.go bad2-good-bad2.go bad2-good-broken.go broken-bad-bad2.go broken-bad-broken.go broken-bad-good.go broken-bad2-bad.go broken-bad2-broken.go broken-bad2-good.go
+ //
+ // produces this output
+ //
+ // .merge_file_a05448 bad-bad-bad2.go
+ // .merge_file_b05448 bad-bad-broken.go
+ // .merge_file_c05448 bad-bad-good.go
+ // .merge_file_d05448 bad-bad2-bad.go
+ // .merge_file_e05448 bad-bad2-broken.go
+ // .merge_file_f05448 bad-bad2-good.go
+ // .merge_file_g05448 bad-broken-bad.go
+ // .merge_file_h05448 bad-broken-bad2.go
+ // .merge_file_i05448 bad-broken-good.go
+ // .merge_file_j05448 bad-good-bad.go
+ // .merge_file_k05448 bad-good-bad2.go
+ // .merge_file_l05448 bad-good-broken.go
+ // .merge_file_m05448 bad2-bad-bad2.go
+ // .merge_file_n05448 bad2-bad-broken.go
+ // .merge_file_o05448 bad2-bad-good.go
+ // .merge_file_p05448 bad2-bad2-bad.go
+ // .merge_file_q05448 bad2-bad2-broken.go
+ // .merge_file_r05448 bad2-bad2-good.go
+ // .merge_file_s05448 bad2-broken-bad.go
+ // .merge_file_t05448 bad2-broken-bad2.go
+ // .merge_file_u05448 bad2-broken-good.go
+ // .merge_file_v05448 bad2-good-bad.go
+ // .merge_file_w05448 bad2-good-bad2.go
+ // .merge_file_x05448 bad2-good-broken.go
+ // .merge_file_y05448 broken-bad-bad2.go
+ // .merge_file_z05448 broken-bad-broken.go
+ // error: unable to create file .merge_file_XXXXXX (No error)
+ // .merge_file_XXXXXX broken-bad-good.go
+ // error: unable to create file .merge_file_XXXXXX (No error)
+ // .merge_file_XXXXXX broken-bad2-bad.go
+ // error: unable to create file .merge_file_XXXXXX (No error)
+ // .merge_file_XXXXXX broken-bad2-broken.go
+ // error: unable to create file .merge_file_XXXXXX (No error)
+ // .merge_file_XXXXXX broken-bad2-good.go
+ //
+ // so limit the number of file arguments to 25.
+ for len(needTemp) > 0 {
+ n := len(needTemp)
+ if n > 25 {
+ n = 25
}
- temp, file := line[:i], line[i+1:]
- temp = filepath.Join(repo, temp)
- file = filepath.Join(pwd, file)
- tempToFile[temp] = file
- fileToTemp[file] = temp
+ args := []string{"checkout-index", "--temp", "--"}
+ args = append(args, needTemp[:n]...)
+ for _, line := range getLines("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)
+ tempToFile[temp] = file
+ fileToTemp[file] = temp
+ }
+ needTemp = needTemp[n:]
}
cleanup = func() {
for temp := range tempToFile {
diff --git a/git-codereview/gofmt_test.go b/git-codereview/gofmt_test.go
index 94fd132..e96c717 100644
--- a/git-codereview/gofmt_test.go
+++ b/git-codereview/gofmt_test.go
@@ -40,10 +40,10 @@
trun(t, gt.client, "git", "add", ".") // make files tracked
testMain(t, "gofmt", "-l")
- testPrintedStdout(t, "bad.go\n", "!good.go", "!test/bad", "test/bench/bad.go")
+ testPrintedStdout(t, "bad.go\n", "!good.go", fromSlash("!test/bad"), fromSlash("test/bench/bad.go"))
testMain(t, "gofmt", "-l")
- testPrintedStdout(t, "bad.go\n", "!good.go", "!test/bad", "test/bench/bad.go")
+ testPrintedStdout(t, "bad.go\n", "!good.go", fromSlash("!test/bad"), fromSlash("test/bench/bad.go"))
testMain(t, "gofmt")
testNoStdout(t)
diff --git a/git-codereview/hook_test.go b/git-codereview/hook_test.go
index 15b04a0..9465b55 100644
--- a/git-codereview/hook_test.go
+++ b/git-codereview/hook_test.go
@@ -71,13 +71,13 @@
testMainDied(t, "hook-invoke", "pre-commit")
testPrintedStderr(t, "gofmt needs to format these files (run 'git gofmt'):",
- "bad.go", "!good.go", "!test/bad", "test/bench/bad.go")
+ "bad.go", "!good.go", fromSlash("!test/bad"), fromSlash("test/bench/bad.go"))
write(t, gt.client+"/broken.go", brokenGo)
trun(t, gt.client, "git", "add", "broken.go")
testMainDied(t, "hook-invoke", "pre-commit")
testPrintedStderr(t, "gofmt needs to format these files (run 'git gofmt'):",
- "bad.go", "!good.go", "!test/bad", "test/bench/bad.go",
+ "bad.go", "!good.go", fromSlash("!test/bad"), fromSlash("test/bench/bad.go"),
"gofmt reported errors:", "broken.go")
}
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 9b3e81f..d9933ad 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -26,9 +26,37 @@
client string // client repo root
}
+// resetReadOnlyFlagAll resets windows read-only flag
+// set on path and any children it contains.
+// The flag is set by git and has to be removed.
+// os.Remove refuses to remove files with read-only flag set.
+func resetReadOnlyFlagAll(path string) error {
+ fi, err := os.Stat(path)
+ if err != nil {
+ return err
+ }
+
+ if !fi.IsDir() {
+ return os.Chmod(path, 0666)
+ }
+
+ fd, err := os.Open(path)
+ if err != nil {
+ return err
+ }
+ defer fd.Close()
+
+ names, _ := fd.Readdirnames(-1)
+ for _, name := range names {
+ resetReadOnlyFlagAll(path + string(filepath.Separator) + name)
+ }
+ return nil
+}
+
func (gt *gitTest) done() {
+ os.Chdir(gt.pwd) // change out of gt.tmpdir first, otherwise following os.RemoveAll fails on windows
+ resetReadOnlyFlagAll(gt.tmpdir)
os.RemoveAll(gt.tmpdir)
- os.Chdir(gt.pwd)
}
func (gt *gitTest) work(t *testing.T) {
@@ -51,10 +79,11 @@
mkdir(t, server)
write(t, server+"/file", "this is master")
+ write(t, server+"/.gitattributes", "* -text\n")
trun(t, server, "git", "init", ".")
trun(t, server, "git", "config", "user.name", "gopher")
trun(t, server, "git", "config", "user.email", "gopher@example.com")
- trun(t, server, "git", "add", "file")
+ trun(t, server, "git", "add", "file", ".gitattributes")
trun(t, server, "git", "commit", "-m", "on master")
for _, name := range []string{"dev.branch", "release.branch"} {
@@ -128,6 +157,14 @@
return string(out)
}
+// fromSlash is like filepath.FromSlash, but it ignores ! at the start of the path.
+func fromSlash(path string) string {
+ if len(path) > 0 && path[0] == '!' {
+ return "!" + filepath.FromSlash(path[1:])
+ }
+ return filepath.FromSlash(path)
+}
+
var (
runLog []string
testStderr *bytes.Buffer
