| Message ID | 9d81d890402f94f4126aedef0845d615d10455bc.1728505840.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pack-bitmap: convert offset to ref deltas where possible | expand |
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? > 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). -Peff
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 @@ -1051,6 +1051,8 @@ static void write_reused_pack_one(struct packed_git *reuse_packfile, uint32_t base_pos; struct object_id base_oid; + BUG("tested"); + if (offset_to_pack_pos(reuse_packfile, base_offset, &base_pos) < 0) die(_("expected object at offset %"PRIuMAX" " "in pack %s"), , and seeing what test(s) fail yields the following. Test Summary Report ------------------- t5326-multi-pack-bitmaps.sh (Wstat: 256 (exited 1) Tests: 357 Failed: 6) Failed tests: 46, 91, 167, 220, 265, 341 Non-zero exit status: 1 t5310-pack-bitmaps.sh (Wstat: 256 (exited 1) Tests: 227 Failed: 6) Failed tests: 46-47, 120-121, 197-198 Non-zero exit status: 1 t5327-multi-pack-bitmaps-rev.sh (Wstat: 256 (exited 1) Tests: 314 Failed: 6) Failed tests: 46, 91, 157, 203, 248, 314 Non-zero exit status: 1 So the OFS_DELTA to REF_DELTA conversion is still tested thoroughly in t5310, t5326, and t5327. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- t/t5332-multi-pack-reuse.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh index 955ea42769b..8bcb736c75a 100755 --- a/t/t5332-multi-pack-reuse.sh +++ b/t/t5332-multi-pack-reuse.sh @@ -43,7 +43,7 @@ test_pack_objects_reused_all () { test_pack_objects_reused () { : >trace2.txt && GIT_TRACE2_EVENT="$PWD/trace2.txt" \ - git pack-objects --stdout --revs >got.pack && + git pack-objects --delta-base-offset --stdout --revs >got.pack && test_pack_reused "$1" <trace2.txt && test_packs_reused "$2" <trace2.txt &&