Subject: Re: [PATCH 6/9] repack: add `--filter=<filter-spec>` option

From: Junio C Hamano <[email-masked]>
Date: Wed, 21 Jun 2023 09:53:50 -0700

Christian Couder <[email-masked]> writes:

> It might indicate that we prefer to be safe, do things in different
> steps and not provide an easy way for users to shoot their own foot.
> For example it seems pretty safe to do things like this:
>
>   1) put all the objects we think should be on the promisor remote in
> a separate packfile
>   2) start checking that each object in that packfile is available on
> the promisor remote
>   3) if an object in that packfile isn't on the promisor remote, try
> to send it there
>   4) if we couldn't send the object, error out
>   5) if we haven't errored out after checking all the objects in the
> packfile, it means all these objects are now available from the
> promisor remote and we can safely delete the packfile

I may be missing something, but to me, the above sound more like a
tail wagging the dog.

Instead of saying "while repacking, we'll create the new pack with
the objects that we suspect that we cannot re-fetch from the
promisor (allowing false positives for safety), and store the rest
in a backup pack (that can immediately be discarded)", the above
says "while repacking, we'll create the new pack with objects that
match the filter, and store the rest to another pack".  But because
the object selection criteria used in the latter is not something
with practical/useful meaning, in other words, it does not exactly
match what we want, we fill the gaps between what we want (i.e. sift
the objects into "refetchable" and "other" bins) and what we
happened to have implemented (i.e. sift the objects into "match
filter" and "other" bints) by sending the objects that we _should_
have included in the new pack (i.e. "not refetchable") to the
promisor to make them refetchable.

I do not know what to think about that.  I do not think there is
even a way to guarantee that the push done for 3) will always be
taken and still leave the resulting promisor usable (e.g.  we can
make them connected by coming up with a random new ref to point
these "we are sending these only because we failed to include them
in the set of objects we should consider local" objects, but then
how would we avoid bloating the refs at the promisor remote side
(which now has become a "dumping ground", rather than holding the
objects needed for histories that project participants care about).

As an argument to salvage this series as (one of the possible
ingredients to) a solution to "slim down a bloated lazy clone"
problem, it sounds a bit weak.

Thanks.