Re: [PATCH] fsck: it is OK for a tag and a commit to lack the body

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2015, 05:42 UTC
Message-ID: <xmqqsi9bcboq.fsf@gitster.dls.corp.google.com>
In-Reply-To: <32aa1979e75ba9bc6dc8a58fe32e9e55@www.dscho.org>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 16 quoted lines

>> +	/*
>> +	 * We did not find double-LF that separates the header
>> +	 * and the body.  Not having a body is not a crime but
>> +	 * we do want to see the terminating LF for the last header
>> +	 * line.
>> +	 */
>> +	if (size && buffer[size - 1] == '\n')
>> +		return 0;
>> +
>>  	return error_func(obj, FSCK_ERROR, "unterminated header");
>>  }
>
> Hmm. Maybe we should still warn when there is no empty line finishing
> the header explicitly, or at least make it FSCK_IGNORE by default so
> that maintainers who like a stricter check can upgrade the condition
> to an error?

Wolfgang, do you know how these old tags without messages were created? I think in the old days we didn't advertise "git tag" as the sole way to create a tag object and more people drove "git mktag" from their script, and "mktag" until e0aaf781 (mktag.c: improve verification of tagger field and tests, 2008-03-27) did not complain if the header were not terminated with double-LF even when the tag did not have a body (hence there is no need for double-LF).

Dscho, I do not think it is reasonable to force all repository owners of projects that started using Git before early 2008 to set configuration variable to ignore warnings for something that was perfectly kosher back when their project started. More importantly, even though Git core itself adds unnecessary double-LF after the header in a tag or a commit that does not have any body, I am not sure if we punish people who use current versions of third-party reimplementations of Git that do not write that unnecessary double-LF at the end an object without a body (I am not saying that there is any known third-party reimplementation to do so---I am saying that I do not think it is their bug if such implementations existed today).

Do we have check marked as FSCK_IGNORE by default? I think a more interesting "stricter check" may be to flag a tag object or a commit object that does not have any body, with or without the double-LF at the end.

And such a check can certainly be added in the future, but what I sent was a fix to a regression that caused us to start whining on a syntactically valid object in the v2.2 timeframe, and is not about adding a new feature.