[PATCH 2/3] commit: check committer identity more strictly
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] commit: check committer identity more strictly
Jeff King p...@peff.net writes:
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 p...@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)
Looks sensible. Is this something we can detect in automated tests,
or is it too cumbersome to set up?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] commit: check committer identity more strictly
On Mon, Jul 23, 2012 at 01:51:25PM -0700, Junio C Hamano wrote:
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)
Looks sensible. Is this something we can detect in automated tests,
or is it too cumbersome to set up?
Sorry, I meant to mention that in the cover letter. No, we can't test
this easily, because the code path in question is triggered by finding a
blank name in /etc/passwd. We'd have to override our getpwent lookup.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
