repack: begin combining cruft packs with `--combine-cruft-below-size`
The previous commit changed the behavior of repack's '--max-cruft-size' to specify a cruft pack-specific override for '--max-pack-size'. Introduce a new flag, '--combine-cruft-below-size' which is a replacement for the old behavior of '--max-cruft-size'. This new flag does explicitly what it says: it combines together cruft packs which are smaller than a given threshold, and leaves alone ones which are larger. This accomplishes the original intent of '--max-cruft-size', which was to avoid repacking cruft packs larger than the given threshold. The new behavior is slightly different. Instead of building up small packs together until the threshold is met, '--combine-cruft-below-size' packs up *all* cruft packs smaller than the threshold. This means that we may make a pack much larger than the given threshold (e.g., if you aggregate 5 packs which are each 99 MiB in size with a threshold of 100 MiB). But that's OK: the point isn't to restrict the size of the cruft packs we generate, it's to avoid working with ones that have already grown too large. If repositories still want to limit the size of the generated cruft pack(s), they may use '--max-cruft-size'. There's some minor test fallout as a result of the slight differences in behavior between the old meaning of '--max-cruft-size' and the behavior of '--combine-cruft-below-size'. In the test which is now called "--combine-cruft-below-size combines packs", we need to use the new flag over the old one to exercise that test's intended behavior. The remainder of the changes there are to improve the clarity of the comments. Suggested-by: Elijah Newren <newren@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Mar 19, 2025 at 18:52 UTC
484d7adcdadbb72a3e0106c4fa49260cf1099b9a
3 files changed
+47
-22
Documentation/git-repack.adoc
+9
@@ -81,6 +81,15 @@ to the new separate pack will be written.
81
`--max-pack-size` (if any) by default. See the documentation for
82
`--max-pack-size` for more details.
83
84
+--combine-cruft-below-size=<n>::
85
+ When generating cruft packs without pruning, only repack
86
+ existing cruft packs whose size is strictly less than `<n>`,
87
+ where `<n>` represents a number of bytes, which can optionally
88
+ be suffixed with "k", "m", or "g". Cruft packs whose size is
89
+ greater than or equal to `<n>` are left as-is and not repacked.
90
+ Useful when you want to avoid repacking large cruft pack(s) in
91
+ repositories that have many and/or large unreachable objects.
92
+
93
--expire-to=<dir>::
94
Write a cruft pack containing pruned objects (if any) to the
95
directory `<dir>`. This option is useful for keeping a copy of
builtin/repack.c
+25
-13
@@ -1022,20 +1022,13 @@ static int write_filtered_pack(const struct pack_objects_args *args,
1022
return finish_pack_objects_cmd(&cmd, names, local);
1023
}
1024
1025
-static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,
1026
- struct existing_packs *existing)
1025
+static void combine_small_cruft_packs(FILE *in, size_t combine_cruft_below_size,
1026
+ struct existing_packs *existing)
1027
{
1028
struct packed_git *p;
1029
struct strbuf buf = STRBUF_INIT;
1030
size_t i;
1031
1032
- /*
1033
- * Squelch a -Wunused-function warning while we rationalize
1034
- * the behavior of --max-cruft-size. This function will become
1035
- * used again in a future commit.
1036
- */
1037
- (void)retain_cruft_pack;
1038
-
1032
for (p = get_all_packs(the_repository); p; p = p->next) {
1033
if (!(p->is_cruft && p->pack_local))
1034
continue;
@@ -1047,7 +1040,12 @@ static void collapse_small_cruft_packs(FILE *in, size_t max_size UNUSED,
1040
if (!string_list_has_string(&existing->cruft_packs, buf.buf))
1041
continue;
1042
1050
- fprintf(in, "-%s.pack\n", buf.buf);
1043
+ if (p->pack_size < combine_cruft_below_size) {
1044
+ fprintf(in, "-%s\n", pack_basename(p));
1045
+ } else {
1046
+ retain_cruft_pack(existing, p);
1047
+ fprintf(in, "%s\n", pack_basename(p));
1048
+ }
1049
}
1050
1051
for (i = 0; i < existing->non_kept_packs.nr; i++)
@@ -1061,6 +1059,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,
1059
const char *destination,
1060
const char *pack_prefix,
1061
const char *cruft_expiration,
1062
+ unsigned long combine_cruft_below_size,
1063
struct string_list *names,
1064
struct existing_packs *existing)
1065
{
@@ -1103,8 +1102,9 @@ static int write_cruft_pack(const struct pack_objects_args *args,
1102
in = xfdopen(cmd.in, "w");
1103
for_each_string_list_item(item, names)
1104
fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
1106
- if (args->max_pack_size && !cruft_expiration) {
1107
- collapse_small_cruft_packs(in, args->max_pack_size, existing);
1105
+ if (combine_cruft_below_size && !cruft_expiration) {
1106
+ combine_small_cruft_packs(in, combine_cruft_below_size,
1107
+ existing);
1108
} else {
1109
for_each_string_list_item(item, &existing->non_kept_packs)
1110
fprintf(in, "-%s.pack\n", item->string);
@@ -1158,6 +1158,7 @@ int cmd_repack(int argc,
1158
const char *opt_window_memory = NULL;
1159
const char *opt_depth = NULL;
1160
const char *opt_threads = NULL;
1161
+ unsigned long combine_cruft_below_size = 0ul;
1162
1163
struct option builtin_repack_options[] = {
1164
OPT_BIT('a', NULL, &pack_everything,
@@ -1170,6 +1171,9 @@ int cmd_repack(int argc,
1171
PACK_CRUFT),
1172
OPT_STRING(0, "cruft-expiration", &cruft_expiration, N_("approxidate"),
1173
N_("with --cruft, expire objects older than this")),
1174
+ OPT_MAGNITUDE(0, "combine-cruft-below-size",
1175
+ &combine_cruft_below_size,
1176
+ N_("with --cruft, only repack cruft packs smaller than this")),
1177
OPT_MAGNITUDE(0, "max-cruft-size", &cruft_po_args.max_pack_size,
1178
N_("with --cruft, limit the size of new cruft packs")),
1179
OPT_BOOL('d', NULL, &delete_redundant,
@@ -1413,7 +1417,8 @@ int cmd_repack(int argc,
1417
cruft_po_args.quiet = po_args.quiet;
1418
1419
ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,
1416
- cruft_expiration, &names,
1420
+ cruft_expiration,
1421
+ combine_cruft_below_size, &names,
1422
&existing);
1423
if (ret)
1424
goto cleanup;
@@ -1440,10 +1445,17 @@ int cmd_repack(int argc,
1445
* generate an empty pack (since every object not in the
1446
* cruft pack generated above will have an mtime older
1447
* than the expiration).
1448
+ *
1449
+ * Pretend we don't have a `--combine-cruft-below-size`
1450
+ * argument, since we're not selectively combining
1451
+ * anything based on size to generate the limbo cruft
1452
+ * pack, but rather removing all cruft packs from the
1453
+ * main repository regardless of size.
1454
*/
1455
ret = write_cruft_pack(&cruft_po_args, expire_to,
1456
pack_prefix,
1457
NULL,
1458
+ 0ul,
1459
&names,
1460
&existing);
1461
if (ret)
t/t7704-repack-cruft.sh
+13
-9
@@ -194,10 +194,13 @@ test_expect_success '--max-cruft-size combines existing packs when not too large
194
)
195
'
196
197
-test_expect_failure '--max-cruft-size combines smaller packs first' '
198
- git init max-cruft-size-consume-small &&
197
+test_expect_success '--combine-cruft-below-size combines packs' '
198
+ repo=combine-cruft-below-size &&
199
+ test_when_finished "rm -fr $repo" &&
200
+
201
+ git init "$repo" &&
202
(
200
- cd max-cruft-size-consume-small &&
203
+ cd "$repo" &&
204
205
test_commit base &&
206
git repack -ad &&
@@ -211,11 +214,11 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '
214
test-tool pack-mtimes "$(basename $cruft_bar)" >>expect.raw &&
215
sort expect.raw >expect.objects &&
216
214
- # repacking with `--max-cruft-size=2M` should combine
215
- # both 0.5 MiB packs together, instead of, say, one of
216
- # the 0.5 MiB packs with the 1.0 MiB pack
217
+ # Repacking with `--combine-cruft-below-size=1M`
218
+ # should combine both 0.5 MiB packs together, but
219
+ # ignore the two packs which are >= 1.0 MiB.
220
ls $packdir/pack-*.mtimes | sort >cruft.before &&
218
- git repack -d --cruft --max-cruft-size=2M &&
221
+ git repack -d --cruft --combine-cruft-below-size=1M &&
222
ls $packdir/pack-*.mtimes | sort >cruft.after &&
223
224
comm -13 cruft.before cruft.after >cruft.new &&
@@ -224,11 +227,12 @@ test_expect_failure '--max-cruft-size combines smaller packs first' '
227
test_line_count = 1 cruft.new &&
228
test_line_count = 2 cruft.removed &&
229
227
- # the two smaller packs should be rolled up first
230
+ # The two packs smaller than 1.0MiB should be repacked
231
+ # together.
232
printf "%s\n" $cruft_foo $cruft_bar | sort >expect.removed &&
233
test_cmp expect.removed cruft.removed &&
234
231
- # ...and contain the set of objects rolled up
235
+ # ...and contain the set of objects rolled up.
236
test-tool pack-mtimes "$(basename $(cat cruft.new))" >actual.raw &&
237
sort actual.raw >actual.objects &&
238