Re: [PATCH 0/6] [RFC] partial-clone: add ability to refetch with expanded filter
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Feb 2, 2022, 18:59 UTC
- Message-ID
- <20220202185957.1928631-1-jonathantanmy@google.com>
- In-Reply-To
- <pull.1138.git.1643730593.gitgitgadget@gmail.com>
"Robert Coup via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 6 quoted lines
> If a filter is changed on a partial clone repository, for example from > blob:none to blob:limit=1m, there is currently no straightforward way to > bulk-refetch the objects that match the new filter for existing local > commits. This is because the client will report commits as "have" during > negotiation and any dependent objects won't be included in the transferred > pack. Another use case is discussed at [1].
Reporting commits as "have" can be solved by forcing the noop negotiator, but there is another issue (which you seem to have discovered, glancing through your patches) in that fetch will abort (and report success) if all wanted commits are present, even if not all objects directly or indirectly referenced by those commits are present.
> This patch series proposes adding a --refilter option to fetch & fetch-pack
> to enable doing a full fetch with a different filter, as if the local has no
> commits in common with the remote. It builds upon cbe566a071
> ("negotiator/noop: add noop fetch negotiator", 2020-08-18).Thanks - I think this is a useful feature. This is useful even in a non-partial-clone repo, to repair objects that were, say, accidentally deleted from the local object store.
If it's acceptable to have a separate command to configure the new filter in the repo config (or to delete it, if we want to convert a partial clone into a regular repo), I think it's clearer to name this option "--repair" or something like that, and explain it as a fetch that does not take into account the contents of the local object store (not as a fetch that changes the filter).
Having said that, the overall feature is worth having. While we decide on the name, the implementation of this will not change much. I'll try to review the implementation by the end of this week (but other reviews are welcome too, needless to say).
> To note: > > 1. This will produce duplicated objects between the existing and newly > fetched packs, but gc will clean them up.
Noted. This might be an argument for naming the option "--repair", since the user would probably understand that there would be duplicate objects.
> 2. This series doesn't check that there's a new filter in any way, whether > configured via config or passed via --filter=. Personally I think that's > fine.
Agreed.
Show 5 quoted lines
> 3. If a user fetches with --refilter applying a more restrictive filter > than previously (eg: blob:limit=1m then blob:limit=1k) the eventual > state is a no-op, since any referenced object already in the local > repository is never removed. Potentially this could be improved in > future by more advanced gc, possibly along the lines discussed at [2].
That's true.