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

[PATCH 2/4] mark_parents_uninteresting(): drop missing object check

From
Jeff King <peff@peff.net>
Date
May 11, 2018, 18:01 UTC
Message-ID
<20180511180158.GB12543@sigill.intra.peff.net>
In-Reply-To
<20180511180029.GA11290@sigill.intra.peff.net>

We allow UNINTERESTING objects in a traversal to be unavailable. As part of this, mark_parents_uninteresting() checks whether we have a particular uninteresting parent; if not, we will mark it as "parsed" so that later code skips it.

This code is redundant and even a little bit harmful, so let's drop it.

It's redundant because when our parse_object() call in add_parents_to_list() fails, we already quietly skip UNINTERESTING parents. This redundancy is a historical artifact. The mark_parents_uninteresting() protection is from 454fbbcde3 (git-rev-list: allow missing objects when the parent is marked UNINTERESTING, 2005-07-10). Much later, aeeae1b771 (revision traversal: allow UNINTERESTING objects to be missing, 2009-01-27) covered more cases by making the actual parse more gentle.

  As an aside, even if this weren't redundant, it would be
  insufficient. The gentle parsing handles both missing and
  corrupted objects, whereas the has_object_file() check
  we're getting rid of covers only missing ones.
And the code we're dropping is harmful for two reasons:
  1. We spend extra time on the object lookup, even though
     we don't actually need the information at this point
     (and will just repeat that lookup later when we parse
     for the common case that we _do_ have the object).
  2. It "lies" about the commit by setting the parsed flag,
     even though we didn't load any useful data into the
     struct. This shouldn't matter for the UNINTERESTING
     case, but we may later clear our flags and do another
     traversal in the same process. While pretty unlikely,
     it's possible that we could then look at the same
     commit without the UNINTERESTING flag, in which case
     we'd produce the wrong result (we'd think it's a commit
     with no parents, when in fact we should probably die
     due to the missing object).
Signed-off-by: Jeff King <peff@peff.net>
---
 revision.c | 12 ------------
 1 file changed, 12 deletions(-)
diff --git a/revision.c b/revision.c
index ef70f69f08..cee4f3a4b4 100644
--- a/revision.c
+++ b/revision.c
@@ -103,18 +103,6 @@ void mark_parents_uninteresting(struct commit *commit)
 		struct commit *commit = pop_commit(&parents);
 
 		while (commit) {
-			/*
-			 * A missing commit is ok iff its parent is marked
-			 * uninteresting.
-			 *
-			 * We just mark such a thing parsed, so that when
-			 * it is popped next time around, we won't be trying
-			 * to parse it and get an error.
-			 */
-			if (!commit->object.parsed &&
-			    !has_object_file(&commit->object.oid))
-				commit->object.parsed = 1;
-
 			if (commit->object.flags & UNINTERESTING)
 				break;
 
-- 
2.17.0.988.gec4b43b3e5
Previous: Jeff KingNext: Junio C Hamano
Message 3 of 11 in “a few mark_parents_uninteresting cleanups”
  1. 0/4 a few mark_parents_uninteresting cleanupsJeff King, May 11, 2018
  2. 1/4 mark_tree_contents_uninteresting(): drop missing object checkJeff King, May 11, 2018
  3. 2/4 mark_parents_uninteresting(): drop missing object checkJeff King, May 11, 2018
  4. Junio C HamanoMay 13, 2018
  5. 3/4 mark_parents_uninteresting(): replace list with stackJeff King, May 11, 2018
  6. 4/4 mark_parents_uninteresting(): avoid most allocationJeff King, May 11, 2018
  7. Derrick StoleeMay 14, 2018
  8. Jeff KingMay 14, 2018
  9. Derrick StoleeMay 14, 2018
  10. Jeff KingMay 14, 2018
  11. Derrick StoleeMay 14, 2018

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.