{"thread":{"id":"63147","subject":"[PATCH 0/5] repack: introduce '--combine-cruft-below-size'","startedAt":"2025-03-17T23:00:15Z","lastAt":"2025-03-20T04:30:36Z","messageCount":19,"participants":["Taylor Blau","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"514458","messageId":"cover.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":null,"subject":"[PATCH 0/5] repack: introduce '--combine-cruft-below-size'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:12Z","receivedAt":"2025-03-17T23:00:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"(This is based on tb/multi-cruft-pack-refresh-fix from Junio's tree,\nwhich is at 08f612ba70 (builtin/pack-objects.c: freshen objects from\nexisting cruft packs, 2025-03-13) at the time of writing).\n\nThis series replaces something close to the existing behavior of\nrepack's '--max-cruft-size' flag with '--combine-cruft-below-size'.\n\nThe new flag is much clearer in its intent and function, and avoids the\nlack of clarity between the two that was discussed in\n\n \n\nThe new behavior is as follows:\n\n - '--max-cruft-size' is a cruft pack-specific override for repack's\n '--max-pack-size' command-line flag.\n\n - '--combine-cruft-below-size' instructs repack to only combine cruft\n packs which are smaller than the given threshold. This will likely\n result in packs which are larger than the threshold. But that is OK:\n the point is to limit the size of the individual packs on input, not\n the size of the outgoing pack.\n\nThis series does break the existing behavior of '--max-cruft-size'. But\nI think breaking backwards compatibility here is OK, since the existing\nbehavior was so broken to begin with. I'm happy to consider other\nalternatives if others feel that this isn't OK.\n\nThe series has an interesting structure that I feel may be worth calling\nout. The first three patches are trivial test cleanups. The fourth patch\nmodifies the existing behavior of '--max-cruft-size', but does so while\nkeeping some of the old infrastructure around.\n\nThat may seem like an unnecessarily complicated approach, but it greatly\nsimplifies introducing the new behavior in the following (and final)\ncommit. I think that this results in a series that is a little easier to\nreview (since we don't see a ton of code being removed in one commit and\nthen re-added in another immediately following it). But if others feel\nlike this was a mistake, please let me know ;-).\n\nThanks in advance for your review!\n\nTaylor Blau (5):\n t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests\n t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests\n t/t7704-repack-cruft.sh: consolidate `write_blob()`\n repack: avoid combining cruft packs with `--max-cruft-size`\n repack: begin combining cruft packs with `--combine-cruft-below-size`\n\n Documentation/git-repack.adoc | 20 ++-\n builtin/repack.c | 62 +++----\n t/t5329-pack-objects-cruft.sh | 302 ++++++----------------------------\n t/t7704-repack-cruft.sh | 293 ++++++++++++++++++++++++++++++---\n 4 files changed, 354 insertions(+), 323 deletions(-)\n\n\nbase-commit: 08f612ba7000bf181ef6d8baed9ece322e567efd\n-- \n2.49.0.rc0.6.g7f120c35e9\n"},{"id":"514459","messageId":"0aa8aa65c130fd62d38b4944c1ce2a97451b0064.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH 1/5] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:16Z","receivedAt":"2025-03-17T23:00:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The cruft pack feature has two primary test scripts which exercise\nvarious parts of it, which are:\n\n - t5329-pack-objects-cruft.sh\n - t7704-repack-cruft.sh\n\nThe former is designed to test low-level pack generation mechanics at\nthe 'git pack-objects --cruft'-level, which is plumbing. The latter, on\nthe other hand, is designed to test the user-facing behavior through\n'git repack --cruft', which is porcelain (under the \"ancillary\nmanipulators\" sub-section).\n\nAt some point a handful of tests which should have been added to the\nlatter script were instead written to the former. This isn't a huge\ndeal, but rectifying it is straightforward. Move a handful of\n'repack'-related tests out of t5329 and into their rightful home in\nt7704.\n\nSigned-off-by: Taylor Blau \n---\n t/t5329-pack-objects-cruft.sh | 250 ----------------------------------\n t/t7704-repack-cruft.sh | 250 ++++++++++++++++++++++++++++++++++\n 2 files changed, 250 insertions(+), 250 deletions(-)\n\ndiff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh\nindex b71a0aef40..60dac8312d 100755\n--- a/t/t5329-pack-objects-cruft.sh\n+++ b/t/t5329-pack-objects-cruft.sh\n@@ -360,43 +360,6 @@ test_expect_success 'expired objects are pruned' '\n \t)\n '\n \n-test_expect_success 'repack --cruft generates a cruft pack' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit branch -M main &&\n-\t\tgit checkout --orphan other &&\n-\t\ttest_commit unreachable &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit tag -d unreachable &&\n-\t\t# objects are not cruft if they are contained in the reflogs\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\tgit rev-list --objects --all --no-object-names >reachable.raw &&\n-\t\tgit cat-file --batch-all-objects --batch-check=\"%(objectname)\" >objects &&\n-\t\tsort reachable &&\n-\t\tcomm -13 reachable objects >unreachable &&\n-\n-\t\tgit repack --cruft -d &&\n-\n-\t\tcruft=$(basename $(ls $packdir/pack-*.mtimes) .mtimes) &&\n-\t\tpack=$(basename $(ls $packdir/pack-*.pack | grep -v $cruft) .pack) &&\n-\n-\t\tgit show-index <$packdir/$pack.idx >actual.raw &&\n-\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n-\t\ttest_cmp reachable actual &&\n-\n-\t\tgit show-index <$packdir/$cruft.idx >actual.raw &&\n-\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n-\t\ttest_cmp unreachable actual\n-\t)\n-'\n-\n test_expect_success 'loose objects mtimes upsert others' '\n \tgit init repo &&\n \ttest_when_finished \"rm -fr repo\" &&\n@@ -470,219 +433,6 @@ test_expect_success 'expiring cruft objects with git gc' '\n \t)\n '\n \n-test_expect_success 'cruft packs are not included in geometric repack' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit repack -Ad &&\n-\t\tgit branch -M main &&\n-\n-\t\tgit checkout --orphan other &&\n-\t\ttest_commit cruft &&\n-\t\tgit repack -d &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit tag -d cruft &&\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\tgit repack --cruft &&\n-\n-\t\tfind $packdir -type f | sort >before &&\n-\t\tgit repack --geometric=2 -d &&\n-\t\tfind $packdir -type f | sort >after &&\n-\n-\t\ttest_cmp before after\n-\t)\n-'\n-\n-test_expect_success 'repack --geometric collects once-cruft objects' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit repack -Ad &&\n-\t\tgit branch -M main &&\n-\n-\t\tgit checkout --orphan other &&\n-\t\tgit rm -rf . &&\n-\t\ttest_commit --no-tag cruft &&\n-\t\tcruft=\"$(git rev-parse HEAD)\" &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\t# Pack the objects created in the previous step into a cruft\n-\t\t# pack. Intentionally leave loose copies of those objects\n-\t\t# around so we can pick them up in a subsequent --geometric\n-\t\t# reapack.\n-\t\tgit repack --cruft &&\n-\n-\t\t# Now make those objects reachable, and ensure that they are\n-\t\t# packed into the new pack created via a --geometric repack.\n-\t\tgit update-ref refs/heads/other $cruft &&\n-\n-\t\t# Without this object, the set of unpacked objects is exactly\n-\t\t# the set of objects already in the cruft pack. Tweak that set\n-\t\t# to ensure we do not overwrite the cruft pack entirely.\n-\t\ttest_commit reachable2 &&\n-\n-\t\tfind $packdir -name \"pack-*.idx\" | sort >before &&\n-\t\tgit repack --geometric=2 -d &&\n-\t\tfind $packdir -name \"pack-*.idx\" | sort >after &&\n-\n-\t\t{\n-\t\t\tgit rev-list --objects --no-object-names $cruft &&\n-\t\t\tgit rev-list --objects --no-object-names reachable..reachable2\n-\t\t} >want.raw &&\n-\t\tsort want.raw >want &&\n-\n-\t\tpack=$(comm -13 before after) &&\n-\t\tgit show-index <$pack >objects.raw &&\n-\n-\t\tcut -d\" \" -f2 objects.raw | sort >got &&\n-\n-\t\ttest_cmp want got\n-\t)\n-'\n-\n-test_expect_success 'cruft repack with no reachable objects' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\t\tgit repack -ad &&\n-\n-\t\tbase=\"$(git rev-parse base)\" &&\n-\n-\t\tgit for-each-ref --format=\"delete %(refname)\" >in &&\n-\t\tgit update-ref --stdin in &&\n-\tgit hash-object -w -t blob in\n-}\n-\n-find_pack () {\n-\tfor idx in $(ls $packdir/pack-*.idx)\n-\tdo\n-\t\tgit show-index <$idx >out &&\n-\t\tif grep -q \"$1\" out\n-\t\tthen\n-\t\t\techo $idx\n-\t\tfi || return 1\n-\tdone\n-}\n-\n-test_expect_success 'cruft repack with --max-pack-size' '\n-\tgit init max-pack-size &&\n-\t(\n-\t\tcd max-pack-size &&\n-\t\ttest_commit base &&\n-\n-\t\t# two cruft objects which exceed the maximum pack size\n-\t\tfoo=$(write_blob foo 1048576) &&\n-\t\tbar=$(write_blob bar 1048576) &&\n-\t\ttest-tool chmtime --get -1000 \\\n-\t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n-\t\ttest-tool chmtime --get -2000 \\\n-\t\t\t\"$objdir/$(test_oid_to_path $bar)\" >bar.mtime &&\n-\t\tgit repack --cruft --max-pack-size=1M &&\n-\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n-\t\ttest_line_count = 2 cruft &&\n-\n-\t\tfoo_mtimes=\"$(basename $(find_pack $foo) .idx).mtimes\" &&\n-\t\tbar_mtimes=\"$(basename $(find_pack $bar) .idx).mtimes\" &&\n-\t\ttest-tool pack-mtimes $foo_mtimes >foo.actual &&\n-\t\ttest-tool pack-mtimes $bar_mtimes >bar.actual &&\n-\n-\t\techo \"$foo $(cat foo.mtime)\" >foo.expect &&\n-\t\techo \"$bar $(cat bar.mtime)\" >bar.expect &&\n-\n-\t\ttest_cmp foo.expect foo.actual &&\n-\t\ttest_cmp bar.expect bar.actual &&\n-\t\ttest \"$foo_mtimes\" != \"$bar_mtimes\"\n-\t)\n-'\n-\n-test_expect_success 'cruft repack with pack.packSizeLimit' '\n-\t(\n-\t\tcd max-pack-size &&\n-\t\t# repack everything back together to remove the existing cruft\n-\t\t# pack (but to keep its objects)\n-\t\tgit repack -adk &&\n-\t\tgit -c pack.packSizeLimit=1M repack --cruft &&\n-\t\t# ensure the same post condition is met when --max-pack-size\n-\t\t# would otherwise be inferred from the configuration\n-\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n-\t\ttest_line_count = 2 cruft &&\n-\t\tfor pack in $(cat cruft)\n-\t\tdo\n-\t\t\ttest-tool pack-mtimes \"$(basename $pack)\" >objects &&\n-\t\t\ttest_line_count = 1 objects || return 1\n-\t\tdone\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects repack.cruftWindow' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\n-\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n-\t\tgit -c pack.window=1 -c repack.cruftWindow=2 repack \\\n-\t\t --cruft --window=3 &&\n-\n-\t\tgrep \"pack-objects.*--window=2.*--cruft\" event.trace\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects --window by default' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\n-\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n-\t\tgit -c pack.window=2 repack --cruft --window=3 &&\n-\n-\t\tgrep \"pack-objects.*--window=3.*--cruft\" event.trace\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects --quiet' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\t\tGIT_PROGRESS_DELAY=0 git repack --cruft --quiet 2>err &&\n-\t\ttest_must_be_empty err\n-\t)\n-'\n-\n test_expect_success 'cruft --local drops unreachable objects' '\n \tgit init alternate &&\n \tgit init repo &&\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 43d2947d28..cd452040ea 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -477,4 +477,254 @@ test_expect_success 'reachable packs are preferred over cruft ones' '\n \t)\n '\n \n+test_expect_success 'repack --cruft generates a cruft pack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit branch -M main &&\n+\t\tgit checkout --orphan other &&\n+\t\ttest_commit unreachable &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit tag -d unreachable &&\n+\t\t# objects are not cruft if they are contained in the reflogs\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\tgit rev-list --objects --all --no-object-names >reachable.raw &&\n+\t\tgit cat-file --batch-all-objects --batch-check=\"%(objectname)\" >objects &&\n+\t\tsort reachable &&\n+\t\tcomm -13 reachable objects >unreachable &&\n+\n+\t\tgit repack --cruft -d &&\n+\n+\t\tcruft=$(basename $(ls $packdir/pack-*.mtimes) .mtimes) &&\n+\t\tpack=$(basename $(ls $packdir/pack-*.pack | grep -v $cruft) .pack) &&\n+\n+\t\tgit show-index <$packdir/$pack.idx >actual.raw &&\n+\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n+\t\ttest_cmp reachable actual &&\n+\n+\t\tgit show-index <$packdir/$cruft.idx >actual.raw &&\n+\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n+\t\ttest_cmp unreachable actual\n+\t)\n+'\n+\n+test_expect_success 'cruft packs are not included in geometric repack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit repack -Ad &&\n+\t\tgit branch -M main &&\n+\n+\t\tgit checkout --orphan other &&\n+\t\ttest_commit cruft &&\n+\t\tgit repack -d &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit tag -d cruft &&\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\tgit repack --cruft &&\n+\n+\t\tfind $packdir -type f | sort >before &&\n+\t\tgit repack --geometric=2 -d &&\n+\t\tfind $packdir -type f | sort >after &&\n+\n+\t\ttest_cmp before after\n+\t)\n+'\n+\n+test_expect_success 'repack --geometric collects once-cruft objects' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit repack -Ad &&\n+\t\tgit branch -M main &&\n+\n+\t\tgit checkout --orphan other &&\n+\t\tgit rm -rf . &&\n+\t\ttest_commit --no-tag cruft &&\n+\t\tcruft=\"$(git rev-parse HEAD)\" &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\t# Pack the objects created in the previous step into a cruft\n+\t\t# pack. Intentionally leave loose copies of those objects\n+\t\t# around so we can pick them up in a subsequent --geometric\n+\t\t# reapack.\n+\t\tgit repack --cruft &&\n+\n+\t\t# Now make those objects reachable, and ensure that they are\n+\t\t# packed into the new pack created via a --geometric repack.\n+\t\tgit update-ref refs/heads/other $cruft &&\n+\n+\t\t# Without this object, the set of unpacked objects is exactly\n+\t\t# the set of objects already in the cruft pack. Tweak that set\n+\t\t# to ensure we do not overwrite the cruft pack entirely.\n+\t\ttest_commit reachable2 &&\n+\n+\t\tfind $packdir -name \"pack-*.idx\" | sort >before &&\n+\t\tgit repack --geometric=2 -d &&\n+\t\tfind $packdir -name \"pack-*.idx\" | sort >after &&\n+\n+\t\t{\n+\t\t\tgit rev-list --objects --no-object-names $cruft &&\n+\t\t\tgit rev-list --objects --no-object-names reachable..reachable2\n+\t\t} >want.raw &&\n+\t\tsort want.raw >want &&\n+\n+\t\tpack=$(comm -13 before after) &&\n+\t\tgit show-index <$pack >objects.raw &&\n+\n+\t\tcut -d\" \" -f2 objects.raw | sort >got &&\n+\n+\t\ttest_cmp want got\n+\t)\n+'\n+\n+test_expect_success 'cruft repack with no reachable objects' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\tgit repack -ad &&\n+\n+\t\tbase=\"$(git rev-parse base)\" &&\n+\n+\t\tgit for-each-ref --format=\"delete %(refname)\" >in &&\n+\t\tgit update-ref --stdin in &&\n+\tgit hash-object -w -t blob in\n+}\n+\n+find_pack () {\n+\tfor idx in $(ls $packdir/pack-*.idx)\n+\tdo\n+\t\tgit show-index <$idx >out &&\n+\t\tif grep -q \"$1\" out\n+\t\tthen\n+\t\t\techo $idx\n+\t\tfi || return 1\n+\tdone\n+}\n+\n+test_expect_success 'cruft repack with --max-pack-size' '\n+\tgit init max-pack-size &&\n+\t(\n+\t\tcd max-pack-size &&\n+\t\ttest_commit base &&\n+\n+\t\t# two cruft objects which exceed the maximum pack size\n+\t\tfoo=$(write_blob foo 1048576) &&\n+\t\tbar=$(write_blob bar 1048576) &&\n+\t\ttest-tool chmtime --get -1000 \\\n+\t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n+\t\ttest-tool chmtime --get -2000 \\\n+\t\t\t\"$objdir/$(test_oid_to_path $bar)\" >bar.mtime &&\n+\t\tgit repack --cruft --max-pack-size=1M &&\n+\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n+\t\ttest_line_count = 2 cruft &&\n+\n+\t\tfoo_mtimes=\"$(basename $(find_pack $foo) .idx).mtimes\" &&\n+\t\tbar_mtimes=\"$(basename $(find_pack $bar) .idx).mtimes\" &&\n+\t\ttest-tool pack-mtimes $foo_mtimes >foo.actual &&\n+\t\ttest-tool pack-mtimes $bar_mtimes >bar.actual &&\n+\n+\t\techo \"$foo $(cat foo.mtime)\" >foo.expect &&\n+\t\techo \"$bar $(cat bar.mtime)\" >bar.expect &&\n+\n+\t\ttest_cmp foo.expect foo.actual &&\n+\t\ttest_cmp bar.expect bar.actual &&\n+\t\ttest \"$foo_mtimes\" != \"$bar_mtimes\"\n+\t)\n+'\n+\n+test_expect_success 'cruft repack with pack.packSizeLimit' '\n+\t(\n+\t\tcd max-pack-size &&\n+\t\t# repack everything back together to remove the existing cruft\n+\t\t# pack (but to keep its objects)\n+\t\tgit repack -adk &&\n+\t\tgit -c pack.packSizeLimit=1M repack --cruft &&\n+\t\t# ensure the same post condition is met when --max-pack-size\n+\t\t# would otherwise be inferred from the configuration\n+\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n+\t\ttest_line_count = 2 cruft &&\n+\t\tfor pack in $(cat cruft)\n+\t\tdo\n+\t\t\ttest-tool pack-mtimes \"$(basename $pack)\" >objects &&\n+\t\t\ttest_line_count = 1 objects || return 1\n+\t\tdone\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects repack.cruftWindow' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\n+\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n+\t\tgit -c pack.window=1 -c repack.cruftWindow=2 repack \\\n+\t\t --cruft --window=3 &&\n+\n+\t\tgrep \"pack-objects.*--window=2.*--cruft\" event.trace\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects --window by default' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\n+\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n+\t\tgit -c pack.window=2 repack --cruft --window=3 &&\n+\n+\t\tgrep \"pack-objects.*--window=3.*--cruft\" event.trace\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects --quiet' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\tGIT_PROGRESS_DELAY=0 git repack --cruft --quiet 2>err &&\n+\t\ttest_must_be_empty err\n+\t)\n+'\n+\n test_done\n-- \n2.49.0.rc0.6.g7f120c35e9\n\n"},{"id":"514460","messageId":"5e8bd3e66e304f42d4e84ec2aa9f06b82321d64b.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH 2/5] t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:19Z","receivedAt":"2025-03-17T23:00:22Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Now that a number of new tests have landed in t7704, make sure that they\nall make sense and are testing the things they say they are.\n\nThings are mostly OK, but a handful of tests needed tweaks. Those tweaks\nare as follows:\n\n - Use the terms \"too large\" or \"too small\" in tests that exercise the\n '--max-cruft-size' behavior. This has historically been treated as a\n threshold beneath which to combine cruft packs, but that will change\n in a subsequent commit. Prepare for that by using a more generic\n term.\n\n - Remove references to \"--max-cruft-size\" in the freshening tests.\n These tests provide coverage of our ability to record updated mtimes\n for objects already in cruft packs whose mtimes are upserted from\n various sources (loose objects, finding that object in a new pack,\n another cruft pack, etc.).\n\n These have nothing to do with the '--max-cruft-size' feature, and in\n fact none of the tests even *use* '--max-cruft-size'. Name them\n appropriately to make it clear that these tests exercise freshening\n behavior, not '--max-cruft-size' behavior.\n\nSigned-off-by: Taylor Blau \n---\n t/t7704-repack-cruft.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex cd452040ea..e6e4c2fad8 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -149,7 +149,7 @@ generate_cruft_pack () {\n \techo \"$packdir/pack-$pack.mtimes\"\n }\n \n-test_expect_success '--max-cruft-size creates new packs when above threshold' '\n+test_expect_success '--max-cruft-size creates new packs when too large' '\n \tgit init max-cruft-size-large &&\n \t(\n \t\tcd max-cruft-size-large &&\n@@ -173,7 +173,7 @@ test_expect_success '--max-cruft-size creates new packs when above threshold' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size combines existing packs when below threshold' '\n+test_expect_success '--max-cruft-size combines existing packs when not too large' '\n \tgit init max-cruft-size-small &&\n \t(\n \t\tcd max-cruft-size-small &&\n@@ -236,10 +236,10 @@ test_expect_success '--max-cruft-size combines smaller packs first' '\n \t)\n '\n \n-test_expect_success 'setup --max-cruft-size with freshened objects' '\n-\tgit init max-cruft-size-freshen &&\n+test_expect_success 'setup cruft with freshened objects' '\n+\tgit init cruft-freshen &&\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\ttest_commit base &&\n \t\tgit repack -ad &&\n@@ -257,9 +257,9 @@ test_expect_success 'setup --max-cruft-size with freshened objects' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (loose)' '\n+test_expect_success 'cruft with freshened objects (loose)' '\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\t# regenerate the object, setting its mtime to be more recent\n \t\tfoo=\"$(generate_random_blob foo 64)\" &&\n@@ -275,9 +275,9 @@ test_expect_success '--max-cruft-size with freshened objects (loose)' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (packed)' '\n+test_expect_success 'cruft with freshened objects (packed)' '\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\t# regenerate the object and store it in a packfile,\n \t\t# setting its mtime to be more recent\n@@ -304,7 +304,7 @@ test_expect_success '--max-cruft-size with freshened objects (packed)' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (previously cruft)' '\n+test_expect_success 'multi-cruft with freshened objects (previously cruft)' '\n \trepo=\"max-cruft-size-threshold\" &&\n \n \ttest_when_finished \"rm -fr $repo\" &&\n-- \n2.49.0.rc0.6.g7f120c35e9\n\n"},{"id":"514461","messageId":"b075ad86015131e4a83bd7bcd707ff2273b375b4.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH 3/5] t/t7704-repack-cruft.sh: consolidate `write_blob()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:22Z","receivedAt":"2025-03-17T23:00:24Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"A previous commit moved a handful of tests from a different script into\nt7704, including one that relies on generating random blobs.\n\nIncidentally, the original home of this test defined its own helper\n\"write_blob\" for doing so, which is identical in function to our\n\"generate_random_blob\" (and is slightly inferior to the latter, which\ncleans up after itself).\n\nRewrite the test that uses \"write_blob\" to no longer do so and then\nremove the function.\n\nSigned-off-by: Taylor Blau \n---\n t/t7704-repack-cruft.sh | 9 ++-------\n 1 file changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex e6e4c2fad8..3fd5aa6089 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -618,11 +618,6 @@ test_expect_success 'cruft repack with no reachable objects' '\n \t)\n '\n \n-write_blob () {\n-\ttest-tool genrandom \"$@\" >in &&\n-\tgit hash-object -w -t blob in\n-}\n-\n find_pack () {\n \tfor idx in $(ls $packdir/pack-*.idx)\n \tdo\n@@ -641,8 +636,8 @@ test_expect_success 'cruft repack with --max-pack-size' '\n \t\ttest_commit base &&\n \n \t\t# two cruft objects which exceed the maximum pack size\n-\t\tfoo=$(write_blob foo 1048576) &&\n-\t\tbar=$(write_blob bar 1048576) &&\n+\t\tfoo=$(generate_random_blob foo 1048576) &&\n+\t\tbar=$(generate_random_blob bar 1048576) &&\n \t\ttest-tool chmtime --get -1000 \\\n \t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n \t\ttest-tool chmtime --get -2000 \\\n-- \n2.49.0.rc0.6.g7f120c35e9\n\n"},{"id":"514462","messageId":"7941997e332828f0005acf97f925efc1e7251a33.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH 4/5] repack: avoid combining cruft packs with `--max-cruft-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:25Z","receivedAt":"2025-03-17T23:00:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In 37dc6d8104 (builtin/repack.c: implement support for\n`--max-cruft-size`, 2023-10-02), we exposed new functionality that\nallowed repositories to specify the behavior of when we should combine\nmultiple cruft packs together.\n\nThis feature was designed to ensure that we never repacked cruft packs\nwhich were larger than the given threshold in order to provide tighter\nI/O bounds for repositories that have many unreachable objects. In\nessence, specifying '--max-cruft-size=N' instructed 'repack' to\naggregate cruft packs together (in order of ascending size) until the\ncombine size grows past 'N', and then make a new cruft pack whose\ncontents includes the packs we rolled up.\n\nBut this isn't quite how it works in practice. Suppose for example that\nwe have two cruft packs which are each 100MiB in size. One might expect\nspecifying \"--max-cruft-size=200M\" would combine these two packs\ntogether, and then avoid repacking them until a pruning GC takes place.\nIn reality, 'repack' would try and aggregate these together, but writing\na pack that is strictly smaller than 200 MiB (since pack-objects'\n\"--max-pack-size\" provides a strict bound for packs containing more than\none object).\n\nSo instead we'll write out a pack that is, say, 199 MiB in size, and\nthen another 1 MiB pack containing the balance. If we later repack the\nrepository without adding any new unreachable objects, we'll repeat the\nsame exercise again, making the same 199 MiB and 1 MiB packs each time.\n\nThis happens because of a poor choice to bolt the '--max-cruft-size'\nfunctionality onto pack-objects' '--max-pack-size', forcing us to\ngenerate packs which are always smaller than the provided threshold and\nthus subject to repacking.\n\nThe following commit will introduce a new flag that implements something\nsimilar to the behavior above. Let's prepare for that by making repack's\n'--max-cruft-size' flag behave as an cruft pack-specific override for\n'--max-pack-size'.\n\nDo so by temporarily repurposing the 'collapse_small_cruft_packs()'\nfunction to instead generate a cruft pack using the same instructions as\nif we didn't specify any maximum pack size. The calling code looks\nsomething like:\n\n if (args->max_pack_size && !cruft_expiration) {\n collapse_small_cruft_packs(in, args->max_pack_size, existing);\n } else {\n for_each_string_list_item(item, &existing->non_kept_packs)\n fprintf(in, \"-%s.pack\\n\", item->string);\n for_each_string_list_item(item, &existing->cruft_packs)\n fprintf(in, \"-%s.pack\\n\", item->string);\n }\n\nThis patch makes collapse_small_cruft_packs() behave identically to the\n'else' arm of the conditional above. This repurposing of\n'collapse_small_cruft_packs()' is intentional, since it will set us up\nnicely to introduce the new behavior in the following commit.\n\nNaturally, there is some test fallout in the test which exercises the\nold meaning of '--max-cruft-size'. Mark that test as failing for now to\nbe dealt with in the following commit. Likewise, add a new test which\nexplicitly tests the behavior of '--max-cruft-size' to place a hard\nlimit on the size of any generated cruft pack(s).\n\nNote that this is a breaking change, as it alters the user-visible\nbehavior of '--max-cruft-size'. But I'm OK changing this behavior in\nthis instance, since the behavior wasn't accurate to begin with.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-repack.adoc | 12 ++------\n builtin/repack.c | 50 ++++++---------------------------\n t/t5329-pack-objects-cruft.sh | 52 +++++++++++++++++++++++++++++++++++\n t/t7704-repack-cruft.sh | 8 ++----\n 4 files changed, 67 insertions(+), 55 deletions(-)\n\ndiff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\nindex 5852a5c973..11db43b1c5 100644\n--- a/Documentation/git-repack.adoc\n+++ b/Documentation/git-repack.adoc\n@@ -77,15 +77,9 @@ to the new separate pack will be written.\n \tOnly useful with `--cruft -d`.\n \n --max-cruft-size=::\n-\tRepack cruft objects into packs as large as `` bytes before\n-\tcreating new packs. As long as there are enough cruft packs\n-\tsmaller than ``, repacking will cause a new cruft pack to\n-\tbe created containing objects from any combined cruft packs,\n-\talong with any new unreachable objects. Cruft packs larger than\n-\t`` will not be modified. When the new cruft pack is larger\n-\tthan `` bytes, it will be split into multiple packs, all of\n-\twhich are guaranteed to be at most `` bytes in size. Only\n-\tuseful with `--cruft -d`.\n+\tOverrides `--max-pack-size` for cruft packs. Inherits the value of\n+\t`--max-pack-size` (if any) by default. See the documentation for\n+\t`--max-pack-size` for more details.\n \n --expire-to=::\n \tWrite a cruft pack containing pruned objects (if any) to the\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 75e3752353..9658f6b354 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -1022,28 +1022,19 @@ static int write_filtered_pack(const struct pack_objects_args *args,\n \treturn finish_pack_objects_cmd(&cmd, names, local);\n }\n \n-static int existing_cruft_pack_cmp(const void *va, const void *vb)\n-{\n-\tstruct packed_git *a = *(struct packed_git **)va;\n-\tstruct packed_git *b = *(struct packed_git **)vb;\n-\n-\tif (a->pack_size < b->pack_size)\n-\t\treturn -1;\n-\tif (a->pack_size > b->pack_size)\n-\t\treturn 1;\n-\treturn 0;\n-}\n-\n-static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n+static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n \t\t\t\t struct existing_packs *existing)\n {\n-\tstruct packed_git **existing_cruft, *p;\n+\tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tsize_t total_size = 0;\n-\tsize_t existing_cruft_nr = 0;\n \tsize_t i;\n \n-\tALLOC_ARRAY(existing_cruft, existing->cruft_packs.nr);\n+\t/*\n+\t * Squelch a -Wunused-function warning while we rationalize\n+\t * the behavior of --max-cruft-size. This function will become\n+\t * used again in a future commit.\n+\t */\n+\t(void)retain_cruft_pack;\n \n \tfor (p = get_all_packs(the_repository); p; p = p->next) {\n \t\tif (!(p->is_cruft && p->pack_local))\n@@ -1056,29 +1047,7 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n \t\tif (!string_list_has_string(&existing->cruft_packs, buf.buf))\n \t\t\tcontinue;\n \n-\t\tif (existing_cruft_nr >= existing->cruft_packs.nr)\n-\t\t\tBUG(\"too many cruft packs (found %\"PRIuMAX\", but knew \"\n-\t\t\t \"of %\"PRIuMAX\")\",\n-\t\t\t (uintmax_t)existing_cruft_nr + 1,\n-\t\t\t (uintmax_t)existing->cruft_packs.nr);\n-\t\texisting_cruft[existing_cruft_nr++] = p;\n-\t}\n-\n-\tQSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);\n-\n-\tfor (i = 0; i < existing_cruft_nr; i++) {\n-\t\tsize_t proposed;\n-\n-\t\tp = existing_cruft[i];\n-\t\tproposed = st_add(total_size, p->pack_size);\n-\n-\t\tif (proposed <= max_size) {\n-\t\t\ttotal_size = proposed;\n-\t\t\tfprintf(in, \"-%s\\n\", pack_basename(p));\n-\t\t} else {\n-\t\t\tretain_cruft_pack(existing, p);\n-\t\t\tfprintf(in, \"%s\\n\", pack_basename(p));\n-\t\t}\n+\t\tfprintf(in, \"-%s.pack\\n\", buf.buf);\n \t}\n \n \tfor (i = 0; i < existing->non_kept_packs.nr; i++)\n@@ -1086,7 +1055,6 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n \t\t\texisting->non_kept_packs.items[i].string);\n \n \tstrbuf_release(&buf);\n-\tfree(existing_cruft);\n }\n \n static int write_cruft_pack(const struct pack_objects_args *args,\ndiff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh\nindex 60dac8312d..25ddda5cf3 100755\n--- a/t/t5329-pack-objects-cruft.sh\n+++ b/t/t5329-pack-objects-cruft.sh\n@@ -695,4 +695,56 @@ test_expect_success 'additional cruft blobs via gc.recentObjectsHook' '\n \t)\n '\n \n+test_expect_success 'split cruft packs with --max-cruft-size' '\n+\trepo=cruft-with--max-cruft-size &&\n+\ttest_when_finished \"rm -fr $repo\" &&\n+\n+\tgit init \"$repo\" &&\n+\n+\t(\n+\t\tcd \"$repo\" &&\n+\n+\t\tgit config core.compression 0 &&\n+\n+\t\tsz=$((1024 * 1024)) && # 1MiB\n+\t\ttest-tool genrandom foo $sz >foo &&\n+\t\ttest-tool genrandom bar $sz >bar &&\n+\t\tfoo=\"$(git hash-object -w -t blob foo)\" &&\n+\t\tbar=\"$(git hash-object -w -t blob bar)\" &&\n+\n+\t\tto=$packdir/pack &&\n+\t\t# Pack together foo and bar into a single 2MiB pack.\n+\t\tpack=\"$(git pack-objects $to <<-EOF\n+\t\t$foo\n+\t\t$bar\n+\t\tEOF\n+\t\t)\" &&\n+\n+\t\t# Then generate a cruft pack containing foo and bar.\n+\t\t#\n+\t\t# Generate the pack with --max-pack-size equal to the\n+\t\t# size of one object, forcing us to write two cruft\n+\t\t# packs.\n+\t\tgit pack-objects --cruft --max-pack-size=$sz $to <<-EOF &&\n+\t\t-pack-$pack.pack\n+\t\tEOF\n+\n+\t\tls $packdir/pack-*.mtimes >crufts &&\n+\t\ttest_line_count = 2 crufts &&\n+\n+\t\tfor cruft in $(cat crufts)\n+\t\tdo\n+\t\t\ttest-tool pack-mtimes \"$(basename \"$cruft\")\" || return 1\n+\t\tdone >actual.raw &&\n+\n+\t\tcut -d\" \" -f1 actual &&\n+\t\tsort >expect <<-EOF &&\n+\t\t$foo\n+\t\t$bar\n+\t\tEOF\n+\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 3fd5aa6089..6debad368d 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -194,7 +194,7 @@ test_expect_success '--max-cruft-size combines existing packs when not too large\n \t)\n '\n \n-test_expect_success '--max-cruft-size combines smaller packs first' '\n+test_expect_failure '--max-cruft-size combines smaller packs first' '\n \tgit init max-cruft-size-consume-small &&\n \t(\n \t\tcd max-cruft-size-consume-small &&\n@@ -354,13 +354,11 @@ test_expect_success 'multi-cruft with freshened objects (previously cruft)' '\n \t\tdone >actual.raw &&\n \t\tsort actual.raw >actual &&\n \n-\t\t# Among the set of all cruft packs, we should see both\n-\t\t# mtimes for object $foo and $bar, as well as the\n+\t\t# Among the set of all cruft packs, we should see the\n+\t\t# new mtimes for object $foo and $bar, as well as the\n \t\t# single new copy of $baz.\n \t\tsort >expect <<-EOF &&\n-\t\t$foo $(cat foo.old)\n \t\t$foo $(cat foo.new)\n-\t\t$bar $(cat bar.old)\n \t\t$bar $(cat bar.new)\n \t\t$baz $(cat baz.old)\n \t\t$quux $(cat quux.new)\n-- \n2.49.0.rc0.6.g7f120c35e9\n\n"},{"id":"514463","messageId":"7f120c35e95dcf41282c87dc2d1b2640ecdc5d84.1742252411.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-17T23:00:28Z","receivedAt":"2025-03-17T23:00:31Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The previous commit changed the behavior of repack's '--max-cruft-size'\nto specify a cruft pack-specific override for '--max-pack-size'.\n\nIntroduce a new flag, '--combine-cruft-below-size' which is a\nreplacement for the old behavior of '--max-cruft-size'. This new flag\ndoes explicitly what it says: it combines together cruft packs which are\nsmaller than a given threshold, and prohibits repacking ones which are\nlarger.\n\nThis accomplishes the original intent of '--max-cruft-size', which was\nto avoid repacking cruft packs larger than the given threshold.\n\nThe new behavior is slightly different. Instead of building up small\npacks together until the threshold is met, '--combine-cruft-below-size'\npacks up *all* cruft packs smaller than the threshold. This means that\nwe may make a pack much larger than the given threshold (e.g., if you\naggregate 5 packs which are each 99 MiB in size with a threshold of 100\nMiB).\n\nBut that's OK: the point isn't to restrict the size of the cruft packs\nwe generate, it's to avoid working with ones that have already grown too\nlarge. If repositories still want to limit the size of the generated\ncruft pack(s), they may use '--max-cruft-size' instead.\n\nThere's some minor test fallout as a result of the slight differences in\nbehavior between the old meaning of '--max-cruft-size' and the behavior\nof '--combine-cruft-below-size'. In the test which is now called\n\"--combine-cruft-below-size combines packs\", we need to use the new flag\nover the old one to exercise that test's intended behavior. The\nremainder of the changes there are to improve the clarity of the\ncomments.\n\nSuggested-by: Elijah Newren \nSigned-off-by: Taylor Blau \n---\n Documentation/git-repack.adoc | 8 ++++++++\n builtin/repack.c | 38 +++++++++++++++++++++++------------\n t/t7704-repack-cruft.sh | 22 +++++++++++---------\n 3 files changed, 46 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\nindex 11db43b1c5..8e6d61aa2f 100644\n--- a/Documentation/git-repack.adoc\n+++ b/Documentation/git-repack.adoc\n@@ -81,6 +81,14 @@ to the new separate pack will be written.\n \t`--max-pack-size` (if any) by default. See the documentation for\n \t`--max-pack-size` for more details.\n \n+--combine-cruft-below-size=::\n+\tWhen generating cruft packs without pruning, only repack\n+\texisting cruft packs whose size is strictly less than ``.\n+\tCruft packs whose size is greater than or equal to `` are\n+\tleft as-is and not repacked. Useful when you want to avoid\n+\trepacking large cruft pack(s) in repositories that have many\n+\tand/or large unreachable objects.\n+\n --expire-to=::\n \tWrite a cruft pack containing pruned objects (if any) to the\n \tdirectory ``. This option is useful for keeping a copy of\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 9658f6b354..f3330ade7b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -1022,20 +1022,13 @@ static int write_filtered_pack(const struct pack_objects_args *args,\n \treturn finish_pack_objects_cmd(&cmd, names, local);\n }\n \n-static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n-\t\t\t\t struct existing_packs *existing)\n+static void combine_small_cruft_packs(FILE *in, size_t combine_cruft_below_size,\n+\t\t\t\t struct existing_packs *existing)\n {\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t i;\n \n-\t/*\n-\t * Squelch a -Wunused-function warning while we rationalize\n-\t * the behavior of --max-cruft-size. This function will become\n-\t * used again in a future commit.\n-\t */\n-\t(void)retain_cruft_pack;\n-\n \tfor (p = get_all_packs(the_repository); p; p = p->next) {\n \t\tif (!(p->is_cruft && p->pack_local))\n \t\t\tcontinue;\n@@ -1047,7 +1040,12 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n \t\tif (!string_list_has_string(&existing->cruft_packs, buf.buf))\n \t\t\tcontinue;\n \n-\t\tfprintf(in, \"-%s.pack\\n\", buf.buf);\n+\t\tif (p->pack_size < combine_cruft_below_size) {\n+\t\t\tfprintf(in, \"-%s\\n\", pack_basename(p));\n+\t\t} else {\n+\t\t\tretain_cruft_pack(existing, p);\n+\t\t\tfprintf(in, \"%s\\n\", pack_basename(p));\n+\t\t}\n \t}\n \n \tfor (i = 0; i < existing->non_kept_packs.nr; i++)\n@@ -1061,6 +1059,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n \t\t\t const char *destination,\n \t\t\t const char *pack_prefix,\n \t\t\t const char *cruft_expiration,\n+\t\t\t unsigned long combine_cruft_below_size,\n \t\t\t struct string_list *names,\n \t\t\t struct existing_packs *existing)\n {\n@@ -1103,8 +1102,9 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n \tin = xfdopen(cmd.in, \"w\");\n \tfor_each_string_list_item(item, names)\n \t\tfprintf(in, \"%s-%s.pack\\n\", pack_prefix, item->string);\n-\tif (args->max_pack_size && !cruft_expiration) {\n-\t\tcollapse_small_cruft_packs(in, args->max_pack_size, existing);\n+\tif (combine_cruft_below_size && !cruft_expiration) {\n+\t\tcombine_small_cruft_packs(in, combine_cruft_below_size,\n+\t\t\t\t\t existing);\n \t} else {\n \t\tfor_each_string_list_item(item, &existing->non_kept_packs)\n \t\t\tfprintf(in, \"-%s.pack\\n\", item->string);\n@@ -1158,6 +1158,7 @@ int cmd_repack(int argc,\n \tconst char *opt_window_memory = NULL;\n \tconst char *opt_depth = NULL;\n \tconst char *opt_threads = NULL;\n+\tunsigned long combine_cruft_below_size = 0ul;\n \n \tstruct option builtin_repack_options[] = {\n \t\tOPT_BIT('a', NULL, &pack_everything,\n@@ -1170,6 +1171,9 @@ int cmd_repack(int argc,\n \t\t\t\t PACK_CRUFT),\n \t\tOPT_STRING(0, \"cruft-expiration\", &cruft_expiration, N_(\"approxidate\"),\n \t\t\t\tN_(\"with --cruft, expire objects older than this\")),\n+\t\tOPT_MAGNITUDE(0, \"combine-cruft-below-size\",\n+\t\t\t &combine_cruft_below_size,\n+\t\t\t N_(\"with --cruft, only repack cruft packs smaller than this\")),\n \t\tOPT_MAGNITUDE(0, \"max-cruft-size\", &cruft_po_args.max_pack_size,\n \t\t\t\tN_(\"with --cruft, limit the size of new cruft packs\")),\n \t\tOPT_BOOL('d', NULL, &delete_redundant,\n@@ -1413,7 +1417,8 @@ int cmd_repack(int argc,\n \t\tcruft_po_args.quiet = po_args.quiet;\n \n \t\tret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,\n-\t\t\t\t cruft_expiration, &names,\n+\t\t\t\t cruft_expiration,\n+\t\t\t\t combine_cruft_below_size, &names,\n \t\t\t\t &existing);\n \t\tif (ret)\n \t\t\tgoto cleanup;\n@@ -1440,10 +1445,17 @@ int cmd_repack(int argc,\n \t\t\t * generate an empty pack (since every object not in the\n \t\t\t * cruft pack generated above will have an mtime older\n \t\t\t * than the expiration).\n+\t\t\t *\n+\t\t\t * Pretend we don't have a `--combine-cruft-below-size`\n+\t\t\t * argument, since we're not selectively combining\n+\t\t\t * anything based on size to generate the limbo cruft\n+\t\t\t * pack, but rather removing all cruft packs from the\n+\t\t\t * main repository regardless of size.\n \t\t\t */\n \t\t\tret = write_cruft_pack(&cruft_po_args, expire_to,\n \t\t\t\t\t pack_prefix,\n \t\t\t\t\t NULL,\n+\t\t\t\t\t 0ul,\n \t\t\t\t\t &names,\n \t\t\t\t\t &existing);\n \t\t\tif (ret)\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 6debad368d..8aebfb45f5 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -194,10 +194,13 @@ test_expect_success '--max-cruft-size combines existing packs when not too large\n \t)\n '\n \n-test_expect_failure '--max-cruft-size combines smaller packs first' '\n-\tgit init max-cruft-size-consume-small &&\n+test_expect_success '--combine-cruft-below-size combines packs' '\n+\trepo=combine-cruft-below-size &&\n+\ttest_when_finished \"rm -fr $repo\" &&\n+\n+\tgit init \"$repo\" &&\n \t(\n-\t\tcd max-cruft-size-consume-small &&\n+\t\tcd \"$repo\" &&\n \n \t\ttest_commit base &&\n \t\tgit repack -ad &&\n@@ -211,11 +214,11 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n \t\ttest-tool pack-mtimes \"$(basename $cruft_bar)\" >>expect.raw &&\n \t\tsort expect.raw >expect.objects &&\n \n-\t\t# repacking with `--max-cruft-size=2M` should combine\n-\t\t# both 0.5 MiB packs together, instead of, say, one of\n-\t\t# the 0.5 MiB packs with the 1.0 MiB pack\n+\t\t# Repacking with `--combine-cruft-below-size=1M`\n+\t\t# should combine both 0.5 MiB packs together, but\n+\t\t# ignore the two packs which are >= 1.0 MiB.\n \t\tls $packdir/pack-*.mtimes | sort >cruft.before &&\n-\t\tgit repack -d --cruft --max-cruft-size=2M &&\n+\t\tgit repack -d --cruft --combine-cruft-below-size=1M &&\n \t\tls $packdir/pack-*.mtimes | sort >cruft.after &&\n \n \t\tcomm -13 cruft.before cruft.after >cruft.new &&\n@@ -224,11 +227,12 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n \t\ttest_line_count = 1 cruft.new &&\n \t\ttest_line_count = 2 cruft.removed &&\n \n-\t\t# the two smaller packs should be rolled up first\n+\t\t# The two packs smaller than 1.0MiB should be repacked\n+\t\t# together.\n \t\tprintf \"%s\\n\" $cruft_foo $cruft_bar | sort >expect.removed &&\n \t\ttest_cmp expect.removed cruft.removed &&\n \n-\t\t# ...and contain the set of objects rolled up\n+\t\t# ...and contain the set of objects rolled up.\n \t\ttest-tool pack-mtimes \"$(basename $(cat cruft.new))\" >actual.raw &&\n \t\tsort actual.raw >actual.objects &&\n \n-- \n2.49.0.rc0.6.g7f120c35e9\n"},{"id":"514535","messageId":"xmqqh63q47zd.fsf@gitster.g","threadId":"63147","inReplyTo":"7f120c35e95dcf41282c87dc2d1b2640ecdc5d84.1742252411.git.me@ttaylorr.com","subject":"Re: [PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-18T16:30:14Z","receivedAt":"2025-03-18T16:30:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> @@ -81,6 +81,14 @@ to the new separate pack will be written.\n> \t`--max-pack-size` (if any) by default. See the documentation for\n> \t`--max-pack-size` for more details.\n> \n> +--combine-cruft-below-size=::\n> +\tWhen generating cruft packs without pruning, only repack\n> +\texisting cruft packs whose size is strictly less than ``.\n> +\tCruft packs whose size is greater than or equal to `` are\n> +\tleft as-is and not repacked. Useful when you want to avoid\n> +\trepacking large cruft pack(s) in repositories that have many\n> +\tand/or large unreachable objects.\n> +\n\nShared with existing entries in this file, but let's strive to make\nsure we explicitly mention units. --max-cruft-size= is explained\nto cramp below '' bytes, which is great, --max-pack-size= says\nit accepts k/m/g suffixes and its minimum size is 1 MiB, which is\nexplicit enough hint that this is counted in bytes. This new entry\nshould hint that this is also counted in bytes.\n\n\n"},{"id":"514642","messageId":"CABPp-BF1ivo2+tR0-KnU-KF6omtswp-ZMWiBgWPmmgOaRu8RhQ@mail.gmail.com","threadId":"63147","inReplyTo":"0aa8aa65c130fd62d38b4944c1ce2a97451b0064.1742252411.git.me@ttaylorr.com","subject":"Re: [PATCH 1/5] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-03-19T14:20:05Z","receivedAt":"2025-03-19T14:20:17Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 17, 2025 at 4:00 PM Taylor Blau wrote:\n>\n> The cruft pack feature has two primary test scripts which exercise\n> various parts of it, which are:\n>\n> - t5329-pack-objects-cruft.sh\n> - t7704-repack-cruft.sh\n>\n> The former is designed to test low-level pack generation mechanics at\n> the 'git pack-objects --cruft'-level, which is plumbing. The latter, on\n> the other hand, is designed to test the user-facing behavior through\n> 'git repack --cruft', which is porcelain (under the \"ancillary\n> manipulators\" sub-section).\n>\n> At some point a handful of tests which should have been added to the\n> latter script were instead written to the former. This isn't a huge\n> deal, but rectifying it is straightforward. Move a handful of\n> 'repack'-related tests out of t5329 and into their rightful home in\n> t7704.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> t/t5329-pack-objects-cruft.sh | 250 ----------------------------------\n> t/t7704-repack-cruft.sh | 250 ++++++++++++++++++++++++++++++++++\n> 2 files changed, 250 insertions(+), 250 deletions(-)\n\nA quick view with --color-moved makes it easy to verify that the tests\nsimply moved.\n"},{"id":"514643","messageId":"CABPp-BHmFCHX85e4SrLpTshMVx2nbAkLhnZoZ5LRX15V8MEs8A@mail.gmail.com","threadId":"63147","inReplyTo":"5e8bd3e66e304f42d4e84ec2aa9f06b82321d64b.1742252411.git.me@ttaylorr.com","subject":"Re: [PATCH 2/5] t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-03-19T14:20:11Z","receivedAt":"2025-03-19T14:20:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 17, 2025 at 4:00 PM Taylor Blau wrote:\n>\n> Now that a number of new tests have landed in t7704, make sure that they\n> all make sense and are testing the things they say they are.\n>\n> Things are mostly OK, but a handful of tests needed tweaks. Those tweaks\n> are as follows:\n>\n> - Use the terms \"too large\" or \"too small\" in tests that exercise the\n> '--max-cruft-size' behavior. This has historically been treated as a\n> threshold beneath which to combine cruft packs, but that will change\n> in a subsequent commit. Prepare for that by using a more generic\n> term.\n>\n> - Remove references to \"--max-cruft-size\" in the freshening tests.\n> These tests provide coverage of our ability to record updated mtimes\n> for objects already in cruft packs whose mtimes are upserted from\n> various sources (loose objects, finding that object in a new pack,\n> another cruft pack, etc.).\n>\n> These have nothing to do with the '--max-cruft-size' feature, and in\n> fact none of the tests even *use* '--max-cruft-size'. Name them\n> appropriately to make it clear that these tests exercise freshening\n> behavior, not '--max-cruft-size' behavior.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> t/t7704-repack-cruft.sh | 20 ++++++++++----------\n> 1 file changed, 10 insertions(+), 10 deletions(-)\n>\n> diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\n> index cd452040ea..e6e4c2fad8 100755\n> --- a/t/t7704-repack-cruft.sh\n> +++ b/t/t7704-repack-cruft.sh\n> @@ -149,7 +149,7 @@ generate_cruft_pack () {\n> echo \"$packdir/pack-$pack.mtimes\"\n> }\n>\n> -test_expect_success '--max-cruft-size creates new packs when above threshold' '\n> +test_expect_success '--max-cruft-size creates new packs when too large' '\n> git init max-cruft-size-large &&\n> (\n> cd max-cruft-size-large &&\n> @@ -173,7 +173,7 @@ test_expect_success '--max-cruft-size creates new packs when above threshold' '\n> )\n> '\n>\n> -test_expect_success '--max-cruft-size combines existing packs when below threshold' '\n> +test_expect_success '--max-cruft-size combines existing packs when not too large' '\n> git init max-cruft-size-small &&\n> (\n> cd max-cruft-size-small &&\n> @@ -236,10 +236,10 @@ test_expect_success '--max-cruft-size combines smaller packs first' '\n> )\n> '\n>\n> -test_expect_success 'setup --max-cruft-size with freshened objects' '\n> - git init max-cruft-size-freshen &&\n> +test_expect_success 'setup cruft with freshened objects' '\n> + git init cruft-freshen &&\n> (\n> - cd max-cruft-size-freshen &&\n> + cd cruft-freshen &&\n>\n> test_commit base &&\n> git repack -ad &&\n> @@ -257,9 +257,9 @@ test_expect_success 'setup --max-cruft-size with freshened objects' '\n> )\n> '\n>\n> -test_expect_success '--max-cruft-size with freshened objects (loose)' '\n> +test_expect_success 'cruft with freshened objects (loose)' '\n> (\n> - cd max-cruft-size-freshen &&\n> + cd cruft-freshen &&\n>\n> # regenerate the object, setting its mtime to be more recent\n> foo=\"$(generate_random_blob foo 64)\" &&\n> @@ -275,9 +275,9 @@ test_expect_success '--max-cruft-size with freshened objects (loose)' '\n> )\n> '\n>\n> -test_expect_success '--max-cruft-size with freshened objects (packed)' '\n> +test_expect_success 'cruft with freshened objects (packed)' '\n> (\n> - cd max-cruft-size-freshen &&\n> + cd cruft-freshen &&\n>\n> # regenerate the object and store it in a packfile,\n> # setting its mtime to be more recent\n> @@ -304,7 +304,7 @@ test_expect_success '--max-cruft-size with freshened objects (packed)' '\n> )\n> '\n>\n> -test_expect_success '--max-cruft-size with freshened objects (previously cruft)' '\n> +test_expect_success 'multi-cruft with freshened objects (previously cruft)' '\n> repo=\"max-cruft-size-threshold\" &&\n>\n> test_when_finished \"rm -fr $repo\" &&\n> --\n> 2.49.0.rc0.6.g7f120c35e9\n\nUsing --color-words for this diff makes it easy to see the small\nwording clarifications; looks good.\n"},{"id":"514644","messageId":"CABPp-BFhZ1JGR_qWSgmcZm=Pix2n6z1AQ+-R-mw9Q_fWFi=_Ew@mail.gmail.com","threadId":"63147","inReplyTo":"7f120c35e95dcf41282c87dc2d1b2640ecdc5d84.1742252411.git.me@ttaylorr.com","subject":"Re: [PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-03-19T14:21:01Z","receivedAt":"2025-03-19T14:21:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 17, 2025 at 4:00 PM Taylor Blau wrote:\n>\n> The previous commit changed the behavior of repack's '--max-cruft-size'\n> to specify a cruft pack-specific override for '--max-pack-size'.\n>\n> Introduce a new flag, '--combine-cruft-below-size' which is a\n> replacement for the old behavior of '--max-cruft-size'. This new flag\n> does explicitly what it says: it combines together cruft packs which are\n> smaller than a given threshold, and prohibits repacking ones which are\n> larger.\n\nTo me \"prohibits\" suggests some kind of stronger action that\npotentially persists beyond the end of this operation. Perhaps this\ncould be reworded to something like\ns/prohibits repacking ones/leaves alone packs/ ?\n\n> This accomplishes the original intent of '--max-cruft-size', which was\n> to avoid repacking cruft packs larger than the given threshold.\n>\n> The new behavior is slightly different. Instead of building up small\n> packs together until the threshold is met, '--combine-cruft-below-size'\n> packs up *all* cruft packs smaller than the threshold. This means that\n> we may make a pack much larger than the given threshold (e.g., if you\n> aggregate 5 packs which are each 99 MiB in size with a threshold of 100\n> MiB).\n>\n> But that's OK: the point isn't to restrict the size of the cruft packs\n> we generate, it's to avoid working with ones that have already grown too\n> large. If repositories still want to limit the size of the generated\n> cruft pack(s), they may use '--max-cruft-size' instead.\n\n...but then they wouldn't get any cruft packs being combined. Did you\nmean s/instead/together with --combine-cruft-below-size/ ?\n\n> There's some minor test fallout as a result of the slight differences in\n> behavior between the old meaning of '--max-cruft-size' and the behavior\n> of '--combine-cruft-below-size'. In the test which is now called\n> \"--combine-cruft-below-size combines packs\", we need to use the new flag\n> over the old one to exercise that test's intended behavior. The\n> remainder of the changes there are to improve the clarity of the\n> comments.\n>\n> Suggested-by: Elijah Newren \n> Signed-off-by: Taylor Blau \n> ---\n> Documentation/git-repack.adoc | 8 ++++++++\n> builtin/repack.c | 38 +++++++++++++++++++++++------------\n> t/t7704-repack-cruft.sh | 22 +++++++++++---------\n> 3 files changed, 46 insertions(+), 22 deletions(-)\n>\n> diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\n> index 11db43b1c5..8e6d61aa2f 100644\n> --- a/Documentation/git-repack.adoc\n> +++ b/Documentation/git-repack.adoc\n> @@ -81,6 +81,14 @@ to the new separate pack will be written.\n> `--max-pack-size` (if any) by default. See the documentation for\n> `--max-pack-size` for more details.\n>\n> +--combine-cruft-below-size=::\n> + When generating cruft packs without pruning, only repack\n> + existing cruft packs whose size is strictly less than ``.\n> + Cruft packs whose size is greater than or equal to `` are\n> + left as-is and not repacked. Useful when you want to avoid\n> + repacking large cruft pack(s) in repositories that have many\n> + and/or large unreachable objects.\n> +\n\nDoes it make sense to modify the documentation for either the\n--max-cruft-szie or --combine-cruft-below-size options to suggest that\nif both are used, it is recommended to make --max-cruft-size twice (or\nmore) the value of --combine-cruft-below-size ?\n\n> --expire-to=::\n> Write a cruft pack containing pruned objects (if any) to the\n> directory ``. This option is useful for keeping a copy of\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index 9658f6b354..f3330ade7b 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -1022,20 +1022,13 @@ static int write_filtered_pack(const struct pack_objects_args *args,\n> return finish_pack_objects_cmd(&cmd, names, local);\n> }\n>\n> -static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n> - struct existing_packs *existing)\n> +static void combine_small_cruft_packs(FILE *in, size_t combine_cruft_below_size,\n> + struct existing_packs *existing)\n> {\n> struct packed_git *p;\n> struct strbuf buf = STRBUF_INIT;\n> size_t i;\n>\n> - /*\n> - * Squelch a -Wunused-function warning while we rationalize\n> - * the behavior of --max-cruft-size. This function will become\n> - * used again in a future commit.\n> - */\n> - (void)retain_cruft_pack;\n> -\n> for (p = get_all_packs(the_repository); p; p = p->next) {\n> if (!(p->is_cruft && p->pack_local))\n> continue;\n> @@ -1047,7 +1040,12 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n> if (!string_list_has_string(&existing->cruft_packs, buf.buf))\n> continue;\n>\n> - fprintf(in, \"-%s.pack\\n\", buf.buf);\n> + if (p->pack_size < combine_cruft_below_size) {\n> + fprintf(in, \"-%s\\n\", pack_basename(p));\n> + } else {\n> + retain_cruft_pack(existing, p);\n> + fprintf(in, \"%s\\n\", pack_basename(p));\n> + }\n> }\n>\n> for (i = 0; i < existing->non_kept_packs.nr; i++)\n> @@ -1061,6 +1059,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n> const char *destination,\n> const char *pack_prefix,\n> const char *cruft_expiration,\n> + unsigned long combine_cruft_below_size,\n> struct string_list *names,\n> struct existing_packs *existing)\n> {\n> @@ -1103,8 +1102,9 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n> in = xfdopen(cmd.in, \"w\");\n> for_each_string_list_item(item, names)\n> fprintf(in, \"%s-%s.pack\\n\", pack_prefix, item->string);\n> - if (args->max_pack_size && !cruft_expiration) {\n> - collapse_small_cruft_packs(in, args->max_pack_size, existing);\n> + if (combine_cruft_below_size && !cruft_expiration) {\n> + combine_small_cruft_packs(in, combine_cruft_below_size,\n> + existing);\n> } else {\n> for_each_string_list_item(item, &existing->non_kept_packs)\n> fprintf(in, \"-%s.pack\\n\", item->string);\n> @@ -1158,6 +1158,7 @@ int cmd_repack(int argc,\n> const char *opt_window_memory = NULL;\n> const char *opt_depth = NULL;\n> const char *opt_threads = NULL;\n> + unsigned long combine_cruft_below_size = 0ul;\n>\n> struct option builtin_repack_options[] = {\n> OPT_BIT('a', NULL, &pack_everything,\n> @@ -1170,6 +1171,9 @@ int cmd_repack(int argc,\n> PACK_CRUFT),\n> OPT_STRING(0, \"cruft-expiration\", &cruft_expiration, N_(\"approxidate\"),\n> N_(\"with --cruft, expire objects older than this\")),\n> + OPT_MAGNITUDE(0, \"combine-cruft-below-size\",\n> + &combine_cruft_below_size,\n> + N_(\"with --cruft, only repack cruft packs smaller than this\")),\n> OPT_MAGNITUDE(0, \"max-cruft-size\", &cruft_po_args.max_pack_size,\n> N_(\"with --cruft, limit the size of new cruft packs\")),\n> OPT_BOOL('d', NULL, &delete_redundant,\n> @@ -1413,7 +1417,8 @@ int cmd_repack(int argc,\n> cruft_po_args.quiet = po_args.quiet;\n>\n> ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,\n> - cruft_expiration, &names,\n> + cruft_expiration,\n> + combine_cruft_below_size, &names,\n> &existing);\n> if (ret)\n> goto cleanup;\n> @@ -1440,10 +1445,17 @@ int cmd_repack(int argc,\n> * generate an empty pack (since every object not in the\n> * cruft pack generated above will have an mtime older\n> * than the expiration).\n> + *\n> + * Pretend we don't have a `--combine-cruft-below-size`\n> + * argument, since we're not selectively combining\n> + * anything based on size to generate the limbo cruft\n> + * pack, but rather removing all cruft packs from the\n> + * main repository regardless of size.\n> */\n> ret = write_cruft_pack(&cruft_po_args, expire_to,\n> pack_prefix,\n> NULL,\n> + 0ul,\n> &names,\n> &existing);\n> if (ret)\n> diff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\n> index 6debad368d..8aebfb45f5 100755\n> --- a/t/t7704-repack-cruft.sh\n> +++ b/t/t7704-repack-cruft.sh\n> @@ -194,10 +194,13 @@ test_expect_success '--max-cruft-size combines existing packs when not too large\n> )\n> '\n>\n> -test_expect_failure '--max-cruft-size combines smaller packs first' '\n> - git init max-cruft-size-consume-small &&\n> +test_expect_success '--combine-cruft-below-size combines packs' '\n> + repo=combine-cruft-below-size &&\n> + test_when_finished \"rm -fr $repo\" &&\n> +\n> + git init \"$repo\" &&\n> (\n> - cd max-cruft-size-consume-small &&\n> + cd \"$repo\" &&\n>\n> test_commit base &&\n> git repack -ad &&\n> @@ -211,11 +214,11 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n> test-tool pack-mtimes \"$(basename $cruft_bar)\" >>expect.raw &&\n> sort expect.raw >expect.objects &&\n>\n> - # repacking with `--max-cruft-size=2M` should combine\n> - # both 0.5 MiB packs together, instead of, say, one of\n> - # the 0.5 MiB packs with the 1.0 MiB pack\n> + # Repacking with `--combine-cruft-below-size=1M`\n> + # should combine both 0.5 MiB packs together, but\n> + # ignore the two packs which are >= 1.0 MiB.\n> ls $packdir/pack-*.mtimes | sort >cruft.before &&\n> - git repack -d --cruft --max-cruft-size=2M &&\n> + git repack -d --cruft --combine-cruft-below-size=1M &&\n> ls $packdir/pack-*.mtimes | sort >cruft.after &&\n>\n> comm -13 cruft.before cruft.after >cruft.new &&\n> @@ -224,11 +227,12 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n> test_line_count = 1 cruft.new &&\n> test_line_count = 2 cruft.removed &&\n>\n> - # the two smaller packs should be rolled up first\n> + # The two packs smaller than 1.0MiB should be repacked\n> + # together.\n> printf \"%s\\n\" $cruft_foo $cruft_bar | sort >expect.removed &&\n> test_cmp expect.removed cruft.removed &&\n>\n> - # ...and contain the set of objects rolled up\n> + # ...and contain the set of objects rolled up.\n> test-tool pack-mtimes \"$(basename $(cat cruft.new))\" >actual.raw &&\n> sort actual.raw >actual.objects &&\n>\n> --\n> 2.49.0.rc0.6.g7f120c35e9\n\nThe rest looks good to me (although I reviewed a preview and you\naddressed that feedback, so perhaps not so surprising...)\n"},{"id":"514691","messageId":"Z9tH5zbZ/eWC1e6p@nand.local","threadId":"63147","inReplyTo":"xmqqh63q47zd.fsf@gitster.g","subject":"Re: [PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:40:39Z","receivedAt":"2025-03-19T22:40:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Mar 18, 2025 at 09:30:14AM -0700, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > @@ -81,6 +81,14 @@ to the new separate pack will be written.\n> > \t`--max-pack-size` (if any) by default. See the documentation for\n> > \t`--max-pack-size` for more details.\n> >\n> > +--combine-cruft-below-size=::\n> > +\tWhen generating cruft packs without pruning, only repack\n> > +\texisting cruft packs whose size is strictly less than ``.\n> > +\tCruft packs whose size is greater than or equal to `` are\n> > +\tleft as-is and not repacked. Useful when you want to avoid\n> > +\trepacking large cruft pack(s) in repositories that have many\n> > +\tand/or large unreachable objects.\n> > +\n>\n> Shared with existing entries in this file, but let's strive to make\n> sure we explicitly mention units. --max-cruft-size= is explained\n> to cramp below '' bytes, which is great, --max-pack-size= says\n> it accepts k/m/g suffixes and its minimum size is 1 MiB, which is\n> explicit enough hint that this is counted in bytes. This new entry\n> should hint that this is also counted in bytes.\n\nDefinitely an oversight on my part, I certainly agree with this\nsentiment.\n\nFWIW, '--max-cruft-size' no longer explicitly says \"in bytes\", but\nmostly because it (a) doesn't describe `` at all, because (b) it\nrefers readers to the documentation on `--max-pack-size`. No need to\ncover it twice there.\n\nThanks,\nTaylor\n"},{"id":"514693","messageId":"Z9tKJ47+3ffiB2fx@nand.local","threadId":"63147","inReplyTo":"CABPp-BFhZ1JGR_qWSgmcZm=Pix2n6z1AQ+-R-mw9Q_fWFi=_Ew@mail.gmail.com","subject":"Re: [PATCH 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:50:15Z","receivedAt":"2025-03-19T22:50:17Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Mar 19, 2025 at 07:21:01AM -0700, Elijah Newren wrote:\n> On Mon, Mar 17, 2025 at 4:00 PM Taylor Blau wrote:\n> >\n> > The previous commit changed the behavior of repack's '--max-cruft-size'\n> > to specify a cruft pack-specific override for '--max-pack-size'.\n> >\n> > Introduce a new flag, '--combine-cruft-below-size' which is a\n> > replacement for the old behavior of '--max-cruft-size'. This new flag\n> > does explicitly what it says: it combines together cruft packs which are\n> > smaller than a given threshold, and prohibits repacking ones which are\n> > larger.\n>\n> To me \"prohibits\" suggests some kind of stronger action that\n> potentially persists beyond the end of this operation. Perhaps this\n> could be reworded to something like\n> s/prohibits repacking ones/leaves alone packs/ ?\n\nFair enough, I went with \"leaves alone ones which are larger\".\n\n> > This accomplishes the original intent of '--max-cruft-size', which was\n> > to avoid repacking cruft packs larger than the given threshold.\n> >\n> > The new behavior is slightly different. Instead of building up small\n> > packs together until the threshold is met, '--combine-cruft-below-size'\n> > packs up *all* cruft packs smaller than the threshold. This means that\n> > we may make a pack much larger than the given threshold (e.g., if you\n> > aggregate 5 packs which are each 99 MiB in size with a threshold of 100\n> > MiB).\n> >\n> > But that's OK: the point isn't to restrict the size of the cruft packs\n> > we generate, it's to avoid working with ones that have already grown too\n> > large. If repositories still want to limit the size of the generated\n> > cruft pack(s), they may use '--max-cruft-size' instead.\n>\n> ...but then they wouldn't get any cruft packs being combined. Did you\n> mean s/instead/together with --combine-cruft-below-size/ ?\n\nOops, yeah; I meant to imply that '--max-cruft-size' was/is still an\noption, not that one excludes the other. I went for s/instead//.\n\n> > There's some minor test fallout as a result of the slight differences in\n> > behavior between the old meaning of '--max-cruft-size' and the behavior\n> > of '--combine-cruft-below-size'. In the test which is now called\n> > \"--combine-cruft-below-size combines packs\", we need to use the new flag\n> > over the old one to exercise that test's intended behavior. The\n> > remainder of the changes there are to improve the clarity of the\n> > comments.\n> >\n> > Suggested-by: Elijah Newren \n> > Signed-off-by: Taylor Blau \n> > ---\n> > Documentation/git-repack.adoc | 8 ++++++++\n> > builtin/repack.c | 38 +++++++++++++++++++++++------------\n> > t/t7704-repack-cruft.sh | 22 +++++++++++---------\n> > 3 files changed, 46 insertions(+), 22 deletions(-)\n> >\n> > diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\n> > index 11db43b1c5..8e6d61aa2f 100644\n> > --- a/Documentation/git-repack.adoc\n> > +++ b/Documentation/git-repack.adoc\n> > @@ -81,6 +81,14 @@ to the new separate pack will be written.\n> > `--max-pack-size` (if any) by default. See the documentation for\n> > `--max-pack-size` for more details.\n> >\n> > +--combine-cruft-below-size=::\n> > + When generating cruft packs without pruning, only repack\n> > + existing cruft packs whose size is strictly less than ``.\n> > + Cruft packs whose size is greater than or equal to `` are\n> > + left as-is and not repacked. Useful when you want to avoid\n> > + repacking large cruft pack(s) in repositories that have many\n> > + and/or large unreachable objects.\n> > +\n>\n> Does it make sense to modify the documentation for either the\n> --max-cruft-szie or --combine-cruft-below-size options to suggest that\n> if both are used, it is recommended to make --max-cruft-size twice (or\n> more) the value of --combine-cruft-below-size ?\n\nEhhh... I kind of want to avoid mentioning it TBH. Part of the reason\nthere is that I think the explanation of \"why\" is a little too detailed\nto spell out meaningfully in the documentation while still keeping it\nconcise. But more importantly I want to avoid encouraging people to use\nthe '--max-{pack,cruft}-size' flags to begin with, since there really is\nno reason to use them outside of filesystem constraints.\n\nThanks,\nTaylor\n"},{"id":"514694","messageId":"cover.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742252411.git.me@ttaylorr.com","subject":"[PATCH v2 0/5] repack: introduce '--combine-cruft-below-size'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:42Z","receivedAt":"2025-03-19T22:52:45Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Here is a small reroll of my series to introduce a new 'repack' flag\ncalled '--combine-cruft-below-size'. There aren't any functional changes\nin this round, and the changes that do exist are limited to\ndocumentation and commit message tweaks in the final patch.\n\nA range-diff is included below, as well as the original cover letter:\n\n(This is based on tb/multi-cruft-pack-refresh-fix from Junio's tree,\nwhich is at 08f612ba70 (builtin/pack-objects.c: freshen objects from\nexisting cruft packs, 2025-03-13) at the time of writing).\n\nThis series replaces something close to the existing behavior of\nrepack's '--max-cruft-size' flag with '--combine-cruft-below-size'.\n\nThe new flag is much clearer in its intent and function, and avoids the\nlack of clarity between the two that was discussed in\n\n \n\nThe new behavior is as follows:\n\n - '--max-cruft-size' is a cruft pack-specific override for repack's\n '--max-pack-size' command-line flag.\n\n - '--combine-cruft-below-size' instructs repack to only combine cruft\n packs which are smaller than the given threshold. This will likely\n result in packs which are larger than the threshold. But that is OK:\n the point is to limit the size of the individual packs on input, not\n the size of the outgoing pack.\n\nThis series does break the existing behavior of '--max-cruft-size'. But\nI think breaking backwards compatibility here is OK, since the existing\nbehavior was so broken to begin with. I'm happy to consider other\nalternatives if others feel that this isn't OK.\n\nThe series has an interesting structure that I feel may be worth calling\nout. The first three patches are trivial test cleanups. The fourth patch\nmodifies the existing behavior of '--max-cruft-size', but does so while\nkeeping some of the old infrastructure around.\n\nThat may seem like an unnecessarily complicated approach, but it greatly\nsimplifies introducing the new behavior in the following (and final)\ncommit. I think that this results in a series that is a little easier to\nreview (since we don't see a ton of code being removed in one commit and\nthen re-added in another immediately following it). But if others feel\nlike this was a mistake, please let me know ;-).\n\nThanks in advance for your review!\n\nTaylor Blau (5):\n t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests\n t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests\n t/t7704-repack-cruft.sh: consolidate `write_blob()`\n repack: avoid combining cruft packs with `--max-cruft-size`\n repack: begin combining cruft packs with `--combine-cruft-below-size`\n\n Documentation/git-repack.adoc | 21 ++-\n builtin/repack.c | 62 +++----\n t/t5329-pack-objects-cruft.sh | 302 ++++++----------------------------\n t/t7704-repack-cruft.sh | 293 ++++++++++++++++++++++++++++++---\n 4 files changed, 355 insertions(+), 323 deletions(-)\n\nRange-diff against v1:\n1: 0aa8aa65c1 = 1: 0aa8aa65c1 t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests\n2: 5e8bd3e66e = 2: 5e8bd3e66e t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests\n3: b075ad8601 = 3: b075ad8601 t/t7704-repack-cruft.sh: consolidate `write_blob()`\n4: 7941997e33 = 4: 7941997e33 repack: avoid combining cruft packs with `--max-cruft-size`\n5: 7f120c35e9 ! 5: dee780a2aa repack: begin combining cruft packs with `--combine-cruft-below-size`\n @@ Commit message\n Introduce a new flag, '--combine-cruft-below-size' which is a\n replacement for the old behavior of '--max-cruft-size'. This new flag\n does explicitly what it says: it combines together cruft packs which are\n - smaller than a given threshold, and prohibits repacking ones which are\n + smaller than a given threshold, and leaves alone ones which are\n larger.\n\n This accomplishes the original intent of '--max-cruft-size', which was\n @@ Commit message\n But that's OK: the point isn't to restrict the size of the cruft packs\n we generate, it's to avoid working with ones that have already grown too\n large. If repositories still want to limit the size of the generated\n - cruft pack(s), they may use '--max-cruft-size' instead.\n + cruft pack(s), they may use '--max-cruft-size'.\n\n There's some minor test fallout as a result of the slight differences in\n behavior between the old meaning of '--max-cruft-size' and the behavior\n @@ Documentation/git-repack.adoc: to the new separate pack will be written.\n\n +--combine-cruft-below-size=::\n +\tWhen generating cruft packs without pruning, only repack\n -+\texisting cruft packs whose size is strictly less than ``.\n -+\tCruft packs whose size is greater than or equal to `` are\n -+\tleft as-is and not repacked. Useful when you want to avoid\n -+\trepacking large cruft pack(s) in repositories that have many\n -+\tand/or large unreachable objects.\n ++\texisting cruft packs whose size is strictly less than ``,\n ++\twhere `` represents a number of bytes, which can optionally\n ++\tbe suffixed with \"k\", \"m\", or \"g\". Cruft packs whose size is\n ++\tgreater than or equal to `` are left as-is and not repacked.\n ++\tUseful when you want to avoid repacking large cruft pack(s) in\n ++\trepositories that have many and/or large unreachable objects.\n +\n --expire-to=::\n \tWrite a cruft pack containing pruned objects (if any) to the\n\nbase-commit: 08f612ba7000bf181ef6d8baed9ece322e567efd\n--\n2.49.0.4.ge59cf92f8d\n"},{"id":"514695","messageId":"0aa8aa65c130fd62d38b4944c1ce2a97451b0064.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"[PATCH v2 1/5] t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:45Z","receivedAt":"2025-03-19T22:52:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The cruft pack feature has two primary test scripts which exercise\nvarious parts of it, which are:\n\n - t5329-pack-objects-cruft.sh\n - t7704-repack-cruft.sh\n\nThe former is designed to test low-level pack generation mechanics at\nthe 'git pack-objects --cruft'-level, which is plumbing. The latter, on\nthe other hand, is designed to test the user-facing behavior through\n'git repack --cruft', which is porcelain (under the \"ancillary\nmanipulators\" sub-section).\n\nAt some point a handful of tests which should have been added to the\nlatter script were instead written to the former. This isn't a huge\ndeal, but rectifying it is straightforward. Move a handful of\n'repack'-related tests out of t5329 and into their rightful home in\nt7704.\n\nSigned-off-by: Taylor Blau \n---\n t/t5329-pack-objects-cruft.sh | 250 ----------------------------------\n t/t7704-repack-cruft.sh | 250 ++++++++++++++++++++++++++++++++++\n 2 files changed, 250 insertions(+), 250 deletions(-)\n\ndiff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh\nindex b71a0aef40..60dac8312d 100755\n--- a/t/t5329-pack-objects-cruft.sh\n+++ b/t/t5329-pack-objects-cruft.sh\n@@ -360,43 +360,6 @@ test_expect_success 'expired objects are pruned' '\n \t)\n '\n \n-test_expect_success 'repack --cruft generates a cruft pack' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit branch -M main &&\n-\t\tgit checkout --orphan other &&\n-\t\ttest_commit unreachable &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit tag -d unreachable &&\n-\t\t# objects are not cruft if they are contained in the reflogs\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\tgit rev-list --objects --all --no-object-names >reachable.raw &&\n-\t\tgit cat-file --batch-all-objects --batch-check=\"%(objectname)\" >objects &&\n-\t\tsort reachable &&\n-\t\tcomm -13 reachable objects >unreachable &&\n-\n-\t\tgit repack --cruft -d &&\n-\n-\t\tcruft=$(basename $(ls $packdir/pack-*.mtimes) .mtimes) &&\n-\t\tpack=$(basename $(ls $packdir/pack-*.pack | grep -v $cruft) .pack) &&\n-\n-\t\tgit show-index <$packdir/$pack.idx >actual.raw &&\n-\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n-\t\ttest_cmp reachable actual &&\n-\n-\t\tgit show-index <$packdir/$cruft.idx >actual.raw &&\n-\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n-\t\ttest_cmp unreachable actual\n-\t)\n-'\n-\n test_expect_success 'loose objects mtimes upsert others' '\n \tgit init repo &&\n \ttest_when_finished \"rm -fr repo\" &&\n@@ -470,219 +433,6 @@ test_expect_success 'expiring cruft objects with git gc' '\n \t)\n '\n \n-test_expect_success 'cruft packs are not included in geometric repack' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit repack -Ad &&\n-\t\tgit branch -M main &&\n-\n-\t\tgit checkout --orphan other &&\n-\t\ttest_commit cruft &&\n-\t\tgit repack -d &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit tag -d cruft &&\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\tgit repack --cruft &&\n-\n-\t\tfind $packdir -type f | sort >before &&\n-\t\tgit repack --geometric=2 -d &&\n-\t\tfind $packdir -type f | sort >after &&\n-\n-\t\ttest_cmp before after\n-\t)\n-'\n-\n-test_expect_success 'repack --geometric collects once-cruft objects' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit reachable &&\n-\t\tgit repack -Ad &&\n-\t\tgit branch -M main &&\n-\n-\t\tgit checkout --orphan other &&\n-\t\tgit rm -rf . &&\n-\t\ttest_commit --no-tag cruft &&\n-\t\tcruft=\"$(git rev-parse HEAD)\" &&\n-\n-\t\tgit checkout main &&\n-\t\tgit branch -D other &&\n-\t\tgit reflog expire --all --expire=all &&\n-\n-\t\t# Pack the objects created in the previous step into a cruft\n-\t\t# pack. Intentionally leave loose copies of those objects\n-\t\t# around so we can pick them up in a subsequent --geometric\n-\t\t# reapack.\n-\t\tgit repack --cruft &&\n-\n-\t\t# Now make those objects reachable, and ensure that they are\n-\t\t# packed into the new pack created via a --geometric repack.\n-\t\tgit update-ref refs/heads/other $cruft &&\n-\n-\t\t# Without this object, the set of unpacked objects is exactly\n-\t\t# the set of objects already in the cruft pack. Tweak that set\n-\t\t# to ensure we do not overwrite the cruft pack entirely.\n-\t\ttest_commit reachable2 &&\n-\n-\t\tfind $packdir -name \"pack-*.idx\" | sort >before &&\n-\t\tgit repack --geometric=2 -d &&\n-\t\tfind $packdir -name \"pack-*.idx\" | sort >after &&\n-\n-\t\t{\n-\t\t\tgit rev-list --objects --no-object-names $cruft &&\n-\t\t\tgit rev-list --objects --no-object-names reachable..reachable2\n-\t\t} >want.raw &&\n-\t\tsort want.raw >want &&\n-\n-\t\tpack=$(comm -13 before after) &&\n-\t\tgit show-index <$pack >objects.raw &&\n-\n-\t\tcut -d\" \" -f2 objects.raw | sort >got &&\n-\n-\t\ttest_cmp want got\n-\t)\n-'\n-\n-test_expect_success 'cruft repack with no reachable objects' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\t\tgit repack -ad &&\n-\n-\t\tbase=\"$(git rev-parse base)\" &&\n-\n-\t\tgit for-each-ref --format=\"delete %(refname)\" >in &&\n-\t\tgit update-ref --stdin in &&\n-\tgit hash-object -w -t blob in\n-}\n-\n-find_pack () {\n-\tfor idx in $(ls $packdir/pack-*.idx)\n-\tdo\n-\t\tgit show-index <$idx >out &&\n-\t\tif grep -q \"$1\" out\n-\t\tthen\n-\t\t\techo $idx\n-\t\tfi || return 1\n-\tdone\n-}\n-\n-test_expect_success 'cruft repack with --max-pack-size' '\n-\tgit init max-pack-size &&\n-\t(\n-\t\tcd max-pack-size &&\n-\t\ttest_commit base &&\n-\n-\t\t# two cruft objects which exceed the maximum pack size\n-\t\tfoo=$(write_blob foo 1048576) &&\n-\t\tbar=$(write_blob bar 1048576) &&\n-\t\ttest-tool chmtime --get -1000 \\\n-\t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n-\t\ttest-tool chmtime --get -2000 \\\n-\t\t\t\"$objdir/$(test_oid_to_path $bar)\" >bar.mtime &&\n-\t\tgit repack --cruft --max-pack-size=1M &&\n-\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n-\t\ttest_line_count = 2 cruft &&\n-\n-\t\tfoo_mtimes=\"$(basename $(find_pack $foo) .idx).mtimes\" &&\n-\t\tbar_mtimes=\"$(basename $(find_pack $bar) .idx).mtimes\" &&\n-\t\ttest-tool pack-mtimes $foo_mtimes >foo.actual &&\n-\t\ttest-tool pack-mtimes $bar_mtimes >bar.actual &&\n-\n-\t\techo \"$foo $(cat foo.mtime)\" >foo.expect &&\n-\t\techo \"$bar $(cat bar.mtime)\" >bar.expect &&\n-\n-\t\ttest_cmp foo.expect foo.actual &&\n-\t\ttest_cmp bar.expect bar.actual &&\n-\t\ttest \"$foo_mtimes\" != \"$bar_mtimes\"\n-\t)\n-'\n-\n-test_expect_success 'cruft repack with pack.packSizeLimit' '\n-\t(\n-\t\tcd max-pack-size &&\n-\t\t# repack everything back together to remove the existing cruft\n-\t\t# pack (but to keep its objects)\n-\t\tgit repack -adk &&\n-\t\tgit -c pack.packSizeLimit=1M repack --cruft &&\n-\t\t# ensure the same post condition is met when --max-pack-size\n-\t\t# would otherwise be inferred from the configuration\n-\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n-\t\ttest_line_count = 2 cruft &&\n-\t\tfor pack in $(cat cruft)\n-\t\tdo\n-\t\t\ttest-tool pack-mtimes \"$(basename $pack)\" >objects &&\n-\t\t\ttest_line_count = 1 objects || return 1\n-\t\tdone\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects repack.cruftWindow' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\n-\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n-\t\tgit -c pack.window=1 -c repack.cruftWindow=2 repack \\\n-\t\t --cruft --window=3 &&\n-\n-\t\tgrep \"pack-objects.*--window=2.*--cruft\" event.trace\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects --window by default' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\n-\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n-\t\tgit -c pack.window=2 repack --cruft --window=3 &&\n-\n-\t\tgrep \"pack-objects.*--window=3.*--cruft\" event.trace\n-\t)\n-'\n-\n-test_expect_success 'cruft repack respects --quiet' '\n-\tgit init repo &&\n-\ttest_when_finished \"rm -fr repo\" &&\n-\t(\n-\t\tcd repo &&\n-\n-\t\ttest_commit base &&\n-\t\tGIT_PROGRESS_DELAY=0 git repack --cruft --quiet 2>err &&\n-\t\ttest_must_be_empty err\n-\t)\n-'\n-\n test_expect_success 'cruft --local drops unreachable objects' '\n \tgit init alternate &&\n \tgit init repo &&\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 43d2947d28..cd452040ea 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -477,4 +477,254 @@ test_expect_success 'reachable packs are preferred over cruft ones' '\n \t)\n '\n \n+test_expect_success 'repack --cruft generates a cruft pack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit branch -M main &&\n+\t\tgit checkout --orphan other &&\n+\t\ttest_commit unreachable &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit tag -d unreachable &&\n+\t\t# objects are not cruft if they are contained in the reflogs\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\tgit rev-list --objects --all --no-object-names >reachable.raw &&\n+\t\tgit cat-file --batch-all-objects --batch-check=\"%(objectname)\" >objects &&\n+\t\tsort reachable &&\n+\t\tcomm -13 reachable objects >unreachable &&\n+\n+\t\tgit repack --cruft -d &&\n+\n+\t\tcruft=$(basename $(ls $packdir/pack-*.mtimes) .mtimes) &&\n+\t\tpack=$(basename $(ls $packdir/pack-*.pack | grep -v $cruft) .pack) &&\n+\n+\t\tgit show-index <$packdir/$pack.idx >actual.raw &&\n+\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n+\t\ttest_cmp reachable actual &&\n+\n+\t\tgit show-index <$packdir/$cruft.idx >actual.raw &&\n+\t\tcut -f2 -d\" \" actual.raw | sort >actual &&\n+\t\ttest_cmp unreachable actual\n+\t)\n+'\n+\n+test_expect_success 'cruft packs are not included in geometric repack' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit repack -Ad &&\n+\t\tgit branch -M main &&\n+\n+\t\tgit checkout --orphan other &&\n+\t\ttest_commit cruft &&\n+\t\tgit repack -d &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit tag -d cruft &&\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\tgit repack --cruft &&\n+\n+\t\tfind $packdir -type f | sort >before &&\n+\t\tgit repack --geometric=2 -d &&\n+\t\tfind $packdir -type f | sort >after &&\n+\n+\t\ttest_cmp before after\n+\t)\n+'\n+\n+test_expect_success 'repack --geometric collects once-cruft objects' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit reachable &&\n+\t\tgit repack -Ad &&\n+\t\tgit branch -M main &&\n+\n+\t\tgit checkout --orphan other &&\n+\t\tgit rm -rf . &&\n+\t\ttest_commit --no-tag cruft &&\n+\t\tcruft=\"$(git rev-parse HEAD)\" &&\n+\n+\t\tgit checkout main &&\n+\t\tgit branch -D other &&\n+\t\tgit reflog expire --all --expire=all &&\n+\n+\t\t# Pack the objects created in the previous step into a cruft\n+\t\t# pack. Intentionally leave loose copies of those objects\n+\t\t# around so we can pick them up in a subsequent --geometric\n+\t\t# reapack.\n+\t\tgit repack --cruft &&\n+\n+\t\t# Now make those objects reachable, and ensure that they are\n+\t\t# packed into the new pack created via a --geometric repack.\n+\t\tgit update-ref refs/heads/other $cruft &&\n+\n+\t\t# Without this object, the set of unpacked objects is exactly\n+\t\t# the set of objects already in the cruft pack. Tweak that set\n+\t\t# to ensure we do not overwrite the cruft pack entirely.\n+\t\ttest_commit reachable2 &&\n+\n+\t\tfind $packdir -name \"pack-*.idx\" | sort >before &&\n+\t\tgit repack --geometric=2 -d &&\n+\t\tfind $packdir -name \"pack-*.idx\" | sort >after &&\n+\n+\t\t{\n+\t\t\tgit rev-list --objects --no-object-names $cruft &&\n+\t\t\tgit rev-list --objects --no-object-names reachable..reachable2\n+\t\t} >want.raw &&\n+\t\tsort want.raw >want &&\n+\n+\t\tpack=$(comm -13 before after) &&\n+\t\tgit show-index <$pack >objects.raw &&\n+\n+\t\tcut -d\" \" -f2 objects.raw | sort >got &&\n+\n+\t\ttest_cmp want got\n+\t)\n+'\n+\n+test_expect_success 'cruft repack with no reachable objects' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\tgit repack -ad &&\n+\n+\t\tbase=\"$(git rev-parse base)\" &&\n+\n+\t\tgit for-each-ref --format=\"delete %(refname)\" >in &&\n+\t\tgit update-ref --stdin in &&\n+\tgit hash-object -w -t blob in\n+}\n+\n+find_pack () {\n+\tfor idx in $(ls $packdir/pack-*.idx)\n+\tdo\n+\t\tgit show-index <$idx >out &&\n+\t\tif grep -q \"$1\" out\n+\t\tthen\n+\t\t\techo $idx\n+\t\tfi || return 1\n+\tdone\n+}\n+\n+test_expect_success 'cruft repack with --max-pack-size' '\n+\tgit init max-pack-size &&\n+\t(\n+\t\tcd max-pack-size &&\n+\t\ttest_commit base &&\n+\n+\t\t# two cruft objects which exceed the maximum pack size\n+\t\tfoo=$(write_blob foo 1048576) &&\n+\t\tbar=$(write_blob bar 1048576) &&\n+\t\ttest-tool chmtime --get -1000 \\\n+\t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n+\t\ttest-tool chmtime --get -2000 \\\n+\t\t\t\"$objdir/$(test_oid_to_path $bar)\" >bar.mtime &&\n+\t\tgit repack --cruft --max-pack-size=1M &&\n+\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n+\t\ttest_line_count = 2 cruft &&\n+\n+\t\tfoo_mtimes=\"$(basename $(find_pack $foo) .idx).mtimes\" &&\n+\t\tbar_mtimes=\"$(basename $(find_pack $bar) .idx).mtimes\" &&\n+\t\ttest-tool pack-mtimes $foo_mtimes >foo.actual &&\n+\t\ttest-tool pack-mtimes $bar_mtimes >bar.actual &&\n+\n+\t\techo \"$foo $(cat foo.mtime)\" >foo.expect &&\n+\t\techo \"$bar $(cat bar.mtime)\" >bar.expect &&\n+\n+\t\ttest_cmp foo.expect foo.actual &&\n+\t\ttest_cmp bar.expect bar.actual &&\n+\t\ttest \"$foo_mtimes\" != \"$bar_mtimes\"\n+\t)\n+'\n+\n+test_expect_success 'cruft repack with pack.packSizeLimit' '\n+\t(\n+\t\tcd max-pack-size &&\n+\t\t# repack everything back together to remove the existing cruft\n+\t\t# pack (but to keep its objects)\n+\t\tgit repack -adk &&\n+\t\tgit -c pack.packSizeLimit=1M repack --cruft &&\n+\t\t# ensure the same post condition is met when --max-pack-size\n+\t\t# would otherwise be inferred from the configuration\n+\t\tfind $packdir -name \"*.mtimes\" >cruft &&\n+\t\ttest_line_count = 2 cruft &&\n+\t\tfor pack in $(cat cruft)\n+\t\tdo\n+\t\t\ttest-tool pack-mtimes \"$(basename $pack)\" >objects &&\n+\t\t\ttest_line_count = 1 objects || return 1\n+\t\tdone\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects repack.cruftWindow' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\n+\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n+\t\tgit -c pack.window=1 -c repack.cruftWindow=2 repack \\\n+\t\t --cruft --window=3 &&\n+\n+\t\tgrep \"pack-objects.*--window=2.*--cruft\" event.trace\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects --window by default' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\n+\t\tGIT_TRACE2_EVENT=$(pwd)/event.trace \\\n+\t\tgit -c pack.window=2 repack --cruft --window=3 &&\n+\n+\t\tgrep \"pack-objects.*--window=3.*--cruft\" event.trace\n+\t)\n+'\n+\n+test_expect_success 'cruft repack respects --quiet' '\n+\tgit init repo &&\n+\ttest_when_finished \"rm -fr repo\" &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\ttest_commit base &&\n+\t\tGIT_PROGRESS_DELAY=0 git repack --cruft --quiet 2>err &&\n+\t\ttest_must_be_empty err\n+\t)\n+'\n+\n test_done\n-- \n2.49.0.4.ge59cf92f8d\n\n"},{"id":"514696","messageId":"5e8bd3e66e304f42d4e84ec2aa9f06b82321d64b.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"[PATCH v2 2/5] t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:48Z","receivedAt":"2025-03-19T22:52:51Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Now that a number of new tests have landed in t7704, make sure that they\nall make sense and are testing the things they say they are.\n\nThings are mostly OK, but a handful of tests needed tweaks. Those tweaks\nare as follows:\n\n - Use the terms \"too large\" or \"too small\" in tests that exercise the\n '--max-cruft-size' behavior. This has historically been treated as a\n threshold beneath which to combine cruft packs, but that will change\n in a subsequent commit. Prepare for that by using a more generic\n term.\n\n - Remove references to \"--max-cruft-size\" in the freshening tests.\n These tests provide coverage of our ability to record updated mtimes\n for objects already in cruft packs whose mtimes are upserted from\n various sources (loose objects, finding that object in a new pack,\n another cruft pack, etc.).\n\n These have nothing to do with the '--max-cruft-size' feature, and in\n fact none of the tests even *use* '--max-cruft-size'. Name them\n appropriately to make it clear that these tests exercise freshening\n behavior, not '--max-cruft-size' behavior.\n\nSigned-off-by: Taylor Blau \n---\n t/t7704-repack-cruft.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex cd452040ea..e6e4c2fad8 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -149,7 +149,7 @@ generate_cruft_pack () {\n \techo \"$packdir/pack-$pack.mtimes\"\n }\n \n-test_expect_success '--max-cruft-size creates new packs when above threshold' '\n+test_expect_success '--max-cruft-size creates new packs when too large' '\n \tgit init max-cruft-size-large &&\n \t(\n \t\tcd max-cruft-size-large &&\n@@ -173,7 +173,7 @@ test_expect_success '--max-cruft-size creates new packs when above threshold' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size combines existing packs when below threshold' '\n+test_expect_success '--max-cruft-size combines existing packs when not too large' '\n \tgit init max-cruft-size-small &&\n \t(\n \t\tcd max-cruft-size-small &&\n@@ -236,10 +236,10 @@ test_expect_success '--max-cruft-size combines smaller packs first' '\n \t)\n '\n \n-test_expect_success 'setup --max-cruft-size with freshened objects' '\n-\tgit init max-cruft-size-freshen &&\n+test_expect_success 'setup cruft with freshened objects' '\n+\tgit init cruft-freshen &&\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\ttest_commit base &&\n \t\tgit repack -ad &&\n@@ -257,9 +257,9 @@ test_expect_success 'setup --max-cruft-size with freshened objects' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (loose)' '\n+test_expect_success 'cruft with freshened objects (loose)' '\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\t# regenerate the object, setting its mtime to be more recent\n \t\tfoo=\"$(generate_random_blob foo 64)\" &&\n@@ -275,9 +275,9 @@ test_expect_success '--max-cruft-size with freshened objects (loose)' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (packed)' '\n+test_expect_success 'cruft with freshened objects (packed)' '\n \t(\n-\t\tcd max-cruft-size-freshen &&\n+\t\tcd cruft-freshen &&\n \n \t\t# regenerate the object and store it in a packfile,\n \t\t# setting its mtime to be more recent\n@@ -304,7 +304,7 @@ test_expect_success '--max-cruft-size with freshened objects (packed)' '\n \t)\n '\n \n-test_expect_success '--max-cruft-size with freshened objects (previously cruft)' '\n+test_expect_success 'multi-cruft with freshened objects (previously cruft)' '\n \trepo=\"max-cruft-size-threshold\" &&\n \n \ttest_when_finished \"rm -fr $repo\" &&\n-- \n2.49.0.4.ge59cf92f8d\n\n"},{"id":"514697","messageId":"b075ad86015131e4a83bd7bcd707ff2273b375b4.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"[PATCH v2 3/5] t/t7704-repack-cruft.sh: consolidate `write_blob()`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:51Z","receivedAt":"2025-03-19T22:52:54Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"A previous commit moved a handful of tests from a different script into\nt7704, including one that relies on generating random blobs.\n\nIncidentally, the original home of this test defined its own helper\n\"write_blob\" for doing so, which is identical in function to our\n\"generate_random_blob\" (and is slightly inferior to the latter, which\ncleans up after itself).\n\nRewrite the test that uses \"write_blob\" to no longer do so and then\nremove the function.\n\nSigned-off-by: Taylor Blau \n---\n t/t7704-repack-cruft.sh | 9 ++-------\n 1 file changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex e6e4c2fad8..3fd5aa6089 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -618,11 +618,6 @@ test_expect_success 'cruft repack with no reachable objects' '\n \t)\n '\n \n-write_blob () {\n-\ttest-tool genrandom \"$@\" >in &&\n-\tgit hash-object -w -t blob in\n-}\n-\n find_pack () {\n \tfor idx in $(ls $packdir/pack-*.idx)\n \tdo\n@@ -641,8 +636,8 @@ test_expect_success 'cruft repack with --max-pack-size' '\n \t\ttest_commit base &&\n \n \t\t# two cruft objects which exceed the maximum pack size\n-\t\tfoo=$(write_blob foo 1048576) &&\n-\t\tbar=$(write_blob bar 1048576) &&\n+\t\tfoo=$(generate_random_blob foo 1048576) &&\n+\t\tbar=$(generate_random_blob bar 1048576) &&\n \t\ttest-tool chmtime --get -1000 \\\n \t\t\t\"$objdir/$(test_oid_to_path $foo)\" >foo.mtime &&\n \t\ttest-tool chmtime --get -2000 \\\n-- \n2.49.0.4.ge59cf92f8d\n\n"},{"id":"514698","messageId":"7941997e332828f0005acf97f925efc1e7251a33.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"[PATCH v2 4/5] repack: avoid combining cruft packs with `--max-cruft-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:54Z","receivedAt":"2025-03-19T22:52:57Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In 37dc6d8104 (builtin/repack.c: implement support for\n`--max-cruft-size`, 2023-10-02), we exposed new functionality that\nallowed repositories to specify the behavior of when we should combine\nmultiple cruft packs together.\n\nThis feature was designed to ensure that we never repacked cruft packs\nwhich were larger than the given threshold in order to provide tighter\nI/O bounds for repositories that have many unreachable objects. In\nessence, specifying '--max-cruft-size=N' instructed 'repack' to\naggregate cruft packs together (in order of ascending size) until the\ncombine size grows past 'N', and then make a new cruft pack whose\ncontents includes the packs we rolled up.\n\nBut this isn't quite how it works in practice. Suppose for example that\nwe have two cruft packs which are each 100MiB in size. One might expect\nspecifying \"--max-cruft-size=200M\" would combine these two packs\ntogether, and then avoid repacking them until a pruning GC takes place.\nIn reality, 'repack' would try and aggregate these together, but writing\na pack that is strictly smaller than 200 MiB (since pack-objects'\n\"--max-pack-size\" provides a strict bound for packs containing more than\none object).\n\nSo instead we'll write out a pack that is, say, 199 MiB in size, and\nthen another 1 MiB pack containing the balance. If we later repack the\nrepository without adding any new unreachable objects, we'll repeat the\nsame exercise again, making the same 199 MiB and 1 MiB packs each time.\n\nThis happens because of a poor choice to bolt the '--max-cruft-size'\nfunctionality onto pack-objects' '--max-pack-size', forcing us to\ngenerate packs which are always smaller than the provided threshold and\nthus subject to repacking.\n\nThe following commit will introduce a new flag that implements something\nsimilar to the behavior above. Let's prepare for that by making repack's\n'--max-cruft-size' flag behave as an cruft pack-specific override for\n'--max-pack-size'.\n\nDo so by temporarily repurposing the 'collapse_small_cruft_packs()'\nfunction to instead generate a cruft pack using the same instructions as\nif we didn't specify any maximum pack size. The calling code looks\nsomething like:\n\n if (args->max_pack_size && !cruft_expiration) {\n collapse_small_cruft_packs(in, args->max_pack_size, existing);\n } else {\n for_each_string_list_item(item, &existing->non_kept_packs)\n fprintf(in, \"-%s.pack\\n\", item->string);\n for_each_string_list_item(item, &existing->cruft_packs)\n fprintf(in, \"-%s.pack\\n\", item->string);\n }\n\nThis patch makes collapse_small_cruft_packs() behave identically to the\n'else' arm of the conditional above. This repurposing of\n'collapse_small_cruft_packs()' is intentional, since it will set us up\nnicely to introduce the new behavior in the following commit.\n\nNaturally, there is some test fallout in the test which exercises the\nold meaning of '--max-cruft-size'. Mark that test as failing for now to\nbe dealt with in the following commit. Likewise, add a new test which\nexplicitly tests the behavior of '--max-cruft-size' to place a hard\nlimit on the size of any generated cruft pack(s).\n\nNote that this is a breaking change, as it alters the user-visible\nbehavior of '--max-cruft-size'. But I'm OK changing this behavior in\nthis instance, since the behavior wasn't accurate to begin with.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-repack.adoc | 12 ++------\n builtin/repack.c | 50 ++++++---------------------------\n t/t5329-pack-objects-cruft.sh | 52 +++++++++++++++++++++++++++++++++++\n t/t7704-repack-cruft.sh | 8 ++----\n 4 files changed, 67 insertions(+), 55 deletions(-)\n\ndiff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\nindex 5852a5c973..11db43b1c5 100644\n--- a/Documentation/git-repack.adoc\n+++ b/Documentation/git-repack.adoc\n@@ -77,15 +77,9 @@ to the new separate pack will be written.\n \tOnly useful with `--cruft -d`.\n \n --max-cruft-size=::\n-\tRepack cruft objects into packs as large as `` bytes before\n-\tcreating new packs. As long as there are enough cruft packs\n-\tsmaller than ``, repacking will cause a new cruft pack to\n-\tbe created containing objects from any combined cruft packs,\n-\talong with any new unreachable objects. Cruft packs larger than\n-\t`` will not be modified. When the new cruft pack is larger\n-\tthan `` bytes, it will be split into multiple packs, all of\n-\twhich are guaranteed to be at most `` bytes in size. Only\n-\tuseful with `--cruft -d`.\n+\tOverrides `--max-pack-size` for cruft packs. Inherits the value of\n+\t`--max-pack-size` (if any) by default. See the documentation for\n+\t`--max-pack-size` for more details.\n \n --expire-to=::\n \tWrite a cruft pack containing pruned objects (if any) to the\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 75e3752353..9658f6b354 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -1022,28 +1022,19 @@ static int write_filtered_pack(const struct pack_objects_args *args,\n \treturn finish_pack_objects_cmd(&cmd, names, local);\n }\n \n-static int existing_cruft_pack_cmp(const void *va, const void *vb)\n-{\n-\tstruct packed_git *a = *(struct packed_git **)va;\n-\tstruct packed_git *b = *(struct packed_git **)vb;\n-\n-\tif (a->pack_size < b->pack_size)\n-\t\treturn -1;\n-\tif (a->pack_size > b->pack_size)\n-\t\treturn 1;\n-\treturn 0;\n-}\n-\n-static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n+static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n \t\t\t\t struct existing_packs *existing)\n {\n-\tstruct packed_git **existing_cruft, *p;\n+\tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tsize_t total_size = 0;\n-\tsize_t existing_cruft_nr = 0;\n \tsize_t i;\n \n-\tALLOC_ARRAY(existing_cruft, existing->cruft_packs.nr);\n+\t/*\n+\t * Squelch a -Wunused-function warning while we rationalize\n+\t * the behavior of --max-cruft-size. This function will become\n+\t * used again in a future commit.\n+\t */\n+\t(void)retain_cruft_pack;\n \n \tfor (p = get_all_packs(the_repository); p; p = p->next) {\n \t\tif (!(p->is_cruft && p->pack_local))\n@@ -1056,29 +1047,7 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n \t\tif (!string_list_has_string(&existing->cruft_packs, buf.buf))\n \t\t\tcontinue;\n \n-\t\tif (existing_cruft_nr >= existing->cruft_packs.nr)\n-\t\t\tBUG(\"too many cruft packs (found %\"PRIuMAX\", but knew \"\n-\t\t\t \"of %\"PRIuMAX\")\",\n-\t\t\t (uintmax_t)existing_cruft_nr + 1,\n-\t\t\t (uintmax_t)existing->cruft_packs.nr);\n-\t\texisting_cruft[existing_cruft_nr++] = p;\n-\t}\n-\n-\tQSORT(existing_cruft, existing_cruft_nr, existing_cruft_pack_cmp);\n-\n-\tfor (i = 0; i < existing_cruft_nr; i++) {\n-\t\tsize_t proposed;\n-\n-\t\tp = existing_cruft[i];\n-\t\tproposed = st_add(total_size, p->pack_size);\n-\n-\t\tif (proposed <= max_size) {\n-\t\t\ttotal_size = proposed;\n-\t\t\tfprintf(in, \"-%s\\n\", pack_basename(p));\n-\t\t} else {\n-\t\t\tretain_cruft_pack(existing, p);\n-\t\t\tfprintf(in, \"%s\\n\", pack_basename(p));\n-\t\t}\n+\t\tfprintf(in, \"-%s.pack\\n\", buf.buf);\n \t}\n \n \tfor (i = 0; i < existing->non_kept_packs.nr; i++)\n@@ -1086,7 +1055,6 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size,\n \t\t\texisting->non_kept_packs.items[i].string);\n \n \tstrbuf_release(&buf);\n-\tfree(existing_cruft);\n }\n \n static int write_cruft_pack(const struct pack_objects_args *args,\ndiff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh\nindex 60dac8312d..25ddda5cf3 100755\n--- a/t/t5329-pack-objects-cruft.sh\n+++ b/t/t5329-pack-objects-cruft.sh\n@@ -695,4 +695,56 @@ test_expect_success 'additional cruft blobs via gc.recentObjectsHook' '\n \t)\n '\n \n+test_expect_success 'split cruft packs with --max-cruft-size' '\n+\trepo=cruft-with--max-cruft-size &&\n+\ttest_when_finished \"rm -fr $repo\" &&\n+\n+\tgit init \"$repo\" &&\n+\n+\t(\n+\t\tcd \"$repo\" &&\n+\n+\t\tgit config core.compression 0 &&\n+\n+\t\tsz=$((1024 * 1024)) && # 1MiB\n+\t\ttest-tool genrandom foo $sz >foo &&\n+\t\ttest-tool genrandom bar $sz >bar &&\n+\t\tfoo=\"$(git hash-object -w -t blob foo)\" &&\n+\t\tbar=\"$(git hash-object -w -t blob bar)\" &&\n+\n+\t\tto=$packdir/pack &&\n+\t\t# Pack together foo and bar into a single 2MiB pack.\n+\t\tpack=\"$(git pack-objects $to <<-EOF\n+\t\t$foo\n+\t\t$bar\n+\t\tEOF\n+\t\t)\" &&\n+\n+\t\t# Then generate a cruft pack containing foo and bar.\n+\t\t#\n+\t\t# Generate the pack with --max-pack-size equal to the\n+\t\t# size of one object, forcing us to write two cruft\n+\t\t# packs.\n+\t\tgit pack-objects --cruft --max-pack-size=$sz $to <<-EOF &&\n+\t\t-pack-$pack.pack\n+\t\tEOF\n+\n+\t\tls $packdir/pack-*.mtimes >crufts &&\n+\t\ttest_line_count = 2 crufts &&\n+\n+\t\tfor cruft in $(cat crufts)\n+\t\tdo\n+\t\t\ttest-tool pack-mtimes \"$(basename \"$cruft\")\" || return 1\n+\t\tdone >actual.raw &&\n+\n+\t\tcut -d\" \" -f1 actual &&\n+\t\tsort >expect <<-EOF &&\n+\t\t$foo\n+\t\t$bar\n+\t\tEOF\n+\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 3fd5aa6089..6debad368d 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -194,7 +194,7 @@ test_expect_success '--max-cruft-size combines existing packs when not too large\n \t)\n '\n \n-test_expect_success '--max-cruft-size combines smaller packs first' '\n+test_expect_failure '--max-cruft-size combines smaller packs first' '\n \tgit init max-cruft-size-consume-small &&\n \t(\n \t\tcd max-cruft-size-consume-small &&\n@@ -354,13 +354,11 @@ test_expect_success 'multi-cruft with freshened objects (previously cruft)' '\n \t\tdone >actual.raw &&\n \t\tsort actual.raw >actual &&\n \n-\t\t# Among the set of all cruft packs, we should see both\n-\t\t# mtimes for object $foo and $bar, as well as the\n+\t\t# Among the set of all cruft packs, we should see the\n+\t\t# new mtimes for object $foo and $bar, as well as the\n \t\t# single new copy of $baz.\n \t\tsort >expect <<-EOF &&\n-\t\t$foo $(cat foo.old)\n \t\t$foo $(cat foo.new)\n-\t\t$bar $(cat bar.old)\n \t\t$bar $(cat bar.new)\n \t\t$baz $(cat baz.old)\n \t\t$quux $(cat quux.new)\n-- \n2.49.0.4.ge59cf92f8d\n\n"},{"id":"514699","messageId":"dee780a2aaebc1055c2fb009325c3d8eb09421cf.1742424671.git.me@ttaylorr.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"[PATCH v2 5/5] repack: begin combining cruft packs with `--combine-cruft-below-size`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2025-03-19T22:52:58Z","receivedAt":"2025-03-19T22:53:00Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"The previous commit changed the behavior of repack's '--max-cruft-size'\nto specify a cruft pack-specific override for '--max-pack-size'.\n\nIntroduce a new flag, '--combine-cruft-below-size' which is a\nreplacement for the old behavior of '--max-cruft-size'. This new flag\ndoes explicitly what it says: it combines together cruft packs which are\nsmaller than a given threshold, and leaves alone ones which are\nlarger.\n\nThis accomplishes the original intent of '--max-cruft-size', which was\nto avoid repacking cruft packs larger than the given threshold.\n\nThe new behavior is slightly different. Instead of building up small\npacks together until the threshold is met, '--combine-cruft-below-size'\npacks up *all* cruft packs smaller than the threshold. This means that\nwe may make a pack much larger than the given threshold (e.g., if you\naggregate 5 packs which are each 99 MiB in size with a threshold of 100\nMiB).\n\nBut that's OK: the point isn't to restrict the size of the cruft packs\nwe generate, it's to avoid working with ones that have already grown too\nlarge. If repositories still want to limit the size of the generated\ncruft pack(s), they may use '--max-cruft-size'.\n\nThere's some minor test fallout as a result of the slight differences in\nbehavior between the old meaning of '--max-cruft-size' and the behavior\nof '--combine-cruft-below-size'. In the test which is now called\n\"--combine-cruft-below-size combines packs\", we need to use the new flag\nover the old one to exercise that test's intended behavior. The\nremainder of the changes there are to improve the clarity of the\ncomments.\n\nSuggested-by: Elijah Newren \nSigned-off-by: Taylor Blau \n---\n Documentation/git-repack.adoc | 9 +++++++++\n builtin/repack.c | 38 +++++++++++++++++++++++------------\n t/t7704-repack-cruft.sh | 22 +++++++++++---------\n 3 files changed, 47 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc\nindex 11db43b1c5..e1cd75eebe 100644\n--- a/Documentation/git-repack.adoc\n+++ b/Documentation/git-repack.adoc\n@@ -81,6 +81,15 @@ to the new separate pack will be written.\n \t`--max-pack-size` (if any) by default. See the documentation for\n \t`--max-pack-size` for more details.\n \n+--combine-cruft-below-size=::\n+\tWhen generating cruft packs without pruning, only repack\n+\texisting cruft packs whose size is strictly less than ``,\n+\twhere `` represents a number of bytes, which can optionally\n+\tbe suffixed with \"k\", \"m\", or \"g\". Cruft packs whose size is\n+\tgreater than or equal to `` are left as-is and not repacked.\n+\tUseful when you want to avoid repacking large cruft pack(s) in\n+\trepositories that have many and/or large unreachable objects.\n+\n --expire-to=::\n \tWrite a cruft pack containing pruned objects (if any) to the\n \tdirectory ``. This option is useful for keeping a copy of\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 9658f6b354..f3330ade7b 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -1022,20 +1022,13 @@ static int write_filtered_pack(const struct pack_objects_args *args,\n \treturn finish_pack_objects_cmd(&cmd, names, local);\n }\n \n-static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n-\t\t\t\t struct existing_packs *existing)\n+static void combine_small_cruft_packs(FILE *in, size_t combine_cruft_below_size,\n+\t\t\t\t struct existing_packs *existing)\n {\n \tstruct packed_git *p;\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t i;\n \n-\t/*\n-\t * Squelch a -Wunused-function warning while we rationalize\n-\t * the behavior of --max-cruft-size. This function will become\n-\t * used again in a future commit.\n-\t */\n-\t(void)retain_cruft_pack;\n-\n \tfor (p = get_all_packs(the_repository); p; p = p->next) {\n \t\tif (!(p->is_cruft && p->pack_local))\n \t\t\tcontinue;\n@@ -1047,7 +1040,12 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,\n \t\tif (!string_list_has_string(&existing->cruft_packs, buf.buf))\n \t\t\tcontinue;\n \n-\t\tfprintf(in, \"-%s.pack\\n\", buf.buf);\n+\t\tif (p->pack_size < combine_cruft_below_size) {\n+\t\t\tfprintf(in, \"-%s\\n\", pack_basename(p));\n+\t\t} else {\n+\t\t\tretain_cruft_pack(existing, p);\n+\t\t\tfprintf(in, \"%s\\n\", pack_basename(p));\n+\t\t}\n \t}\n \n \tfor (i = 0; i < existing->non_kept_packs.nr; i++)\n@@ -1061,6 +1059,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n \t\t\t const char *destination,\n \t\t\t const char *pack_prefix,\n \t\t\t const char *cruft_expiration,\n+\t\t\t unsigned long combine_cruft_below_size,\n \t\t\t struct string_list *names,\n \t\t\t struct existing_packs *existing)\n {\n@@ -1103,8 +1102,9 @@ static int write_cruft_pack(const struct pack_objects_args *args,\n \tin = xfdopen(cmd.in, \"w\");\n \tfor_each_string_list_item(item, names)\n \t\tfprintf(in, \"%s-%s.pack\\n\", pack_prefix, item->string);\n-\tif (args->max_pack_size && !cruft_expiration) {\n-\t\tcollapse_small_cruft_packs(in, args->max_pack_size, existing);\n+\tif (combine_cruft_below_size && !cruft_expiration) {\n+\t\tcombine_small_cruft_packs(in, combine_cruft_below_size,\n+\t\t\t\t\t existing);\n \t} else {\n \t\tfor_each_string_list_item(item, &existing->non_kept_packs)\n \t\t\tfprintf(in, \"-%s.pack\\n\", item->string);\n@@ -1158,6 +1158,7 @@ int cmd_repack(int argc,\n \tconst char *opt_window_memory = NULL;\n \tconst char *opt_depth = NULL;\n \tconst char *opt_threads = NULL;\n+\tunsigned long combine_cruft_below_size = 0ul;\n \n \tstruct option builtin_repack_options[] = {\n \t\tOPT_BIT('a', NULL, &pack_everything,\n@@ -1170,6 +1171,9 @@ int cmd_repack(int argc,\n \t\t\t\t PACK_CRUFT),\n \t\tOPT_STRING(0, \"cruft-expiration\", &cruft_expiration, N_(\"approxidate\"),\n \t\t\t\tN_(\"with --cruft, expire objects older than this\")),\n+\t\tOPT_MAGNITUDE(0, \"combine-cruft-below-size\",\n+\t\t\t &combine_cruft_below_size,\n+\t\t\t N_(\"with --cruft, only repack cruft packs smaller than this\")),\n \t\tOPT_MAGNITUDE(0, \"max-cruft-size\", &cruft_po_args.max_pack_size,\n \t\t\t\tN_(\"with --cruft, limit the size of new cruft packs\")),\n \t\tOPT_BOOL('d', NULL, &delete_redundant,\n@@ -1413,7 +1417,8 @@ int cmd_repack(int argc,\n \t\tcruft_po_args.quiet = po_args.quiet;\n \n \t\tret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,\n-\t\t\t\t cruft_expiration, &names,\n+\t\t\t\t cruft_expiration,\n+\t\t\t\t combine_cruft_below_size, &names,\n \t\t\t\t &existing);\n \t\tif (ret)\n \t\t\tgoto cleanup;\n@@ -1440,10 +1445,17 @@ int cmd_repack(int argc,\n \t\t\t * generate an empty pack (since every object not in the\n \t\t\t * cruft pack generated above will have an mtime older\n \t\t\t * than the expiration).\n+\t\t\t *\n+\t\t\t * Pretend we don't have a `--combine-cruft-below-size`\n+\t\t\t * argument, since we're not selectively combining\n+\t\t\t * anything based on size to generate the limbo cruft\n+\t\t\t * pack, but rather removing all cruft packs from the\n+\t\t\t * main repository regardless of size.\n \t\t\t */\n \t\t\tret = write_cruft_pack(&cruft_po_args, expire_to,\n \t\t\t\t\t pack_prefix,\n \t\t\t\t\t NULL,\n+\t\t\t\t\t 0ul,\n \t\t\t\t\t &names,\n \t\t\t\t\t &existing);\n \t\t\tif (ret)\ndiff --git a/t/t7704-repack-cruft.sh b/t/t7704-repack-cruft.sh\nindex 6debad368d..8aebfb45f5 100755\n--- a/t/t7704-repack-cruft.sh\n+++ b/t/t7704-repack-cruft.sh\n@@ -194,10 +194,13 @@ test_expect_success '--max-cruft-size combines existing packs when not too large\n \t)\n '\n \n-test_expect_failure '--max-cruft-size combines smaller packs first' '\n-\tgit init max-cruft-size-consume-small &&\n+test_expect_success '--combine-cruft-below-size combines packs' '\n+\trepo=combine-cruft-below-size &&\n+\ttest_when_finished \"rm -fr $repo\" &&\n+\n+\tgit init \"$repo\" &&\n \t(\n-\t\tcd max-cruft-size-consume-small &&\n+\t\tcd \"$repo\" &&\n \n \t\ttest_commit base &&\n \t\tgit repack -ad &&\n@@ -211,11 +214,11 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n \t\ttest-tool pack-mtimes \"$(basename $cruft_bar)\" >>expect.raw &&\n \t\tsort expect.raw >expect.objects &&\n \n-\t\t# repacking with `--max-cruft-size=2M` should combine\n-\t\t# both 0.5 MiB packs together, instead of, say, one of\n-\t\t# the 0.5 MiB packs with the 1.0 MiB pack\n+\t\t# Repacking with `--combine-cruft-below-size=1M`\n+\t\t# should combine both 0.5 MiB packs together, but\n+\t\t# ignore the two packs which are >= 1.0 MiB.\n \t\tls $packdir/pack-*.mtimes | sort >cruft.before &&\n-\t\tgit repack -d --cruft --max-cruft-size=2M &&\n+\t\tgit repack -d --cruft --combine-cruft-below-size=1M &&\n \t\tls $packdir/pack-*.mtimes | sort >cruft.after &&\n \n \t\tcomm -13 cruft.before cruft.after >cruft.new &&\n@@ -224,11 +227,12 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '\n \t\ttest_line_count = 1 cruft.new &&\n \t\ttest_line_count = 2 cruft.removed &&\n \n-\t\t# the two smaller packs should be rolled up first\n+\t\t# The two packs smaller than 1.0MiB should be repacked\n+\t\t# together.\n \t\tprintf \"%s\\n\" $cruft_foo $cruft_bar | sort >expect.removed &&\n \t\ttest_cmp expect.removed cruft.removed &&\n \n-\t\t# ...and contain the set of objects rolled up\n+\t\t# ...and contain the set of objects rolled up.\n \t\ttest-tool pack-mtimes \"$(basename $(cat cruft.new))\" >actual.raw &&\n \t\tsort actual.raw >actual.objects &&\n \n-- \n2.49.0.4.ge59cf92f8d\n"},{"id":"514712","messageId":"CABPp-BF-i5Npbnp+tHoW1QbvrYzCngwny9dMTOnf+0-y5yxc2g@mail.gmail.com","threadId":"63147","inReplyTo":"cover.1742424671.git.me@ttaylorr.com","subject":"Re: [PATCH v2 0/5] repack: introduce '--combine-cruft-below-size'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-03-20T04:30:24Z","receivedAt":"2025-03-20T04:30:36Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Mar 19, 2025 at 3:52 PM Taylor Blau wrote:\n>\n> Here is a small reroll of my series to introduce a new 'repack' flag\n> called '--combine-cruft-below-size'. There aren't any functional changes\n> in this round, and the changes that do exist are limited to\n> documentation and commit message tweaks in the final patch.\n>\n> A range-diff is included below, as well as the original cover letter:\n>\n> (This is based on tb/multi-cruft-pack-refresh-fix from Junio's tree,\n> which is at 08f612ba70 (builtin/pack-objects.c: freshen objects from\n> existing cruft packs, 2025-03-13) at the time of writing).\n>\n> This series replaces something close to the existing behavior of\n> repack's '--max-cruft-size' flag with '--combine-cruft-below-size'.\n>\n> The new flag is much clearer in its intent and function, and avoids the\n> lack of clarity between the two that was discussed in\n>\n> \n>\n> The new behavior is as follows:\n>\n> - '--max-cruft-size' is a cruft pack-specific override for repack's\n> '--max-pack-size' command-line flag.\n>\n> - '--combine-cruft-below-size' instructs repack to only combine cruft\n> packs which are smaller than the given threshold. This will likely\n> result in packs which are larger than the threshold. But that is OK:\n> the point is to limit the size of the individual packs on input, not\n> the size of the outgoing pack.\n>\n> This series does break the existing behavior of '--max-cruft-size'. But\n> I think breaking backwards compatibility here is OK, since the existing\n> behavior was so broken to begin with. I'm happy to consider other\n> alternatives if others feel that this isn't OK.\n>\n> The series has an interesting structure that I feel may be worth calling\n> out. The first three patches are trivial test cleanups. The fourth patch\n> modifies the existing behavior of '--max-cruft-size', but does so while\n> keeping some of the old infrastructure around.\n>\n> That may seem like an unnecessarily complicated approach, but it greatly\n> simplifies introducing the new behavior in the following (and final)\n> commit. I think that this results in a series that is a little easier to\n> review (since we don't see a ton of code being removed in one commit and\n> then re-added in another immediately following it). But if others feel\n> like this was a mistake, please let me know ;-).\n>\n> Thanks in advance for your review!\n>\n> Taylor Blau (5):\n> t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests\n> t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests\n> t/t7704-repack-cruft.sh: consolidate `write_blob()`\n> repack: avoid combining cruft packs with `--max-cruft-size`\n> repack: begin combining cruft packs with `--combine-cruft-below-size`\n>\n> Documentation/git-repack.adoc | 21 ++-\n> builtin/repack.c | 62 +++----\n> t/t5329-pack-objects-cruft.sh | 302 ++++++----------------------------\n> t/t7704-repack-cruft.sh | 293 ++++++++++++++++++++++++++++++---\n> 4 files changed, 355 insertions(+), 323 deletions(-)\n>\n> Range-diff against v1:\n> 1: 0aa8aa65c1 = 1: 0aa8aa65c1 t/t5329-pack-objects-cruft.sh: evict 'repack'-related tests\n> 2: 5e8bd3e66e = 2: 5e8bd3e66e t/t7704-repack-cruft.sh: clarify wording in --max-cruft-size tests\n> 3: b075ad8601 = 3: b075ad8601 t/t7704-repack-cruft.sh: consolidate `write_blob()`\n> 4: 7941997e33 = 4: 7941997e33 repack: avoid combining cruft packs with `--max-cruft-size`\n> 5: 7f120c35e9 ! 5: dee780a2aa repack: begin combining cruft packs with `--combine-cruft-below-size`\n> @@ Commit message\n> Introduce a new flag, '--combine-cruft-below-size' which is a\n> replacement for the old behavior of '--max-cruft-size'. This new flag\n> does explicitly what it says: it combines together cruft packs which are\n> - smaller than a given threshold, and prohibits repacking ones which are\n> + smaller than a given threshold, and leaves alone ones which are\n> larger.\n>\n> This accomplishes the original intent of '--max-cruft-size', which was\n> @@ Commit message\n> But that's OK: the point isn't to restrict the size of the cruft packs\n> we generate, it's to avoid working with ones that have already grown too\n> large. If repositories still want to limit the size of the generated\n> - cruft pack(s), they may use '--max-cruft-size' instead.\n> + cruft pack(s), they may use '--max-cruft-size'.\n>\n> There's some minor test fallout as a result of the slight differences in\n> behavior between the old meaning of '--max-cruft-size' and the behavior\n> @@ Documentation/git-repack.adoc: to the new separate pack will be written.\n>\n> +--combine-cruft-below-size=::\n> + When generating cruft packs without pruning, only repack\n> -+ existing cruft packs whose size is strictly less than ``.\n> -+ Cruft packs whose size is greater than or equal to `` are\n> -+ left as-is and not repacked. Useful when you want to avoid\n> -+ repacking large cruft pack(s) in repositories that have many\n> -+ and/or large unreachable objects.\n> ++ existing cruft packs whose size is strictly less than ``,\n> ++ where `` represents a number of bytes, which can optionally\n> ++ be suffixed with \"k\", \"m\", or \"g\". Cruft packs whose size is\n> ++ greater than or equal to `` are left as-is and not repacked.\n> ++ Useful when you want to avoid repacking large cruft pack(s) in\n> ++ repositories that have many and/or large unreachable objects.\n> +\n> --expire-to=::\n> Write a cruft pack containing pruned objects (if any) to the\n>\n> base-commit: 08f612ba7000bf181ef6d8baed9ece322e567efd\n> --\n> 2.49.0.4.ge59cf92f8d\n\nv2 + your comments address all my feedback from v1, so this round\nlooks good to me.\n"}]}