git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] commit: check committer identity more strictly

From
Jeff King <peff@peff.net>
Date
Jul 23, 2012, 18:50 UTC
Message-ID
<20120723185035.GB27588@sigill.intra.peff.net>
In-Reply-To
<20120723184634.GA31905@sigill.intra.peff.net>

The identity of the committer will ultimately be pulled from the ident code by commit_tree(). However, we make an attempt to check the author and committer identity early, before the user has done any manual work like inputting a commit message. That lets us abort without them having to worry about salvaging the work from .git/COMMIT_EDITMSG.

The early check for committer ident does not use the IDENT_STRICT flag, meaning that it would not find an empty name field. The motivation was presumably because we did not want to be too restrictive, as later calls might be more lax (for example, when we create the reflog entry, we do not care too much about a real name). However, because commit_tree will always get a strict identity to put in the commit object itself, there is no point in being lax only to die later (and in fact it is harmful, because the user will have wasted time typing their commit message).

Incidentally, this bug was masked prior to 060d4bb, as the initial loose call would taint the later strict call. So the commit would succeed (albeit with a bogus committer line in the commit object), and nobody noticed that our early check did not match the later one.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 95eeab1..20cef95 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -725,7 +725,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	strbuf_release(&sb);
 
 	/* This checks if committer ident is explicitly given */
-	strbuf_addstr(&committer_ident, git_committer_info(0));
+	strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
 	if (use_editor && include_status) {
 		char *ai_tmp, *ci_tmp;
 		if (whence != FROM_COMMIT)
-- 
1.7.10.5.40.g059818d
Previous: Jeff KingNext: Junio C Hamano
Message 6 of 16 in “Re: empty ident name trashes commit message”
  1. Ramana KumarJul 21, 2012
  2. Jeff KingJul 23, 2012
  3. Ramana KumarJul 23, 2012
  4. Jeff KingJul 23, 2012
  5. 1/3 advice: pass varargs to strbuf_vaddf, not strbuf_addfJeff King, Jul 23, 2012
  6. 2/3 commit: check committer identity more strictlyJeff King, Jul 23, 2012
  7. Junio C HamanoJul 23, 2012
  8. Jeff KingJul 23, 2012
  9. 3/3 commit: give a hint when a commit message has been abandonedJeff King, Jul 23, 2012
  10. Junio C HamanoJul 23, 2012
  11. Jeff KingJul 23, 2012
  12. Junio C HamanoJul 23, 2012
  13. Jeff KingJul 23, 2012
  14. Junio C HamanoJul 23, 2012
  15. Jeff KingJul 23, 2012
  16. Junio C HamanoJul 23, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.