Subject: Re: [PATCH v2 0/4] [RFC] repack: add --filter=

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Feb 2022 11:31:23 -0800

Christian Couder <[email-masked]> writes:

>> For what it's worth, I am fine having a mode of repack which allows us
>> to remove objects that we know are stored by a promisor remote. But this
>> series doesn't do that, so users could easily run `git repack -d
>> --filter=...` and find that they have irrecoverably corrupted their
>> repository.
>
> In some cases we just know the objects we are removing are stored by a
> promisor remote or are replicated on different physical machines or
> both, so you should be fine with this.

So, we need to decide if an object we have that is outside the
narrowed filter definition was (and still is, but let's keep the
assumption the whole lazy clone mechanism makes: promisor remotes
will never shed objects that they once served) available at the
promisor remote, but I suspect we have too little information to
reliably do so.  It is OK to assume that objects in existing packs
taken from the promisor remotes and everything reachable from them
(but missing from our object store) will be available to us from
there.  But if we see an object that is outside of _new_ filter spec
(e.g. you fetched with "max 100MB", now you are refiltering with
"max 50MB", narrowing the spec, and you need to decide for an object
that weigh 70MB), can we tell if that can be retrieved from the
promisor or is it unique to our repository until we push it out?  I
am not sure.  For that matter, do we even have a way to compare if
the new filter spec is a subset, a superset, or neither, of the
original filter spec?

> If you are not fine with this because sometimes a user might use it
> without knowing, then why are you ok with commands deleting refs not
> checking that there isn't a regular repack removing dangling objects?

Sorry, I do not follow this argument.  Your user may do "branch -D"
because the branch deleted is no longer needed, which may mean that
objects only reachable from the deleted branch are no longer needed.
I do not see what repack has anything to do with that.  As long as
the filter spec does not change (in other words, before this series
is applied), the repack that discards objects that are known to be
reachable from objects in packs retrieved from promisor remote, the
objects that are no longer reachable may be removed and that will
not lose objects that we do not know to be retrievable from there
(which is different from objects that we know are unretrievable).
But with filter spec changing after the fact, I am not sure if that
is safe.  IOW, "commands deleting refs" may have been OK without
this series, but this series may be what makes it not OK, no?

Puzzled.