Re: Corrupted (?) commit 6e6db85e confusing gitk

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 2, 2007, 20:48 UTC
Message-ID: <7vmyssvn55.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071202193918.GQ6212@lavos.net>

bdowning@lavos.net (Brian Downing) writes:

Show 29 quoted lines

> It looks like the "guilty" commit that allowed this behavior was:
>
> commit 13208572fbe8838fd8835548d7502202d1f7b21d
> Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Date:   Sun Nov 11 17:35:58 2007 +0000
>
>     builtin-commit: fix --signoff
>
>     The Signed-off-by: line contained a spurious timestamp.  The reason was
>     a call to git_committer_info(1), which automatically added the
>     timestamp.
>
>     Instead, fmt_ident() was taught to interpret an empty string for the
>     date (as opposed to NULL, which still triggers the default behavior)
>     as "do not bother with the timestamp", and builtin-commit.c uses it.
>
>     Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> With the above, something like:
>
> echo msg | GIT_AUTHOR_DATE='' git commit-tree sha1
>
> will produce a broken commit without a timestamp, since fmt_ident is
> also used for the committer and author lines.
>
> Personally, I think if the date_str is not NULL, it should die() on
> anything that can't successfully be parsed as a date, rather than simply
> falling back to the current time.  But maybe that's a bit extreme.

Yeah, that change does look like a hack now we look at it again. It would have been much cleaner to make the caller accept the default behaviour of fmt_ident() and strip out the part it does not want from the result. That way, the damage would have been much contained.

The next issue would be to find who could pass an empty GIT_AUTHOR_DATE without noticing...