From 7744f6d7ac193720bb483dc2a2c266504b33d1a9 Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Mon, 25 Mar 2019 11:35:57 +1100 Subject: 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 --- git-codereview/hook.go | 31 +++++++++++++++++++++++-------- git-codereview/hook_test.go | 10 ++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) 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 @@ func hookCommitMsg(args []string) { // 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 @@ func hookCommitMsg(args []string) { } } +// 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 ". +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 @@ func TestHookCommitMsgGerrit(t *testing.T) { 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) { -- cgit v1.3