{"thread":{"id":"11097","subject":"Corrupted (?) commit 6e6db85e confusing gitk","startedAt":"2007-12-02T16:06:07Z","lastAt":"2007-12-02T23:05:04Z","messageCount":18,"participants":["Steffen Prohaska","Wincent Colaiuta","Junio C Hamano","Brian Downing","Linus Torvalds","Johannes Schindelin","Michael Gebetsroither"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"61649","messageId":"5F1A20CC-7427-4E7A-AB95-E89C9FA17951@zib.de","threadId":"11097","inReplyTo":null,"subject":"Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-12-02T16:06:07Z","receivedAt":"2007-12-02T16:06:07Z","isPatch":false,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"When I run\n\n    gitk 6e6db85ea9423eea755cf5acf7a563c0d9559063\n\ngitk complaints with 'Error: expected integer but got \"Hamano\"'.\n\nI tracked the problem down to the raw content of the commit object.\nThe author line is lacking time and timezone information:\n\n     $ git-cat-file -p 6e6db85ea9423eea755cf5acf7a563c0d9559063\n     tree 5265f13d094e7c453a06f097add25eaefb843a79\n     parent d25430c5f88c7e7b4ce24c1b08e409f4345c4eb9\n     author Junio C Hamano <gitster@pobox.com>\n     committer Junio C Hamano <gitster@pobox.com> 1196466497 -0800\n\n     Run the specified perl in Documentation/\n\n     Makefile uses $(PERL_PATH) but Documentation/Makefile uses \"perl\";\n     that means the two Makefiles actually use two different\n     Perl installations.\n\n     Teach Documentation/Makefile to use PERL_PATH that is exported  \nfrom the\n     toplevel Makefile, and give a sane fallback for people who run  \n\"make\"\n     from Documentation directory.\n\n     Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\ngitk fails to parse this because it expects the time to be\nthe second item from the end of a line (look for \"set audate\"\nin function parsecommit of gitk).  For the commit above, gitk\nfinds \"Hamano\" instead of the correct time.\n\nI'm pretty convinced that the original commit is reported\ncorrectly.  I verified that with two different versions of git\n(1.5.3.7.949.g2221a6 on mac and 1.5.3.6.1889.g98603 on mingw).\nBoth report the raw commit without time and timezone.\n\nI'd like to conclude with some questions:\n  - Is this commit corrupted?\n  - How was the commit created?\n  - Should \"git fsck\" detect such corruption?\n  - Should gitk more gracefully handle corrupted commits?\n\nI do not have solutions yet.\n\n\tSteffen\n"},{"id":"61650","messageId":"C1CE0786-FB89-4E5D-9FEA-1F2FFFD47D12@wincent.com","threadId":"11097","inReplyTo":"5F1A20CC-7427-4E7A-AB95-E89C9FA17951@zib.de","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-02T16:12:52Z","receivedAt":"2007-12-02T16:12:52Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 2/12/2007, a las 17:06, Steffen Prohaska escribió:\n\n> When I run\n>\n>   gitk 6e6db85ea9423eea755cf5acf7a563c0d9559063\n>\n> gitk complaints with 'Error: expected integer but got \"Hamano\"'.\n>\n> I tracked the problem down to the raw content of the commit object.\n> The author line is lacking time and timezone information:\n>\n>    $ git-cat-file -p 6e6db85ea9423eea755cf5acf7a563c0d9559063\n>    tree 5265f13d094e7c453a06f097add25eaefb843a79\n>    parent d25430c5f88c7e7b4ce24c1b08e409f4345c4eb9\n>    author Junio C Hamano <gitster@pobox.com>\n>    committer Junio C Hamano <gitster@pobox.com> 1196466497 -0800\n\nYeah, and:\n\n$ git show 6e6db85ea9423eea755cf5acf7a563c0d9559063 | head -3\ncommit 6e6db85ea9423eea755cf5acf7a563c0d9559063\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Thu Jan 1 00:00:00 1970 +0000\n\nCheers,\nWincent\n"},{"id":"61652","messageId":"1196613383337-git-send-email-prohaska@zib.de","threadId":"11097","inReplyTo":"5F1A20CC-7427-4E7A-AB95-E89C9FA17951@zib.de","subject":"[PATCH] gitk: Add workaround to handle corrupted author date","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-12-02T16:36:23Z","receivedAt":"2007-12-02T16:36:23Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"6e6db85ea9423eea755cf5acf7a563c0d9559063 contains a corrupted\nauthor line, which is lacking the time and timezone information.\n\nThis commit adds a workaround to handle this situation.  If the\ntime cannot be parsed, it is assumed to be 0 and the full line\nis assumed to be the author's name.\n---\n gitk |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\nThis works around the issue for me.  However, I don't think\nthis patch should be applied.\n\nThe best was if such a corrupted commit wouldn't enter the\nrepository in the first place.  But once it is there, I think git\nshould verify the format of a commit and report an approriate\nerror.  gitk could continue to assume well formed commits.\n\n    Steffen\n\ndiff --git a/gitk b/gitk\nindex 1da0b0a..873766c 100755\n--- a/gitk\n+++ b/gitk\n@@ -439,7 +439,12 @@ proc parsecommit {id contents listed} {\n \tset tag [lindex $line 0]\n \tif {$tag == \"author\"} {\n \t    set audate [lindex $line end-1]\n-\t    set auname [lrange $line 1 end-2]\n+\t    if {[catch {formatdate $audate}]} {\n+\t\tset audate 0\n+\t\tset auname [lrange $line 1 end]\n+\t    } else {\n+\t\tset auname [lrange $line 1 end-2]\n+\t    }\n \t} elseif {$tag == \"committer\"} {\n \t    set comdate [lindex $line end-1]\n \t    set comname [lrange $line 1 end-2]\n-- \n1.5.3.7.949.g2221a6\n"},{"id":"61670","messageId":"7vir3hx70y.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"5F1A20CC-7427-4E7A-AB95-E89C9FA17951@zib.de","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T18:53:33Z","receivedAt":"2007-12-02T18:53:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska <prohaska@zib.de> writes:\n\n> I'd like to conclude with some questions:\n>  - Is this commit corrupted?\n>  - How was the commit created?\n>  - Should \"git fsck\" detect such corruption?\n>  - Should gitk more gracefully handle corrupted commits?\n\nYeah, I was wondering what that commit that records the change older\nthan git or myself come to life ;-)\n\nI did rewrite the commit a few times, and it was some interaction\nbetween the built-in commit series, git-rebase -i and git-am, but I do\nnot have the details, sorry.\n"},{"id":"61675","messageId":"20071202193918.GQ6212@lavos.net","threadId":"11097","inReplyTo":"7vir3hx70y.fsf@gitster.siamese.dyndns.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2007-12-02T19:39:18Z","receivedAt":"2007-12-02T19:39:18Z","isPatch":false,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Sun, Dec 02, 2007 at 10:53:33AM -0800, Junio C Hamano wrote:\n> Yeah, I was wondering what that commit that records the change older\n> than git or myself come to life ;-)\n> \n> I did rewrite the commit a few times, and it was some interaction\n> between the built-in commit series, git-rebase -i and git-am, but I do\n> not have the details, sorry.\n\nIt looks like the \"guilty\" commit that allowed this behavior was:\n\ncommit 13208572fbe8838fd8835548d7502202d1f7b21d\nAuthor: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nDate:   Sun Nov 11 17:35:58 2007 +0000\n\n    builtin-commit: fix --signoff\n\n    The Signed-off-by: line contained a spurious timestamp.  The reason was\n    a call to git_committer_info(1), which automatically added the\n    timestamp.\n\n    Instead, fmt_ident() was taught to interpret an empty string for the\n    date (as opposed to NULL, which still triggers the default behavior)\n    as \"do not bother with the timestamp\", and builtin-commit.c uses it.\n\n    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nWith the above, something like:\n\necho msg | GIT_AUTHOR_DATE='' git commit-tree sha1\n\nwill produce a broken commit without a timestamp, since fmt_ident is\nalso used for the committer and author lines.\n\nPersonally, I think if the date_str is not NULL, it should die() on\nanything that can't successfully be parsed as a date, rather than simply\nfalling back to the current time.  But maybe that's a bit extreme.\n\n-bcd\n"},{"id":"61677","messageId":"alpine.LFD.0.9999.0712021229300.8458@woody.linux-foundation.org","threadId":"11097","inReplyTo":"20071202193918.GQ6212@lavos.net","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-02T20:34:57Z","receivedAt":"2007-12-02T20:34:57Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 2 Dec 2007, Brian Downing wrote:\n>\n> With the above, something like:\n> \n> echo msg | GIT_AUTHOR_DATE='' git commit-tree sha1\n> \n> will produce a broken commit without a timestamp, since fmt_ident is\n> also used for the committer and author lines.\n\nOuch. And I notice that fsck doesn't even warn about the resulting broken \ncommit. Partly because I was lazy, but partly because originally I was \nthinking that maybe we'll have more header lines, so fsck basically just \nchecks the ones that git *really* cares about (parenthood and tree), and \nthe rest is not really even looked at (well, it does check that the next \nline starts with \"author\", but that's it).\n\nI guess the breakage is pretty benign, but this is still very wrong.\n\nJunio: that broken commit seems to be in \"pu\" only - we should make sure \nthat it never makes it into next or master, so that it will eventually get \npruned out of history.\n\n\t\tLinus\n"},{"id":"61678","messageId":"7vmyssvn55.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"20071202193918.GQ6212@lavos.net","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T20:48:22Z","receivedAt":"2007-12-02T20:48:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"bdowning@lavos.net (Brian Downing) writes:\n\n> It looks like the \"guilty\" commit that allowed this behavior was:\n>\n> commit 13208572fbe8838fd8835548d7502202d1f7b21d\n> Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Date:   Sun Nov 11 17:35:58 2007 +0000\n>\n>     builtin-commit: fix --signoff\n>\n>     The Signed-off-by: line contained a spurious timestamp.  The reason was\n>     a call to git_committer_info(1), which automatically added the\n>     timestamp.\n>\n>     Instead, fmt_ident() was taught to interpret an empty string for the\n>     date (as opposed to NULL, which still triggers the default behavior)\n>     as \"do not bother with the timestamp\", and builtin-commit.c uses it.\n>\n>     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>     Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>\n> With the above, something like:\n>\n> echo msg | GIT_AUTHOR_DATE='' git commit-tree sha1\n>\n> will produce a broken commit without a timestamp, since fmt_ident is\n> also used for the committer and author lines.\n>\n> Personally, I think if the date_str is not NULL, it should die() on\n> anything that can't successfully be parsed as a date, rather than simply\n> falling back to the current time.  But maybe that's a bit extreme.\n\nYeah, that change does look like a hack now we look at it again.  It\nwould have been much cleaner to make the caller accept the default\nbehaviour of fmt_ident() and strip out the part it does not want from\nthe result.  That way, the damage would have been much contained.\n\nThe next issue would be to find who could pass an empty GIT_AUTHOR_DATE\nwithout noticing...\n"},{"id":"61681","messageId":"alpine.LFD.0.9999.0712021322580.8458@woody.linux-foundation.org","threadId":"11097","inReplyTo":"7vmyssvn55.fsf@gitster.siamese.dyndns.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-02T21:25:12Z","receivedAt":"2007-12-02T21:25:12Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 2 Dec 2007, Junio C Hamano wrote:\n>\n> The next issue would be to find who could pass an empty GIT_AUTHOR_DATE\n> without noticing...\n\nIn the meantime, here's a not-very-well-tested patch to fsck to at least \nnotice this.\n\nOf course, in the name of containment it would probably be even better if \nparse_commit() did it, because then people would be unable to pull from \nsuch a corrupt repository! But this would seem to be at least a slight \nstep in the right direction.\n\n\t\tLinus\n\n---\n builtin-fsck.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex e4874f6..309212c 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -351,8 +351,48 @@ static int fsck_tree(struct tree *item)\n \treturn retval;\n }\n \n+static int parse_commit_line(struct commit *commit, const char *expect, const char *buffer)\n+{\n+\tchar *end;\n+\tconst char *p;\n+\tint len = strlen(expect);\n+\tint saw_lt = 0;\n+\n+\tif (memcmp(buffer, expect, len))\n+\t\tgoto bad;\n+\tp = (char *)buffer + len;\n+\tif (*p != ' ')\n+\t\tgoto bad;\n+\twhile (*++p != '>') {\n+\t\tif (*p == '<')\n+\t\t\tsaw_lt++;\n+\t\tif (!*p)\n+\t\t\tgoto bad;\n+\t}\n+\tif (saw_lt != 1)\n+\t\tgoto bad;\n+\tif (*++p != ' ')\n+\t\tgoto bad;\n+\n+\t/* Date in seconds since the epoch (UTC) */\n+\tif (strtoul(p, &end, 10) == ULONG_MAX)\n+\t\tgoto bad;\n+\tif (*end++ != ' ')\n+\t\tgoto bad;\n+\n+\t/* TZ that date was done in */\n+\tif (strtoul(end, &end, 10) == ULONG_MAX)\n+\t\tgoto bad;\n+\tif (*end++ != '\\n')\n+\t\tgoto bad;\n+\treturn end - buffer;\n+bad:\n+\treturn objerror(&commit->object, \"invalid format - missing or corrupt '%s'\", expect);\n+}\n+\n static int fsck_commit(struct commit *commit)\n {\n+\tint len;\n \tchar *buffer = commit->buffer;\n \tunsigned char tree_sha1[20], sha1[20];\n \n@@ -370,8 +410,21 @@ static int fsck_commit(struct commit *commit)\n \t\t\treturn objerror(&commit->object, \"invalid 'parent' line format - bad sha1\");\n \t\tbuffer += 48;\n \t}\n-\tif (memcmp(buffer, \"author \", 7))\n-\t\treturn objerror(&commit->object, \"invalid format - expected 'author' line\");\n+\n+\t/*\n+\t * We check the author/committer lines for completeness.\n+\t * But errors here aren't fatal to the rest of the parsing.\n+\t */\n+\tlen = parse_commit_line(commit, \"author\", buffer);\n+\tif (len >= 0) {\n+\t\tbuffer += len;\n+\t\tlen = parse_commit_line(commit, \"committer\", buffer);\n+\t\tif (len >= 0) {\n+\t\t\tbuffer += len;\n+\t\t\tif (*buffer != '\\n')\n+\t\t\t\tobjerror(&commit->object, \"invalid format - missing or corrupt end-of-headers\");\n+\t\t}\n+\t}\n \tfree(commit->buffer);\n \tcommit->buffer = NULL;\n \tif (!commit->tree)\n"},{"id":"61682","messageId":"Pine.LNX.4.64.0712022132060.27959@racer.site","threadId":"11097","inReplyTo":"20071202193918.GQ6212@lavos.net","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-02T21:34:06Z","receivedAt":"2007-12-02T21:34:06Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Brian Downing wrote:\n\n> On Sun, Dec 02, 2007 at 10:53:33AM -0800, Junio C Hamano wrote:\n> > Yeah, I was wondering what that commit that records the change older\n> > than git or myself come to life ;-)\n> > \n> > I did rewrite the commit a few times, and it was some interaction\n> > between the built-in commit series, git-rebase -i and git-am, but I do\n> > not have the details, sorry.\n> \n> It looks like the \"guilty\" commit that allowed this behavior was:\n> \n> commit 13208572fbe8838fd8835548d7502202d1f7b21d\n> Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Date:   Sun Nov 11 17:35:58 2007 +0000\n> \n>     builtin-commit: fix --signoff\n> \n>     The Signed-off-by: line contained a spurious timestamp.  The reason was\n>     a call to git_committer_info(1), which automatically added the\n>     timestamp.\n> \n>     Instead, fmt_ident() was taught to interpret an empty string for the\n>     date (as opposed to NULL, which still triggers the default behavior)\n>     as \"do not bother with the timestamp\", and builtin-commit.c uses it.\n> \n>     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>     Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> \n> With the above, something like:\n> \n> echo msg | GIT_AUTHOR_DATE='' git commit-tree sha1\n\nDarn.  But when can \"GIT_AUTHOR_DATE\" be set to the empty string?  I mean, \nI understand unset'ing it.  But setting it to \"\"?\n\nCiao,\nDscho\n"},{"id":"61683","messageId":"7v63zgvkl5.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"7vmyssvn55.fsf@gitster.siamese.dyndns.org","subject":"Fix --signoff in builtin-commit differently.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T21:43:34Z","receivedAt":"2007-12-02T21:43:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Introduce fmt_name() specifically meant for formatting the name and\nemail pair, to add signed-off-by value.  This reverts parts of\n13208572fbe8838fd8835548d7502202d1f7b21d (builtin-commit: fix --signoff)\nso that an empty datestamp string given to fmt_ident() by mistake will\nerror out as before.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n Junio C Hamano <gitster@pobox.com> writes:\n\n >> Personally, I think if the date_str is not NULL, it should die() on\n >> anything that can't successfully be parsed as a date, rather than simply\n >> falling back to the current time.  But maybe that's a bit extreme.\n >\n > Yeah, that change does look like a hack now we look at it again.  It\n > would have been much cleaner to make the caller accept the default\n > behaviour of fmt_ident() and strip out the part it does not want from\n > the result.  That way, the damage would have been much contained.\n >\n > The next issue would be to find who could pass an empty GIT_AUTHOR_DATE\n > without noticing...\n\n Perhaps like this...\n\n builtin-commit.c |    6 ++----\n cache.h          |    1 +\n ident.c          |   34 ++++++++++++++++++++++++----------\n 3 files changed, 27 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 96cb544..2319cc1 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -346,11 +346,9 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \n \t\tstrbuf_init(&sob, 0);\n \t\tstrbuf_addstr(&sob, sign_off_header);\n-\t\tstrbuf_addstr(&sob, fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t\t\t      getenv(\"GIT_COMMITTER_EMAIL\"),\n-\t\t\t\t\t      \"\", 1));\n+\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n \t\tstrbuf_addch(&sob, '\\n');\n-\n \t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n \t\t\t; /* do nothing */\n \t\tif (prefixcmp(sb.buf + i, sob.buf)) {\ndiff --git a/cache.h b/cache.h\nindex cf0bdc6..43cfebb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -444,6 +444,7 @@ enum date_mode parse_date_format(const char *format);\n extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\n+extern const char *fmt_name(const char *name, const char *email);\n \n struct checkout {\n \tconst char *base_dir;\ndiff --git a/ident.c b/ident.c\nindex 5be7533..021d79b 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -192,12 +192,14 @@ static const char *env_hint =\n \"Omit --global to set the identity only in this repository.\\n\"\n \"\\n\";\n \n-const char *fmt_ident(const char *name, const char *email,\n-\t\t      const char *date_str, int error_on_no_name)\n+static const char *fmt_ident_1(const char *name, const char *email,\n+\t\t\t       const char *date_str, int flag)\n {\n \tstatic char buffer[1000];\n \tchar date[50];\n \tint i;\n+\tint error_on_no_name = !!(flag & 01);\n+\tint name_addr_only = !!(flag & 02);\n \n \tsetup_ident();\n \tif (!name)\n@@ -224,24 +226,36 @@ const char *fmt_ident(const char *name, const char *email,\n \t}\n \n \tstrcpy(date, git_default_date);\n-\tif (date_str) {\n-\t\tif (*date_str)\n-\t\t\tparse_date(date_str, date, sizeof(date));\n-\t\telse\n-\t\t\tdate[0] = '\\0';\n-\t}\n+\tif (!name_addr_only && date_str)\n+\t\tparse_date(date_str, date, sizeof(date));\n \n \ti = copy(buffer, sizeof(buffer), 0, name);\n \ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n \ti = copy(buffer, sizeof(buffer), i, email);\n-\ti = add_raw(buffer, sizeof(buffer), i, date[0] ? \"> \" : \">\");\n-\ti = copy(buffer, sizeof(buffer), i, date);\n+\tif (!name_addr_only) {\n+\t\ti = add_raw(buffer, sizeof(buffer), i,  \"> \");\n+\t\ti = copy(buffer, sizeof(buffer), i, date);\n+\t} else {\n+\t\ti = add_raw(buffer, sizeof(buffer), i, \">\");\n+\t}\n \tif (i >= sizeof(buffer))\n \t\tdie(\"Impossibly long personal identifier\");\n \tbuffer[i] = 0;\n \treturn buffer;\n }\n \n+const char *fmt_ident(const char *name, const char *email,\n+\t\t      const char *date_str, int error_on_no_name)\n+{\n+\tint flag = (error_on_no_name ? 01 : 0);\n+\treturn fmt_ident_1(name, email, date_str, flag);\n+}\n+\n+const char *fmt_name(const char *name, const char *email)\n+{\n+\treturn fmt_ident_1(name, email, NULL, 03);\n+}\n+\n const char *git_author_info(int error_on_no_name)\n {\n \treturn fmt_ident(getenv(\"GIT_AUTHOR_NAME\"),\n"},{"id":"61684","messageId":"7v1wa4vkev.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"alpine.LFD.0.9999.0712021322580.8458@woody.linux-foundation.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T21:47:20Z","receivedAt":"2007-12-02T21:47:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sun, 2 Dec 2007, Junio C Hamano wrote:\n>>\n>> The next issue would be to find who could pass an empty GIT_AUTHOR_DATE\n>> without noticing...\n>\n> In the meantime, here's a not-very-well-tested patch to fsck to at least \n> notice this.\n\nThanks.\n\nI recall that the very initial git did not use the current format for\ntimestamp but ctime() return value, and this will also notice them (and\nconvert-objects will be there for us).\n"},{"id":"61685","messageId":"alpine.LFD.0.9999.0712021348020.8458@woody.linux-foundation.org","threadId":"11097","inReplyTo":"Pine.LNX.4.64.0712022132060.27959@racer.site","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-02T21:49:40Z","receivedAt":"2007-12-02T21:49:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 2 Dec 2007, Johannes Schindelin wrote:\n> \n> Darn.  But when can \"GIT_AUTHOR_DATE\" be set to the empty string?  I mean, \n> I understand unset'ing it.  But setting it to \"\"?\n\nWell, regardless, I think we should make sure that git-commit-tree never \nwrites out an invalid commit - no matter *how* insane input it gets. \nMaking it complain loudly (with a 'die(\"Oh, no, you don't!\")') would be a \ngood idea.\n\n\t\tLinus\n"},{"id":"61688","messageId":"7vfxyku4kw.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"Pine.LNX.4.64.0712022132060.27959@racer.site","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T22:14:39Z","receivedAt":"2007-12-02T22:14:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> With the above, something like:\n>> \n>> echo msg | GIT_AUTHOR_DATE='' git commit-tree sha1\n>\n> Darn.  But when can \"GIT_AUTHOR_DATE\" be set to the empty string?  I mean, \n> I understand unset'ing it.  But setting it to \"\"?\n\nMaybe something like this would catch such a breakage earlier, but with\nthe re-fix for --signoff I just sent to make fmt_ident() safer, I think\nthis patch would fall into belt-and-suspender category.\n\n---\n\n git-am.sh |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 4126f0e..bab6f68 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -307,9 +307,11 @@ do\n \tGIT_AUTHOR_EMAIL=\"$(sed -n '/^Email/ s/Email: //p' \"$dotest/info\")\"\n \tGIT_AUTHOR_DATE=\"$(sed -n '/^Date/ s/Date: //p' \"$dotest/info\")\"\n \n-\tif test -z \"$GIT_AUTHOR_EMAIL\"\n+\tif test -z \"$GIT_AUTHOR_EMAIL\" ||\n+\t   test -z \"$GIT_AUTHOR_NAME\" ||\n+\t   test -z \"$GIT_AUTHOR_DATE\"\n \tthen\n-\t\techo \"Patch does not have a valid e-mail address.\"\n+\t\techo \"Patch does not have a valid authorship information.\"\n \t\tstop_here $this\n \tfi\n \n"},{"id":"61689","messageId":"Pine.LNX.4.64.0712022230480.27959@racer.site","threadId":"11097","inReplyTo":"7v63zgvkl5.fsf@gitster.siamese.dyndns.org","subject":"Re: Fix --signoff in builtin-commit differently.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-02T22:31:42Z","receivedAt":"2007-12-02T22:31:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Junio C Hamano wrote:\n\n> Introduce fmt_name() specifically meant for formatting the name and\n> email pair, to add signed-off-by value.  This reverts parts of\n> 13208572fbe8838fd8835548d7502202d1f7b21d (builtin-commit: fix --signoff)\n> so that an empty datestamp string given to fmt_ident() by mistake will\n> error out as before.\n\n>From a quick glance, looks good to me.\n\nSorry for the breakage,\nDscho\n"},{"id":"61691","messageId":"7vbq98u3l1.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"alpine.LFD.0.9999.0712021348020.8458@woody.linux-foundation.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T22:36:10Z","receivedAt":"2007-12-02T22:36:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sun, 2 Dec 2007, Johannes Schindelin wrote:\n>> \n>> Darn.  But when can \"GIT_AUTHOR_DATE\" be set to the empty string?  I mean, \n>> I understand unset'ing it.  But setting it to \"\"?\n>\n> Well, regardless, I think we should make sure that git-commit-tree never \n> writes out an invalid commit - no matter *how* insane input it gets. \n> Making it complain loudly (with a 'die(\"Oh, no, you don't!\")') would be a \n> good idea.\n\nFWIW, fmt_ident() records the current time (before the kh/commit series,\nand after the fix I sent on top of kh/commit series), which may be a\nreasonable alternative.\n"},{"id":"61693","messageId":"alpine.LFD.0.9999.0712021441110.8458@woody.linux-foundation.org","threadId":"11097","inReplyTo":"7v1wa4vkev.fsf@gitster.siamese.dyndns.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-02T22:43:41Z","receivedAt":"2007-12-02T22:43:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 2 Dec 2007, Junio C Hamano wrote:\n> \n> I recall that the very initial git did not use the current format for\n> timestamp but ctime() return value\n\nYes, but..\n\n> and this will also notice them (and convert-objects will be there for \n> us).\n\n.. I don't think we have actually accepted the ctime-string format since \nswitchng over, so no existing git repositories will have them. The commit \ndate parsing just does a \"strtoul()\" in parse_commit_date().\n\nSo I wouldn't expect convert-objects to be needed - or rather, it was \nneeded 2.5 _years_ ago, and we've not supported those early broken formats \nsince, afaik.\n\n\t\t\tLinus\n"},{"id":"61694","messageId":"fivdfu$smr$1@ger.gmane.org","threadId":"11097","inReplyTo":"7vmyssvn55.fsf@gitster.siamese.dyndns.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Michael Gebetsroither","fromEmail":"gebi@sbox.tugraz.at","sentAt":"2007-12-02T22:59:22Z","receivedAt":"2007-12-02T22:59:22Z","isPatch":false,"sender":{"key":"gebi@sbox.tugraz.at","avatar":"https://gravatar.com/avatar/b3168bab3f94cb1f09343b408b618ff2982aea3c20789c387f6d5c9b3b73999b?d=mp&s=160"},"body":"* Junio C Hamano <gitster@pobox.com> wrote:\n\n> Yeah, that change does look like a hack now we look at it again.  It\n> would have been much cleaner to make the caller accept the default\n> behaviour of fmt_ident() and strip out the part it does not want from\n> the result.  That way, the damage would have been much contained.\n\nLast hg2git from repo.or.cz does it (it uses git-fast-import).\n\nJust tried to convert lastes mercurial repos to git and the resulting\ngit repos can't be displayed with gitk.\n\ngit fsck --full gives many \"bad commit date in xxxx\"\n\n% git cat-file commit f7900d59930d796c4739452cf68ca9c08a921b5d | sed\n% '/^$/,//d'\ntree 4fb3636148433dcd368fc2dfa20247cb281e2ff8\nparent 6ea0491f4ebb13003d15c6fc478d92cbe1201902\nauthor Mathieu Clabaut <mathieu.clabaut@gmail.com> <\"Mathieu Clabaut\n<mathieu.clabaut@gmail.com>\"> 1153937514 +0200\ncommitter Mathieu Clabaut <mathieu.clabaut@gmail.com> <\"Mathieu Clabaut\n<mathieu.clabaut@gmail.com>\"> 1153937514 +0200\n\nBut the new hg2git is _amazingly_ fast, converts the whole mercurial\nrepos got git in just 1min :).\n\ncu,\nmichael\n-- \nIt's already too late!\n"},{"id":"61695","messageId":"7v3auku28v.fsf@gitster.siamese.dyndns.org","threadId":"11097","inReplyTo":"fivdfu$smr$1@ger.gmane.org","subject":"Re: Corrupted (?) commit 6e6db85e confusing gitk","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T23:05:04Z","receivedAt":"2007-12-02T23:05:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Gebetsroither <gebi@sbox.tugraz.at> writes:\n\n> Last hg2git from repo.or.cz does it (it uses git-fast-import).\n>\n> Just tried to convert lastes mercurial repos to git and the resulting\n> git repos can't be displayed with gitk.\n>\n> author Mathieu Clabaut <mathieu.clabaut@gmail.com> <\"Mathieu Clabaut\n> <mathieu.clabaut@gmail.com>\"> 1153937514 +0200\n> committer Mathieu Clabaut <mathieu.clabaut@gmail.com> <\"Mathieu Clabaut\n> <mathieu.clabaut@gmail.com>\"> 1153937514 +0200\n\nThat's totally broken author/committer information from a broken\nhg2git, with what appears to be correct timestamps.\n"}]}