Re: [PATCH 08/11] t5332: enable OFS_DELTAs via test_pack_objects_reused

[Taylor Blau <me@ttaylorr.com>]

On Fri, Oct 11, 2024 at 04:19:39AM -0400, Jeff King wrote:
> On Wed, Oct 09, 2024 at 04:31:24PM -0400, Taylor Blau wrote:
>
> > Back when test_pack_objects_reused was introduced via commit
> > 7c01878eeb (t5332-multi-pack-reuse.sh: extract pack-objects helper
> > functions, 2024-02-05), we converted bare pack-objects invocations
> > into one of two wrapped variants, either test_pack_objects_reused or
> > test_pack_objects_reused_all.
> >
> > The latter passes `--delta-base-offset`, allowing pack-objects to
> > generate OFS_DELTAs in its output pack. But the former does not, for
> > no good reason.
> >
> > As we do not want to convert OFS_DELTAs to REF_DELTAs unnecessarily,
> > let's unify these two and pass `--delta-base-offset` to both.
>
> I think that matches what happens in the real world. I am puzzled that
> your BUG() instrumenting turns up some conversion cases. Why are we
> still converting in those cases?

These are cases where we're calling pack-objects directly without
passing the --delta-base-offset flag, so all deltas get converted into
REF_DELTAs.

> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index 0fc0680b402..0f1b22b8674 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
>
> You need to indent or otherwise comment-out this diff. Otherwise "git
> am" will pick it up as the start of the actual diff, adding the bogus
> BUG() call to the applied patch (and dropping the rest of your commit
> message).

Oops, of course. Thanks for spotting again.

Thanks,
Taylor