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

2012-07-23 Thread Junio C Hamano 
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

2012-07-23 Thread Jeff King 
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