git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Jeff King <peff@peff.net>, Wolfgang Denk <wd@denx.de>,
	git@vger.kernel.org
Subject: Re: [PATCH] fsck: it is OK for a tag and a commit to lack the body
Date: Sun, 28 Jun 2015 22:42:45 -0700	[thread overview]
Message-ID: <xmqqsi9bcboq.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <32aa1979e75ba9bc6dc8a58fe32e9e55@www.dscho.org> (Johannes Schindelin's message of "Mon, 29 Jun 2015 07:12:38 +0200")

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

>> +	/*
>> +	 * 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.

  reply	other threads:[~2015-06-29  5:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-25 15:51 git error in tag ...: unterminated header Wolfgang Denk
2015-06-25 17:32 ` Junio C Hamano
2015-06-25 20:13   ` Wolfgang Denk
2015-06-25 20:24     ` Junio C Hamano
2015-06-25 21:07       ` Johannes Schindelin
2015-06-25 21:21         ` Junio C Hamano
2015-06-25 22:29           ` Junio C Hamano
2015-06-26  8:06             ` Johannes Schindelin
2015-06-26 15:52               ` Jeff King
2015-06-26 17:37                 ` Junio C Hamano
2015-06-27  8:57                   ` Johannes Schindelin
2015-06-27 18:36                     ` Junio C Hamano
2015-06-28 18:18                 ` [PATCH] fsck: it is OK for a tag and a commit to lack the body Junio C Hamano
2015-06-28 18:21                   ` Eric Sunshine
2015-06-29  5:12                   ` Johannes Schindelin
2015-06-29  5:42                     ` Junio C Hamano [this message]
2015-06-29 14:51                       ` Johannes Schindelin
2015-06-25 20:48     ` git error in tag ...: unterminated header Junio C Hamano
2015-06-25 17:38 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsi9bcboq.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=wd@denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).