@matheusbsbs / matheus-bagliano / commits / 9f6714ab3e

builtin/gc: pack refs when using `git maintenance run --auto`

When running `git maintenance run --auto`, then the various subtasks will only run as needed. Thus, we for example end up only packing loose objects if we hit a certain threshold. Interestingly enough, the "pack-refs" task is actually _never_ executed when the auto-flag is set because it does not have a condition at all. As 41abfe15d9 (maintenance: add pack-refs task, 2021-02-09) mentions: The 'auto_condition' function pointer is left NULL for now. We could extend this in the future to have a condition check if pack-refs should be run during 'git maintenance run --auto'. It is not quite clear from that quote whether it is actually intended that the task doesn't run at all in this mode. Also, no test was added to verify this behaviour. Ultimately though, it feels quite surprising that `git maintenance run --auto --task=pack-refs` would quietly never do anything at all. In any case, now that we do have the logic in place to let ref backends decide whether or not to repack refs, it does make sense to wire it up accordingly. With the "reftable" backend we will thus now perform auto-compaction, which optimizes the refdb as needed. But for the "files" backend we now unconditionally pack refs as it does not yet know to handle the "auto" flag. Arguably, this can be seen as a bug fix given that previously the task never did anything at all. Eventually though we should amend the "files" backend to use some heuristics for auto compaction, as well. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 25, 2024 at 11:03 UTC 9f6714ab3e61ad58c4532077d4b8dc807ff0410d

3 files changed +22 -2

builtin/gc.c

+11 -1

@@ -206,6 +206,16 @@ struct maintenance_run_opts {

206 enum schedule_priority schedule;

207 };

208

209 +static int pack_refs_condition(void)

210 +{

211 + /*

212 + * The auto-repacking logic for refs is handled by the ref backends and

213 + * exposed via `git pack-refs --auto`. We thus always return truish

214 + * here and let the backend decide for us.

215 + */

216 + return 1;

217 +}

218 +

219 static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)

220 {

221 struct child_process cmd = CHILD_PROCESS_INIT;

@@ -1298,7 +1308,7 @@ static struct maintenance_task tasks[] = {

1308 [TASK_PACK_REFS] = {

1309 "pack-refs",

1310 maintenance_task_pack_refs,

1301 - NULL,

1311 + pack_refs_condition,

1312 },

1313 };

1314

t/t0601-reffiles-pack-refs.sh

+10

@@ -370,4 +370,14 @@ test_expect_success 'pack-refs does not drop broken refs during deletion' '

370 test_cmp expect actual

371 '

372

373 +test_expect_success 'maintenance --auto unconditionally packs loose refs' '

374 + git update-ref refs/heads/something HEAD &&

375 + test_path_is_file .git/refs/heads/something &&

376 + git rev-parse refs/heads/something >expect &&

377 + git maintenance run --task=pack-refs --auto &&

378 + test_path_is_missing .git/refs/heads/something &&

379 + git rev-parse refs/heads/something >actual &&

380 + test_cmp expect actual

381 +'

382 +

383 test_done

t/t0610-reftable-basics.sh

+1 -1

@@ -387,7 +387,7 @@ test_expect_success 'pack-refs: compaction raises locking errors' '

387 test_cmp expect err

388 '

389

390 -for command in pack-refs gc

390 +for command in pack-refs gc "maintenance run --task=pack-refs"

391 do

392 test_expect_success "$command: auto compaction" '

393 test_when_finished "rm -rf repo" &&