Re: [External] Re: [PATCH 1/1] revision: don't set parents as uninteresting if exclude promisor objects

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

韩仰 <hanyang.tony@bytedance.com> writes:

> Thanks, I agree process_parents() isn't the right place to fix the issue.
>
>> It apepars to me that its approach to exclude the objects that
>> appear in the promisor packs may be sound, but the design and
>> implementation of it is dubious.  Shouldn't it be getting the list
>> of objects with get_object_list() WITHOUT paying any attention to
>> --exclude-promisor-objects flag, and then filtering objects that
>> appear in the promisor packs out of that list, without mucking with
>> the object and commit traversal in revision.c at all?
>
> The problem is --exclude-promisor-objects is an option in revision.c,
> and this option is used by pack-objects, prune, midx-write and rev-list.
> I see there are two ways to fix this issue, one is to remove the
> --exclude-promisor-objects from revision.c, and filter objects in show_commit
> or show_objects functions. The other place to filter objects is probably
> in revision walk, maybe in traverse_commit_list?

Perhaps another simpler approach may be to use is_promisor_object()
function and get rid of this initial marking of these objects in
prepare_revision_walk() with the for_each_packed_object() loop,
which abuses the UNINTERESTING bit.  The feature wants to exclude
objects contained in these packs, but does not want to exclude
objects that are referred to and outside of these packs, so
UNINTERESTING bit whose natural behaviour is to propagate down the
history is a very bad fit for it.  We may be able to lose a lot of
existing code paths that say "if exclude_promisor_objects then do
this", and filter objects out with "is_promisor_object()" at the
output phase near get_revision().

Jonathan, if I am not mistaken, this is almost all your code.  Any
insights to lend us, even though you may not be very active around
here lately?

Thanks.