Re: [PATCH] fsck: do not crash on tag objects which do not contain an empty line

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 8, 2007, 05:30 UTC
Message-ID: <Pine.LNX.4.64.0706080621420.4059@racer.site>
In-Reply-To: <7vmyzbm3cx.fsf@assigned-by-dhcp.cox.net>

Hi,

On Thu, 7 Jun 2007, Junio C Hamano wrote:

Show 9 quoted lines

> Recent git-tag does this
> 
>     ( printf 'object %s\ntype %s\ntag %s\ntagger %s\n\n' \
> 	"$object" "$type" "$name" "$tagger";
>       cat "$GIT_DIR"/TAG_FINALMSG ) >"$GIT_DIR"/TAG_TMP
> 
> so even if TAG_FINALMSG (which is -m or edited message after
> filtering out the comments and git-stripspace) is empty, you
> would have the two LFs at the end of the header.

I could imagine that there is still a git-stripspace after that. Undoing the last LF.

> Did this tag come from cvsimport (or svnimport) perhaps?

Yep. cvsimport.

But nevertheless, I know the convention of "LFLF" being the end of the header from several places: HTTP, mail, proxies.

_None_ of them fail as badly as what I experienced. Heck, Git used to be the _most_ stable software I ever used, _including_ the Linux kernel. And all of a sudden, when I really needed it just to work properly, and did not bother to install a "stable" version, but just took "next", it failed on me.

I don't like this at all.

And I have to say, even if I was responsible for my share of not-quite-perfect patches, this patch _series_ is bad IMHO.

Not only does already the _first_ patch contain a serious error. I did not even bother to read it in total, because it was _huge_, contained _white-space_ fixes in combination with some dubious comment changes, did not clean up _obvious_ errors (like lookup_object(sha1)->object!!), but at the same time _changed_ behaviour.

And it is totally unreadable, with 120+ characters per line.

Frankly, I am angry at myself that I did not have a look and shot that patch down on the merits of its _line size_ alone.

I have come to expect bad code from bad style and I would have been right again, had I read the patch in time.

> I agree with you that if something does not have body, we should not 
> require an empty trailing line after the header.

It is just _one_ issue about it.

We should not fail to process a fetch _at all_, just because a single _tag_ is not really conforming to some obscure convention.

Ciao, Dscho "still somewhat upset"