Re: Possible regression in ref advertisement

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2013, 20:07 UTC
Message-ID: <7v1uc4ximl.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1361822092.30765.12.camel@centaur.cmartin.tk>

Carlos Martín Nieto <cmn@elego.de> writes:

Show 8 quoted lines

>> A shot in the dark, as I do not seem to be able to reproduce the issue
>> with anything that contains the commit.  Perhaps your .git/packed-refs
>> is corrupt?
>
> My packed-refs file did not end with LF. It seems it must or the parser
> won't consider the last tag in the file to have a peeled target and mine
> didn't. I don't how the ended up this way but it looks like there's is a
> bug in the parser (or does the format force you to have LF at the end?)

It is unclear what kind of corruption your packed-refs file had, as there are multiple ways for a file not to "end with LF", but anyway.

As packed-refs file is expected to be a text file, it is not surprising to get an undefined result if the it ends with an incomplete line.

I do not offhand recall if we tolerate lines with CRLF endings; as far as I know we do not _write_ CRLF out, and because we do not expect people to muck directly with editor bypassing "pack-refs", I would not be surprised if we didn't do anything special for people who make the lines end with with CRLF the file, either.

I'd be more worried about the possibly lost entries after that incomplete line (and also possibly truncated end part of the tagname on the last, incomplete line). Perhaps fsck should diagnose such an anomaly as repository corruption? Perhaps we should enhance the file format a bit (right now, the first "capability" line should say something like "# pack-refs with: peeled" to say it was created with the version of pack-refs that can record peeled tags, but we can add other capabilities to extend the format) to add checksums to detect corruption?