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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 23, 2022, 19:31 UTC
Message-ID: <xmqqv8x5v0qc.fsf@gitster.g>
In-Reply-To: <CAP8UFD3U4t-inWC5mZYhybWpjVwkqA7v4hYZ5voBOEJ=+_Y1kQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 9 quoted lines

>> 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.