git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Tuncer Ayaz <tuncer.ayaz@gmail.com>
Subject: [PATCH] fsck: fix bogus commit header check
Date: Wed, 26 May 2010 16:50:34 -0500	[thread overview]
Message-ID: <20100526215034.GA6872@progeny.tock> (raw)

daae1922 (fsck: check ident lines in commit objects, 2010-04-24)
taught fsck to expect commit objects to have the form

  tree <object name>
  <parents>
  author <valid ident string>
  committer <valid ident string>

  log message

The check is overly strict: for example, it errors out with the
message “expected blank line” for perfectly valid commits with an
"encoding ISO-8859-1" line.

Later it might make sense to teach fsck about the rest of the header
and warn about unrecognized header lines, but for simplicity, let’s
accept arbitrary trailing lines for now.

Reported-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Patch is against jn/fsck-ident (daae1922).

As you might expect, this triggers for some real-world repos.
Many thanks to Tuncer for the catch.

 fsck.c          |    2 --
 t/t1450-fsck.sh |    8 ++++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fsck.c b/fsck.c
index ae9ae1a..3d05d4a 100644
--- a/fsck.c
+++ b/fsck.c
@@ -311,8 +311,6 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)
 	err = fsck_ident(&buffer, &commit->object, error_func);
 	if (err)
 		return err;
-	if (*buffer != '\n')
-		return error_func(&commit->object, FSCK_ERROR, "invalid format - expected blank line");
 	if (!commit->tree)
 		return error_func(&commit->object, FSCK_ERROR, "could not load commit's tree %s", sha1_to_hex(tree_sha1));
 
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 22a80c8..759cf12 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -5,7 +5,9 @@ test_description='git fsck random collection of tests'
 . ./test-lib.sh
 
 test_expect_success setup '
+	git config i18n.commitencoding ISO-8859-1 &&
 	test_commit A fileA one &&
+	git config --unset i18n.commitencoding &&
 	git checkout HEAD^0 &&
 	test_commit B fileB two &&
 	git tag -d A B &&
@@ -28,6 +30,12 @@ test_expect_success 'loose objects borrowed from alternate are not missing' '
 	)
 '
 
+test_expect_success 'valid objects appear valid' '
+	{ git fsck 2>out; true; } &&
+	! grep error out &&
+	! grep fatal out
+'
+
 # Corruption tests follow.  Make sure to remove all traces of the
 # specific corruption you test afterwards, lest a later test trip over
 # it.
-- 
1.7.1.232.g3072.dirty

             reply	other threads:[~2010-05-26 21:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-26 21:50 Jonathan Nieder [this message]
2010-05-29  1:10 ` [PATCH] fsck: fix bogus commit header check 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

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

  git send-email \
    --in-reply-to=20100526215034.GA6872@progeny.tock \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tuncer.ayaz@gmail.com \
    /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.
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 NNTP newsgroup(s).