git-codereview: make commit-msg hook play nicely with other systems
Gerrit requires 'Change-Id:' lines to be included in each commit
message, but they are not the only kind of 'metadata line' that might
appear in a commit message. Metadata lines observed by other systems
include 'Bug:' and 'Signed-off-by:'. This change ensures that the
commit-msg hook adds its 'Change-Id:' line to the set of metadata lines,
separated only by a single linefeed, rather than creating a new set by
inserting two line feeds.
Change-Id: Ia3bdce6f52f663685eea1e648874ef81ddb2bd91
Reviewed-on: https://go-review.googlesource.com/c/review/+/169097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/git-codereview/hook.go b/git-codereview/hook.go
index b80c567..09bed8b 100644
--- a/git-codereview/hook.go
+++ b/git-codereview/hook.go
@@ -189,15 +189,12 @@
// Add Change-Id to commit message if not present.
if nChangeId == 0 {
- n := len(data)
- for n > 0 && data[n-1] == '\n' {
- n--
+ data = bytes.TrimRight(data, "\n")
+ sep := "\n\n"
+ if endsWithMetadataLine(data) {
+ sep = "\n"
}
- var id [20]byte
- if _, err := io.ReadFull(rand.Reader, id[:]); err != nil {
- dief("generating Change-Id: %v", err)
- }
- data = append(data[:n], fmt.Sprintf("\n\nChange-Id: I%x\n", id[:])...)
+ data = append(data, fmt.Sprintf("%sChange-Id: I%x\n", sep, randomBytes())...)
}
// Add branch prefix to commit message if not present and on a
@@ -221,6 +218,24 @@
}
}
+// randomBytes returns 20 random bytes suitable for use in a Change-Id line.
+func randomBytes() []byte {
+ var id [20]byte
+ if _, err := io.ReadFull(rand.Reader, id[:]); err != nil {
+ dief("generating Change-Id: %v", err)
+ }
+ return id[:]
+}
+
+var metadataLineRE = regexp.MustCompile(`^[a-zA-Z0-9-]+: `)
+
+// endsWithMetadataLine reports whether the given commit message ends with a
+// metadata line such as "Bug: #42" or "Signed-off-by: Al <al@example.com>".
+func endsWithMetadataLine(msg []byte) bool {
+ i := bytes.LastIndexByte(msg, '\n')
+ return i >= 0 && metadataLineRE.Match(msg[i+1:])
+}
+
var (
fixupBang = []byte("fixup!")
squashBang = []byte("squash!")
diff --git a/git-codereview/hook_test.go b/git-codereview/hook_test.go
index 8cb04f9..d1cc0c6 100644
--- a/git-codereview/hook_test.go
+++ b/git-codereview/hook_test.go
@@ -42,6 +42,16 @@
if got := testStderr.String(); got != multiple {
t.Fatalf("unexpected output:\ngot: %q\nwant: %q", got, multiple)
}
+
+ // Check that hook doesn't add two line feeds before Change-Id
+ // if the exsting message ends with a metadata line.
+ write(t, gt.client+"/msg.txt", "Test message.\n\nBug: 1234\n")
+ testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
+ data = read(t, gt.client+"/msg.txt")
+ if !bytes.Contains(data, []byte("Bug: 1234\nChange-Id: ")) {
+ t.Fatalf("after hook-invoke commit-msg, missing Change-Id: directly after Bug line\n%s", data)
+ }
+
}
func TestHookCommitMsg(t *testing.T) {
