git-codereview: reject commits with multiple Change-Id lines
This should help prevent the situation in CL 8653.
Change-Id: I03e4868656a86e69e11f3e0545feb1681fb0f628
Reviewed-on: https://go-review.googlesource.com/8671
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
diff --git a/git-codereview/hook.go b/git-codereview/hook.go
index 3d4ff9c..18994c8 100644
--- a/git-codereview/hook.go
+++ b/git-codereview/hook.go
@@ -153,9 +153,17 @@
data = issueRefRE.ReplaceAll(data, []byte("${space}"+issueRepo+"${ref}"))
}
+ // Complain if two Change-Ids are present.
+ // This can happen during an interactive rebase;
+ // it is easy to forget to remove one of them.
+ nChangeId := bytes.Count(data, []byte("\nChange-Id: "))
+ if nChangeId > 1 {
+ dief("multiple Change-Id lines")
+ }
+
// Add Change-Id to commit message if not present.
edited := false
- if !bytes.Contains(data, []byte("\nChange-Id: ")) {
+ if nChangeId == 0 {
edited = true
n := len(data)
for n > 0 && data[n-1] == '\n' {
diff --git a/git-codereview/hook_test.go b/git-codereview/hook_test.go
index 34772d0..b3686a4 100644
--- a/git-codereview/hook_test.go
+++ b/git-codereview/hook_test.go
@@ -23,30 +23,32 @@
// Check that hook adds Change-Id.
write(t, gt.client+"/msg.txt", "Test message.\n")
testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
- data, err := ioutil.ReadFile(gt.client + "/msg.txt")
- if err != nil {
- t.Fatal(err)
- }
+ data := read(t, gt.client+"/msg.txt")
if !bytes.Contains(data, []byte("\n\nChange-Id: ")) {
t.Fatalf("after hook-invoke commit-msg, missing Change-Id:\n%s", data)
}
// Check that hook is no-op when Change-Id is already present.
testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
- data1, err := ioutil.ReadFile(gt.client + "/msg.txt")
- if err != nil {
- t.Fatal(err)
- }
+ data1 := read(t, gt.client+"/msg.txt")
if !bytes.Equal(data, data1) {
t.Fatalf("second hook-invoke commit-msg changed Change-Id:\nbefore:\n%s\n\nafter:\n%s", data, data1)
}
+ // Check that hook rejects multiple Change-Ids.
+ write(t, gt.client+"/msgdouble.txt", string(data)+string(data))
+ testMainDied(t, "hook-invoke", "commit-msg", gt.client+"/msgdouble.txt")
+ const multiple = "git-codereview: multiple Change-Id lines\n"
+ if got := testStderr.String(); got != multiple {
+ t.Fatalf("unexpected output:\ngot: %q\nwant: %q", got, multiple)
+ }
+
// Check that hook fails when message is empty.
write(t, gt.client+"/empty.txt", "\n\n# just a file with\n# comments\n")
testMainDied(t, "hook-invoke", "commit-msg", gt.client+"/empty.txt")
- const want = "git-codereview: empty commit message\n"
- if got := testStderr.String(); got != want {
- t.Fatalf("unexpected output:\ngot: %q\nwant: %q", got, want)
+ const empty = "git-codereview: empty commit message\n"
+ if got := testStderr.String(); got != empty {
+ t.Fatalf("unexpected output:\ngot: %q\nwant: %q", got, empty)
}
// Check that hook inserts a blank line after the first line as needed.
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 939590f..034d052 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -184,6 +184,14 @@
}
}
+func read(t *testing.T, file string) []byte {
+ b, err := ioutil.ReadFile(file)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return b
+}
+
func remove(t *testing.T, file string) {
if err := os.RemoveAll(file); err != nil {
t.Fatal(err)
