From 577dd0d29bf7cbbdf1f00c20a90caac50b1c603f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 May 2018 14:01:59 -0400 Subject: [PATCH] mark_parents_uninteresting(): drop missing object check 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 Signed-off-by: Junio C Hamano --- revision.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/revision.c b/revision.c index 9de92bb5e5..e1fa9b13b9 100644 --- a/revision.c +++ b/revision.c @@ -102,18 +102,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;