git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] revision traversal and pack: notice and die on missing commit

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 11, 2009, 09:27 UTC
Message-ID
<7vfxilgw6o.fsf_-_@gitster.siamese.dyndns.org>
In-Reply-To
<20090209092113.GB1320@atjola.homenet>

cc0e6c5 (Handle return code of parse_commit in revision machinery, 2007-05-04) attempted to tighten error checking in the revision machinery, but it wasn't enough. When get_revision_1() was asked for the next commit to return, it tries to read and simplify the parents of the commit to be returned, but an error while doing so was silently ignored and reported as a truncated history to the caller instead.

This resulted in an early end of "git log" output or a pack that lacks older commits from "git pack-objects", without any error indication in the exit status from these commands, even though the underlying parse_commit() issues an error message to the end user.

Note that the codepath in add_parents_list() that paints parents of an UNINTERESTING commit UNINTERESTING silently ignores the error when parse_commit() fails; this is deliberate and in line with aeeae1b (revision traversal: allow UNINTERESTING objects to be missing, 2009-01-27).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Dscho, do not complain that I did not use test_commit, because this is
   meant to apply against maint-1.5.4, which happens to be the oldest
   maintenance track I am still keeping around.  The breakage actually
   goes even older; cc0e6c5 is *not* the culprit, but merely an incomplete
   attempt to fix this issue.
   There is a fairly obvious comflict when this patch is applied to maint
   and master, but I'll be merging the commit upward after a bit more
   testing.
 revision.c                     |    6 ++++--
 t/t5307-pack-missing-commit.sh |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100755 t/t5307-pack-missing-commit.sh
diff --git a/revision.c b/revision.c
index d79f2b3..b8c015e 100644
--- a/revision.c
+++ b/revision.c
@@ -1484,14 +1484,16 @@ static struct commit *get_revision_1(struct rev_info *revs)
 			    (commit->date < revs->max_age))
 				continue;
 			if (add_parents_to_list(revs, commit, &revs->commits) < 0)
-				return NULL;
+				die("Failed to traverse parents of commit %s",
+				    sha1_to_hex(commit->object.sha1));
 		}
 
 		switch (simplify_commit(revs, commit)) {
 		case commit_ignore:
 			continue;
 		case commit_error:
-			return NULL;
+			die("Failed to simplify parents of commit %s",
+			    sha1_to_hex(commit->object.sha1));
 		default:
 			return commit;
 		}
diff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh
new file mode 100755
index 0000000..ae52a18
--- /dev/null
+++ b/t/t5307-pack-missing-commit.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='pack should notice missing commit objects'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	for i in 1 2 3 4 5
+	do
+		echo "$i" >"file$i" &&
+		git add "file$i" &&
+		test_tick &&
+		git commit -m "$i" &&
+		git tag "tag$i"
+	done &&
+	obj=$(git rev-parse --verify tag3) &&
+	fanout=$(expr "$obj" : "\(..\)") &&
+	remainder=$(expr "$obj" : "..\(.*\)") &&
+	rm -f ".git/objects/$fanout/$remainder"
+'
+
+test_expect_success 'check corruption' '
+	test_must_fail git fsck
+'
+
+test_expect_success 'rev-list notices corruption (1)' '
+	test_must_fail git rev-list HEAD
+'
+
+test_expect_success 'rev-list notices corruption (2)' '
+	test_must_fail git rev-list --objects HEAD
+'
+
+test_expect_success 'pack-objects notices corruption' '
+	echo HEAD |
+	test_must_fail git pack-objects --revs pack
+'
+
+test_done
-- 
1.6.2.rc0.50.g0a27
Previous: Björn Steinbrink
Message 4 of 4 in “"repack" when a needed commit object missing "kills" the repo”
  1. Björn SteinbrinkFeb 9, 2009
  2. Björn SteinbrinkFeb 9, 2009
  3. Björn SteinbrinkFeb 9, 2009
  4. revision traversal and pack: notice and die on missing commitJunio C Hamano, Feb 11, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.