git-codereview: reject empty commits in mail

Fixes golang/go#10753.

Change-Id: I224a411177b3d198576a06bc69da9894b7b67c34
Reviewed-on: https://go-review.googlesource.com/19799
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

4 files changed, 37 insertions, 1 deletions

diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index 5ba9d35..8cf1566 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -355,6 +355,11 @@ func (b *Branch) DefaultCommit(action string) *Commit {
 	return work[0]
 }
 
+// ListFiles returns the list of files in a given commit.
+func ListFiles(c *Commit) []string {
+	return nonBlankLines(cmdOutput("git", "diff", "--name-only", c.Parent, c.Hash, "--"))
+}
+
 func cmdBranchpoint(args []string) {
 	expectZeroArgs(args, "sync")
 	fmt.Fprintf(stdout(), "%s\n", CurrentBranch().Branchpoint())
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index c83a5d8..4607bc1 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -47,6 +47,10 @@ func cmdMail(args []string) {
 		return
 	}
 
+	if len(ListFiles(c)) == 0 {
+		dief("cannot mail: commit %s is empty", c.ShortHash)
+	}
+
 	if !*force && HasStagedChanges() {
 		dief("there are staged changes; aborting.\n"+
 			"Use '%s change' to include them or '%s mail -f' to force it.", os.Args[0], os.Args[0])
diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go
index fa006db..c24370f 100644
--- a/git-codereview/mail_test.go
+++ b/git-codereview/mail_test.go
@@ -162,3 +162,30 @@ func TestMailTopic(t *testing.T) {
 		"git push -q origin HEAD:refs/for/master%topic=test-topic",
 		"git tag -f work.mailed "+h)
 }
+
+func TestMailEmpty(t *testing.T) {
+	gt := newGitTest(t)
+	defer gt.done()
+
+	// fake auth information to avoid Gerrit error
+	auth.host = "gerrit.fake"
+	auth.user = "not-a-user"
+	defer func() {
+		auth.host = ""
+		auth.user = ""
+	}()
+
+	testMain(t, "change", "work")
+	testRan(t, "git checkout -q -b work",
+		"git branch -q --set-upstream-to origin/master")
+
+	t.Logf("creating empty change")
+	testCommitMsg = "foo: this commit will be empty"
+	testMain(t, "change")
+	testRan(t, "git commit -q --allow-empty -m foo: this commit will be empty")
+
+	h := CurrentBranch().Pending()[0].ShortHash
+
+	testMainDied(t, "mail")
+	testPrintedStderr(t, "cannot mail: commit "+h+" is empty")
+}
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index a3b4db8..80c90d8 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -43,7 +43,7 @@ func (b *pendingBranch) load() {
 		b.staged, b.unstaged, b.untracked = LocalChanges()
 	}
 	for _, c := range b.Pending() {
-		c.committed = nonBlankLines(cmdOutput("git", "diff", "--name-only", c.Parent, c.Hash, "--"))
+		c.committed = ListFiles(c)
 		if !pendingLocal {
 			c.g, c.gerr = b.GerritChange(c, "DETAILED_LABELS", "CURRENT_REVISION", "MESSAGES", "DETAILED_ACCOUNTS")
 		}