Re: [PATCH 03/18] revert: Simplify and inline add_message_to_msg

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:

>> Unfortunately it fails for me even after the patch.  If the test looks
>> reasonable to you, it could be worth adding marked with
>> "test_expect_failure".
>
> Perhaps as part of a different series which addresses the issue of
> malformed objects in general.  I don't think it is relevant for this
> series.  The "other" series I'm thinking about should contain:

I don't want to work on that, personally...

But git already has a strategy for this.  It is quite simple: git can
give insane results when fed malformed objects, but it does not blindly
trust it and let scripts segfault, corrupt something else, escalate
privileges, etc.  Remember that malformed objects might even have been
received from a remote machine with "git fetch" --- one simply cannot
trust objects beyond the assertion "each object is a stream of bytes".

And on the other hand, git maintains sanity by _preserving_ invariants
and providing good behavior when it deals with valid objects.  To a
rough approximation, "valid object" is a synonym for "git fsck accepts
it" (but that approximation is only modulo bugs --- git fsck has both
false positives and false negatives).  It is perfectly legitimate for
commands to get confused and give a wrong result when working with
invalid objects, and this ability is a nice thing because it allows
for some optimizations.

As for the example at hand: perhaps we should teach "git fsck" to call
commit objects without a blank line after the header invalid.  After
all, historical implementations of commands like "git cherry-pick" are
not ready to cope with them.  But that is no excuse to pretend they
don't exist!  While the best thing about your patch (and the most
invasive aspect of it) is that it improves that situation, for some
reason you don't want to document this important aspect of its impact.
It leaves me completely puzzled.