From 368565e55d5cff377efaee5cf86f092709480314 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:04 +0200 Subject: [PATCH 01/25] t7900: simplify how we check for maintenance tasks We have several tests in t7900 that verify whether specific maintenance tasks did or did not run. This is done rather ad-hoc by checking for spawned Git commands, which is awfully fragile: - We have to adjust tests whenever arguments to the spawned Git commands change. - We don't have a way to verify that negative matches are still working as expected. - We rely on maintenance tasks spawning a Git command in the first place. We can do much better though, as we already have trace2 regions for each of the maintenance tasks. Introduce a helper function that extracts all such regions so that we can get a direct list of all maintenance tasks that a certain command ran. Adapt tests that care about whether or not a specific task ran to use this new helper. Note that many tests still use `test_subcommand` though, as they really care about the exact command that was executed. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t7900-maintenance.sh | 194 ++++++++++++++++++++++------------------- 1 file changed, 102 insertions(+), 92 deletions(-) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index d7f82e1bec163f..129829f1f42f34 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -23,6 +23,12 @@ test_xmllint () { fi } +test_maintenance_tasks () { + cat >expect && + sed -ne "s/.*\"region_enter\".*\"category\":\"maintenance\([^\"]*\)\".*\"label\":\"\([^\"][^\"]*\)\".*/\2\1/p" "$1" >actual && + test_cmp expect actual +} + test_lazy_prereq SYSTEMD_ANALYZE ' systemd-analyze verify /lib/systemd/system/basic.target ' @@ -180,8 +186,9 @@ test_expect_success 'maintenance..enabled' ' git config maintenance.gc.enabled false && git config maintenance.commit-graph.enabled true && GIT_TRACE2_EVENT="$(pwd)/run-config.txt" git maintenance run 2>err && - test_subcommand ! git gc --quiet ' ' @@ -189,16 +196,20 @@ test_expect_success 'run --task=' ' git maintenance run --task=commit-graph 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-gc.txt" \ git maintenance run --task=gc 2>/dev/null && - GIT_TRACE2_EVENT="$(pwd)/run-commit-graph.txt" \ - git maintenance run --task=commit-graph 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \ git maintenance run --task=commit-graph --task=gc 2>/dev/null && - test_subcommand ! git gc --quiet --no-detach --skip-foreground-tasks daily -> hourly' ' GIT_TRACE2_EVENT="$(pwd)/hourly.txt" \ git maintenance run --schedule=hourly 2>/dev/null && - test_subcommand git prune-packed --quiet /dev/null && - test_subcommand git prune-packed --quiet /dev/null && - test_subcommand git prune-packed --quiet expect && rm -f trace2.txt && GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ git -c maintenance.strategy=$STRATEGY maintenance run --quiet "$@" && - sed -n 's/{"event":"child_start","sid":"[^/"]*",.*,"argv":\["\(.*\)\"]}/\1/p' actual - test_cmp expect actual + test_maintenance_tasks trace2.txt } test_expect_success 'maintenance.strategy is respected' ' @@ -1017,48 +1031,44 @@ test_expect_success 'maintenance.strategy is respected' ' test_grep "unknown maintenance strategy: .unknown." err && test_strategy incremental <<-\EOF && - git pack-refs --all --prune - git reflog expire --all - git gc --quiet --no-detach --skip-foreground-tasks + gc foreground + gc EOF test_strategy incremental --schedule=weekly <<-\EOF && - git pack-refs --all --prune - git prune-packed --quiet - git multi-pack-index write --no-progress - git multi-pack-index expire --no-progress - git multi-pack-index repack --no-progress --batch-size=1 - git commit-graph write --split --reachable --no-progress + pack-refs foreground + prefetch + loose-objects + incremental-repack + commit-graph EOF test_strategy gc <<-\EOF && - git pack-refs --all --prune - git reflog expire --all - git gc --quiet --no-detach --skip-foreground-tasks + gc foreground + gc EOF test_strategy gc --schedule=weekly <<-\EOF && - git pack-refs --all --prune - git reflog expire --all - git gc --quiet --no-detach --skip-foreground-tasks + gc foreground + gc EOF test_strategy geometric <<-\EOF && - git pack-refs --all --prune - git reflog expire --all - git repack -d -l --geometric=2 --quiet --write-midx - git commit-graph write --split --reachable --no-progress - git worktree prune --expire 3.months.ago - git rerere gc + pack-refs foreground + reflog-expire foreground + geometric-repack + commit-graph + worktree-prune + rerere-gc EOF test_strategy geometric --schedule=weekly <<-\EOF - git pack-refs --all --prune - git reflog expire --all - git repack -d -l --geometric=2 --quiet --write-midx - git commit-graph write --split --reachable --no-progress - git worktree prune --expire 3.months.ago - git rerere gc + pack-refs foreground + reflog-expire foreground + geometric-repack + commit-graph + worktree-prune + rerere-gc EOF ) ' From 37deb9b4be807643ef264738f7fa3dc97588e33d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:05 +0200 Subject: [PATCH 02/25] odb: run "pre-auto-gc" hook for all maintenance tasks The "pre-auto-gc" hook is supposed to run before auto-maintenance starts. The intent of this is to give users the ability to intercept running maintenance in case there's for example an event that is not supposed to run in parallel with repository maintenance. This hook runs via `need_to_gc()`, which is invoked via two paths: - It is called directly by git-gc(1). - It is called indirectly by git-maintenance(1) via the "gc" task. While the former makes sense, the latter is somewhat off. While the hook is indeed strongly tied to gc'ing a repository, the original intent of the hook is rather to inhibit any kind of automated garbage collection. That noticeably also includes all the other maintenance tasks that our new infrastructure may run, but those aren't getting intercepted at all. The move towards our new maintenance strategy has thus somewhat neutered the effectiveness of the hook. Fix this issue by running the hook before the first auto-maintenance task that would run as determined by the tasks's auto condition. Note that this requires us to lift the call to `run_hooks()` out of `needs_to_gc()`, as the hook would otherwise potentially run multiple times. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 35 +++++++++--- t/t7900-maintenance.sh | 126 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 9 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index d32af422af5e58..77d0a5c9484375 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -709,8 +709,6 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args) else return 0; - if (run_hooks(the_repository, "pre-auto-gc")) - return 0; return 1; } @@ -933,7 +931,8 @@ int cmd_gc(int argc, /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(&cfg, &repack_args)) { + if (!need_to_gc(&cfg, &repack_args) || + run_hooks(the_repository, "pre-auto-gc")) { ret = 0; goto out; } @@ -1755,11 +1754,18 @@ enum task_phase { TASK_PHASE_BACKGROUND, }; +enum auto_gc_hook_result { + AUTO_GC_HOOK_UNDECIDED = 0, + AUTO_GC_HOOK_RUN = 1, + AUTO_GC_HOOK_SKIP = 2, +}; + static int maybe_run_task(const struct maintenance_task *task, struct repository *repo, struct maintenance_run_opts *opts, struct gc_config *cfg, - enum task_phase phase) + enum task_phase phase, + enum auto_gc_hook_result *auto_gc_hook_result) { int foreground = (phase == TASK_PHASE_FOREGROUND); maintenance_task_fn fn = foreground ? task->foreground : task->background; @@ -1768,9 +1774,19 @@ static int maybe_run_task(const struct maintenance_task *task, if (!fn) return 0; - if (opts->auto_flag && - (!task->auto_condition || !task->auto_condition(cfg))) - return 0; + if (opts->auto_flag) { + if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP) + return 0; + + if (!task->auto_condition || !task->auto_condition(cfg)) + return 0; + + if (*auto_gc_hook_result == AUTO_GC_HOOK_UNDECIDED) + *auto_gc_hook_result = run_hooks(repo, "pre-auto-gc") ? + AUTO_GC_HOOK_SKIP : AUTO_GC_HOOK_RUN; + if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP) + return 0; + } trace2_region_enter(region, task->name, repo); if (fn(opts, cfg)) { @@ -1789,6 +1805,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, struct lock_file lk; struct repository *r = the_repository; char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path); + enum auto_gc_hook_result auto_gc_hook_result = AUTO_GC_HOOK_UNDECIDED; if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { /* @@ -1808,7 +1825,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, for (size_t i = 0; i < opts->tasks_nr; i++) if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, - TASK_PHASE_FOREGROUND)) + TASK_PHASE_FOREGROUND, &auto_gc_hook_result)) result = 1; /* Failure to daemonize is ok, we'll continue in foreground. */ @@ -1820,7 +1837,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts, for (size_t i = 0; i < opts->tasks_nr; i++) if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, - TASK_PHASE_BACKGROUND)) + TASK_PHASE_BACKGROUND, &auto_gc_hook_result)) result = 1; rollback_lock_file(&lk); diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 129829f1f42f34..2d52e7918a33ff 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -758,6 +758,132 @@ test_expect_success 'geometric repacking honors configured split factor' ' ) ' +test_expect_success 'pre-auto-gc hook runs exactly once' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + write_script .git/hooks/pre-auto-gc <<-\EOF && + echo hook >>hook.log + EOF + + # Satisfy the auto condition for multiple tasks, both in the + # foreground and in the background phase. + git config set maintenance.reflog-expire.auto -1 && + git config set maintenance.geometric-repack.auto -1 && + git config set maintenance.rerere-gc.auto -1 && + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git maintenance run --auto 2>/dev/null && + + # The successful hook does not inhibit any of the tasks... + test_maintenance_tasks trace2.txt <<-\EOF && + reflog-expire foreground + geometric-repack + rerere-gc + EOF + # ... but it must only have been executed a single time. + test_line_count = 1 hook.log + ) +' + +test_expect_success 'pre-auto-gc hook can inhibit geometric strategy' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + write_script .git/hooks/pre-auto-gc <<-\EOF && + echo hook >>hook.log + exit 1 + EOF + + git config set maintenance.reflog-expire.auto -1 && + git config set maintenance.geometric-repack.auto -1 && + git config set maintenance.rerere-gc.auto -1 && + + # Maintenance would be required... + git maintenance is-needed --auto && + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git maintenance run --auto 2>/dev/null && + + # ... but the failing hook inhibits all tasks. The hook itself + # is expected to be the only child process being spawned, and + # it must only run a single time. + test_grep "child_start.*pre-auto-gc" trace2.txt && + test_maintenance_tasks trace2.txt <<-\EOF && + EOF + test_line_count = 1 hook.log + ) +' + +test_expect_success 'pre-auto-gc hook can inhibit gc strategy' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + write_script .git/hooks/pre-auto-gc <<-\EOF && + echo hook >>hook.log + exit 1 + EOF + + git config set maintenance.strategy gc && + git config set maintenance.auto false && + git config set gc.auto 3 && + + test_oid_init && + + # We need to create two objects whose hashes start with 17 + # since this is what the gc task counts. + test_commit "$(test_oid blob17_1)" && + test_commit "$(test_oid blob17_2)" && + + # Maintenance would be required... + git maintenance is-needed --auto && + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git maintenance run --auto 2>/dev/null && + + # ... but the failing hook inhibits all tasks. The hook itself + # is expected to be the only child process being spawned, and + # it must only run a single time. + test_grep "child_start.*pre-auto-gc" trace2.txt && + test_maintenance_tasks trace2.txt <<-\EOF && + EOF + test_subcommand_flex ! git trace2 && + test_line_count = 1 hook.log + ) +' + +test_expect_success 'pre-auto-gc hook does not run when no maintenance is needed' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + write_script .git/hooks/pre-auto-gc <<-\EOF && + echo hook >>hook.log + EOF + test_must_fail git maintenance is-needed --auto && + git maintenance run --auto 2>/dev/null && + test_path_is_missing hook.log + ) +' + +test_expect_success 'pre-auto-gc hook does not run without --auto' ' + test_when_finished "rm -rf repo" && + git init repo && + test_hook -C repo pre-auto-gc <<-\EOF && + echo hook >>hook.log + EOF + ( + cd repo && + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ + git maintenance run 2>/dev/null && + test_grep "\[\"git\",\"repack\"," trace2.txt && + test_path_is_missing hook.log + ) +' + test_expect_success 'pack-refs task' ' for n in $(test_seq 1 5) do From 3f8696804331ee9dacf49fa0c6fec4300a37b709 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:06 +0200 Subject: [PATCH 03/25] builtin/gc: move worktree and rerere tasks before object optimizations In subsequent patches we'll consolidate all tasks that relate to maintenance of the object database and move it into the "files" backend. The relevant code is somewhat scattered though, as several other tasks are interspersed between. Refactor the code so that all object database optimizations are grouped together, which requires us to move worktree pruning and rerere garbage collection around. In theory, rearranging this code can have an effect on the object database optimizations: - Rerere entries really shouldn't impact garbage collection at all, as these entries are not stored in the object database. - The index and HEAD reference of pruned worktrees may reference objects that become unreachable. That being said, the impact should be overall rather negligible. If the user was asking us to prune objects with immediate expiration time then we might now prune objects that were previously still kept alive by the worktree. But besides being a very specific edge case, it's arguably not even the wrong thing to also prune any potentially-unreachable objects immediately. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 77d0a5c9484375..8f568003eef52f 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1011,6 +1011,13 @@ int cmd_gc(int argc, if (opts.detach <= 0 && !skip_foreground_tasks) gc_foreground_tasks(&opts, &cfg); + if (cfg.prune_worktrees_expire && + maintenance_task_worktree_prune(&opts, &cfg)) + die(FAILED_RUN, "worktree"); + + if (maintenance_task_rerere_gc(&opts, &cfg)) + die(FAILED_RUN, "rerere"); + if (!the_repository->repository_format_precious_objects) { struct child_process repack_cmd = CHILD_PROCESS_INIT; @@ -1038,13 +1045,6 @@ int cmd_gc(int argc, } } - if (cfg.prune_worktrees_expire && - maintenance_task_worktree_prune(&opts, &cfg)) - die(FAILED_RUN, "worktree"); - - if (maintenance_task_rerere_gc(&opts, &cfg)) - die(FAILED_RUN, "rerere"); - report_garbage = report_pack_garbage; odb_reprepare(the_repository->objects); if (pack_garbage.nr > 0) { From e86fe6fe62d0fb5b54a1afed6166d3e27d28f081 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:07 +0200 Subject: [PATCH 04/25] builtin/gc: extract object database optimizations into separate function Extract the object database optimization logic from `cmd_gc()` into a new `maintenance_task_odb()` helper function. This is a pure refactoring with no intended functional change. Note that the message that notifies the user about too many loose objects is moved into the new function, as well. It is inherently an implementation detail of how the "files" source works, and as a consequence we'll move it around in a later commit, as well. This reordering means that the warning may now be printed at a different point in time, but it's not expected that this will have any practical implications. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 79 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 30 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 8f568003eef52f..2ff98fa727b1e0 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -839,6 +839,53 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, return 0; } +static int maintenance_task_odb(struct maintenance_run_opts *opts, + struct gc_config *cfg, + struct strvec *repack_args) +{ + struct child_process repack_cmd = CHILD_PROCESS_INIT; + int ret; + + if (the_repository->repository_format_precious_objects) + return 0; + + repack_cmd.git_cmd = 1; + repack_cmd.odb_to_close = the_repository->objects; + strvec_pushv(&repack_cmd.args, repack_args->v); + if (run_command(&repack_cmd)) { + ret = error(FAILED_RUN, repack_args->v[0]); + goto out; + } + + if (cfg->prune_expire) { + struct child_process prune_cmd = CHILD_PROCESS_INIT; + + strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); + /* run `git prune` even if using cruft packs */ + strvec_push(&prune_cmd.args, cfg->prune_expire); + if (opts->quiet) + strvec_push(&prune_cmd.args, "--no-progress"); + if (repo_has_promisor_remote(the_repository)) + strvec_push(&prune_cmd.args, + "--exclude-promisor-objects"); + prune_cmd.git_cmd = 1; + + if (run_command(&prune_cmd)) { + ret = error(FAILED_RUN, prune_cmd.args.v[0]); + goto out; + } + } + + if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold)) + warning(_("There are too many unreachable loose objects; " + "run 'git prune' to remove them.")); + + ret = 0; + +out: + return ret; +} + int cmd_gc(int argc, const char **argv, const char *prefix, @@ -1018,32 +1065,8 @@ int cmd_gc(int argc, if (maintenance_task_rerere_gc(&opts, &cfg)) die(FAILED_RUN, "rerere"); - if (!the_repository->repository_format_precious_objects) { - struct child_process repack_cmd = CHILD_PROCESS_INIT; - - repack_cmd.git_cmd = 1; - repack_cmd.odb_to_close = the_repository->objects; - strvec_pushv(&repack_cmd.args, repack_args.v); - if (run_command(&repack_cmd)) - die(FAILED_RUN, repack_args.v[0]); - - if (cfg.prune_expire) { - struct child_process prune_cmd = CHILD_PROCESS_INIT; - - strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); - /* run `git prune` even if using cruft packs */ - strvec_push(&prune_cmd.args, cfg.prune_expire); - if (opts.quiet) - strvec_push(&prune_cmd.args, "--no-progress"); - if (repo_has_promisor_remote(the_repository)) - strvec_push(&prune_cmd.args, - "--exclude-promisor-objects"); - prune_cmd.git_cmd = 1; - - if (run_command(&prune_cmd)) - die(FAILED_RUN, prune_cmd.args.v[0]); - } - } + if (maintenance_task_odb(&opts, &cfg, &repack_args)) + die(NULL); report_garbage = report_pack_garbage; odb_reprepare(the_repository->objects); @@ -1057,10 +1080,6 @@ int cmd_gc(int argc, !opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0, NULL); - if (opts.auto_flag && too_many_loose_objects(cfg.gc_auto_threshold)) - warning(_("There are too many unreachable loose objects; " - "run 'git prune' to remove them.")); - if (!daemonized) { char *path = repo_git_path(the_repository, "gc.log"); unlink(path); From 4d93bf9873696a33a9959c912e7a3fa4fb56a82b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:08 +0200 Subject: [PATCH 05/25] builtin/gc: make repack arguments self-contained When optimizing the object database most of the heavy-lifting is done by git-repack(1). The arguments we pass to this function are assembled in global scope, which is hard to follow. Refactor the logic by moving the vector into `maintenance_task_odb()`. While that means we have to pass more arguments to this function, it has the upside that the logic becomes self-contained without any kind of global interdependencies. This is a pure refactoring with no intended functional change. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 156 +++++++++++++++++++++++++-------------------------- 1 file changed, 75 insertions(+), 81 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 2ff98fa727b1e0..25a59caea696c9 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -661,7 +661,7 @@ static void add_repack_incremental_option(struct strvec *args) strvec_push(args, "--no-write-bitmap-index"); } -static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args) +static int need_to_gc(struct gc_config *cfg) { /* * Setting gc.auto to 0 or negative can disable the @@ -669,46 +669,8 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args) */ if (cfg->gc_auto_threshold <= 0) return 0; - - /* - * If there are too many loose objects, but not too many - * packs, we run "repack -d -l". If there are too many packs, - * we run "repack -A -d -l". Otherwise we tell the caller - * there is no need. - */ - if (too_many_packs(cfg)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (cfg->big_pack_threshold) { - find_base_packs(&keep_pack, cfg->big_pack_threshold); - if (keep_pack.nr >= cfg->gc_auto_pack_limit) { - cfg->big_pack_threshold = 0; - string_list_clear(&keep_pack, 0); - find_base_packs(&keep_pack, 0); - } - } else { - struct packed_git *p = find_base_packs(&keep_pack, 0); - uint64_t mem_have, mem_want; - - mem_have = total_ram(); - mem_want = estimate_repack_memory(cfg, p); - - /* - * Only allow 1/2 of memory for pack-objects, leave - * the rest for the OS and other processes in the - * system. - */ - if (!mem_have || mem_want < mem_have / 2) - string_list_clear(&keep_pack, 0); - } - - add_repack_all_option(cfg, &keep_pack, repack_args); - string_list_clear(&keep_pack, 0); - } else if (too_many_loose_objects(cfg->gc_auto_threshold)) - add_repack_incremental_option(repack_args); - else + if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold)) return 0; - return 1; } @@ -841,7 +803,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, static int maintenance_task_odb(struct maintenance_run_opts *opts, struct gc_config *cfg, - struct strvec *repack_args) + int keep_largest_pack, + int aggressive) { struct child_process repack_cmd = CHILD_PROCESS_INIT; int ret; @@ -851,9 +814,75 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, repack_cmd.git_cmd = 1; repack_cmd.odb_to_close = the_repository->objects; - strvec_pushv(&repack_cmd.args, repack_args->v); + + strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); + if (aggressive) { + strvec_push(&repack_cmd.args, "-f"); + if (cfg->aggressive_depth > 0) + strvec_pushf(&repack_cmd.args, "--depth=%d", cfg->aggressive_depth); + if (cfg->aggressive_window > 0) + strvec_pushf(&repack_cmd.args, "--window=%d", cfg->aggressive_window); + } + if (opts->quiet) + strvec_push(&repack_cmd.args, "-q"); + + /* + * There's three cases we need to consider: + * + * - If we're invoked without `--auto` we'll need to perform a full + * repack. + * + * - If we're invoked with `--auto` and there's too many packs, then + * we perform a full repack, as well. + * + * - Otherwise we perform an incremental repack. + */ + if (!opts->auto_flag) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (keep_largest_pack != -1) { + if (keep_largest_pack) + find_base_packs(&keep_pack, 0); + } else if (cfg->big_pack_threshold) { + find_base_packs(&keep_pack, cfg->big_pack_threshold); + } + + add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else if (too_many_packs(cfg)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (cfg->big_pack_threshold) { + find_base_packs(&keep_pack, cfg->big_pack_threshold); + if (keep_pack.nr >= cfg->gc_auto_pack_limit) { + cfg->big_pack_threshold = 0; + string_list_clear(&keep_pack, 0); + find_base_packs(&keep_pack, 0); + } + } else { + struct packed_git *p = find_base_packs(&keep_pack, 0); + uint64_t mem_have, mem_want; + + mem_have = total_ram(); + mem_want = estimate_repack_memory(cfg, p); + + /* + * Only allow 1/2 of memory for pack-objects, leave + * the rest for the OS and other processes in the + * system. + */ + if (!mem_have || mem_want < mem_have / 2) + string_list_clear(&keep_pack, 0); + } + + add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); + } + if (run_command(&repack_cmd)) { - ret = error(FAILED_RUN, repack_args->v[0]); + ret = error(FAILED_RUN, repack_cmd.args.v[0]); goto out; } @@ -899,7 +928,6 @@ int cmd_gc(int argc, int keep_largest_pack = -1; int skip_foreground_tasks = 0; timestamp_t dummy; - struct strvec repack_args = STRVEC_INIT; struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; struct gc_config cfg = GC_CONFIG_INIT; const char *prune_expire_sentinel = "sentinel"; @@ -939,8 +967,6 @@ int cmd_gc(int argc, show_usage_with_options_if_asked(argc, argv, builtin_gc_usage, builtin_gc_options); - strvec_pushl(&repack_args, "repack", "-d", "-l", NULL); - gc_config(&cfg); if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time)) @@ -961,16 +987,6 @@ int cmd_gc(int argc, if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy)) die(_("failed to parse prune expiry value %s"), cfg.prune_expire); - if (aggressive) { - strvec_push(&repack_args, "-f"); - if (cfg.aggressive_depth > 0) - strvec_pushf(&repack_args, "--depth=%d", cfg.aggressive_depth); - if (cfg.aggressive_window > 0) - strvec_pushf(&repack_args, "--window=%d", cfg.aggressive_window); - } - if (opts.quiet) - strvec_push(&repack_args, "-q"); - if (opts.auto_flag) { if (cfg.detach_auto && opts.detach < 0) opts.detach = 1; @@ -978,8 +994,7 @@ int cmd_gc(int argc, /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(&cfg, &repack_args) || - run_hooks(the_repository, "pre-auto-gc")) { + if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) { ret = 0; goto out; } @@ -991,18 +1006,6 @@ int cmd_gc(int argc, fprintf(stderr, _("Auto packing the repository for optimum performance.\n")); fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n")); } - } else { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (keep_largest_pack != -1) { - if (keep_largest_pack) - find_base_packs(&keep_pack, 0); - } else if (cfg.big_pack_threshold) { - find_base_packs(&keep_pack, cfg.big_pack_threshold); - } - - add_repack_all_option(&cfg, &keep_pack, &repack_args); - string_list_clear(&keep_pack, 0); } if (opts.detach > 0) { @@ -1065,7 +1068,7 @@ int cmd_gc(int argc, if (maintenance_task_rerere_gc(&opts, &cfg)) die(FAILED_RUN, "rerere"); - if (maintenance_task_odb(&opts, &cfg, &repack_args)) + if (maintenance_task_odb(&opts, &cfg, keep_largest_pack, aggressive)) die(NULL); report_garbage = report_pack_garbage; @@ -1088,7 +1091,6 @@ int cmd_gc(int argc, out: maintenance_run_opts_release(&opts); - strvec_clear(&repack_args); gc_config_release(&cfg); return 0; } @@ -1291,15 +1293,7 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts, static int gc_condition(struct gc_config *cfg) { - /* - * Note that it's fine to drop the repack arguments here, as we execute - * git-gc(1) as a separate child process anyway. So it knows to compute - * these arguments again. - */ - struct strvec repack_args = STRVEC_INIT; - int ret = need_to_gc(cfg, &repack_args); - strvec_clear(&repack_args); - return ret; + return need_to_gc(cfg); } static int prune_packed(struct maintenance_run_opts *opts) From 57ca517baca6c28ed11e923ed7f25bb5c7af8909 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:09 +0200 Subject: [PATCH 06/25] builtin/gc: inline config values specific to the "files" backend The `struct gc_config` contains a set of values that we read via the Git repository's configuration. Several of those values that are consumed by the object database optimization logic are inherently specific to the "files" config. In a later commit we'll make the logic to optimize object databases pluggable. So by carrying these "files"-backend specific values in the generic config struct means that other backends would have to worry about these values, too. This feels somewhat dirty, as implementation- specific details should live with the backends themselves. Inline these values directly at the call sites that need them instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 115 ++++++++++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 62 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 25a59caea696c9..5d445edaa06923 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -130,22 +130,11 @@ struct gc_config { unsigned long max_cruft_size; int aggressive_depth; int aggressive_window; - int gc_auto_threshold; - int gc_auto_pack_limit; int detach_auto; char *gc_log_expire; char *prune_expire; char *prune_worktrees_expire; - char *repack_filter; - char *repack_filter_to; char *repack_expire_to; - unsigned long big_pack_threshold; - unsigned long max_delta_cache_size; - /* - * Remove this member from gc_config once repo_settings is passed - * through the callchain. - */ - size_t delta_base_cache_limit; }; #define GC_CONFIG_INIT { \ @@ -154,14 +143,10 @@ struct gc_config { .cruft_packs = 1, \ .aggressive_depth = 50, \ .aggressive_window = 250, \ - .gc_auto_threshold = 6700, \ - .gc_auto_pack_limit = 50, \ .detach_auto = 1, \ .gc_log_expire = xstrdup("1.day.ago"), \ .prune_expire = xstrdup("2.weeks.ago"), \ .prune_worktrees_expire = xstrdup("3.months.ago"), \ - .max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \ - .delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \ } static void gc_config_release(struct gc_config *cfg) @@ -169,15 +154,12 @@ static void gc_config_release(struct gc_config *cfg) free(cfg->gc_log_expire); free(cfg->prune_expire); free(cfg->prune_worktrees_expire); - free(cfg->repack_filter); - free(cfg->repack_filter_to); } static void gc_config(struct gc_config *cfg) { const char *value; char *owned = NULL; - unsigned long ulongval; if (!repo_config_get_value(the_repository, "gc.packrefs", &value)) { if (value && !strcmp(value, "notbare")) @@ -192,8 +174,6 @@ static void gc_config(struct gc_config *cfg) repo_config_get_int(the_repository, "gc.aggressivewindow", &cfg->aggressive_window); repo_config_get_int(the_repository, "gc.aggressivedepth", &cfg->aggressive_depth); - repo_config_get_int(the_repository, "gc.auto", &cfg->gc_auto_threshold); - repo_config_get_int(the_repository, "gc.autopacklimit", &cfg->gc_auto_pack_limit); repo_config_get_bool(the_repository, "gc.autodetach", &cfg->detach_auto); repo_config_get_bool(the_repository, "gc.cruftpacks", &cfg->cruft_packs); repo_config_get_ulong(the_repository, "gc.maxcruftsize", &cfg->max_cruft_size); @@ -213,22 +193,6 @@ static void gc_config(struct gc_config *cfg) cfg->gc_log_expire = owned; } - repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &cfg->big_pack_threshold); - repo_config_get_ulong(the_repository, "pack.deltacachesize", &cfg->max_delta_cache_size); - - if (!repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &ulongval)) - cfg->delta_base_cache_limit = ulongval; - - if (!repo_config_get_string(the_repository, "gc.repackfilter", &owned)) { - free(cfg->repack_filter); - cfg->repack_filter = owned; - } - - if (!repo_config_get_string(the_repository, "gc.repackfilterto", &owned)) { - free(cfg->repack_filter_to); - cfg->repack_filter_to = owned; - } - repo_config(the_repository, git_default_config, NULL); } @@ -504,12 +468,12 @@ static struct packed_git *find_base_packs(struct string_list *packs, return base; } -static int too_many_packs(struct gc_config *cfg) +static int too_many_packs(int gc_auto_pack_limit) { struct packed_git *p; int cnt = 0; - if (cfg->gc_auto_pack_limit <= 0) + if (gc_auto_pack_limit <= 0) return 0; repo_for_each_pack(the_repository, p) { @@ -523,7 +487,7 @@ static int too_many_packs(struct gc_config *cfg) */ cnt++; } - return cfg->gc_auto_pack_limit < cnt; + return gc_auto_pack_limit < cnt; } static uint64_t total_ram(void) @@ -571,9 +535,10 @@ static uint64_t total_ram(void) return 0; } -static uint64_t estimate_repack_memory(struct gc_config *cfg, - struct packed_git *pack) +static uint64_t estimate_repack_memory(struct packed_git *pack) { + unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE; + unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT; unsigned long nr_objects; size_t os_cache, heap; @@ -584,6 +549,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg, if (!pack || !nr_objects) return 0; + repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size); + repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit); + /* * First we have to scan through at least one pack. * Assume enough room in OS file cache to keep the entire pack @@ -611,9 +579,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg, * read_sha1_file() (either at delta calculation phase, or * writing phase) also fills up the delta base cache */ - heap += cfg->delta_base_cache_limit; + heap += delta_base_cache_limit; /* and of course pack-objects has its own delta cache */ - heap += cfg->max_delta_cache_size; + heap += max_delta_cache_size; return os_cache + heap; } @@ -629,6 +597,12 @@ static void add_repack_all_option(struct gc_config *cfg, struct string_list *keep_pack, struct strvec *args) { + char *repack_filter = NULL; + char *repack_filter_to = NULL; + + repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter); + repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to); + if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now") && !(cfg->cruft_packs && cfg->repack_expire_to)) strvec_push(args, "-a"); @@ -650,10 +624,13 @@ static void add_repack_all_option(struct gc_config *cfg, if (keep_pack) for_each_string_list(keep_pack, keep_one_pack, args); - if (cfg->repack_filter && *cfg->repack_filter) - strvec_pushf(args, "--filter=%s", cfg->repack_filter); - if (cfg->repack_filter_to && *cfg->repack_filter_to) - strvec_pushf(args, "--filter-to=%s", cfg->repack_filter_to); + if (repack_filter && *repack_filter) + strvec_pushf(args, "--filter=%s", repack_filter); + if (repack_filter_to && *repack_filter_to) + strvec_pushf(args, "--filter-to=%s", repack_filter_to); + + free(repack_filter); + free(repack_filter_to); } static void add_repack_incremental_option(struct strvec *args) @@ -661,16 +638,24 @@ static void add_repack_incremental_option(struct strvec *args) strvec_push(args, "--no-write-bitmap-index"); } -static int need_to_gc(struct gc_config *cfg) +static int need_to_gc(struct repository *repo) { + int gc_auto_threshold = 6700; + int gc_auto_pack_limit = 50; + + repo_config_get_int(repo, "gc.auto", &gc_auto_threshold); + repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit); + /* * Setting gc.auto to 0 or negative can disable the * automatic gc. */ - if (cfg->gc_auto_threshold <= 0) + if (gc_auto_threshold <= 0) return 0; - if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold)) + if (!too_many_packs(gc_auto_pack_limit) && + !too_many_loose_objects(gc_auto_threshold)) return 0; + return 1; } @@ -807,8 +792,15 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, int aggressive) { struct child_process repack_cmd = CHILD_PROCESS_INIT; + unsigned long big_pack_threshold = 0; + int gc_auto_threshold = 6700; + int gc_auto_pack_limit = 50; int ret; + repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold); + repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit); + repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold); + if (the_repository->repository_format_precious_objects) return 0; @@ -843,19 +835,18 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, if (keep_largest_pack != -1) { if (keep_largest_pack) find_base_packs(&keep_pack, 0); - } else if (cfg->big_pack_threshold) { - find_base_packs(&keep_pack, cfg->big_pack_threshold); + } else if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); } add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); string_list_clear(&keep_pack, 0); - } else if (too_many_packs(cfg)) { + } else if (too_many_packs(gc_auto_pack_limit)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; - if (cfg->big_pack_threshold) { - find_base_packs(&keep_pack, cfg->big_pack_threshold); - if (keep_pack.nr >= cfg->gc_auto_pack_limit) { - cfg->big_pack_threshold = 0; + if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); + if (keep_pack.nr >= gc_auto_pack_limit) { string_list_clear(&keep_pack, 0); find_base_packs(&keep_pack, 0); } @@ -864,7 +855,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, uint64_t mem_have, mem_want; mem_have = total_ram(); - mem_want = estimate_repack_memory(cfg, p); + mem_want = estimate_repack_memory(p); /* * Only allow 1/2 of memory for pack-objects, leave @@ -905,7 +896,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, } } - if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold)) + if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold)) warning(_("There are too many unreachable loose objects; " "run 'git prune' to remove them.")); @@ -994,7 +985,7 @@ int cmd_gc(int argc, /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) { + if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) { ret = 0; goto out; } @@ -1291,9 +1282,9 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts, return run_command(&child); } -static int gc_condition(struct gc_config *cfg) +static int gc_condition(struct gc_config *cfg UNUSED) { - return need_to_gc(cfg); + return need_to_gc(the_repository); } static int prune_packed(struct maintenance_run_opts *opts) From cc78c5029e62e6e1283c7d05bf60d845922aac9b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:10 +0200 Subject: [PATCH 07/25] builtin/gc: introduce object database optimization options Introduce `struct odb_optimize_options` to decouple the options that are specific to optimizing the object database from `struct gc_config`. This structure will be moved into the object database layer in a subsequent commit. Note that there are a small set of backend-specific options in this structure. In an ideal world those of course wouldn't exist, but as we're introducing the object database abstractions retroactively we are somewhat forced to keep them. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 181 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 120 insertions(+), 61 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 5d445edaa06923..17490106fc9c91 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -593,7 +593,39 @@ static int keep_one_pack(struct string_list_item *item, void *data) return 0; } -static void add_repack_all_option(struct gc_config *cfg, +enum odb_optimize_flags { + /* Enable verbose logging and progress reporting. */ + ODB_OPTIMIZE_VERBOSE = (1 << 0), + + /* Perform auto-maintenance, only optimizing objects as required. */ + ODB_OPTIMIZE_AUTO = (1 << 1), + + /* Recompute existing deltas. */ + ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2), +}; + +struct odb_optimize_options { + enum odb_optimize_flags flags; + const char *prune_expire; + const char *expire_to; + int depth; + int window; + + /* Backend-specific options. */ + int keep_largest_pack; + int cruft_packs; + unsigned long max_cruft_size; +}; + +#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \ + .prune_expire = (cfg)->prune_expire, \ + .expire_to = (cfg)->repack_expire_to, \ + .cruft_packs = (cfg)->cruft_packs, \ + .max_cruft_size = (cfg)->max_cruft_size, \ + .window = (aggressive) ? (cfg)->aggressive_window : 0, \ + .depth = (aggressive) ? (cfg)->aggressive_depth : 0 + +static void add_repack_all_option(const struct odb_optimize_options *opts, struct string_list *keep_pack, struct strvec *args) { @@ -603,22 +635,22 @@ static void add_repack_all_option(struct gc_config *cfg, repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter); repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to); - if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now") - && !(cfg->cruft_packs && cfg->repack_expire_to)) + if (opts->prune_expire && !strcmp(opts->prune_expire, "now") && + !(opts->cruft_packs && opts->expire_to)) strvec_push(args, "-a"); - else if (cfg->cruft_packs) { + else if (opts->cruft_packs) { strvec_push(args, "--cruft"); - if (cfg->prune_expire) - strvec_pushf(args, "--cruft-expiration=%s", cfg->prune_expire); - if (cfg->max_cruft_size) + if (opts->prune_expire) + strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire); + if (opts->max_cruft_size) strvec_pushf(args, "--max-cruft-size=%lu", - cfg->max_cruft_size); - if (cfg->repack_expire_to) - strvec_pushf(args, "--expire-to=%s", cfg->repack_expire_to); + opts->max_cruft_size); + if (opts->expire_to) + strvec_pushf(args, "--expire-to=%s", opts->expire_to); } else { strvec_push(args, "-A"); - if (cfg->prune_expire) - strvec_pushf(args, "--unpack-unreachable=%s", cfg->prune_expire); + if (opts->prune_expire) + strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire); } if (keep_pack) @@ -786,10 +818,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, return 0; } -static int maintenance_task_odb(struct maintenance_run_opts *opts, - struct gc_config *cfg, - int keep_largest_pack, - int aggressive) +static int odb_optimize(struct object_database *odb, + const struct odb_optimize_options *opts) { struct child_process repack_cmd = CHILD_PROCESS_INIT; unsigned long big_pack_threshold = 0; @@ -801,21 +831,20 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit); repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold); - if (the_repository->repository_format_precious_objects) + if (odb->repo->repository_format_precious_objects) return 0; repack_cmd.git_cmd = 1; repack_cmd.odb_to_close = the_repository->objects; strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); - if (aggressive) { + if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS) strvec_push(&repack_cmd.args, "-f"); - if (cfg->aggressive_depth > 0) - strvec_pushf(&repack_cmd.args, "--depth=%d", cfg->aggressive_depth); - if (cfg->aggressive_window > 0) - strvec_pushf(&repack_cmd.args, "--window=%d", cfg->aggressive_window); - } - if (opts->quiet) + if (opts->depth > 0) + strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth); + if (opts->window > 0) + strvec_pushf(&repack_cmd.args, "--window=%d", opts->window); + if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) strvec_push(&repack_cmd.args, "-q"); /* @@ -829,47 +858,49 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, * * - Otherwise we perform an incremental repack. */ - if (!opts->auto_flag) { + if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; - if (keep_largest_pack != -1) { - if (keep_largest_pack) + if (opts->keep_largest_pack != -1) { + if (opts->keep_largest_pack) find_base_packs(&keep_pack, 0); } else if (big_pack_threshold) { find_base_packs(&keep_pack, big_pack_threshold); } - add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); + add_repack_all_option(opts, &keep_pack, &repack_cmd.args); string_list_clear(&keep_pack, 0); - } else if (too_many_packs(gc_auto_pack_limit)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - if (keep_pack.nr >= gc_auto_pack_limit) { - string_list_clear(&keep_pack, 0); - find_base_packs(&keep_pack, 0); + } else { + if (too_many_packs(gc_auto_pack_limit)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); + if (keep_pack.nr >= gc_auto_pack_limit) { + string_list_clear(&keep_pack, 0); + find_base_packs(&keep_pack, 0); + } + } else { + struct packed_git *p = find_base_packs(&keep_pack, 0); + uint64_t mem_have, mem_want; + + mem_have = total_ram(); + mem_want = estimate_repack_memory(p); + + /* + * Only allow 1/2 of memory for pack-objects, leave + * the rest for the OS and other processes in the + * system. + */ + if (!mem_have || mem_want < mem_have / 2) + string_list_clear(&keep_pack, 0); } - } else { - struct packed_git *p = find_base_packs(&keep_pack, 0); - uint64_t mem_have, mem_want; - - mem_have = total_ram(); - mem_want = estimate_repack_memory(p); - /* - * Only allow 1/2 of memory for pack-objects, leave - * the rest for the OS and other processes in the - * system. - */ - if (!mem_have || mem_want < mem_have / 2) - string_list_clear(&keep_pack, 0); + add_repack_all_option(opts, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); } - - add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); - string_list_clear(&keep_pack, 0); - } else { - add_repack_incremental_option(&repack_cmd.args); } if (run_command(&repack_cmd)) { @@ -877,13 +908,13 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, goto out; } - if (cfg->prune_expire) { + if (opts->prune_expire) { struct child_process prune_cmd = CHILD_PROCESS_INIT; strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); /* run `git prune` even if using cruft packs */ - strvec_push(&prune_cmd.args, cfg->prune_expire); - if (opts->quiet) + strvec_push(&prune_cmd.args, opts->prune_expire); + if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) strvec_push(&prune_cmd.args, "--no-progress"); if (repo_has_promisor_remote(the_repository)) strvec_push(&prune_cmd.args, @@ -896,7 +927,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, } } - if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold)) + if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold)) warning(_("There are too many unreachable loose objects; " "run 'git prune' to remove them.")); @@ -906,6 +937,26 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, return ret; } +static int maintenance_task_odb(struct maintenance_run_opts *opts, + struct gc_config *cfg, + int keep_largest_pack, + int aggressive) +{ + struct odb_optimize_options odb_opts = { + .keep_largest_pack = keep_largest_pack, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive), + }; + + if (opts->auto_flag) + odb_opts.flags |= ODB_OPTIMIZE_AUTO; + if (!opts->quiet) + odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; + if (aggressive) + odb_opts.flags |= ODB_OPTIMIZE_NO_REUSE_DELTAS; + + return odb_optimize(the_repository->objects, &odb_opts); +} + int cmd_gc(int argc, const char **argv, const char *prefix, @@ -1596,11 +1647,19 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts, child.odb_to_close = the_repository->objects; strvec_pushl(&child.args, "repack", "-d", "-l", NULL); - if (geometry.split < geometry.pack_nr) + if (geometry.split < geometry.pack_nr) { strvec_pushf(&child.args, "--geometric=%d", geometry.split_factor); - else - add_repack_all_option(cfg, NULL, &child.args); + } else { + struct odb_optimize_options odb_opts = { + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), + }; + + if (!opts->quiet) + odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; + + add_repack_all_option(&odb_opts, NULL, &child.args); + } if (opts->quiet) strvec_push(&child.args, "--quiet"); if (the_repository->settings.core_multi_pack_index) From 7d663dd83ba2ae459178c98f96c0761d044698b4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:11 +0200 Subject: [PATCH 08/25] builtin/gc: move geometric repacking into `odb_optimize()` We have two major object database optimization strategies: - The legacy strategy used by git-gc(1), which absorbs loose objects into packfiles, and eventually merges all packfiles once we have too many of them. - The more recent "geometric" strategy used by git-maintenance(1), which merges packfiles using a geometric sequence. These two strategies are still using completely separate code paths. In a subsequent commit we'll want to make both strategies pluggable though. Prepare for this change by merging the "geometric" strategy into `odb_optimize()`. This also allows us to reuse some of the logic we have in that function. Note that this change requires us to adapt tests because we're now using "-q" instead of "--quiet". Naturally though, these invocations are of course equivalent to one another. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 171 +++++++++++++++++++++-------------------- t/t7900-maintenance.sh | 18 ++--- 2 files changed, 96 insertions(+), 93 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 17490106fc9c91..c8504f4456b0d0 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -593,6 +593,11 @@ static int keep_one_pack(struct string_list_item *item, void *data) return 0; } +enum odb_optimize_strategy { + ODB_OPTIMIZE_INCREMENTAL, + ODB_OPTIMIZE_GEOMETRIC, +}; + enum odb_optimize_flags { /* Enable verbose logging and progress reporting. */ ODB_OPTIMIZE_VERBOSE = (1 << 0), @@ -605,6 +610,7 @@ enum odb_optimize_flags { }; struct odb_optimize_options { + enum odb_optimize_strategy strategy; enum odb_optimize_flags flags; const char *prune_expire; const char *expire_to; @@ -858,49 +864,87 @@ static int odb_optimize(struct object_database *odb, * * - Otherwise we perform an incremental repack. */ - if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (opts->keep_largest_pack != -1) { - if (opts->keep_largest_pack) - find_base_packs(&keep_pack, 0); - } else if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - } - - add_repack_all_option(opts, &keep_pack, &repack_cmd.args); - string_list_clear(&keep_pack, 0); - } else { - if (too_many_packs(gc_auto_pack_limit)) { + switch (opts->strategy) { + case ODB_OPTIMIZE_INCREMENTAL: + if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; - if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - if (keep_pack.nr >= gc_auto_pack_limit) { - string_list_clear(&keep_pack, 0); + if (opts->keep_largest_pack != -1) { + if (opts->keep_largest_pack) find_base_packs(&keep_pack, 0); - } - } else { - struct packed_git *p = find_base_packs(&keep_pack, 0); - uint64_t mem_have, mem_want; - - mem_have = total_ram(); - mem_want = estimate_repack_memory(p); - - /* - * Only allow 1/2 of memory for pack-objects, leave - * the rest for the OS and other processes in the - * system. - */ - if (!mem_have || mem_want < mem_have / 2) - string_list_clear(&keep_pack, 0); + } else if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); } add_repack_all_option(opts, &keep_pack, &repack_cmd.args); string_list_clear(&keep_pack, 0); } else { - add_repack_incremental_option(&repack_cmd.args); + if (too_many_packs(gc_auto_pack_limit)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); + if (keep_pack.nr >= gc_auto_pack_limit) { + string_list_clear(&keep_pack, 0); + find_base_packs(&keep_pack, 0); + } + } else { + struct packed_git *p = find_base_packs(&keep_pack, 0); + uint64_t mem_have, mem_want; + + mem_have = total_ram(); + mem_want = estimate_repack_memory(p); + + /* + * Only allow 1/2 of memory for pack-objects, leave + * the rest for the OS and other processes in the + * system. + */ + if (!mem_have || mem_want < mem_have / 2) + string_list_clear(&keep_pack, 0); + } + + add_repack_all_option(opts, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); + } } + + break; + case ODB_OPTIMIZE_GEOMETRIC: { + struct pack_geometry geometry = { + .split_factor = 2, + }; + struct pack_objects_args po_args = { + .local = 1, + }; + struct existing_packs existing_packs = EXISTING_PACKS_INIT; + struct string_list kept_packs = STRING_LIST_INIT_DUP; + + repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", + &geometry.split_factor); + + existing_packs.repo = the_repository; + existing_packs_collect(&existing_packs, &kept_packs); + pack_geometry_init(&geometry, &existing_packs, &po_args); + pack_geometry_split(&geometry); + + if (geometry.split < geometry.pack_nr) { + strvec_pushf(&repack_cmd.args, "--geometric=%d", + geometry.split_factor); + } else { + add_repack_all_option(opts, NULL, &repack_cmd.args); + } + if (the_repository->settings.core_multi_pack_index) + strvec_push(&repack_cmd.args, "--write-midx"); + + existing_packs_release(&existing_packs); + pack_geometry_release(&geometry); + break; + } + default: + die("unknown maintenance strategy '%d'", opts->strategy); } if (run_command(&repack_cmd)) { @@ -908,7 +952,8 @@ static int odb_optimize(struct object_database *odb, goto out; } - if (opts->prune_expire) { + /* Geometric repacking uses cruft packs, so we don't have to prune separately. */ + if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) { struct child_process prune_cmd = CHILD_PROCESS_INIT; strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); @@ -943,6 +988,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, int aggressive) { struct odb_optimize_options odb_opts = { + .strategy = ODB_OPTIMIZE_INCREMENTAL, .keep_largest_pack = keep_largest_pack, OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive), }; @@ -1624,58 +1670,15 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts, struct gc_config *cfg) { - struct pack_geometry geometry = { - .split_factor = 2, - }; - struct pack_objects_args po_args = { - .local = 1, + struct odb_optimize_options odb_opts = { + .strategy = ODB_OPTIMIZE_GEOMETRIC, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), }; - struct existing_packs existing_packs = EXISTING_PACKS_INIT; - struct string_list kept_packs = STRING_LIST_INIT_DUP; - struct child_process child = CHILD_PROCESS_INIT; - int ret; - - repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", - &geometry.split_factor); - - existing_packs.repo = the_repository; - existing_packs_collect(&existing_packs, &kept_packs); - pack_geometry_init(&geometry, &existing_packs, &po_args); - pack_geometry_split(&geometry); - - child.git_cmd = 1; - child.odb_to_close = the_repository->objects; - - strvec_pushl(&child.args, "repack", "-d", "-l", NULL); - if (geometry.split < geometry.pack_nr) { - strvec_pushf(&child.args, "--geometric=%d", - geometry.split_factor); - } else { - struct odb_optimize_options odb_opts = { - OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), - }; - if (!opts->quiet) - odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; - - add_repack_all_option(&odb_opts, NULL, &child.args); - } - if (opts->quiet) - strvec_push(&child.args, "--quiet"); - if (the_repository->settings.core_multi_pack_index) - strvec_push(&child.args, "--write-midx"); - - if (run_command(&child)) { - ret = error(_("failed to perform geometric repack")); - goto out; - } - - ret = 0; + if (!opts->quiet) + odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; -out: - existing_packs_release(&existing_packs); - pack_geometry_release(&geometry); - return ret; + return odb_optimize(the_repository->objects, &odb_opts); } static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 2d52e7918a33ff..6d87da2ae4d508 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -574,8 +574,8 @@ run_and_verify_geometric_pack () { rm -f "trace2.txt" && GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --geometric=2 \ - --quiet --write-midx packfiles && @@ -606,8 +606,8 @@ test_expect_success 'geometric repacking task' ' # The initial repack causes an all-into-one repack. GIT_TRACE2_EVENT="$(pwd)/initial-repack.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx /dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx /dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx packs && test_line_count = 2 packs && ls .git/objects/pack/*.mtimes >cruft && @@ -754,7 +754,7 @@ test_expect_success 'geometric repacking honors configured split factor' ' test_geometric_repack_needed false splitFactor=2 && test_geometric_repack_needed true splitFactor=3 && - test_subcommand git repack -d -l --geometric=3 --quiet --write-midx Date: Mon, 13 Jul 2026 07:52:12 +0200 Subject: [PATCH 09/25] builtin/gc: introduce `odb_optimize_required()` When invoking either git-gc(1) or git-maintenance(1) with the "--auto" flag then we only perform those maintenance tasks that are actually required. This logic is inherently an implementation detail of the object database backend that's in use. But the logic is scattered around multiple different functions, which makes it hard to make the logic pluggable. Introduce a new `odb_optimize_required()` function that allows us to check these conditions in a generic way. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 160 +++++++++++++++++++++++++++++---------------------- 1 file changed, 92 insertions(+), 68 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c8504f4456b0d0..e119930adc816e 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -676,25 +676,84 @@ static void add_repack_incremental_option(struct strvec *args) strvec_push(args, "--no-write-bitmap-index"); } -static int need_to_gc(struct repository *repo) +static bool odb_optimize_required(struct object_database *odb, + const struct odb_optimize_options *opts) { - int gc_auto_threshold = 6700; - int gc_auto_pack_limit = 50; + switch (opts->strategy) { + case ODB_OPTIMIZE_INCREMENTAL: { + int gc_auto_threshold = 6700; + int gc_auto_pack_limit = 50; - repo_config_get_int(repo, "gc.auto", &gc_auto_threshold); - repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit); + repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold); + repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit); - /* - * Setting gc.auto to 0 or negative can disable the - * automatic gc. - */ - if (gc_auto_threshold <= 0) - return 0; - if (!too_many_packs(gc_auto_pack_limit) && - !too_many_loose_objects(gc_auto_threshold)) - return 0; + /* + * Setting gc.auto to 0 or negative can disable the + * automatic gc. + */ + if (gc_auto_threshold <= 0) + return false; + if (!too_many_packs(gc_auto_pack_limit) && + !too_many_loose_objects(gc_auto_threshold)) + return false; - return 1; + return true; + } + case ODB_OPTIMIZE_GEOMETRIC: { + struct pack_geometry geometry = { + .split_factor = 2, + }; + struct pack_objects_args po_args = { + .local = 1, + }; + struct existing_packs existing_packs = EXISTING_PACKS_INIT; + struct string_list kept_packs = STRING_LIST_INIT_DUP; + int auto_value = 100; + bool ret; + + repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto", + &auto_value); + if (!auto_value) + return false; + if (auto_value < 0) + return true; + + repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor", + &geometry.split_factor); + + existing_packs.repo = odb->repo; + existing_packs_collect(&existing_packs, &kept_packs); + pack_geometry_init(&geometry, &existing_packs, &po_args); + pack_geometry_split(&geometry); + + /* + * When we'd merge at least two packs with one another we always + * perform the repack. + */ + if (geometry.split) { + ret = true; + goto out; + } + + /* + * Otherwise, we estimate the number of loose objects to determine + * whether we want to create a new packfile or not. + */ + if (too_many_loose_objects(auto_value)) { + ret = true; + goto out; + } + + ret = false; + + out: + existing_packs_release(&existing_packs); + pack_geometry_release(&geometry); + return ret; + } + default: + BUG("unknown maintenance strategy '%d'", opts->strategy); + } } /* return NULL on success, else hostname running the gc */ @@ -1076,13 +1135,19 @@ int cmd_gc(int argc, die(_("failed to parse prune expiry value %s"), cfg.prune_expire); if (opts.auto_flag) { + struct odb_optimize_options optimize_opts = { + .strategy = ODB_OPTIMIZE_INCREMENTAL, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(&cfg, 0), + }; + if (cfg.detach_auto && opts.detach < 0) opts.detach = 1; /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) { + if (!odb_optimize_required(the_repository->objects, &optimize_opts) || + run_hooks(the_repository, "pre-auto-gc")) { ret = 0; goto out; } @@ -1379,9 +1444,13 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts, return run_command(&child); } -static int gc_condition(struct gc_config *cfg UNUSED) +static int gc_condition(struct gc_config *cfg) { - return need_to_gc(the_repository); + struct odb_optimize_options opts = { + .strategy = ODB_OPTIMIZE_INCREMENTAL, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), + }; + return odb_optimize_required(the_repository->objects, &opts); } static int prune_packed(struct maintenance_run_opts *opts) @@ -1681,58 +1750,13 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts, return odb_optimize(the_repository->objects, &odb_opts); } -static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED) +static int geometric_repack_auto_condition(struct gc_config *cfg) { - struct pack_geometry geometry = { - .split_factor = 2, - }; - struct pack_objects_args po_args = { - .local = 1, + struct odb_optimize_options opts = { + .strategy = ODB_OPTIMIZE_GEOMETRIC, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), }; - struct existing_packs existing_packs = EXISTING_PACKS_INIT; - struct string_list kept_packs = STRING_LIST_INIT_DUP; - int auto_value = 100; - int ret; - - repo_config_get_int(the_repository, "maintenance.geometric-repack.auto", - &auto_value); - if (!auto_value) - return 0; - if (auto_value < 0) - return 1; - - repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", - &geometry.split_factor); - - existing_packs.repo = the_repository; - existing_packs_collect(&existing_packs, &kept_packs); - pack_geometry_init(&geometry, &existing_packs, &po_args); - pack_geometry_split(&geometry); - - /* - * When we'd merge at least two packs with one another we always - * perform the repack. - */ - if (geometry.split) { - ret = 1; - goto out; - } - - /* - * Otherwise, we estimate the number of loose objects to determine - * whether we want to create a new packfile or not. - */ - if (too_many_loose_objects(auto_value)) { - ret = 1; - goto out; - } - - ret = 0; - -out: - existing_packs_release(&existing_packs); - pack_geometry_release(&geometry); - return ret; + return odb_optimize_required(the_repository->objects, &opts); } typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts, From 0a778894b9676a1e93870589851731dad05e086a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:13 +0200 Subject: [PATCH 10/25] builtin/gc: refactor ODB optimizations to operate on "files" source We have a couple of functions that are implementation details of how the "files" object database source performs optimizations. These functions often use global state like `the_repository` and implicitly derive the source they are supposed to optimize. Refactor these interfaces to accept a "files" source directly. This will make it easier to move around the whole logic into "odb/source-files.c" in a subsequent step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 79 +++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index e119930adc816e..32071824887cda 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -428,9 +428,8 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED) return should_gc; } -static int too_many_loose_objects(int limit) +static int too_many_loose_objects(struct odb_source_files *files, int limit) { - struct odb_source_files *files = odb_source_files_downcast(the_repository->objects->sources); /* * This is weird, but stems from legacy behaviour: the GC auto * threshold was always essentially interpreted as if it was rounded up @@ -446,19 +445,21 @@ static int too_many_loose_objects(int limit) return loose_count > auto_threshold; } -static struct packed_git *find_base_packs(struct string_list *packs, +static struct packed_git *find_base_packs(struct odb_source_files *files, + struct string_list *packs, unsigned long limit) { - struct packed_git *p, *base = NULL; + struct packfile_list_entry *e; + struct packed_git *base = NULL; - repo_for_each_pack(the_repository, p) { - if (!p->pack_local || p->is_cruft) + for (e = packfile_store_get_packs(files->packed); e; e = e->next) { + if (e->pack->is_cruft) continue; if (limit) { - if (p->pack_size >= limit) - string_list_append(packs, p->pack_name); - } else if (!base || base->pack_size < p->pack_size) { - base = p; + if (e->pack->pack_size >= limit) + string_list_append(packs, e->pack->pack_name); + } else if (!base || base->pack_size < e->pack->pack_size) { + base = e->pack; } } @@ -468,18 +469,16 @@ static struct packed_git *find_base_packs(struct string_list *packs, return base; } -static int too_many_packs(int gc_auto_pack_limit) +static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit) { - struct packed_git *p; + struct packfile_list_entry *e; int cnt = 0; if (gc_auto_pack_limit <= 0) return 0; - repo_for_each_pack(the_repository, p) { - if (!p->pack_local) - continue; - if (p->pack_keep) + for (e = packfile_store_get_packs(files->packed); e; e = e->next) { + if (e->pack->pack_keep) continue; /* * Perhaps check the size of the pack and count only @@ -535,15 +534,16 @@ static uint64_t total_ram(void) return 0; } -static uint64_t estimate_repack_memory(struct packed_git *pack) +static uint64_t estimate_repack_memory(struct odb_source_files *files, + struct packed_git *pack) { unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE; unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT; unsigned long nr_objects; size_t os_cache, heap; - if (odb_count_objects(the_repository->objects, - ODB_COUNT_OBJECTS_APPROXIMATE, &nr_objects) < 0) + if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE, + &nr_objects) < 0) return 0; if (!pack || !nr_objects) @@ -679,6 +679,8 @@ static void add_repack_incremental_option(struct strvec *args) static bool odb_optimize_required(struct object_database *odb, const struct odb_optimize_options *opts) { + struct odb_source_files *files = odb_source_files_downcast(odb->sources); + switch (opts->strategy) { case ODB_OPTIMIZE_INCREMENTAL: { int gc_auto_threshold = 6700; @@ -693,8 +695,8 @@ static bool odb_optimize_required(struct object_database *odb, */ if (gc_auto_threshold <= 0) return false; - if (!too_many_packs(gc_auto_pack_limit) && - !too_many_loose_objects(gc_auto_threshold)) + if (!too_many_packs(files, gc_auto_pack_limit) && + !too_many_loose_objects(files, gc_auto_threshold)) return false; return true; @@ -739,7 +741,7 @@ static bool odb_optimize_required(struct object_database *odb, * Otherwise, we estimate the number of loose objects to determine * whether we want to create a new packfile or not. */ - if (too_many_loose_objects(auto_value)) { + if (too_many_loose_objects(files, auto_value)) { ret = true; goto out; } @@ -886,21 +888,22 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, static int odb_optimize(struct object_database *odb, const struct odb_optimize_options *opts) { + struct odb_source_files *files = odb_source_files_downcast(odb->sources); struct child_process repack_cmd = CHILD_PROCESS_INIT; unsigned long big_pack_threshold = 0; int gc_auto_threshold = 6700; int gc_auto_pack_limit = 50; int ret; - repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold); - repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit); - repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold); + repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold); + repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit); + repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold); if (odb->repo->repository_format_precious_objects) return 0; repack_cmd.git_cmd = 1; - repack_cmd.odb_to_close = the_repository->objects; + repack_cmd.odb_to_close = odb->repo->objects; strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS) @@ -930,29 +933,29 @@ static int odb_optimize(struct object_database *odb, if (opts->keep_largest_pack != -1) { if (opts->keep_largest_pack) - find_base_packs(&keep_pack, 0); + find_base_packs(files, &keep_pack, 0); } else if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); + find_base_packs(files, &keep_pack, big_pack_threshold); } add_repack_all_option(opts, &keep_pack, &repack_cmd.args); string_list_clear(&keep_pack, 0); } else { - if (too_many_packs(gc_auto_pack_limit)) { + if (too_many_packs(files, gc_auto_pack_limit)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); + find_base_packs(files, &keep_pack, big_pack_threshold); if (keep_pack.nr >= gc_auto_pack_limit) { string_list_clear(&keep_pack, 0); - find_base_packs(&keep_pack, 0); + find_base_packs(files, &keep_pack, 0); } } else { - struct packed_git *p = find_base_packs(&keep_pack, 0); + struct packed_git *p = find_base_packs(files, &keep_pack, 0); uint64_t mem_have, mem_want; mem_have = total_ram(); - mem_want = estimate_repack_memory(p); + mem_want = estimate_repack_memory(files, p); /* * Only allow 1/2 of memory for pack-objects, leave @@ -981,10 +984,10 @@ static int odb_optimize(struct object_database *odb, struct existing_packs existing_packs = EXISTING_PACKS_INIT; struct string_list kept_packs = STRING_LIST_INIT_DUP; - repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", + repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor", &geometry.split_factor); - existing_packs.repo = the_repository; + existing_packs.repo = odb->repo; existing_packs_collect(&existing_packs, &kept_packs); pack_geometry_init(&geometry, &existing_packs, &po_args); pack_geometry_split(&geometry); @@ -995,7 +998,7 @@ static int odb_optimize(struct object_database *odb, } else { add_repack_all_option(opts, NULL, &repack_cmd.args); } - if (the_repository->settings.core_multi_pack_index) + if (odb->repo->settings.core_multi_pack_index) strvec_push(&repack_cmd.args, "--write-midx"); existing_packs_release(&existing_packs); @@ -1020,7 +1023,7 @@ static int odb_optimize(struct object_database *odb, strvec_push(&prune_cmd.args, opts->prune_expire); if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) strvec_push(&prune_cmd.args, "--no-progress"); - if (repo_has_promisor_remote(the_repository)) + if (repo_has_promisor_remote(odb->repo)) strvec_push(&prune_cmd.args, "--exclude-promisor-objects"); prune_cmd.git_cmd = 1; @@ -1031,7 +1034,7 @@ static int odb_optimize(struct object_database *odb, } } - if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold)) + if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold)) warning(_("There are too many unreachable loose objects; " "run 'git prune' to remove them.")); From 7534d456816d49f20716e49900d43cedb02e8c42 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:14 +0200 Subject: [PATCH 11/25] builtin/gc: fix signedness issues in ODB-related functionality There are a couple of signedness issues in ODB-related functionality. These are not a problem because we disable -Wsign-compare in this file, but once we move these functions into "odb/source-files.c" they will result in warnings. Fix those issues: - In `too_many_loose_objects()` we receive a signed limit, but compare it with the unsigned actual number of loose objects. This is fixed by bailing out immediately when the limit is smaller than or equal to zero, which we also do similarly in other places. The warning is then squelched via a cast. - In `find_base_packs()` we compare the signed size of the pack against the unsigned limit. As the pack size is always going to be a positive file size it's safe to cast it to an unsigned value. - In `odb_optimize()` we compare the unsigned `keep_pack.nr` value against the signed `gc_auto_pack_limit`. We only reach this code when `too_many_packs()` returns true-ish, and that can only happen when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning by casting the limit to an unsigned value. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 32071824887cda..8cf3781313bc53 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED) static int too_many_loose_objects(struct odb_source_files *files, int limit) { - /* - * This is weird, but stems from legacy behaviour: the GC auto - * threshold was always essentially interpreted as if it was rounded up - * to the next multiple 256 of, so we retain this behaviour for now. - */ - int auto_threshold = DIV_ROUND_UP(limit, 256) * 256; unsigned long loose_count; + if (limit <= 0) + return 0; + if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE, &loose_count) < 0) return 0; - return loose_count > auto_threshold; + /* + * This is weird, but stems from legacy behaviour: the GC auto + * threshold was always essentially interpreted as if it was rounded up + * to the next multiple 256 of, so we retain this behaviour for now. + */ + return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256); } static struct packed_git *find_base_packs(struct odb_source_files *files, @@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files, if (e->pack->is_cruft) continue; if (limit) { - if (e->pack->pack_size >= limit) + if ((uintmax_t) e->pack->pack_size >= limit) string_list_append(packs, e->pack->pack_name); } else if (!base || base->pack_size < e->pack->pack_size) { base = e->pack; @@ -946,7 +948,7 @@ static int odb_optimize(struct object_database *odb, if (big_pack_threshold) { find_base_packs(files, &keep_pack, big_pack_threshold); - if (keep_pack.nr >= gc_auto_pack_limit) { + if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) { string_list_clear(&keep_pack, 0); find_base_packs(files, &keep_pack, 0); } From 56f3fc9520a52a43aef2cabaf30b71a79c9eca57 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 13 Jul 2026 07:52:15 +0200 Subject: [PATCH 12/25] odb: make optimizations pluggable Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c" into the "files" source and wire them up via newly introduced vtable pointers for the object database sources. This makes the logic pluggable and thus allows other backends to have their own, custom implementation. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 490 +-------------------------------------------- odb.c | 12 ++ odb.h | 45 +++++ odb/source-files.c | 470 +++++++++++++++++++++++++++++++++++++++++++ odb/source-files.h | 15 ++ odb/source.h | 36 ++++ 6 files changed, 579 insertions(+), 489 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 8cf3781313bc53..ac1a21e91267f7 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -30,16 +30,11 @@ #include "commit-graph.h" #include "packfile.h" #include "object-file.h" -#include "pack.h" -#include "pack-objects.h" +#include "odb.h" #include "path.h" #include "reflog.h" -#include "repack.h" #include "rerere.h" #include "revision.h" -#include "blob.h" -#include "tree.h" -#include "promisor-remote.h" #include "refs.h" #include "remote.h" #include "exec-cmd.h" @@ -428,203 +423,6 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED) return should_gc; } -static int too_many_loose_objects(struct odb_source_files *files, int limit) -{ - unsigned long loose_count; - - if (limit <= 0) - return 0; - - if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE, - &loose_count) < 0) - return 0; - - /* - * This is weird, but stems from legacy behaviour: the GC auto - * threshold was always essentially interpreted as if it was rounded up - * to the next multiple 256 of, so we retain this behaviour for now. - */ - return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256); -} - -static struct packed_git *find_base_packs(struct odb_source_files *files, - struct string_list *packs, - unsigned long limit) -{ - struct packfile_list_entry *e; - struct packed_git *base = NULL; - - for (e = packfile_store_get_packs(files->packed); e; e = e->next) { - if (e->pack->is_cruft) - continue; - if (limit) { - if ((uintmax_t) e->pack->pack_size >= limit) - string_list_append(packs, e->pack->pack_name); - } else if (!base || base->pack_size < e->pack->pack_size) { - base = e->pack; - } - } - - if (base) - string_list_append(packs, base->pack_name); - - return base; -} - -static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit) -{ - struct packfile_list_entry *e; - int cnt = 0; - - if (gc_auto_pack_limit <= 0) - return 0; - - for (e = packfile_store_get_packs(files->packed); e; e = e->next) { - if (e->pack->pack_keep) - continue; - /* - * Perhaps check the size of the pack and count only - * very small ones here? - */ - cnt++; - } - return gc_auto_pack_limit < cnt; -} - -static uint64_t total_ram(void) -{ -#if defined(HAVE_SYSINFO) - struct sysinfo si; - - if (!sysinfo(&si)) { - uint64_t total = si.totalram; - - if (si.mem_unit > 1) - total *= (uint64_t)si.mem_unit; - return total; - } -#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64)) - uint64_t physical_memory; - int mib[2]; - size_t length; - - mib[0] = CTL_HW; -# if defined(HW_MEMSIZE) - mib[1] = HW_MEMSIZE; -# elif defined(HW_PHYSMEM64) - mib[1] = HW_PHYSMEM64; -# else - mib[1] = HW_PHYSMEM; -# endif - length = sizeof(physical_memory); - if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) { - if (length == 4) { - uint32_t mem; - - if (!sysctl(mib, 2, &mem, &length, NULL, 0)) - physical_memory = mem; - } - return physical_memory; - } -#elif defined(GIT_WINDOWS_NATIVE) - MEMORYSTATUSEX memInfo; - - memInfo.dwLength = sizeof(MEMORYSTATUSEX); - if (GlobalMemoryStatusEx(&memInfo)) - return memInfo.ullTotalPhys; -#endif - return 0; -} - -static uint64_t estimate_repack_memory(struct odb_source_files *files, - struct packed_git *pack) -{ - unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE; - unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT; - unsigned long nr_objects; - size_t os_cache, heap; - - if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE, - &nr_objects) < 0) - return 0; - - if (!pack || !nr_objects) - return 0; - - repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size); - repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit); - - /* - * First we have to scan through at least one pack. - * Assume enough room in OS file cache to keep the entire pack - * or we may accidentally evict data of other processes from - * the cache. - */ - os_cache = pack->pack_size + pack->index_size; - /* then pack-objects needs lots more for book keeping */ - heap = sizeof(struct object_entry) * nr_objects; - /* - * internal rev-list --all --objects takes up some memory too, - * let's say half of it is for blobs - */ - heap += sizeof(struct blob) * nr_objects / 2; - /* - * and the other half is for trees (commits and tags are - * usually insignificant) - */ - heap += sizeof(struct tree) * nr_objects / 2; - /* and then obj_hash[], underestimated in fact */ - heap += sizeof(struct object *) * nr_objects; - /* revindex is used also */ - heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects; - /* - * read_sha1_file() (either at delta calculation phase, or - * writing phase) also fills up the delta base cache - */ - heap += delta_base_cache_limit; - /* and of course pack-objects has its own delta cache */ - heap += max_delta_cache_size; - - return os_cache + heap; -} - -static int keep_one_pack(struct string_list_item *item, void *data) -{ - struct strvec *args = data; - strvec_pushf(args, "--keep-pack=%s", basename(item->string)); - return 0; -} - -enum odb_optimize_strategy { - ODB_OPTIMIZE_INCREMENTAL, - ODB_OPTIMIZE_GEOMETRIC, -}; - -enum odb_optimize_flags { - /* Enable verbose logging and progress reporting. */ - ODB_OPTIMIZE_VERBOSE = (1 << 0), - - /* Perform auto-maintenance, only optimizing objects as required. */ - ODB_OPTIMIZE_AUTO = (1 << 1), - - /* Recompute existing deltas. */ - ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2), -}; - -struct odb_optimize_options { - enum odb_optimize_strategy strategy; - enum odb_optimize_flags flags; - const char *prune_expire; - const char *expire_to; - int depth; - int window; - - /* Backend-specific options. */ - int keep_largest_pack; - int cruft_packs; - unsigned long max_cruft_size; -}; - #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \ .prune_expire = (cfg)->prune_expire, \ .expire_to = (cfg)->repack_expire_to, \ @@ -633,133 +431,6 @@ struct odb_optimize_options { .window = (aggressive) ? (cfg)->aggressive_window : 0, \ .depth = (aggressive) ? (cfg)->aggressive_depth : 0 -static void add_repack_all_option(const struct odb_optimize_options *opts, - struct string_list *keep_pack, - struct strvec *args) -{ - char *repack_filter = NULL; - char *repack_filter_to = NULL; - - repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter); - repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to); - - if (opts->prune_expire && !strcmp(opts->prune_expire, "now") && - !(opts->cruft_packs && opts->expire_to)) - strvec_push(args, "-a"); - else if (opts->cruft_packs) { - strvec_push(args, "--cruft"); - if (opts->prune_expire) - strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire); - if (opts->max_cruft_size) - strvec_pushf(args, "--max-cruft-size=%lu", - opts->max_cruft_size); - if (opts->expire_to) - strvec_pushf(args, "--expire-to=%s", opts->expire_to); - } else { - strvec_push(args, "-A"); - if (opts->prune_expire) - strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire); - } - - if (keep_pack) - for_each_string_list(keep_pack, keep_one_pack, args); - - if (repack_filter && *repack_filter) - strvec_pushf(args, "--filter=%s", repack_filter); - if (repack_filter_to && *repack_filter_to) - strvec_pushf(args, "--filter-to=%s", repack_filter_to); - - free(repack_filter); - free(repack_filter_to); -} - -static void add_repack_incremental_option(struct strvec *args) -{ - strvec_push(args, "--no-write-bitmap-index"); -} - -static bool odb_optimize_required(struct object_database *odb, - const struct odb_optimize_options *opts) -{ - struct odb_source_files *files = odb_source_files_downcast(odb->sources); - - switch (opts->strategy) { - case ODB_OPTIMIZE_INCREMENTAL: { - int gc_auto_threshold = 6700; - int gc_auto_pack_limit = 50; - - repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold); - repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit); - - /* - * Setting gc.auto to 0 or negative can disable the - * automatic gc. - */ - if (gc_auto_threshold <= 0) - return false; - if (!too_many_packs(files, gc_auto_pack_limit) && - !too_many_loose_objects(files, gc_auto_threshold)) - return false; - - return true; - } - case ODB_OPTIMIZE_GEOMETRIC: { - struct pack_geometry geometry = { - .split_factor = 2, - }; - struct pack_objects_args po_args = { - .local = 1, - }; - struct existing_packs existing_packs = EXISTING_PACKS_INIT; - struct string_list kept_packs = STRING_LIST_INIT_DUP; - int auto_value = 100; - bool ret; - - repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto", - &auto_value); - if (!auto_value) - return false; - if (auto_value < 0) - return true; - - repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor", - &geometry.split_factor); - - existing_packs.repo = odb->repo; - existing_packs_collect(&existing_packs, &kept_packs); - pack_geometry_init(&geometry, &existing_packs, &po_args); - pack_geometry_split(&geometry); - - /* - * When we'd merge at least two packs with one another we always - * perform the repack. - */ - if (geometry.split) { - ret = true; - goto out; - } - - /* - * Otherwise, we estimate the number of loose objects to determine - * whether we want to create a new packfile or not. - */ - if (too_many_loose_objects(files, auto_value)) { - ret = true; - goto out; - } - - ret = false; - - out: - existing_packs_release(&existing_packs); - pack_geometry_release(&geometry); - return ret; - } - default: - BUG("unknown maintenance strategy '%d'", opts->strategy); - } -} - /* return NULL on success, else hostname running the gc */ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) { @@ -887,165 +558,6 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, return 0; } -static int odb_optimize(struct object_database *odb, - const struct odb_optimize_options *opts) -{ - struct odb_source_files *files = odb_source_files_downcast(odb->sources); - struct child_process repack_cmd = CHILD_PROCESS_INIT; - unsigned long big_pack_threshold = 0; - int gc_auto_threshold = 6700; - int gc_auto_pack_limit = 50; - int ret; - - repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold); - repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit); - repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold); - - if (odb->repo->repository_format_precious_objects) - return 0; - - repack_cmd.git_cmd = 1; - repack_cmd.odb_to_close = odb->repo->objects; - - strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); - if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS) - strvec_push(&repack_cmd.args, "-f"); - if (opts->depth > 0) - strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth); - if (opts->window > 0) - strvec_pushf(&repack_cmd.args, "--window=%d", opts->window); - if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) - strvec_push(&repack_cmd.args, "-q"); - - /* - * There's three cases we need to consider: - * - * - If we're invoked without `--auto` we'll need to perform a full - * repack. - * - * - If we're invoked with `--auto` and there's too many packs, then - * we perform a full repack, as well. - * - * - Otherwise we perform an incremental repack. - */ - switch (opts->strategy) { - case ODB_OPTIMIZE_INCREMENTAL: - if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (opts->keep_largest_pack != -1) { - if (opts->keep_largest_pack) - find_base_packs(files, &keep_pack, 0); - } else if (big_pack_threshold) { - find_base_packs(files, &keep_pack, big_pack_threshold); - } - - add_repack_all_option(opts, &keep_pack, &repack_cmd.args); - string_list_clear(&keep_pack, 0); - } else { - if (too_many_packs(files, gc_auto_pack_limit)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (big_pack_threshold) { - find_base_packs(files, &keep_pack, big_pack_threshold); - if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) { - string_list_clear(&keep_pack, 0); - find_base_packs(files, &keep_pack, 0); - } - } else { - struct packed_git *p = find_base_packs(files, &keep_pack, 0); - uint64_t mem_have, mem_want; - - mem_have = total_ram(); - mem_want = estimate_repack_memory(files, p); - - /* - * Only allow 1/2 of memory for pack-objects, leave - * the rest for the OS and other processes in the - * system. - */ - if (!mem_have || mem_want < mem_have / 2) - string_list_clear(&keep_pack, 0); - } - - add_repack_all_option(opts, &keep_pack, &repack_cmd.args); - string_list_clear(&keep_pack, 0); - } else { - add_repack_incremental_option(&repack_cmd.args); - } - } - - break; - case ODB_OPTIMIZE_GEOMETRIC: { - struct pack_geometry geometry = { - .split_factor = 2, - }; - struct pack_objects_args po_args = { - .local = 1, - }; - struct existing_packs existing_packs = EXISTING_PACKS_INIT; - struct string_list kept_packs = STRING_LIST_INIT_DUP; - - repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor", - &geometry.split_factor); - - existing_packs.repo = odb->repo; - existing_packs_collect(&existing_packs, &kept_packs); - pack_geometry_init(&geometry, &existing_packs, &po_args); - pack_geometry_split(&geometry); - - if (geometry.split < geometry.pack_nr) { - strvec_pushf(&repack_cmd.args, "--geometric=%d", - geometry.split_factor); - } else { - add_repack_all_option(opts, NULL, &repack_cmd.args); - } - if (odb->repo->settings.core_multi_pack_index) - strvec_push(&repack_cmd.args, "--write-midx"); - - existing_packs_release(&existing_packs); - pack_geometry_release(&geometry); - break; - } - default: - die("unknown maintenance strategy '%d'", opts->strategy); - } - - if (run_command(&repack_cmd)) { - ret = error(FAILED_RUN, repack_cmd.args.v[0]); - goto out; - } - - /* Geometric repacking uses cruft packs, so we don't have to prune separately. */ - if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) { - struct child_process prune_cmd = CHILD_PROCESS_INIT; - - strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); - /* run `git prune` even if using cruft packs */ - strvec_push(&prune_cmd.args, opts->prune_expire); - if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) - strvec_push(&prune_cmd.args, "--no-progress"); - if (repo_has_promisor_remote(odb->repo)) - strvec_push(&prune_cmd.args, - "--exclude-promisor-objects"); - prune_cmd.git_cmd = 1; - - if (run_command(&prune_cmd)) { - ret = error(FAILED_RUN, prune_cmd.args.v[0]); - goto out; - } - } - - if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold)) - warning(_("There are too many unreachable loose objects; " - "run 'git prune' to remove them.")); - - ret = 0; - -out: - return ret; -} - static int maintenance_task_odb(struct maintenance_run_opts *opts, struct gc_config *cfg, int keep_largest_pack, diff --git a/odb.c b/odb.c index 7d555be09feaea..89660981feac02 100644 --- a/odb.c +++ b/odb.c @@ -1003,6 +1003,18 @@ int odb_write_object_stream(struct object_database *odb, return odb_source_write_object_stream(odb->sources, stream, len, oid); } +int odb_optimize(struct object_database *odb, + const struct odb_optimize_options *opts) +{ + return odb_source_optimize(odb->sources, opts); +} + +bool odb_optimize_required(struct object_database *odb, + const struct odb_optimize_options *opts) +{ + return odb_source_optimize_required(odb->sources, opts); +} + struct object_database *odb_new(struct repository *repo, const char *primary_source, const char *secondary_sources) diff --git a/odb.h b/odb.h index 3834a0dcbf033b..7e1c85c22e844f 100644 --- a/odb.h +++ b/odb.h @@ -117,6 +117,51 @@ struct object_database *odb_new(struct repository *repo, /* Free the object database and release all resources. */ void odb_free(struct object_database *o); +enum odb_optimize_strategy { + ODB_OPTIMIZE_INCREMENTAL, + ODB_OPTIMIZE_GEOMETRIC, +}; + +enum odb_optimize_flags { + /* Enable verbose logging and progress reporting. */ + ODB_OPTIMIZE_VERBOSE = (1 << 0), + + /* Perform auto-maintenance, only optimizing objects as required. */ + ODB_OPTIMIZE_AUTO = (1 << 1), + + /* Recompute existing deltas. */ + ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2), +}; + +struct odb_optimize_options { + enum odb_optimize_strategy strategy; + enum odb_optimize_flags flags; + const char *prune_expire; + const char *expire_to; + int depth; + int window; + + /* Backend-specific options. */ + int keep_largest_pack; + int cruft_packs; + unsigned long max_cruft_size; +}; + +/* + * Optimize the object database. Returns 0 on success, a negative error code + * otherwise. + */ +int odb_optimize(struct object_database *odb, + const struct odb_optimize_options *opts); + +/* + * Check whether optimization of the object database is required given the + * provided options. Returns true if optimization should be performed, false + * otherwise. + */ +bool odb_optimize_required(struct object_database *odb, + const struct odb_optimize_options *opts); + /* * Close the object database and all of its sources so that any held resources * will be released. The database can still be used after closing it, in which diff --git a/odb/source-files.c b/odb/source-files.c index bbd1784b337c6d..82cf61da4af9dd 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -1,6 +1,8 @@ #include "git-compat-util.h" #include "abspath.h" +#include "blob.h" #include "chdir-notify.h" +#include "config.h" #include "gettext.h" #include "lockfile.h" #include "object-file.h" @@ -8,8 +10,16 @@ #include "odb/source.h" #include "odb/source-files.h" #include "odb/source-loose.h" +#include "pack-objects.h" #include "packfile.h" +#include "path.h" +#include "promisor-remote.h" +#include "repack.h" +#include "run-command.h" #include "strbuf.h" +#include "string-list.h" +#include "strvec.h" +#include "tree.h" #include "write-or-die.h" static void odb_source_files_reparent(const char *name UNUSED, @@ -260,6 +270,464 @@ static int odb_source_files_write_alternate(struct odb_source *source, return ret; } +static int too_many_loose_objects(struct odb_source_files *files, int limit) +{ + unsigned long loose_count; + + if (limit <= 0) + return 0; + + if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE, + &loose_count) < 0) + return 0; + + /* + * This is weird, but stems from legacy behaviour: the GC auto + * threshold was always essentially interpreted as if it was rounded up + * to the next multiple 256 of, so we retain this behaviour for now. + */ + return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256); +} + +static struct packed_git *find_base_packs(struct odb_source_files *files, + struct string_list *packs, + unsigned long limit) +{ + struct packfile_list_entry *e; + struct packed_git *base = NULL; + + for (e = packfile_store_get_packs(files->packed); e; e = e->next) { + if (e->pack->is_cruft) + continue; + if (limit) { + if ((uintmax_t) e->pack->pack_size >= limit) + string_list_append(packs, e->pack->pack_name); + } else if (!base || base->pack_size < e->pack->pack_size) { + base = e->pack; + } + } + + if (base) + string_list_append(packs, base->pack_name); + + return base; +} + +static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit) +{ + struct packfile_list_entry *e; + int cnt = 0; + + if (gc_auto_pack_limit <= 0) + return 0; + + for (e = packfile_store_get_packs(files->packed); e; e = e->next) { + if (e->pack->pack_keep) + continue; + /* + * Perhaps check the size of the pack and count only + * very small ones here? + */ + cnt++; + } + return gc_auto_pack_limit < cnt; +} + +static uint64_t total_ram(void) +{ +#if defined(HAVE_SYSINFO) + struct sysinfo si; + + if (!sysinfo(&si)) { + uint64_t total = si.totalram; + + if (si.mem_unit > 1) + total *= (uint64_t)si.mem_unit; + return total; + } +#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64)) + uint64_t physical_memory; + int mib[2]; + size_t length; + + mib[0] = CTL_HW; +# if defined(HW_MEMSIZE) + mib[1] = HW_MEMSIZE; +# elif defined(HW_PHYSMEM64) + mib[1] = HW_PHYSMEM64; +# else + mib[1] = HW_PHYSMEM; +# endif + length = sizeof(physical_memory); + if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) { + if (length == 4) { + uint32_t mem; + + if (!sysctl(mib, 2, &mem, &length, NULL, 0)) + physical_memory = mem; + } + return physical_memory; + } +#elif defined(GIT_WINDOWS_NATIVE) + MEMORYSTATUSEX memInfo; + + memInfo.dwLength = sizeof(MEMORYSTATUSEX); + if (GlobalMemoryStatusEx(&memInfo)) + return memInfo.ullTotalPhys; +#endif + return 0; +} + +static uint64_t estimate_repack_memory(struct odb_source_files *files, + struct packed_git *pack) +{ + unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE; + unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT; + unsigned long nr_objects; + size_t os_cache, heap; + + if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE, + &nr_objects) < 0) + return 0; + + if (!pack || !nr_objects) + return 0; + + repo_config_get_ulong(files->base.odb->repo, "pack.deltacachesize", + &max_delta_cache_size); + repo_config_get_ulong(files->base.odb->repo, "core.deltabasecachelimit", + &delta_base_cache_limit); + + /* + * First we have to scan through at least one pack. + * Assume enough room in OS file cache to keep the entire pack + * or we may accidentally evict data of other processes from + * the cache. + */ + os_cache = pack->pack_size + pack->index_size; + /* then pack-objects needs lots more for book keeping */ + heap = sizeof(struct object_entry) * nr_objects; + /* + * internal rev-list --all --objects takes up some memory too, + * let's say half of it is for blobs + */ + heap += sizeof(struct blob) * nr_objects / 2; + /* + * and the other half is for trees (commits and tags are + * usually insignificant) + */ + heap += sizeof(struct tree) * nr_objects / 2; + /* and then obj_hash[], underestimated in fact */ + heap += sizeof(struct object *) * nr_objects; + /* revindex is used also */ + heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects; + /* + * read_sha1_file() (either at delta calculation phase, or + * writing phase) also fills up the delta base cache + */ + heap += delta_base_cache_limit; + /* and of course pack-objects has its own delta cache */ + heap += max_delta_cache_size; + + return os_cache + heap; +} + +static int keep_one_pack(struct string_list_item *item, void *data) +{ + struct strvec *args = data; + strvec_pushf(args, "--keep-pack=%s", basename(item->string)); + return 0; +} + +static void add_repack_all_option(struct repository *repo, + const struct odb_optimize_options *opts, + struct string_list *keep_pack, + struct strvec *args) +{ + char *repack_filter = NULL; + char *repack_filter_to = NULL; + + repo_config_get_string(repo, "gc.repackfilter", &repack_filter); + repo_config_get_string(repo, "gc.repackfilterto", &repack_filter_to); + + if (opts->prune_expire && !strcmp(opts->prune_expire, "now") && + !(opts->cruft_packs && opts->expire_to)) + strvec_push(args, "-a"); + else if (opts->cruft_packs) { + strvec_push(args, "--cruft"); + if (opts->prune_expire) + strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire); + if (opts->max_cruft_size) + strvec_pushf(args, "--max-cruft-size=%lu", + opts->max_cruft_size); + if (opts->expire_to) + strvec_pushf(args, "--expire-to=%s", opts->expire_to); + } else { + strvec_push(args, "-A"); + if (opts->prune_expire) + strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire); + } + + if (keep_pack) + for_each_string_list(keep_pack, keep_one_pack, args); + + if (repack_filter && *repack_filter) + strvec_pushf(args, "--filter=%s", repack_filter); + if (repack_filter_to && *repack_filter_to) + strvec_pushf(args, "--filter-to=%s", repack_filter_to); + + free(repack_filter); + free(repack_filter_to); +} + +static void add_repack_incremental_option(struct strvec *args) +{ + strvec_push(args, "--no-write-bitmap-index"); +} + +bool odb_source_files_optimize_required(struct odb_source *source, + const struct odb_optimize_options *opts) +{ + struct odb_source_files *files = odb_source_files_downcast(source); + struct repository *repo = source->odb->repo; + + switch (opts->strategy) { + case ODB_OPTIMIZE_INCREMENTAL: { + int gc_auto_threshold = 6700; + int gc_auto_pack_limit = 50; + + repo_config_get_int(repo, "gc.auto", &gc_auto_threshold); + repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit); + + /* + * Setting gc.auto to 0 or negative can disable the + * automatic gc. + */ + if (gc_auto_threshold <= 0) + return false; + if (!too_many_packs(files, gc_auto_pack_limit) && + !too_many_loose_objects(files, gc_auto_threshold)) + return false; + + return true; + } + case ODB_OPTIMIZE_GEOMETRIC: { + struct pack_geometry geometry = { + .split_factor = 2, + }; + struct pack_objects_args po_args = { + .local = 1, + }; + struct existing_packs existing_packs = EXISTING_PACKS_INIT; + struct string_list kept_packs = STRING_LIST_INIT_DUP; + int auto_value = 100; + bool ret; + + repo_config_get_int(repo, "maintenance.geometric-repack.auto", + &auto_value); + if (!auto_value) + return false; + if (auto_value < 0) + return true; + + repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor", + &geometry.split_factor); + + existing_packs.repo = repo; + existing_packs_collect(&existing_packs, &kept_packs); + pack_geometry_init(&geometry, &existing_packs, &po_args); + pack_geometry_split(&geometry); + + /* + * When we'd merge at least two packs with one another we always + * perform the repack. + */ + if (geometry.split) { + ret = true; + goto out; + } + + /* + * Otherwise, we estimate the number of loose objects to determine + * whether we want to create a new packfile or not. + */ + if (too_many_loose_objects(files, auto_value)) { + ret = true; + goto out; + } + + ret = false; + + out: + existing_packs_release(&existing_packs); + pack_geometry_release(&geometry); + return ret; + } + default: + BUG("unknown maintenance strategy '%d'", opts->strategy); + } +} + +int odb_source_files_optimize(struct odb_source *source, + const struct odb_optimize_options *opts) +{ + struct odb_source_files *files = odb_source_files_downcast(source); + struct repository *repo = source->odb->repo; + struct child_process repack_cmd = CHILD_PROCESS_INIT; + unsigned long big_pack_threshold = 0; + int gc_auto_threshold = 6700; + int gc_auto_pack_limit = 50; + int ret; + + repo_config_get_int(repo, "gc.auto", &gc_auto_threshold); + repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit); + repo_config_get_ulong(repo, "gc.bigpackthreshold", &big_pack_threshold); + + if (repo->repository_format_precious_objects) + return 0; + + repack_cmd.git_cmd = 1; + repack_cmd.odb_to_close = repo->objects; + + strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); + if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS) + strvec_push(&repack_cmd.args, "-f"); + if (opts->depth > 0) + strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth); + if (opts->window > 0) + strvec_pushf(&repack_cmd.args, "--window=%d", opts->window); + if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) + strvec_push(&repack_cmd.args, "-q"); + + /* + * There's three cases we need to consider: + * + * - If we're invoked without `--auto` we'll need to perform a full + * repack. + * + * - If we're invoked with `--auto` and there's too many packs, then + * we perform a full repack, as well. + * + * - Otherwise we perform an incremental repack. + */ + switch (opts->strategy) { + case ODB_OPTIMIZE_INCREMENTAL: + if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (opts->keep_largest_pack != -1) { + if (opts->keep_largest_pack) + find_base_packs(files, &keep_pack, 0); + } else if (big_pack_threshold) { + find_base_packs(files, &keep_pack, big_pack_threshold); + } + + add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + if (too_many_packs(files, gc_auto_pack_limit)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (big_pack_threshold) { + find_base_packs(files, &keep_pack, big_pack_threshold); + if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) { + string_list_clear(&keep_pack, 0); + find_base_packs(files, &keep_pack, 0); + } + } else { + struct packed_git *p = find_base_packs(files, &keep_pack, 0); + uint64_t mem_have, mem_want; + + mem_have = total_ram(); + mem_want = estimate_repack_memory(files, p); + + /* + * Only allow 1/2 of memory for pack-objects, leave + * the rest for the OS and other processes in the + * system. + */ + if (!mem_have || mem_want < mem_have / 2) + string_list_clear(&keep_pack, 0); + } + + add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); + } + } + + break; + case ODB_OPTIMIZE_GEOMETRIC: { + struct pack_geometry geometry = { + .split_factor = 2, + }; + struct pack_objects_args po_args = { + .local = 1, + }; + struct existing_packs existing_packs = EXISTING_PACKS_INIT; + struct string_list kept_packs = STRING_LIST_INIT_DUP; + + repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor", + &geometry.split_factor); + + existing_packs.repo = repo; + existing_packs_collect(&existing_packs, &kept_packs); + pack_geometry_init(&geometry, &existing_packs, &po_args); + pack_geometry_split(&geometry); + + if (geometry.split < geometry.pack_nr) { + strvec_pushf(&repack_cmd.args, "--geometric=%d", + geometry.split_factor); + } else { + add_repack_all_option(repo, opts, NULL, &repack_cmd.args); + } + if (repo->settings.core_multi_pack_index) + strvec_push(&repack_cmd.args, "--write-midx"); + + existing_packs_release(&existing_packs); + pack_geometry_release(&geometry); + break; + } + default: + die("unknown maintenance strategy '%d'", opts->strategy); + } + + if (run_command(&repack_cmd)) { + ret = error("failed to run %s", repack_cmd.args.v[0]); + goto out; + } + + /* Geometric repacking uses cruft packs, so we don't have to prune separately. */ + if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) { + struct child_process prune_cmd = CHILD_PROCESS_INIT; + + strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); + /* run `git prune` even if using cruft packs */ + strvec_push(&prune_cmd.args, opts->prune_expire); + if (!(opts->flags & ODB_OPTIMIZE_VERBOSE)) + strvec_push(&prune_cmd.args, "--no-progress"); + if (repo_has_promisor_remote(repo)) + strvec_push(&prune_cmd.args, + "--exclude-promisor-objects"); + prune_cmd.git_cmd = 1; + + if (run_command(&prune_cmd)) { + ret = error("failed to run %s", prune_cmd.args.v[0]); + goto out; + } + } + + if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold)) + warning(_("There are too many unreachable loose objects; " + "run 'git prune' to remove them.")); + + ret = 0; + +out: + return ret; +} + struct odb_source_files *odb_source_files_new(struct object_database *odb, const char *path, bool local) @@ -285,6 +753,8 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb, files->base.begin_transaction = odb_source_files_begin_transaction; files->base.read_alternates = odb_source_files_read_alternates; files->base.write_alternate = odb_source_files_write_alternate; + files->base.optimize = odb_source_files_optimize; + files->base.optimize_required = odb_source_files_optimize_required; /* * Ideally, we would only ever store absolute paths in the source. This diff --git a/odb/source-files.h b/odb/source-files.h index d7ac3c1c81d892..044242bc36e4a7 100644 --- a/odb/source-files.h +++ b/odb/source-files.h @@ -21,6 +21,21 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb, const char *path, bool local); +/* + * Optimize the files object database source by repacking loose objects and + * packfiles as needed. Returns 0 on success, a negative error code otherwise. + */ +int odb_source_files_optimize(struct odb_source *source, + const struct odb_optimize_options *opts); + +/* + * Check whether optimization of the files object database source is required + * given the provided options. Returns true if optimization should be + * performed, false otherwise. + */ +bool odb_source_files_optimize_required(struct odb_source *source, + const struct odb_optimize_options *opts); + /* * Cast the given object database source to the files backend. This will cause * a BUG in case the source doesn't use this backend. diff --git a/odb/source.h b/odb/source.h index 8767708c9c769c..88a48ba3c3075b 100644 --- a/odb/source.h +++ b/odb/source.h @@ -258,6 +258,21 @@ struct odb_source { */ int (*write_alternate)(struct odb_source *source, const char *alternate); + + /* + * This callback is expected to optimize the object database source. + * Returns 0 on success, a negative error code otherwise. + */ + int (*optimize)(struct odb_source *source, + const struct odb_optimize_options *opts); + + /* + * This callback is expected to check whether optimization of the + * object database source is required given the provided options. + * Returns true if optimization should be performed, false otherwise. + */ + bool (*optimize_required)(struct odb_source *source, + const struct odb_optimize_options *opts); }; /* @@ -475,4 +490,25 @@ static inline int odb_source_begin_transaction(struct odb_source *source, return source->begin_transaction(source, out); } +/* + * Optimize the object database source. Returns 0 on success, a negative error + * code otherwise. + */ +static inline int odb_source_optimize(struct odb_source *source, + const struct odb_optimize_options *opts) +{ + return source->optimize(source, opts); +} + +/* + * Check whether optimization of the object database source is required given + * the provided options. Returns true if optimization should be performed, + * false otherwise. + */ +static inline bool odb_source_optimize_required(struct odb_source *source, + const struct odb_optimize_options *opts) +{ + return source->optimize_required(source, opts); +} + #endif From 88efab5c3b7b7585a3b4f45db89c108fef9a04f2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 15 Jul 2026 02:05:23 -0400 Subject: [PATCH 13/25] diff: ignore unmerged paths outside prefix with --relative --cached A diff using --relative ignores entries outside the current directory. This results in a segfault when we try to process an unmerged entry that's outside of our prefix, since we end up with a NULL diff_filepair and use it without checking that it's valid. I think this bug goes back to 76399c0195 (diff.c: return filepair from diff_unmerge(), 2011-04-22). Prior to that, diff_unmerge() knew to skip entries outside of our prefix, due to cd676a5136 (diff --relative: output paths as relative to the current subdirectory, 2008-02-12). Back then the caller didn't care that we hadn't added anything to the queue. In 76399c0195 that changed; we now returned the pair (or NULL), and the caller in do_oneway_diff() was then called fill_filespec() itself. And it does so without checking for NULL, causing a segfault. The obvious fix is to skip the fill_filespec() call (after which we just return), which this patch does. There's another call to diff_unmerge() in run_diff_files(). That case was already fixed by 8174627b3d (diff-lib: ignore paths that are outside $cwd if --relative asked, 2021-08-22), but of course it didn't help us for --cached. That commit also claims that checking the result of diff_unmerge() is not enough, as we'd want other code paths to skip the entry, too (even if they wouldn't segfault). But as far as I can tell, that is not true for --cached. We eventually end up in diff_queue_addremove() or in diff_queue_change(), both of which know to return early when we're outside of the prefix. Arguably we could be checking at the top of oneway_diff() whether the path is interesting at all. That would not only avoid this code path entirely, but would also possibly save a small amount of work. But since everything else appears to work OK, I went for the smallest fix here to avoid any regression. Specifically, a comment in oneway_diff() claims we're supposed to advance o->pos, which we might fail to do if we return early. Though that "advance" seems to have gone away in da165f470e (unpack-trees.c: prepare for looking ahead in the index, 2010-01-07), so it is possible the comment is simply out of date. We can explore that separately; checking for a NULL return from diff_unmerge() seems like a sensible thing to do regardless. We can piggy-back on the tests added by 8174627b3d; we're just checking the --cached variant. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff-lib.c | 2 +- t/t4045-diff-relative.sh | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/diff-lib.c b/diff-lib.c index ae91027a024eec..a23119b8522012 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -467,7 +467,7 @@ static void do_oneway_diff(struct unpack_trees_options *o, if (cached && idx && ce_stage(idx)) { struct diff_filepair *pair; pair = diff_unmerge(&revs->diffopt, idx->name); - if (tree) + if (pair && tree) fill_filespec(pair->one, &tree->oid, 1, tree->ce_mode); return; diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh index 2c8493fe66c441..167be0bdcce586 100755 --- a/t/t4045-diff-relative.sh +++ b/t/t4045-diff-relative.sh @@ -245,4 +245,13 @@ test_expect_failure 'diff --relative with change in subdir' ' test_cmp expected out ' +test_expect_success 'diff --relative --cached with change in subdir' ' + git switch br3 && + test_when_finished "git merge --abort" && + test_must_fail git merge sub1 && + echo file0 >expected && + git -C subdir diff --relative --name-only --cached >out && + test_cmp expected out +' + test_done From 7780bff8d161fe42d65954e026dcc621c50bd128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 25 Jul 2026 12:41:07 +0200 Subject: [PATCH 14/25] branch: report active bisect run when rejecting delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git branch refuses to delete branches that are currently checked out with a message like this: "error: cannot delete branch 'foo' used by worktree at '/path/of/worktree'". This can be confusing if it's an internal checkout for git bisect. Report a more specific error in that case to help users that might have forgotten their bisect run. Suggested-by: stsp Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- branch.c | 80 +++++++++++++++++++++++++++++++++-------------- branch.h | 6 ++++ builtin/branch.c | 7 +++++ t/t3200-branch.sh | 4 +-- 4 files changed, 72 insertions(+), 25 deletions(-) diff --git a/branch.c b/branch.c index 243db7d0fc0226..a9fc79081847f0 100644 --- a/branch.c +++ b/branch.c @@ -385,6 +385,39 @@ int validate_branchname(const char *name, struct strbuf *ref) static int initialized_checked_out_branches; static struct strmap current_checked_out_branches = STRMAP_INIT; +enum branch_checkout_kind { + BRANCH_CHECKOUT_KIND_CHECKOUT, + BRANCH_CHECKOUT_KIND_REBASE, + BRANCH_CHECKOUT_KIND_BISECT, + BRANCH_CHECKOUT_KIND_UPDATE_REF, +}; + +struct checked_out_branch { + char *refname; + char *path; + enum branch_checkout_kind kind; +}; + +static struct checked_out_branch *checked_out_branches; +static size_t checked_out_branches_alloc, checked_out_branches_nr; + +static void register_checked_out_branch(const char *prefix, const char *name, + const char *path, + enum branch_checkout_kind kind) +{ + char *refname = xstrfmt("%s%s", prefix, name); + char *path_copy = xstrdup(path); + + ALLOC_GROW(checked_out_branches, checked_out_branches_nr + 1, + checked_out_branches_alloc); + checked_out_branches[checked_out_branches_nr].refname = refname; + checked_out_branches[checked_out_branches_nr].path = path_copy; + checked_out_branches[checked_out_branches_nr].kind = kind; + checked_out_branches_nr++; + + strmap_put(¤t_checked_out_branches, refname, path_copy); +} + static void prepare_checked_out_branches(void) { int i = 0; @@ -397,7 +430,7 @@ static void prepare_checked_out_branches(void) worktrees = get_worktrees(); while (worktrees[i]) { - char *old, *wt_gitdir; + char *wt_gitdir; struct wt_status_state state = { 0 }; struct worktree *wt = worktrees[i++]; struct string_list update_refs = STRING_LIST_INIT_DUP; @@ -406,34 +439,25 @@ static void prepare_checked_out_branches(void) continue; if (wt->head_ref) { - old = strmap_put(¤t_checked_out_branches, - wt->head_ref, - xstrdup(wt->path)); - free(old); + register_checked_out_branch("", wt->head_ref, wt->path, + BRANCH_CHECKOUT_KIND_CHECKOUT); } if (wt_status_check_rebase(wt, &state) && (state.rebase_in_progress || state.rebase_interactive_in_progress) && state.branch) { - struct strbuf ref = STRBUF_INIT; - strbuf_addf(&ref, "refs/heads/%s", state.branch); - old = strmap_put(¤t_checked_out_branches, - ref.buf, - xstrdup(wt->path)); - free(old); - strbuf_release(&ref); + register_checked_out_branch("refs/heads/", state.branch, + wt->path, + BRANCH_CHECKOUT_KIND_REBASE); } wt_status_state_free_buffers(&state); if (wt_status_check_bisect(wt, &state) && state.bisecting_from) { - struct strbuf ref = STRBUF_INIT; - strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from); - old = strmap_put(¤t_checked_out_branches, - ref.buf, - xstrdup(wt->path)); - free(old); - strbuf_release(&ref); + register_checked_out_branch("refs/heads/", + state.bisecting_from, + wt->path, + BRANCH_CHECKOUT_KIND_BISECT); } wt_status_state_free_buffers(&state); @@ -442,10 +466,9 @@ static void prepare_checked_out_branches(void) &update_refs)) { struct string_list_item *item; for_each_string_list_item(item, &update_refs) { - old = strmap_put(¤t_checked_out_branches, - item->string, - xstrdup(wt->path)); - free(old); + register_checked_out_branch("", item->string, + wt->path, + BRANCH_CHECKOUT_KIND_UPDATE_REF); } string_list_clear(&update_refs, 1); } @@ -462,6 +485,17 @@ const char *branch_checked_out(const char *refname) return strmap_get(¤t_checked_out_branches, refname); } +const char *branch_bisecting(const char *refname) +{ + prepare_checked_out_branches(); + for (size_t i = 0; i < checked_out_branches_nr; i++) { + if (!strcmp(refname, checked_out_branches[i].refname) && + checked_out_branches[i].kind == BRANCH_CHECKOUT_KIND_BISECT) + return checked_out_branches[i].path; + } + return NULL; +} + /* * Check if a branch 'name' can be created as a new branch; die otherwise. * 'force' can be used when it is OK for the named branch already exists. diff --git a/branch.h b/branch.h index 3dc6e2a0ffe635..e9b1f7b37df06f 100644 --- a/branch.h +++ b/branch.h @@ -106,6 +106,12 @@ void create_branches_recursively(struct repository *r, const char *name, */ const char *branch_checked_out(const char *refname); +/* + * If the branch at 'refname' is currently used for bisecting in a + * worktree, then return the path to that worktree. + */ +const char *branch_bisecting(const char *refname); + /* * Check if 'name' can be a valid name for a branch; die otherwise. * Return 1 if the named branch already exists; return 0 otherwise. diff --git a/builtin/branch.c b/builtin/branch.c index dede60d27b69ea..29e4ec6c672f75 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -265,6 +265,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, if (kinds == FILTER_REFS_BRANCHES) { const char *path; + if ((path = branch_bisecting(name))) { + error(_("cannot delete branch '%s' " + "used by worktree at '%s' for bisect"), + bname.buf, path); + ret = 1; + continue; + } if ((path = branch_checked_out(name))) { error(_("cannot delete branch '%s' " "used by worktree at '%s'"), diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 1ecbafbee18e03..051434d9c6ae99 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -930,7 +930,7 @@ test_expect_success 'deleting currently checked out branch fails' ' git worktree add -b my7 my7 && test_must_fail git -C my7 branch -d my7 && test_must_fail git branch -d my7 2>actual && - test_grep "^error: cannot delete branch .my7. used by worktree at " actual && + test_grep "^error: cannot delete branch '"'"'my7'"'"' used by worktree at '"'.*'\$"'" actual && rm -r my7 && git worktree prune ' @@ -941,7 +941,7 @@ test_expect_success 'deleting in-use branch fails' ' git -C my7 bisect start HEAD HEAD~2 && test_must_fail git -C my7 branch -d my7 && test_must_fail git branch -d my7 2>actual && - test_grep "^error: cannot delete branch .my7. used by worktree at " actual && + test_grep "^error: cannot delete branch '"'"'my7'"'"' used by worktree at '"'.*' for bisect\$"'" actual && rm -r my7 && git worktree prune ' From ae0780def7353ec713a96ef498d10e4ef70692a2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Jul 2026 04:37:27 -0400 Subject: [PATCH 15/25] bloom: silence CHECK_ASSERTION_SIDE_EFFECTS false positive Using gcc 15, compiling with CHECK_ASSERTION_SIDE_EFFECTS=1 causes a complaint about this line in bloom.c having a side effect: assert(version == 1 || version == 2); I think this is pretty clearly a false positive, as those comparisons should not have side effects. The side-effect checker uses a magic definition of assert() that relies on the compiler's optimizer to drop a reference to an otherwise unused variable. And for whatever reason, gcc chooses not to do so here under -O2 (side note: if you have -O0 in your CFLAGS, that naturally creates many more false positives!). This code has been around for a while, but nobody seems to have noticed because we use an older version of the compiler in our static-analysis ci job, and it does not complain. Presumably very few people run this check locally on their more modern compilers. Let's silence the false positive to avoid confusion for anyone running locally, and to make it possible to upgrade the image we use for our static-analysis job. We could just switch to our custom ASSERT() here, but I think we can improve the code by integrating the assertion into the if/else cascade. That avoids repeating the logic about which versions are acceptable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- bloom.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bloom.c b/bloom.c index a805ac0c296b37..aac8f448c9fc95 100644 --- a/bloom.c +++ b/bloom.c @@ -605,10 +605,10 @@ int bloom_filter_contains_vec(const struct bloom_filter *filter, uint32_t test_bloom_murmur3_seeded(uint32_t seed, const char *data, size_t len, int version) { - assert(version == 1 || version == 2); - if (version == 2) return murmur3_seeded_v2(seed, data, len); - else + else if (version == 1) return murmur3_seeded_v1(seed, data, len); + else + BUG("unexpected bloom version: %d", version); } From 1a1579c42d9d4c78d4d4df5e4d3d8bc73e3ddf9b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 26 Jul 2026 04:39:05 -0400 Subject: [PATCH 16/25] ci: bump ubuntu image version for static-analysis job We recently ran into a case[1] where old versions of coccinelle ran very slowly, but newer ones are fine. The version we use in GitHub's CI was the old slow version, leading to timeouts of the static-analysis job. We get the old version because we ask for the ubuntu-22.04 image. That has coccinelle 1.1.1, but the "fast" improvement is in coccinelle 1.3.0, specifically their 58619b8fe (break up envs for e1 & e2, 2024-08-18). Bumping to ubuntu-25.10 would be enough to get that new version. But I don't see any need to ask for a specific version at all. We originally used a specific version because coccinelle wasn't available in ubuntu 20.04, so we pinned to 18.04 in d051ed77ee (.github/workflows/main.yml: run static-analysis on bionic, 2021-02-08). Later that got bumped in ef46584831 (ci: update 'static-analysis' to Ubuntu 22.04, 2022-08-23) when 18.04 support was dropped. It seems like the absence of coccinelle was a blip in 20.04, and we can just stick with "latest" going forward. I tested the result on GitHub's CI. I bumped the matching line in the GitLab definition, but didn't have a simple means of testing (but it's such a trivial change nothing could go wrong, right?). [1] https://lore.kernel.org/git/20260724091152.27794-2-tnyman@openai.com/ Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .github/workflows/main.yml | 4 ++-- .gitlab-ci.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cf341d74dbff21..9896a817eb8805 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -458,8 +458,8 @@ jobs: if: needs.ci-config.outputs.enabled == 'yes' env: jobname: StaticAnalysis - CI_JOB_IMAGE: ubuntu-22.04 - runs-on: ubuntu-22.04 + CI_JOB_IMAGE: ubuntu-latest + runs-on: ubuntu-latest concurrency: group: static-analysis-${{ github.ref }} cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }} diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1a8e90932cc292..c4953480986310 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -226,7 +226,7 @@ test:fuzz-smoke-tests: - ./ci/run-build-and-minimal-fuzzers.sh static-analysis: - image: ubuntu:22.04 + image: ubuntu:latest stage: analyze needs: [ ] variables: From 2f5ff2c339d90422dd4010b5d980d33d1959fdd9 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Sun, 26 Jul 2026 16:38:59 +0100 Subject: [PATCH 17/25] rebase -i: fix counting of fixups after rebase --skip When the sequencer processes a chain of "fixup" and "squash" commands it keeps a list of the commands that have been executed. If there are conflicts, then the list is saved when the rebase stops for the user to resolve them. When the rebase resumes, the list is loaded and is used to initialize the count of how many "fixup" and "squash" commands have been processed; if a command has been skipped with "git rebase --skip", then the last command needs to be popped off the end of the list. To count the number of commands, commit_staged_changes() uses the number of newlines in the file plus one. This is due to the slightly unusual way the list is constructed - instead of appending a newline when a command is added, a newline is inserted before the command if the current count is greater than zero. Therefore, when we pop a skipped command off the list, we should also remove the newline that precedes it. Otherwise, when a new command is added, a blank line will be left before it, which will contribute to the fixup count the next time the file is read. Unfortunately, the preceding newline is not removed, leading to an incorrect count. Fix this by removing the newline that appears before the skipped command. In addition to fixing the code that removes a skipped command from the list, the code that reads the list is fixed to skip blank lines. We have had reports of users starting a rebase with one version of git and continuing it with another. Often this happens because the version of git bundled with an IDE or TUI differs from the one used at the command line. By fixing both the reading and writing ends of the problem we ensure the count is correct when an older version of git reads the fixup file written by a newer version and vice versa. Triggering the incorrect count requires the user to skip two "fixup" or "squash" commands before the final command in the chain. An existing test is extended to prevent future regressions. The consequence of miscounting is not serious: we just print the wrong count in the header of the commit message template. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 11 ++++++++++- t/t3418-rebase-continue.sh | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index 1355a99a092268..4640ee9b7f536b 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3281,7 +3281,13 @@ static int read_populate_opts(struct replay_opts *opts) const char *p = ctx->current_fixups.buf; ctx->current_fixup_count = 1; while ((p = strchr(p, '\n'))) { - ctx->current_fixup_count++; + /* + * Older versions of git accidentally + * inserted blank lines when a fixup + * was skipped. + */ + if (p[1] && p[1] != '\n') + ctx->current_fixup_count++; p++; } } @@ -5354,6 +5360,9 @@ static int commit_staged_changes(struct repository *r, BUG("Incorrect current_fixups:\n%s", p); while (len && p[len - 1] != '\n') len--; + /* Remove trailing newline */ + if (len) + len--; strbuf_setlen(&ctx->current_fixups, len); if (write_message(p, len, rebase_path_current_fixups(), 0) < 0) { diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh index f9b8999db50f1b..3c248e973649ea 100755 --- a/t/t3418-rebase-continue.sh +++ b/t/t3418-rebase-continue.sh @@ -134,6 +134,7 @@ test_expect_success '--skip after failed fixup cleans commit message' ' EOF : skip and continue && + test_config commit.status false && echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh && (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) && @@ -145,7 +146,8 @@ test_expect_success '--skip after failed fixup cleans commit message' ' : now, let us ensure that "squash" is handled correctly && git reset --hard wants-fixup-3 && - test_must_fail env FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1" \ + test_must_fail env \ + FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1 squash 4 squash 1" \ git rebase -i HEAD~4 && : the second squash failed, but there are two more in the chain && @@ -171,6 +173,32 @@ test_expect_success '--skip after failed fixup cleans commit message' ' fixup 2 EOF + (test_set_editor "$PWD/copy-editor.sh" && + test_must_fail git rebase --skip) && + : not the final squash, no need to edit the commit message && + test_path_is_missing .git/copy.txt && + + : The first, third and fifth squashes succeeded, therefore: && + cat >expect <<-\EOF && + # This is a combination of 4 commits. + # This is the 1st commit message: + + wants-fixup + + # This is the commit message #2: + + fixup 1 + + # This is the commit message #3: + + fixup 2 + + # This is the commit message #4: + + fixup 3 + EOF + test_commit_message HEAD expect && + (test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) && test_commit_message HEAD <<-\EOF && wants-fixup @@ -178,12 +206,12 @@ test_expect_success '--skip after failed fixup cleans commit message' ' fixup 1 fixup 2 + + fixup 3 EOF : Final squash failed, but there was still a squash && - head -n1 .git/copy.txt >first-line && - test_grep "# This is a combination of 3 commits" first-line && - test_grep "# This is the commit message #3:" .git/copy.txt + test_cmp expect .git/copy.txt ' test_expect_success 'setup rerere database' ' From 4a3a8ee96c7c749b99c91345db190a2c3720abeb Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Sun, 26 Jul 2026 16:39:00 +0100 Subject: [PATCH 18/25] rebase: remember fixup -c after skipping fixup/squash When the final command in a chain of "fixup" and "squash" commands is skipped, we should prompt the user to edit the commit message if the chain contains a "fixup -c" command that was not skipped. Unfortunately, commit_staged_changes() only looks for completed "squash" commands and so does not prompt the user to edit the message. Fix this by recording whether a fixup command has the "-c" flag set and then checking whether we have seen either a "fixup -c" or a "squash" command. Add regression tests for skipping a command in the middle of the chain (which currently works but has no test coverage), and for skipping the final command (which is fixed by this patch). Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 20 +++++++++++--- t/t3437-rebase-fixup-options.sh | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index 4640ee9b7f536b..1a0a283b42c3c2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1926,6 +1926,13 @@ static int seen_squash(struct replay_ctx *ctx) strstr(ctx->current_fixups.buf, "\nsquash"); } +/* Does the current fixup chain contain a "fixup -c" command? */ +static int seen_fixup_edit_msg(struct replay_ctx *ctx) +{ + return starts_with(ctx->current_fixups.buf, "fixup -c") || + strstr(ctx->current_fixups.buf, "\nfixup -c"); +} + static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n) { strbuf_setlen(buf1, strlen(comment_line_str) + 1); @@ -2148,9 +2155,14 @@ static int update_squash_messages(struct repository *r, strbuf_release(&buf); if (!res) { - strbuf_addf(&ctx->current_fixups, "%s%s %s", + const char *fixup_flag = ""; + + if (is_fixup_flag(command, flag) && (flag & TODO_EDIT_FIXUP_MSG)) + fixup_flag = " -c"; + + strbuf_addf(&ctx->current_fixups, "%s%s%s %s", ctx->current_fixups.len ? "\n" : "", - command_to_string(command), + command_to_string(command), fixup_flag, oid_to_hex(&commit->object.oid)); res = write_message(ctx->current_fixups.buf, ctx->current_fixups.len, @@ -5391,8 +5403,8 @@ static int commit_staged_changes(struct repository *r, * message, no need to bother the user with * opening the commit message in the editor. */ - if (!starts_with(p, "squash ") && - !strstr(p, "\nsquash ")) + if (!seen_squash(ctx) && + !seen_fixup_edit_msg(ctx)) flags = (flags & ~EDIT_MSG) | CLEANUP_MSG; } else if (is_fixup(peek_command(todo_list, 0))) { /* diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh index 5d306a476928b1..a4b2a631654f1c 100755 --- a/t/t3437-rebase-fixup-options.sh +++ b/t/t3437-rebase-fixup-options.sh @@ -186,6 +186,53 @@ test_expect_success 'multiple fixup -c opens editor once' ' test_commit_message HEAD expected-message ' +test_expect_success 'fixup -c is remembered after skipping final fixup' ' + test_when_finished "test_might_fail git rebase --abort" && + cat >todo <<-\EOF && + pick B + fixup -c A1 + fixup A3 + EOF + ( + set_fake_editor && + set_replace_editor todo && + test_must_fail git rebase -i A A && + git show && cat .git/rebase-merge/message-squash && + FAKE_COMMIT_AMEND=edited git rebase --skip + ) && + test_commit_message HEAD <<-\EOF + new subject + + new + body + + edited + EOF +' +test_expect_success 'fixup -c is remembered after skipping later fixup' ' + test_when_finished "test_might_fail git rebase --abort" && + cat >todo <<-\EOF && + pick B + fixup -c A1 + fixup A3 + fixup A2 + EOF + ( + set_fake_editor && + set_replace_editor todo && + test_must_fail git rebase -i A A && + FAKE_COMMIT_AMEND=edited git rebase --skip + ) && + test_commit_message HEAD <<-\EOF + new subject + + new + body + + edited + EOF +' + test_expect_success 'sequence squash, fixup & fixup -c gives combined message' ' test_when_finished "test_might_fail git rebase --abort" && git checkout --detach A3 && From 447126ed7df66b396235766547a57649f925aac2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Jul 2026 11:14:58 -0400 Subject: [PATCH 19/25] diff-lib: add idx/tree sanity check to oneway_diff When looking just at the code in oneway_diff(), it seems possible for both "idx" and "tree" to be NULL, in which case we'd potentially segfault while checking the relative prefix. But if you consider what these items actually mean, it shouldn't be possible for both to be NULL. Let's add an assertion and a comment documenting this. It might help human readers, but should also silence static analyzers like Coverity which complain about the potential segfault. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff-lib.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/diff-lib.c b/diff-lib.c index a23119b8522012..0e868b28b6b004 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -530,6 +530,16 @@ static int oneway_diff(const struct cache_entry * const *src, if (tree == o->df_conflict_entry) tree = NULL; + /* + * We should only see a NULL idx when the entry was present in the tree + * but deleted in the idx. In which case it should be impossible + * that a NULL tree was passed in (there would have been no entry at + * all) or that we got a df conflict above (you need a directory and a + * file to get such a conflict, which implies both sides are present). + */ + if (!idx && !tree) + BUG("oneway_diff with neither idx nor tree"); + if (ce_path_match(revs->diffopt.repo->index, idx ? idx : tree, &revs->prune_data, NULL)) { From c57c052ae8d8486d88f93f983db4439241669c8a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 28 Jul 2026 11:46:35 +0000 Subject: [PATCH 20/25] mingw: skip symlink type auto-detection for network share targets On Windows, symbolic links come in two flavors: file symlinks and directory symlinks. Since Git was born on Linux where this distinction does not exist, Git for Windows has to auto-detect the type by looking at the target. When the target does not yet exist at symlink creation time, Git for Windows creates a "phantom" file symlink and later, once checkout is complete, calls `CreateFileW()` on the target to check whether it is actually a directory. If the symlink target is a UNC path (e.g. `\\attacker\share`), this auto-detection triggers an SMB connection to the remote host. Windows performs NTLM authentication by default for such connections, which means a crafted repository can exfiltrate the cloning user's NTLMv2 hash to an attacker-controlled server without any user interaction beyond `git clone -c core.symlinks=true `. There are ways to specify UNC paths that start with only a single backslash (e.g. `\??\UNC\host\share`); All of them do start like that, though, so let's use that as a tell-tale that we should skip the auto-detection in `process_phantom_symlink()`. The symlink is then left as a file symlink (the `mklink` default), and a warning is emitted suggesting the user set the `symlink` gitattribute to `dir` if a directory symlink is needed. When the attribute is already set, auto-detection is never invoked in the first place, so that code path is unaffected. This is the same class of vulnerability as CVE-2025-66413 (https://github.com/git-for-windows/git/security/advisories/GHSA-hv9c-4jm9-jh3x) and follows the same general mitigation pattern that MinTTY adopted for ANSI escape sequences referencing network share paths (https://github.com/mintty/mintty/security/advisories/GHSA-jf4m-m6rv-p6c5). Note that there are legitimate paths starting with a single backslash that are _not_ network paths: drive-less absolute paths are interpreted as relative to the current working directory's drive. In practice, these are highly uncommon (and brittle, just one working directory change away from breaking). In any case, the only consequence is now that the symlink type of those has to be specified via Git attributes, is all. Reported-by: Justin Lee Addresses: CVE-2026-32631 Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index 3eca3a7f2e87b2..2b0d162d498381 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -352,6 +352,29 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink) wchar_t relative[MAX_PATH]; const wchar_t *rel; + /* + * Do not follow symlinks to network shares, to avoid NTLM credential + * leak from crafted repositories (e.g. \\attacker-server\share). + * Since paths come in all kind of enterprising shapes and forms (in + * addition to the canonical `\\host\share` form, there's also + * `\??\UNC\host\share`, `\GLOBAL??\UNC\host\share` and also + * `\Device\Mup\host\share`, just to name a few), we simply avoid + * following every symlink target that starts with a slash. + * + * This also catches drive-less absolute paths, of course. These are + * uncommon in practice (and also fragile because they are relative to + * the current working directory's drive). The only "harm" this does + * is that it now requires users to specify via the Git attributes if + * they have such an uncommon symbolic link and need it to be a + * directory type link. + */ + if (is_wdir_sep(wtarget[0])) { + warning("created file symlink '%ls' pointing to '%ls';\n" + "set the `symlink` gitattribute to `dir` if a " + "directory symlink is required", wlink, wtarget); + return PHANTOM_SYMLINK_DONE; + } + /* check that wlink is still a file symlink */ if ((GetFileAttributesW(wlink) & (FILE_ATTRIBUTE_REPARSE_POINT | FILE_ATTRIBUTE_DIRECTORY)) From b678bb728331fbc575b8eee7948f08eec167d951 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Jul 2026 10:37:26 -0400 Subject: [PATCH 21/25] t0014: factor out choice of deprecated commands We have a few tests related to aliasing deprecated commands which use "whatchanged" and "pack-redundant", as these are the only two deprecated commands we have. Let's pull those names into variables so that we can refactor the tests without relying on the specific names. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0014-alias.sh | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh index 5144b0effd78aa..9d7c7373552350 100755 --- a/t/t0014-alias.sh +++ b/t/t0014-alias.sh @@ -27,17 +27,20 @@ test_expect_success 'looping aliases - internal execution' ' test_grep "^fatal: alias loop detected: expansion of" output ' +deprecated1=whatchanged +deprecated2=pack-redundant + test_expect_success 'looping aliases - deprecated builtins' ' - test_config alias.whatchanged pack-redundant && - test_config alias.pack-redundant whatchanged && + test_config alias.$deprecated1 $deprecated2 && + test_config alias.$deprecated2 $deprecated1 && cat >expect <<-EOF && - ${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ} - ${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ} - fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate: - whatchanged <== - pack-redundant ==> + ${SQ}$deprecated1${SQ} is aliased to ${SQ}$deprecated2${SQ} + ${SQ}$deprecated2${SQ} is aliased to ${SQ}$deprecated1${SQ} + fatal: alias loop detected: expansion of ${SQ}$deprecated1${SQ} does not terminate: + $deprecated1 <== + $deprecated2 ==> EOF - test_must_fail git whatchanged -h 2>actual && + test_must_fail git $deprecated1 -h 2>actual && test_cmp expect actual ' @@ -90,8 +93,8 @@ test_expect_success 'can alias-shadow via two deprecated builtins' ' # some git(1) commands will fail... (see above) test_might_fail git status -h >expect && test_file_not_empty expect && - test_might_fail git -c alias.whatchanged=pack-redundant \ - -c alias.pack-redundant=status whatchanged -h >actual && + test_might_fail git -c alias.$deprecated1=$deprecated2 \ + -c alias.$deprecated2=status $deprecated1 -h >actual && test_cmp expect actual ' From bc57ecb91537c776d0f34233746b09a88000bd26 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Jul 2026 10:38:45 -0400 Subject: [PATCH 22/25] t0014: generate deprecated command names dynamically We have a few tests related to aliasing of deprecated commands. They use whatchanged and pack-redundant because those are the only two deprecated commands we have. Eventually those commands will be removed, at which point these tests will be checking nothing useful (they'll just be regular aliases, which we already cover in other tests). We could remove them at that point, but the code to handle deprecated commands will still remain. We probably do want to keep the tests around for the eventual day that we deprecate more commands. So let's ask Git for its list of deprecated commands, and if we don't have any, skip those tests. This also prevents an annoying corner case when your build directory contains old build products. Right now those commands are marked as deprecated builtins and treated specially; we allow aliases and never look for them as dashed external commands. But after they are removed, they aren't special anymore. If your directory happens to contain hardlinks from the build of an older version, that confuses Git: it sees the old hardlinks in place, thinks those are actual external commands, and refuses to allow aliasing. You can see that today like this: make make WITH_BREAKING_CHANGES=1 test The first "make" creates git-whatchanged as a hardlink to Git, and the second does not clean it up (it doesn't know about the whatchanged command at all anymore). t0014 fails because Git won't create an alias to the "external" whatchanged command. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0014-alias.sh | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh index 9d7c7373552350..cbc447b4814e78 100755 --- a/t/t0014-alias.sh +++ b/t/t0014-alias.sh @@ -27,10 +27,15 @@ test_expect_success 'looping aliases - internal execution' ' test_grep "^fatal: alias loop detected: expansion of" output ' -deprecated1=whatchanged -deprecated2=pack-redundant +test_expect_success 'detect deprecated commands' ' + git --list-cmds=deprecated >deprecated && + if read deprecated1 && read deprecated2 + then + test_set_prereq HAVE_DEPRECATED + fi expect <<-EOF && @@ -89,7 +94,7 @@ test_expect_success 'can alias-shadow deprecated builtins' ' done ' -test_expect_success 'can alias-shadow via two deprecated builtins' ' +test_expect_success HAVE_DEPRECATED 'can alias-shadow via two deprecated builtins' ' # some git(1) commands will fail... (see above) test_might_fail git status -h >expect && test_file_not_empty expect && From 6375b40aea9ebcfdd8deda50a11f8cc336b28e09 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Mon, 3 Aug 2026 19:09:52 +0200 Subject: [PATCH 23/25] mailmap: change primary address for Christian Couder The `chriscool@tuxfamily.org` address is an old one that I don't use anymore, while `christian.couder@gmail.com` is the address I have been sending patches from for a long time. Let's swap the two addresses in the existing entry, so that the Gmail address becomes the primary one and the old tuxfamily.org address is mapped to it. This way both addresses still resolve to the same person, and the address I actually use is the canonical one. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- .mailmap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index f8ede075ea172f..d518b388fe4b1c 100644 --- a/.mailmap +++ b/.mailmap @@ -39,7 +39,7 @@ Chris Shoemaker Chris Wright Christian Ludwig Cord Seele -Christian Couder +Christian Couder Christian Stimming Christopher Díaz Riveros Christopher Diaz Riveros Clemens Buchacher From 7a403c3ee5519770b2630b49a59eba7815c8c210 Mon Sep 17 00:00:00 2001 From: "D. Ben Knoble" Date: Wed, 5 Aug 2026 08:19:32 -0400 Subject: [PATCH 24/25] mailmap: change primary address for D. Ben Knoble Ben uses the +github GMail trick to identify emails sent to him by folks that found his GitHub profile. At the time, that also meant he had to commit under the same email for GitHub to recognize his commits. He has since found out that GitHub can be configured with more than one email for identification, and he would prefer his canonical email to omit mention of GitHub (where it's not relevant) going forward. Signed-off-by: D. Ben Knoble Signed-off-by: Junio C Hamano --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index d518b388fe4b1c..48c34797b94a94 100644 --- a/.mailmap +++ b/.mailmap @@ -45,6 +45,7 @@ Christopher Díaz Riveros Christopher Diaz Riveros Clemens Buchacher Clemens Buchacher Csaba Henk +D. Ben Knoble Dan Johnson Dana L. How Dana L. How Dana How From 2c78326f810173a4f3aefd8021f1e07575412481 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 5 Aug 2026 11:10:39 -0700 Subject: [PATCH 25/25] The 11th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.56.0.adoc | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index a57ca5426fb4ca..87ac9067f194eb 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -76,6 +76,9 @@ UI, Workflows & Features filtered on the client based on server-advertised capabilities, returning empty strings for inapplicable or unsupported fields. + * 'git branch -d' has been taught to report when a branch cannot be + deleted because it is being used in an active bisect run. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -280,6 +283,21 @@ Performance, Internal Implementation, Development Support etc. first refactoring force_object_loose() to use generic ODB write interfaces instead of loose-backend internals. + * Object database housekeeping in 'git gc' and 'git maintenance' has + been refactored to be pluggable. The files-backend-specific logic, + including incremental and geometric repacking as well as object + pruning, has been moved out of the command implementation and into the + files object database source, enabling future alternative object + database backends to implement their own housekeeping services. + + * The image version used by the static-analysis CI job has been bumped + to ubuntu-latest (Ubuntu 24.04), which brings in a newer Coccinelle + version that resolves a severe performance regression. A false + positive warning from the 'CHECK_ASSERTION_SIDE_EFFECTS' build with + GCC 15 in the Bloom filter code has also been silenced to facilitate + the image upgrade. + (merge 1a1579c42d jk/ci-static-analysis-image-bump later to maint). + Fixes since v2.55 ----------------- @@ -466,3 +484,26 @@ Fixes since v2.55 * The remote-matching logic for submodules has been corrected to resolve 'url.*.insteadOf' aliases before comparing the inventoried URL from '.gitmodules' with the URLs of configured remotes. + + * 'git diff --relative' running with '--cached' has been corrected to + avoid a segfault when encountering unmerged paths outside the + prefix. + (merge 447126ed7d jk/diff-relative-cached-unmerged later to maint). + + * Two bugs in how 'git rebase' handles skipped 'fixup' and 'squash' + commands have been fixed. One bug caused an incorrect commit count to + be shown in the template message when multiple commands were skipped, + and another prevented the editor from opening when the final command + in a chain containing 'fixup -c' was skipped. + + * The alias tests in 't/t0014-alias.sh' have been updated to dynamically + query the list of deprecated commands using 'git + --list-cmds=deprecated' to avoid test failures when running with + 'WITH_BREAKING_CHANGES' in a build directory that contains stale + executables of formerly deprecated commands. + (merge bc57ecb915 jk/t0014-dynamic-deprecated-cmds later to maint). + + * Git for Windows has been updated to avoid auto-detecting the symlink + type if the target path starts with a slash, preventing NTLM + credential leaks when checking out repositories with crafted + symbolic links pointing to network shares.