Re: [PATCH v2 3/8] pack-bitmap: allow aborting iteration of bitmapped objects

[Taylor Blau <ttaylorr@openai.com>]

On Mon, Jul 13, 2026 at 11:53:50AM +0200, Patrick Steinhardt wrote:
> On Sat, Jul 11, 2026 at 04:01:14AM -0400, Jeff King wrote:
> > On Fri, Jul 10, 2026 at 03:34:53PM -0700, Taylor Blau wrote:
> >
> > > However, the remaining `show_objects_for_type()` callers from within
> > > `traverse_bitmap_commit_list()` do *not* bother to inspect the return
> > > value, despite taking in an arbitrary 'show_reachable_fn', which itself
> > > may return a non-zero value.
> > >
> > > I guess this must be effectively OK in practice with respect to the
> > > existing code for the same reason you indicate in the commit message
> > > above, but we should change this function to *also* propagate non-zero
> > > return values to eliminate the foot-gun completely.
> >
> > The matching non-bitmap traverse_commit_list() does not allow aborting
> > based on callback returns, either. In fact, its callbacks return void!
> >
> > Whichever direction we go, those two should probably stay in sync (so
> > either both should allow aborting early with a non-zero return, or both
> > should return void).
>
> That's fair. But adapting `traverse_commit_list()` requires tons of
> changes all over the tree, so I'm inclined to rather leave both
> `traverse_bitmap_commit_list()` and `traverse_commit_list()` as-is.
> Does that work for both of you?

I think that it's fine to leave it as-is for the purpose of this series,
though I would like to address it.

I don't think we need to adapt `traverse_commit_list()`, though. We can
go in the other direction Peff suggested, which would be to split the
callback type used by `for_each_bitmapped_object()` from
`show_reachable_fn`, keep the former abortable, and make the latter
return void.

That keeps `traverse_bitmap_commit_list()` in sync with
`traverse_commit_list()` without changing the non-bitmap traversal
machinery. I have a small two-patch follow-up on top of v3 that does
this, which I'll send separately.

Thanks,
Taylor