Re: [PATCH 1/2] parse_commit_buffer: don't parse invalid commits

[Junio C Hamano <gitster@pobox.com>]

mkoegler@auto.tuwien.ac.at (Martin Koegler) writes:

> You seem to have missed my reply to your last mail:
> http://marc.info/?l=git&m=119969163624138&w=2

Yeah, apparently I did.

> On the other hand, it is possible, that somebody pushed such a commit
> out, if he does not notice it. Then its difficult to get rid of the
> broken commit. [Hasn't happend a broken commit on pu recently?]

That's true, but I wonder if there is a reasonable middle
ground.  For example, we could keep the warning you added (by
calling error() to identify which commit object is suspicious)
without making the whole function return that error.  Then a
broken commit could have been catched while reviewing the series
before the commit gets pushed out.

>> I do not think the checks done by fsck and parse_commit should
>> share the same strictness.  They serve different purposes.
>
> Maybe I can improve fsck.

Thanks.  It should notice bogus commits but it should allow
checking objects referenced by them unless the particular
bogosity is severe enough to make "tree " and "parent " lines in
them invalid.

IOW, the information on the "author" line is not that important.
It has zero importance from the structural viewpoint, just like
the commit log message contents does not have any structural
importance (it does not even affect how the log output is sorted
at all, while the timestamp on the "committer" line has some say
in it).

Noticing a bogus "author" or "committer" line is like noticing
that your commit log message has garbage characters in it.
Maybe a future option to fsck might want to check that all log
messages are encoded in UTF-8, and you may catch some that are
in ISO-8859-1.  After flagging them as such, the program should
still continue digging the history down to check its ancestors,
as such a breakage does not affect the structural integrity of
the history.