builtin/pack-objects.c: support `--max-pack-size` with `--cruft`
When pack-objects learned the `--cruft` option back in b757353676 (builtin/pack-objects.c: --cruft without expiration, 2022-05-20), we explicitly forbade `--cruft` with `--max-pack-size`. At the time, there was no specific rationale given in the patch for not supporting the `--max-pack-size` option with `--cruft`. (As best I can remember, it's because we were trying to push users towards only ever having a single cruft pack, but I cannot be sure). However, `--max-pack-size` is flexible enough that it already works with `--cruft` and can shard unreachable objects across multiple cruft packs, creating separate ".mtimes" files as appropriate. In fact, the `--max-pack-size` option worked with `--cruft` as far back as b757353676! This is because we overwrite the `written_list`, and pass down the appropriate length, i.e. the number of objects written in each pack shard. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Aug 28, 2023 at 18:49 UTC
61568efa95608fdafffe67967a82e88bcd90fade
4 files changed
+48
-17
Documentation/git-pack-objects.txt
+1
-3
@@ -116,9 +116,7 @@ unreachable object whose mtime is newer than the `--cruft-expiration`).
116
+
117
Incompatible with `--unpack-unreachable`, `--keep-unreachable`,
118
`--pack-loose-unreachable`, `--stdin-packs`, as well as any other
119
-options which imply `--revs`. Also incompatible with `--max-pack-size`;
120
-when this option is set, the maximum pack size is not inferred from
121
-`pack.packSizeLimit`.
119
+options which imply `--revs`.
120
121
--cruft-expiration=<approxidate>::
122
If specified, objects are eliminated from the cruft pack if they
builtin/pack-objects.c
+1
-3
@@ -4382,7 +4382,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
4382
4383
if (!HAVE_THREADS && delta_search_threads != 1)
4384
warning(_("no threads support, ignoring --threads"));
4385
- if (!pack_to_stdout && !pack_size_limit && !cruft)
4385
+ if (!pack_to_stdout && !pack_size_limit)
4386
pack_size_limit = pack_size_limit_cfg;
4387
if (pack_to_stdout && pack_size_limit)
4388
die(_("--max-pack-size cannot be used to build a pack for transfer"));
@@ -4414,8 +4414,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
4414
die(_("cannot use internal rev list with --cruft"));
4415
if (stdin_packs)
4416
die(_("cannot use --stdin-packs with --cruft"));
4417
- if (pack_size_limit)
4418
- die(_("cannot use --max-pack-size with --cruft"));
4417
}
4418
4419
/*
builtin/repack.c
+2
-1
@@ -720,7 +720,6 @@ static int write_cruft_pack(const struct pack_objects_args *args,
720
721
strvec_push(&cmd.args, "--honor-pack-keep");
722
strvec_push(&cmd.args, "--non-empty");
723
- strvec_push(&cmd.args, "--max-pack-size=0");
723
724
cmd.in = -1;
725
@@ -1048,6 +1047,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1047
cruft_po_args.depth = po_args.depth;
1048
if (!cruft_po_args.threads)
1049
cruft_po_args.threads = po_args.threads;
1050
+ if (!cruft_po_args.max_pack_size)
1051
+ cruft_po_args.max_pack_size = po_args.max_pack_size;
1052
1053
cruft_po_args.local = po_args.local;
1054
cruft_po_args.quiet = po_args.quiet;
t/t5329-pack-objects-cruft.sh
+44
-10
@@ -573,23 +573,54 @@ test_expect_success 'cruft repack with no reachable objects' '
573
)
574
'
575
576
-test_expect_success 'cruft repack ignores --max-pack-size' '
576
+write_blob () {
577
+ test-tool genrandom "$@" >in &&
578
+ git hash-object -w -t blob in
579
+}
580
+
581
+find_pack () {
582
+ for idx in $(ls $packdir/pack-*.idx)
583
+ do
584
+ git show-index <$idx >out &&
585
+ if grep -q "$1" out
586
+ then
587
+ echo $idx
588
+ fi || return 1
589
+ done
590
+}
591
+
592
+test_expect_success 'cruft repack with --max-pack-size' '
593
git init max-pack-size &&
594
(
595
cd max-pack-size &&
596
test_commit base &&
597
+
598
# two cruft objects which exceed the maximum pack size
582
- test-tool genrandom foo 1048576 | git hash-object --stdin -w &&
583
- test-tool genrandom bar 1048576 | git hash-object --stdin -w &&
599
+ foo=$(write_blob foo 1048576) &&
600
+ bar=$(write_blob bar 1048576) &&
601
+ test-tool chmtime --get -1000 \
602
+ "$objdir/$(test_oid_to_path $foo)" >foo.mtime &&
603
+ test-tool chmtime --get -2000 \
604
+ "$objdir/$(test_oid_to_path $bar)" >bar.mtime &&
605
git repack --cruft --max-pack-size=1M &&
606
find $packdir -name "*.mtimes" >cruft &&
586
- test_line_count = 1 cruft &&
587
- test-tool pack-mtimes "$(basename "$(cat cruft)")" >objects &&
588
- test_line_count = 2 objects
607
+ test_line_count = 2 cruft &&
608
+
609
+ foo_mtimes="$(basename $(find_pack $foo) .idx).mtimes" &&
610
+ bar_mtimes="$(basename $(find_pack $bar) .idx).mtimes" &&
611
+ test-tool pack-mtimes $foo_mtimes >foo.actual &&
612
+ test-tool pack-mtimes $bar_mtimes >bar.actual &&
613
+
614
+ echo "$foo $(cat foo.mtime)" >foo.expect &&
615
+ echo "$bar $(cat bar.mtime)" >bar.expect &&
616
+
617
+ test_cmp foo.expect foo.actual &&
618
+ test_cmp bar.expect bar.actual &&
619
+ test "$foo_mtimes" != "$bar_mtimes"
620
)
621
'
622
592
-test_expect_success 'cruft repack ignores pack.packSizeLimit' '
623
+test_expect_success 'cruft repack with pack.packSizeLimit' '
624
(
625
cd max-pack-size &&
626
# repack everything back together to remove the existing cruft
@@ -599,9 +630,12 @@ test_expect_success 'cruft repack ignores pack.packSizeLimit' '
630
# ensure the same post condition is met when --max-pack-size
631
# would otherwise be inferred from the configuration
632
find $packdir -name "*.mtimes" >cruft &&
602
- test_line_count = 1 cruft &&
603
- test-tool pack-mtimes "$(basename "$(cat cruft)")" >objects &&
604
- test_line_count = 2 objects
633
+ test_line_count = 2 cruft &&
634
+ for pack in $(cat cruft)
635
+ do
636
+ test-tool pack-mtimes "$(basename $pack)" >objects &&
637
+ test_line_count = 1 objects || return 1
638
+ done
639
)
640
'
641