builtin/repack.c: support writing a MIDX while repacking
Teach `git repack` a new `--write-midx` option for callers that wish to persist a multi-pack index in their repository while repacking. There are two existing alternatives to this new flag, but they don't cover our particular use-case. These alternatives are: - Call 'git multi-pack-index write' after running 'git repack', or - Set 'GIT_TEST_MULTI_PACK_INDEX=1' in your environment when running 'git repack'. The former works, but introduces a gap in bitmap coverage between repacking and writing a new MIDX (since the repack may have deleted a pack included in the existing MIDX, invalidating it altogether). Setting the 'GIT_TEST_' environment variable is obviously unsupported. In fact, even if it were supported officially, it still wouldn't work, because it generates the MIDX *after* redundant packs have been dropped, leading to the same issue as above. Introduce a new option which eliminates this race by teaching `git repack` to generate the MIDX at the critical point: after the new packs have been written and moved into place, but before the redundant packs have been removed. This option is compatible with `git repack`'s '--bitmap' option (it changes the interpretation to be: "write a bitmap corresponding to the MIDX after one has been generated"). There is a little bit of additional noise in the patch below to avoid repeating ourselves when selecting which packs to delete. Instead of a single loop as before (where we iterate over 'existing_packs', decide if a pack is worth deleting, and if so, delete it), we have two loops (the first where we decide which ones are worth deleting, and the second where we actually do the deleting). This makes it so we have a single check we can make consistently when (1) telling the MIDX which packs we want to exclude, and (2) actually unlinking the redundant packs. There is also a tiny change to short-circuit the body of write_midx_included_packs() when no packs remain in the case of an empty repository. The MIDX code does not handle this, so avoid trying to generate a MIDX covering zero packs in the first place. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Sep 28, 2021 at 21:55 UTC
1d89d88d37d18cd3af37ef4742aa39282441ec14
5 files changed
+234
-24
Documentation/git-repack.txt
+10
-4
@@ -9,7 +9,7 @@ git-repack - Pack unpacked objects in a repository
9
SYNOPSIS
10
--------
11
[verse]
12
-'git repack' [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]
12
+'git repack' [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m] [--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>] [--write-midx]
13
14
DESCRIPTION
15
-----------
@@ -128,10 +128,11 @@ depth is 4095.
128
-b::
129
--write-bitmap-index::
130
Write a reachability bitmap index as part of the repack. This
131
- only makes sense when used with `-a` or `-A`, as the bitmaps
131
+ only makes sense when used with `-a`, `-A` or `-m`, as the bitmaps
132
must be able to refer to all reachable objects. This option
133
- overrides the setting of `repack.writeBitmaps`. This option
134
- has no effect if multiple packfiles are created.
133
+ overrides the setting of `repack.writeBitmaps`. This option
134
+ has no effect if multiple packfiles are created, unless writing a
135
+ MIDX (in which case a multi-pack bitmap is created).
136
137
--pack-kept-objects::
138
Include objects in `.keep` files when repacking. Note that we
@@ -190,6 +191,11 @@ to change in the future. This option (implying a drastically different
191
repack mode) is not guaranteed to work with all other combinations of
192
option to `git repack`.
193
194
+-m::
195
+--write-midx::
196
+ Write a multi-pack index (see linkgit:git-multi-pack-index[1])
197
+ containing the non-redundant packs.
198
+
199
CONFIGURATION
200
-------------
201
builtin/repack.c
+119
-19
@@ -434,6 +434,76 @@ static void clear_pack_geometry(struct pack_geometry *geometry)
434
geometry->split = 0;
435
}
436
437
+static void midx_included_packs(struct string_list *include,
438
+ struct string_list *existing_nonkept_packs,
439
+ struct string_list *existing_kept_packs,
440
+ struct string_list *names,
441
+ struct pack_geometry *geometry)
442
+{
443
+ struct string_list_item *item;
444
+
445
+ for_each_string_list_item(item, existing_kept_packs)
446
+ string_list_insert(include, xstrfmt("%s.idx", item->string));
447
+ for_each_string_list_item(item, names)
448
+ string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
449
+ if (geometry) {
450
+ struct strbuf buf = STRBUF_INIT;
451
+ uint32_t i;
452
+ for (i = geometry->split; i < geometry->pack_nr; i++) {
453
+ struct packed_git *p = geometry->pack[i];
454
+
455
+ strbuf_addstr(&buf, pack_basename(p));
456
+ strbuf_strip_suffix(&buf, ".pack");
457
+ strbuf_addstr(&buf, ".idx");
458
+
459
+ string_list_insert(include, strbuf_detach(&buf, NULL));
460
+ }
461
+ } else {
462
+ for_each_string_list_item(item, existing_nonkept_packs) {
463
+ if (item->util)
464
+ continue;
465
+ string_list_insert(include, xstrfmt("%s.idx", item->string));
466
+ }
467
+ }
468
+}
469
+
470
+static int write_midx_included_packs(struct string_list *include,
471
+ int show_progress, int write_bitmaps)
472
+{
473
+ struct child_process cmd = CHILD_PROCESS_INIT;
474
+ struct string_list_item *item;
475
+ FILE *in;
476
+ int ret;
477
+
478
+ if (!include->nr)
479
+ return 0;
480
+
481
+ cmd.in = -1;
482
+ cmd.git_cmd = 1;
483
+
484
+ strvec_push(&cmd.args, "multi-pack-index");
485
+ strvec_pushl(&cmd.args, "write", "--stdin-packs", NULL);
486
+
487
+ if (show_progress)
488
+ strvec_push(&cmd.args, "--progress");
489
+ else
490
+ strvec_push(&cmd.args, "--no-progress");
491
+
492
+ if (write_bitmaps)
493
+ strvec_push(&cmd.args, "--bitmap");
494
+
495
+ ret = start_command(&cmd);
496
+ if (ret)
497
+ return ret;
498
+
499
+ in = xfdopen(cmd.in, "w");
500
+ for_each_string_list_item(item, include)
501
+ fprintf(in, "%s\n", item->string);
502
+ fclose(in);
503
+
504
+ return finish_command(&cmd);
505
+}
506
+
507
int cmd_repack(int argc, const char **argv, const char *prefix)
508
{
509
struct child_process cmd = CHILD_PROCESS_INIT;
@@ -457,6 +527,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
527
int no_update_server_info = 0;
528
struct pack_objects_args po_args = {NULL};
529
int geometric_factor = 0;
530
+ int write_midx = 0;
531
532
struct option builtin_repack_options[] = {
533
OPT_BIT('a', NULL, &pack_everything,
@@ -499,6 +570,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
570
N_("do not repack this pack")),
571
OPT_INTEGER('g', "geometric", &geometric_factor,
572
N_("find a geometric progression with factor <N>")),
573
+ OPT_BOOL('m', "write-midx", &write_midx,
574
+ N_("write a multi-pack index of the resulting packs")),
575
OPT_END()
576
};
577
@@ -515,8 +588,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
588
die(_("--keep-unreachable and -A are incompatible"));
589
590
if (write_bitmaps < 0) {
518
- if (!(pack_everything & ALL_INTO_ONE) ||
519
- !is_bare_repository())
591
+ if (!write_midx &&
592
+ (!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
593
write_bitmaps = 0;
594
} else if (write_bitmaps &&
595
git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0) &&
@@ -526,7 +599,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
599
if (pack_kept_objects < 0)
600
pack_kept_objects = write_bitmaps > 0;
601
529
- if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
602
+ if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)
603
die(_(incremental_bitmap_conflict_error));
604
605
if (geometric_factor) {
@@ -568,10 +641,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
641
}
642
if (has_promisor_remote())
643
strvec_push(&cmd.args, "--exclude-promisor-objects");
571
- if (write_bitmaps > 0)
572
- strvec_push(&cmd.args, "--write-bitmap-index");
573
- else if (write_bitmaps < 0)
574
- strvec_push(&cmd.args, "--write-bitmap-index-quiet");
644
+ if (!write_midx) {
645
+ if (write_bitmaps > 0)
646
+ strvec_push(&cmd.args, "--write-bitmap-index");
647
+ else if (write_bitmaps < 0)
648
+ strvec_push(&cmd.args, "--write-bitmap-index-quiet");
649
+ }
650
if (use_delta_islands)
651
strvec_push(&cmd.args, "--delta-islands");
652
@@ -684,22 +759,47 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
759
}
760
/* End of pack replacement. */
761
762
+ if (delete_redundant && pack_everything & ALL_INTO_ONE) {
763
+ const int hexsz = the_hash_algo->hexsz;
764
+ string_list_sort(&names);
765
+ for_each_string_list_item(item, &existing_nonkept_packs) {
766
+ char *sha1;
767
+ size_t len = strlen(item->string);
768
+ if (len < hexsz)
769
+ continue;
770
+ sha1 = item->string + len - hexsz;
771
+ /*
772
+ * Mark this pack for deletion, which ensures that this
773
+ * pack won't be included in a MIDX (if `--write-midx`
774
+ * was given) and that we will actually delete this pack
775
+ * (if `-d` was given).
776
+ */
777
+ item->util = (void*)(intptr_t)!string_list_has_string(&names, sha1);
778
+ }
779
+ }
780
+
781
+ if (write_midx) {
782
+ struct string_list include = STRING_LIST_INIT_NODUP;
783
+ midx_included_packs(&include, &existing_nonkept_packs,
784
+ &existing_kept_packs, &names, geometry);
785
+
786
+ ret = write_midx_included_packs(&include,
787
+ show_progress, write_bitmaps > 0);
788
+
789
+ string_list_clear(&include, 0);
790
+
791
+ if (ret)
792
+ return ret;
793
+ }
794
+
795
reprepare_packed_git(the_repository);
796
797
if (delete_redundant) {
798
int opts = 0;
691
- if (pack_everything & ALL_INTO_ONE) {
692
- const int hexsz = the_hash_algo->hexsz;
693
- string_list_sort(&names);
694
- for_each_string_list_item(item, &existing_nonkept_packs) {
695
- char *sha1;
696
- size_t len = strlen(item->string);
697
- if (len < hexsz)
698
- continue;
699
- sha1 = item->string + len - hexsz;
700
- if (!string_list_has_string(&names, sha1))
701
- remove_redundant_pack(packdir, item->string);
702
- }
799
+ for_each_string_list_item(item, &existing_nonkept_packs) {
800
+ if (!item->util)
801
+ continue;
802
+ remove_redundant_pack(packdir, item->string);
803
}
804
805
if (geometry) {
t/lib-midx.sh
new
+8
@@ -0,0 +1,8 @@
1
+# test_midx_consistent <objdir>
2
+test_midx_consistent () {
3
+ ls $1/pack/pack-*.idx | xargs -n 1 basename | sort >expect &&
4
+ test-tool read-midx $1 | grep ^pack-.*\.idx$ | sort >actual &&
5
+
6
+ test_cmp expect actual &&
7
+ git multi-pack-index --object-dir=$1 verify
8
+}
t/t7700-repack.sh
+96
@@ -3,6 +3,8 @@
3
test_description='git repack works correctly'
4
5
. ./test-lib.sh
6
+. "${TEST_DIRECTORY}/lib-bitmap.sh"
7
+. "${TEST_DIRECTORY}/lib-midx.sh"
8
9
commit_and_pack () {
10
test_commit "$@" 1>&2 &&
@@ -234,4 +236,98 @@ test_expect_success 'auto-bitmaps do not complain if unavailable' '
236
test_must_be_empty actual
237
'
238
239
+objdir=.git/objects
240
+midx=$objdir/pack/multi-pack-index
241
+
242
+test_expect_success 'setup for --write-midx tests' '
243
+ git init midx &&
244
+ (
245
+ cd midx &&
246
+ git config core.multiPackIndex true &&
247
+
248
+ test_commit base
249
+ )
250
+'
251
+
252
+test_expect_success '--write-midx unchanged' '
253
+ (
254
+ cd midx &&
255
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack &&
256
+ test_path_is_missing $midx &&
257
+ test_path_is_missing $midx-*.bitmap &&
258
+
259
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack --write-midx &&
260
+
261
+ test_path_is_file $midx &&
262
+ test_path_is_missing $midx-*.bitmap &&
263
+ test_midx_consistent $objdir
264
+ )
265
+'
266
+
267
+test_expect_success '--write-midx with a new pack' '
268
+ (
269
+ cd midx &&
270
+ test_commit loose &&
271
+
272
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack --write-midx &&
273
+
274
+ test_path_is_file $midx &&
275
+ test_path_is_missing $midx-*.bitmap &&
276
+ test_midx_consistent $objdir
277
+ )
278
+'
279
+
280
+test_expect_success '--write-midx with -b' '
281
+ (
282
+ cd midx &&
283
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -mb &&
284
+
285
+ test_path_is_file $midx &&
286
+ test_path_is_file $midx-*.bitmap &&
287
+ test_midx_consistent $objdir
288
+ )
289
+'
290
+
291
+test_expect_success '--write-midx with -d' '
292
+ (
293
+ cd midx &&
294
+ test_commit repack &&
295
+
296
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -Ad --write-midx &&
297
+
298
+ test_path_is_file $midx &&
299
+ test_path_is_missing $midx-*.bitmap &&
300
+ test_midx_consistent $objdir
301
+ )
302
+'
303
+
304
+test_expect_success 'cleans up MIDX when appropriate' '
305
+ (
306
+ cd midx &&
307
+
308
+ test_commit repack-2 &&
309
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adb --write-midx &&
310
+
311
+ checksum=$(midx_checksum $objdir) &&
312
+ test_path_is_file $midx &&
313
+ test_path_is_file $midx-$checksum.bitmap &&
314
+ test_path_is_file $midx-$checksum.rev &&
315
+
316
+ test_commit repack-3 &&
317
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adb --write-midx &&
318
+
319
+ test_path_is_file $midx &&
320
+ test_path_is_missing $midx-$checksum.bitmap &&
321
+ test_path_is_missing $midx-$checksum.rev &&
322
+ test_path_is_file $midx-$(midx_checksum $objdir).bitmap &&
323
+ test_path_is_file $midx-$(midx_checksum $objdir).rev &&
324
+
325
+ test_commit repack-4 &&
326
+ GIT_TEST_MULTI_PACK_INDEX=0 git repack -Adb &&
327
+
328
+ find $objdir/pack -type f -name "multi-pack-index*" >files &&
329
+ test_must_be_empty files
330
+ )
331
+'
332
+
333
test_done
t/t7703-repack-geometric.sh
+1
-1
@@ -15,7 +15,7 @@ test_expect_success '--geometric with no packs' '
15
(
16
cd geometric &&
17
18
- git repack --geometric 2 >out &&
18
+ git repack --write-midx --geometric 2 >out &&
19
test_i18ngrep "Nothing new to pack" out
20
)
21
'