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

[PATCH] revision.c: ignore broken tags with ignore_missing_links

From
Jeff King <peff@peff.net>
Date
May 20, 2017, 08:30 UTC
Message-ID
<20170520083025.nv7hqwy6of3mkkjs@sigill.intra.peff.net>
In-Reply-To
<20170520080312.nwb277swvmpq7iq6@sigill.intra.peff.net>

When peeling a tag for prepare_revision_walk(), we do not respect the ignore_missing_links flag. This can lead to a bogus error when pack-objects walks the possibly-broken unreachable-but-recent part of the object graph.

The other link-following all happens via traverse_commit_list(), which explains why this case was missed. And our tests covered only broken links from commits. Let's be more comprehensive and cover broken tree entries (which do work) and tags (which shows off this bug).

Signed-off-by: Jeff King <peff@peff.net>
---
So here's the fix I showed earlier polished up as a real patch. I still
think your patch to improve the error message is a good suggestion. I
was going to just re-send it out on top of this, but I realized it was
missing your signoff. Do you want to resend?
 revision.c                 |  2 +-
 t/t6501-freshen-objects.sh | 27 ++++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 8a8c1789c..610638f2e 100644
--- a/revision.c
+++ b/revision.c
@@ -230,7 +230,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 			die("bad tag");
 		object = parse_object(tag->tagged->oid.hash);
 		if (!object) {
-			if (flags & UNINTERESTING)
+			if (revs->ignore_missing_links || (flags & UNINTERESTING))
 				return NULL;
 			die("bad object %s", oid_to_hex(&tag->tagged->oid));
 		}
diff --git a/t/t6501-freshen-objects.sh b/t/t6501-freshen-objects.sh
index cf076dcd9..394b169ea 100755
--- a/t/t6501-freshen-objects.sh
+++ b/t/t6501-freshen-objects.sh
@@ -129,7 +129,7 @@ for repack in '' true; do
 	'
 done
 
-test_expect_success 'do not complain about existing broken links' '
+test_expect_success 'do not complain about existing broken links (commit)' '
 	cat >broken-commit <<-\EOF &&
 	tree 0000000000000000000000000000000000000001
 	parent 0000000000000000000000000000000000000002
@@ -144,4 +144,29 @@ test_expect_success 'do not complain about existing broken links' '
 	test_must_be_empty stderr
 '
 
+test_expect_success 'do not complain about existing broken links (tree)' '
+	cat >broken-tree <<-\EOF &&
+	100644 blob 0000000000000000000000000000000000000003	foo
+	EOF
+	tree=$(git mktree --missing <broken-tree) &&
+	git gc 2>stderr &&
+	git cat-file -e $tree &&
+	test_must_be_empty stderr
+'
+
+test_expect_success 'do not complain about existing broken links (tag)' '
+	cat >broken-tag <<-\EOF &&
+	object 0000000000000000000000000000000000000004
+	type commit
+	tag broken
+	tagger whatever <whatever@example.com> 1234 -0000
+
+	this is a broken tag
+	EOF
+	tag=$(git hash-object -t tag -w broken-tag) &&
+	git gc 2>stderr &&
+	git cat-file -e $tag &&
+	test_must_be_empty stderr
+'
+
 test_done
-- 
2.13.0.234.g029c1392a
Previous: Jeff King
Message 3 of 3 in “die("bad object.. for duplicate tagged tag in remote”
  1. Chris WestMay 19, 2017
  2. Jeff KingMay 20, 2017
  3. revision.c: ignore broken tags with ignore_missing_linksJeff King, May 20, 2017

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.