> > However, in the case that --ignore-missing is not set but
> > --exclude-promisor-objects is set, there is still no distinction between
> > the case wherein the missing object is a promisor object and the case
> > wherein it is not. This is unnecessarily restrictive, since if a missing
> > promisor object is encountered in traversal, it is ignored; likewise it
> > should be ignored if provided through the CLI. Therefore, distinguish
> > between these 2 cases. (As a bonus, the code is now simpler.)
>
> nit about tenses, not worth a reroll on its own: "As a bonus, this
> makes the code simpler" (since commit messages describe the status quo
> before the patch in the present tense).
OK.
> > --- a/revision.c
> > +++ b/revision.c
> > @@ -370,8 +370,18 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
> > if (!repo_parse_commit(revs->repo, c))
> > object = (struct object *) c;
> > else
> > + /*
> > + * There is something wrong with the commit.
> > + * repo_parse_commit() will have already printed an
> > + * error message. For our purposes, treat as missing.
> > + */
> > object = NULL;
> > } else {
> > + /*
> > + * There is something wrong with the object. parse_object()
>
> If we got here, we are in the !commit case, which is not inherently wrong,
> right?
[snip]
Ah, good catch. It should be "If parse_object() returns NULL, ..."
> By the way, why doesn't parse_object itself check the commit graph for
> commit objects?
Because that's how I coded it in ec0c5798ee ("revision: use commit graph
in get_reference()", 2018-12-28). In the commit message, I said:
> Another way to accomplish this effect would be to modify parse_object()
> to use the commit graph if possible; however, I did not want to change
> parse_object()'s current behavior of always checking the object
> signature of the returned object.
But that doesn't mean that we cannot change it.
> By "and treats it as missing" does this mean "and we should treat it
> as missing"? (Forgive my ignorance.)
I don't fully know if we should, hence my specific wording :-P but see
my answer to your next question.
> Why do we treat corrupt objects as missing? Is this for graceful
> degredation when trying to recover data from a corrupt repository?
This (at least, treating wrong-hash objects the same as missing) has
been true since acdeec62cb ("Don't ever return corrupt objects from
"parse_object()"", 2007-03-20) (and that commit message has no
explanation). I think this is best - I consider corrupt objects to be
similar to missing objects with respect to repository corruption, so for
me it is logical to treat them the same way.
