path: root/builtin/commit.c
diff options
authorJeff King <>2014-12-10 15:42:10 (GMT)
committerJunio C Hamano <>2014-12-11 23:34:35 (GMT)
commitfac908389d4571ab5004872a54e50349272b63fc (patch)
tree314519fd3e5a7713d3ce3ccfffbf678d0861ad14 /builtin/commit.c
parent14ac2864dcd566a025e449ab9db6b5dc57744a32 (diff)
commit: loosen ident checks when generating template
When we generate the commit-message template, we try to report an author or committer ident that will be of interest to the user: an author that does not match the committer, or a committer that was auto-configured. When doing so, if we encounter what we consider to be a bogus ident, we immediately die. This is a bad idea, because our use of the idents here is purely informational. Any ident rules should be enforced elsewhere, because commits that do not invoke the editor will not even hit this code path (e.g., "git commit -mfoo" would work, but "git commit" would not). So at best, we are redundant with other checks, and at worse, we actively prevent commits that should otherwise be allowed. We should therefore do the minimal parsing we can to get a value and not do any validation (i.e., drop the call to sane_ident_split()). In theory we could notice when even our minimal parsing fails to work, and do the sane thing for each check (e.g., if we have an author but can't parse the committer, assume they are different and print the author). But we can actually simplify this even further. We know that the author and committer strings we are parsing have been generated by us earlier in the program, and therefore they must be parseable. We could just call split_ident_line without even checking its return value, knowing that it will put _something_ in the name/mail fields. Of course, to protect ourselves against future changes to the code, it makes sense to turn this into an assert, so we are not surprised if our assumption fails. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin/commit.c')
1 files changed, 14 insertions, 9 deletions
diff --git a/builtin/commit.c b/builtin/commit.c
index d1c90db..d551f20 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -502,6 +502,12 @@ static int is_a_merge(const struct commit *current_head)
return !!(current_head->parents && current_head->parents->next);
+static void assert_split_ident(struct ident_split *id, const struct strbuf *buf)
+ if (split_ident_line(id, buf->buf, buf->len))
+ die("BUG: unable to parse our own ident: %s", buf->buf);
static void export_one(const char *var, const char *s, const char *e, int hack)
struct strbuf buf = STRBUF_INIT;
@@ -608,13 +614,6 @@ static void determine_author_info(struct strbuf *author_ident)
-static void split_ident_or_die(struct ident_split *id, const struct strbuf *buf)
- if (split_ident_line(id, buf->buf, buf->len) ||
- !sane_ident_split(id))
- die(_("Malformed ident string: '%s'"), buf->buf);
static int author_date_is_interesting(void)
return author_message || force_date;
@@ -822,8 +821,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
status_printf_ln(s, GIT_COLOR_NORMAL,
"%s", only_include_assumed);
- split_ident_or_die(&ai, author_ident);
- split_ident_or_die(&ci, &committer_ident);
+ /*
+ * These should never fail because they come from our own
+ * fmt_ident. They may fail the sane_ident test, but we know
+ * that the name and mail pointers will at least be valid,
+ * which is enough for our tests and printing here.
+ */
+ assert_split_ident(&ai, author_ident);
+ assert_split_ident(&ci, &committer_ident);
if (ident_cmp(&ai, &ci))
status_printf_ln(s, GIT_COLOR_NORMAL,