Re: [PATCH v3 2/3] shallow: offer to prune only non-existing entries

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Jonathan,

On Thu, 25 Oct 2018, Jonathan Tan wrote:

> > On Wed, 24 Oct 2018, Johannes Schindelin wrote:
> > 
> > Coming back to my question whether there is a better way to check for
> > the presence of a local commit, I figured that I can use
> > `has_object_file()` instead of looking up and parsing the commit, as
> > this code does not really need to verify that the shallow entry refers
> > to a commit, but only that it refers to a local object.
> 
> Note that has_object_file() also triggers the lazy fetch if needed, but
> I agree that it's better because you don't really need to parse the
> commit.

Thanks for confirming.

So even better would be to use `is_promisor_object(oid) ||
has_object_file(oid)`, right?

This is something that is probably not even needed: as I mentioned, the
shallow commits are *expected* to be local. It should not ever happen that
they are fetched.

> There is the possibility of just checking for loose objects (which does
> not trigger any lazy fetches), which works for builtin/prune since it
> only prunes loose objects, but doesn't work in the general case, I
> guess.

In the test case I added, the commit object is actually packed. And then,
because it became unreachable, it is dropped.

If I only checked for loose objects here, the shallow line would already
be removed when the commit gets packed, which would be the wrong thing to
do.

In short: thank you for confirming that the current version of the patch
seems to be the best we can do for now.

Ciao,
Dscho