Re: [GSoC PATCH v2 0/7] repack: add --drop-filtered to reclaim space in partial clones

Hi,
thanks for the review Junio!

On Fri, 31 Jul 2026 at 21:04, Junio C Hamano <[email protected]> wrote:
> By 'by construction', do you mean 'It is guaranteed recoverable, as
> long as ODB_FOR_EACH_OBJECT_PROMISOR_ONLY is working correctly'?

yes, that is what I meant. the object is recoverable because it came from a
promisor pack (due to a .promisor file), so the remote has promised to
give it back.
"by construction" means "recoverable as long as the promisor-only walk correctly
picks out promisor objects".

> Since I do not use it, I do not personally trust promisor-based
> traversal all that much, and it would be great if we could hear from
> other practitioners that this really works well.

i'll also be glad to hear from people who actually use partial clone
about whether
leaning on the promisor-only walk here is a good idea for now, until the
remote-object-info side of the cat-file protocol lands, which would let us
verify against the remote directly.

> This is sensible, as long as this repacking is done only with
> locally available data, without dynamically pulling in lazy objects
> from the promisor (which would defeat the whole point ;-)).

right, i made sure of that :)
enumeration passes OBJECT_INFO_SKIP_FETCH_OBJECT on
every object-info lookup, so it never triggers a lazy fetch.
The rebuild is local too: it only repacks promisor objects that are
already present.
I confirmed this by tracing a real drop and by moving the promisor remote away
entirely before a drop, it still completed, so it clearly did not need
the remote

> Presumably, this rebuilding is done without an extra traversal,
> driven instead by the list of enumerated promisor objects we
> constructed above (excluding the unwanted ones)?

not quite, there are two walks right now. First,
enumerate_promisor_blobs() walks the
promisor objects to figure out what to drop. Then
repack_promisor_objects() does its own
promisor-only walk to rebuild the pack, skipping anything in that drop set.
so the rebuild does use the drop set, but through a second walk, not by directly
reusing the first list.

> I wonder whether size is the only criterion we would want to use
> when choosing what to discard among objects we know the promisor can
> give us on-demand.  It is, of course, perfectly fine to make it the
> only condition in this first effort, but it would help to imagine
> what other criteria we might want in the future and how they would
> fit into the framework you establish with this series.  Ensuring
> that the framework is easily extensible with a future set of rules
> will keep us from painting ourselves into a corner.

true, i agree..
size (blob:limit) is the only rule for now, but its easy to imagine others:
how old an object is or when it was last used, its path, its type, or whether
its still reachable from the current branch. The design should handle
those without
much trouble. Enumeration builds a set of promisor objects, and then one step
narrows that set down to what we actually drop.
Right now that step is just the blob:limit filter. A new rule would
plug in at the same spot,
narrowing the same set, so the overall "list them, then pick what to drop" shape
would not change

> I assume you do not mean a race where an operation wants to write a
> blob, finds that an identical one that came from the promisor remote
> already exists locally, refrains from writing another copy, and the
> drop-filtered operation removes the blob at the right moment.
> Rather, you likely have in mind an operation that stops, gives
> control back to the user, and, while the user ponders the situation,
> the drop-filtered operation kicks in and removes the blobs involved
> in the operation in progress.  Am I reading you correctly?

um yes, the case i had in mind is the second one: an operation stops halfway,
hands control back to the user, and drop-filtered runs in that gap and
removes blobs the paused operation was using

> Even in either of these situations, I do not quite see why the
> safeguards are necessary.  The operation completes, or stays stopped
> in the middle.  The user's next move (whether they issue a new
> command after completion or resume the interrupted operation) will
> automatically lazy-refetch what the drop-filtered operation
> discarded as needed, will it not?

yes, you got that right. Since the objects are promised, whatever gets dropped
will just be  lazy-refetched when the user runs the next command or resumes the
operation.
the guards avoid immediately re-downloading something we just dropped,
(which we can call as some wasted work : )), and a network fetch in
the middle of,
say, resolving a merge.
The index guard is the same story- the blob it protects would just be
re-fetched by
the next command anyway.
So they are a convenience to avoid pointless re-fetching, not a
correctness measure.
I am happy to drop them or keep them clearly documented as just that,
whichever the list prefers.

Thanks,
Siddharth Shrimali