git-codereview: do not gofmt committed files during merge
Change-Id: Ie772b8f46587ad0d56350b697e75601f45b4c393
Reviewed-on: https://go-review.googlesource.com/2785
Reviewed-by: Andrew Gerrand <adg@golang.org>
diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index f9644a0..28edf5d 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -119,7 +119,15 @@
}
// Find files modified in the index compared to the branchpoint.
- indexFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM", "--cached", b.Branchpoint(), "--")))
+ branchpt := b.Branchpoint()
+ if strings.Contains(getOutput("git", "branch", "-r", "--contains", b.Name), "origin/") {
+ // This is a branch tag move, not an actual change.
+ // Use HEAD as branch point, so nothing will appear changed.
+ // We don't want to think about gofmt on already published
+ // commits.
+ branchpt = "HEAD"
+ }
+ indexFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM", "--cached", branchpt, "--")))
localFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM")))
localFilesMap := stringMap(localFiles)
isUnstaged := func(file string) bool {
diff --git a/git-codereview/gofmt_test.go b/git-codereview/gofmt_test.go
index e96c717..62184ab 100644
--- a/git-codereview/gofmt_test.go
+++ b/git-codereview/gofmt_test.go
@@ -179,3 +179,42 @@
testMain(t, "gofmt")
}
+
+func TestGofmtFastForwardMerge(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ // merge dev.branch into master
+ write(t, gt.server+"/file", "more work")
+ trun(t, gt.server, "git", "commit", "-m", "work", "file")
+ trun(t, gt.server, "git", "merge", "-m", "merge", "dev.branch")
+
+ // add bad go file on master
+ write(t, gt.server+"/bad.go", "package {\n")
+ trun(t, gt.server, "git", "add", "bad.go")
+ trun(t, gt.server, "git", "commit", "-m", "bad go")
+
+ // update client
+ trun(t, gt.client, "git", "checkout", "master")
+ trun(t, gt.client, "git", "pull")
+ trun(t, gt.client, "git", "change", "dev.branch")
+ trun(t, gt.client, "git", "pull")
+
+ // merge master into dev.branch, fast forward merge
+ trun(t, gt.client, "git", "merge", "--ff-only", "master")
+
+ // verify that now client is in a state where just the tag is changing; there's no new commit.
+ masterHash := strings.TrimSpace(trun(t, gt.server, "git", "rev-parse", "master"))
+ devHash := strings.TrimSpace(trun(t, gt.client, "git", "rev-parse", "HEAD"))
+
+ if masterHash != devHash {
+ t.Logf("branches:\n%s", trun(t, gt.client, "git", "branch", "-a", "-v"))
+ t.Logf("log:\n%s", trun(t, gt.client, "git", "log", "--graph", "--decorate"))
+ t.Fatalf("setup wrong - got different commit hashes on master and dev branch")
+ }
+
+ // check that gofmt finds nothing to do, ignoring the bad (but committed) file1.go.
+ testMain(t, "gofmt")
+ testNoStdout(t)
+ testNoStderr(t)
+}
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index a5f1418..76a835d 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -92,10 +92,12 @@
for _, name := range []string{"dev.branch", "release.branch"} {
trun(t, server, "git", "checkout", "master")
- trun(t, server, "git", "branch", name)
- write(t, server+"/file", "this is "+name)
- trun(t, server, "git", "commit", "-a", "-m", "on "+name)
+ trun(t, server, "git", "checkout", "-b", name)
+ write(t, server+"/file."+name, "this is "+name)
+ trun(t, server, "git", "add", "file."+name)
+ trun(t, server, "git", "commit", "-m", "on "+name)
}
+ trun(t, server, "git", "checkout", "master")
client := tmpdir + "/git-client"
mkdir(t, client)
