repack: honor `-l` when calculating pack geometry
When the user passes `-l` to git-repack(1), then they essentially ask us to only repack objects part of the local object database while ignoring any packfiles part of an alternate object database. And we in fact honor this bit when doing a geometric repack as the resulting packfile will only ever contain local objects. What we're missing though is that we don't take locality of packfiles into account when computing whether the geometric sequence is intact or not. So even though we would only ever roll up local packfiles anyway, we could end up trying to repack because of non-local packfiles. This does not make much sense, and in the worst case it can cause us to try and do the geometric repack over and over again because we're never able to restore the geometric sequence. Fix this bug by honoring whether the user has passed `-l`. If so, we skip adding any non-local packfiles to the pack geometry. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Apr 14, 2023 at 08:02 UTC
932c16c04b5e41ee1c76d5640ec3ae67e1900c07
2 files changed
+68
-2
builtin/repack.c
+11
-2
@@ -321,7 +321,8 @@ static int geometry_cmp(const void *va, const void *vb)
321
}
322
323
static void init_pack_geometry(struct pack_geometry **geometry_p,
324
- struct string_list *existing_kept_packs)
324
+ struct string_list *existing_kept_packs,
325
+ const struct pack_objects_args *args)
326
{
327
struct packed_git *p;
328
struct pack_geometry *geometry;
@@ -331,6 +332,14 @@ static void init_pack_geometry(struct pack_geometry **geometry_p,
332
geometry = *geometry_p;
333
334
for (p = get_all_packs(the_repository); p; p = p->next) {
335
+ if (args->local && !p->pack_local)
336
+ /*
337
+ * When asked to only repack local packfiles we skip
338
+ * over any packfiles that are borrowed from alternate
339
+ * object directories.
340
+ */
341
+ continue;
342
+
343
if (!pack_kept_objects) {
344
/*
345
* Any pack that has its pack_keep bit set will appear
@@ -898,7 +907,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
907
if (geometric_factor) {
908
if (pack_everything)
909
die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a");
901
- init_pack_geometry(&geometry, &existing_kept_packs);
910
+ init_pack_geometry(&geometry, &existing_kept_packs, &po_args);
911
split_pack_geometry(geometry, geometric_factor);
912
}
913
t/t7703-repack-geometric.sh
+57
@@ -10,6 +10,12 @@ objdir=.git/objects
10
packdir=$objdir/pack
11
midx=$objdir/pack/multi-pack-index
12
13
+packed_objects () {
14
+ git show-index <"$1" >tmp-object-list &&
15
+ cut -d' ' -f2 tmp-object-list | sort &&
16
+ rm tmp-object-list
17
+ }
18
+
19
test_expect_success '--geometric with no packs' '
20
git init geometric &&
21
test_when_finished "rm -fr geometric" &&
@@ -361,4 +367,55 @@ test_expect_success '--geometric with same pack in main and alternate ODB' '
367
test_cmp expected-files actual-files
368
'
369
370
+test_expect_success '--geometric -l with non-intact geometric sequence across ODBs' '
371
+ test_when_finished "rm -fr shared member" &&
372
+
373
+ git init shared &&
374
+ test_commit_bulk -C shared --start=1 1 &&
375
+
376
+ git clone --shared shared member &&
377
+ test_commit_bulk -C member --start=2 1 &&
378
+
379
+ # Verify that our assumptions actually hold: both generated packfiles
380
+ # should have three objects and should be non-equal.
381
+ packed_objects shared/.git/objects/pack/pack-*.idx >shared-objects &&
382
+ packed_objects member/.git/objects/pack/pack-*.idx >member-objects &&
383
+ test_line_count = 3 shared-objects &&
384
+ test_line_count = 3 member-objects &&
385
+ ! test_cmp shared-objects member-objects &&
386
+
387
+ # Perform the geometric repack. With `-l`, we should only see the local
388
+ # packfile and thus arrive at the conclusion that the geometric
389
+ # sequence is intact. We thus expect no changes.
390
+ #
391
+ # Note that we are tweaking mtimes of the packfiles so that we can
392
+ # verify they did not change. This is done in order to detect the case
393
+ # where we do repack objects, but the resulting packfile is the same.
394
+ test-tool chmtime --verbose =0 member/.git/objects/pack/* >expected-member-packs &&
395
+ git -C member repack --geometric=2 -l -d &&
396
+ test-tool chmtime --verbose member/.git/objects/pack/* >actual-member-packs &&
397
+ test_cmp expected-member-packs actual-member-packs &&
398
+
399
+ {
400
+ packed_objects shared/.git/objects/pack/pack-*.idx &&
401
+ packed_objects member/.git/objects/pack/pack-*.idx
402
+ } | sort >expected-objects &&
403
+
404
+ # On the other hand, when doing a non-local geometric repack we should
405
+ # see both packfiles and thus repack them. We expect that the shared
406
+ # object database was not changed.
407
+ test-tool chmtime --verbose =0 shared/.git/objects/pack/* >expected-shared-packs &&
408
+ git -C member repack --geometric=2 -d &&
409
+ test-tool chmtime --verbose shared/.git/objects/pack/* >actual-shared-packs &&
410
+ test_cmp expected-shared-packs actual-shared-packs &&
411
+
412
+ # Furthermore, we expect that the member repository now has a single
413
+ # packfile that contains the combined shared and non-shared objects.
414
+ ls member/.git/objects/pack/pack-*.idx >actual &&
415
+ test_line_count = 1 actual &&
416
+ packed_objects member/.git/objects/pack/pack-*.idx >actual-objects &&
417
+ test_line_count = 6 actual-objects &&
418
+ test_cmp expected-objects actual-objects
419
+'
420
+
421
test_done