Re: [PATCH 0/1] revision: fix reachable objects being gc'ed in no blob clone repo

[Junio C Hamano <gitster@pobox.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

> Jonathan Tan <jonathantanmy@google.com> writes:
>> Solutions I can think of:
>
> One more thing that I just thought of regarding the solution in this
> patch. It seems to be to have a different separation of packs: all
> objects currently in promisor packs and all objects currently not
> in promisor packs. And the way it is done is to only exclude (in
> this patch, mark UNINTERESTING, although it might be better to have
> a separate flag for it) objects in promisor packs, but not their
> ancestors.

You're right to mention two separate bits, especially because you do
not want the "I am in a promisor pack" bit to propagate down to the
ancestry chain like UNINTERESTING bit does.  But isn't the approach
to enumerate all objects in promisor packs in an oidset and give a
quick way for is_promisor_object() to answer if an object is or is
not in promisor pack sufficient to replace the need to use _any_
object flag bits to manage objects in promisor packs?

> There are two ways we can go from here:
>
>  - Do not iterate past this object, just like for UNINTERESTING. This
>    would end up not packing objects that we need to pack (e.g. {C,T,B}2
>    below, if we only have a ref pointing to C3).
>
>   commit  tree  blob
>    C3 ---- T3 -- B3 (fetched from remote, in promisor pack)
>    |
>    C2 ---- T2 -- B2 (created locally, in non-promisor pack)
>    |
>    C1 ---- T1 -- B1 (fetched from remote, in promisor pack)
>
>  - Iterate past this object (I think this is the path this patch took,
>    but I didn't look at it closely). This works, but seems to be very
>    slow. We would need to walk through all reachable objects (promisor
>    object or not), unlike currently in which we stop once we have
>    reached a promisor object.

Thanks for helping Han & Xinxin.