韩仰 <hanyang.tony@xxxxxxxxxxxxx> 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.
