From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Derrick Stolee <dstolee@microsoft.com>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 2/4] mark_parents_uninteresting(): drop missing object check
Date: Fri, 11 May 2018 14:01:59 -0400 [thread overview]
Message-ID: <20180511180158.GB12543@sigill.intra.peff.net> (raw)
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
next prev parent reply other threads:[~2018-05-11 18:02 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-11 18:00 [PATCH 0/4] a few mark_parents_uninteresting cleanups Jeff King
2018-05-11 18:00 ` [PATCH 1/4] mark_tree_contents_uninteresting(): drop missing object check Jeff King
2018-05-11 18:01 ` Jeff King [this message]
2018-05-13 2:23 ` [PATCH 2/4] mark_parents_uninteresting(): " Junio C Hamano
2018-05-11 18:02 ` [PATCH 3/4] mark_parents_uninteresting(): replace list with stack Jeff King
2018-05-11 18:03 ` [PATCH 4/4] mark_parents_uninteresting(): avoid most allocation Jeff King
2018-05-14 12:47 ` Derrick Stolee
2018-05-14 13:09 ` Jeff King
2018-05-14 13:25 ` Derrick Stolee
2018-05-14 14:09 ` Jeff King
2018-05-14 12:50 ` [PATCH 0/4] a few mark_parents_uninteresting cleanups Derrick Stolee
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180511180158.GB12543@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).