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