From c9ef360403e0f64a5d4c70f66a9b25f02a98fda6 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:16 +0200 Subject: [PATCH 01/40] t1800: add hook output stream tests Lack of test coverage in this area led to some regressions while converting the remaining hooks to the newer hook.[ch] API. Add some tests to verify hooks write to the expected output streams. Suggested-by: Patrick Steinhardt Suggested-by: Junio C Hamano Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- t/t1800-hook.sh | 137 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 4feaf0d7bef71f..ed28a2fadb7ec6 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -184,4 +184,141 @@ test_expect_success 'stdin to hooks' ' test_cmp expect actual ' +check_stdout_separate_from_stderr () { + for hook in "$@" + do + # Ensure hook's stdout is only in stdout, not stderr + test_grep "Hook $hook stdout" stdout.actual || return 1 + test_grep ! "Hook $hook stdout" stderr.actual || return 1 + + # Ensure hook's stderr is only in stderr, not stdout + test_grep "Hook $hook stderr" stderr.actual || return 1 + test_grep ! "Hook $hook stderr" stdout.actual || return 1 + done +} + +check_stdout_merged_to_stderr () { + for hook in "$@" + do + # Ensure hook's stdout is only in stderr, not stdout + test_grep "Hook $hook stdout" stderr.actual || return 1 + test_grep ! "Hook $hook stdout" stdout.actual || return 1 + + # Ensure hook's stderr is only in stderr, not stdout + test_grep "Hook $hook stderr" stderr.actual || return 1 + test_grep ! "Hook $hook stderr" stdout.actual || return 1 + done +} + +setup_hooks () { + for hook in "$@" + do + test_hook $hook <<-EOF + echo >&1 Hook $hook stdout + echo >&2 Hook $hook stderr + EOF + done +} + +test_expect_success 'client hooks: pre-push expects separate stdout and stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote && + git remote add origin remote && + test_commit A && + setup_hooks pre-push && + git push origin HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_separate_from_stderr pre-push +' + +test_expect_success 'client hooks: commit hooks expect stdout redirected to stderr' ' + hooks="pre-commit prepare-commit-msg \ + commit-msg post-commit \ + reference-transaction" && + setup_hooks $hooks && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -B main && + git checkout -b branch-a && + test_commit commit-on-branch-a && + git commit --allow-empty -m "Test" >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr $hooks +' + +test_expect_success 'client hooks: checkout hooks expect stdout redirected to stderr' ' + setup_hooks post-checkout reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b new-branch main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-checkout reference-transaction +' + +test_expect_success 'client hooks: merge hooks expect stdout redirected to stderr' ' + setup_hooks pre-merge-commit post-merge reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + test_commit new-branch-commit && + git merge --no-ff branch-a >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-merge-commit post-merge reference-transaction +' + +test_expect_success 'client hooks: post-rewrite hooks expect stdout redirected to stderr' ' + setup_hooks post-rewrite reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + git commit --amend --allow-empty --no-edit >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-rewrite reference-transaction +' + +test_expect_success 'client hooks: applypatch hooks expect stdout redirected to stderr' ' + setup_hooks applypatch-msg pre-applypatch post-applypatch && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b branch-b main && + test_commit branch-b && + git format-patch -1 --stdout >patch && + git checkout -b branch-c main && + git am patch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr applypatch-msg pre-applypatch post-applypatch +' + +test_expect_success 'client hooks: rebase hooks expect stdout redirected to stderr' ' + setup_hooks pre-rebase && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b branch-d main && + test_commit branch-d && + git checkout main && + test_commit diverge-main && + git checkout branch-d && + git rebase main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-rebase +' + +test_expect_success 'client hooks: post-index-change expects stdout redirected to stderr' ' + setup_hooks post-index-change && + test_when_finished "rm -f stdout.actual stderr.actual" && + oid=$(git hash-object -w --stdin stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-index-change +' + +test_expect_success 'server hooks expect stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote-server && + git remote add origin-server remote-server && + cd remote-server && + setup_hooks pre-receive update post-receive post-update && + cd .. && + git push origin-server HEAD:new-branch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-receive update post-receive post-update +' + +test_expect_success 'server push-to-checkout hook expects stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init server && + git -C server checkout -b main && + test_config -C server receive.denyCurrentBranch updateInstead && + git remote add origin-server-2 server && + cd server && + setup_hooks push-to-checkout && + cd .. && + git push origin-server-2 HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr push-to-checkout +' + test_done From dee82860688cdcd35cbf0ce7e74736e473c5b697 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:17 +0200 Subject: [PATCH 02/40] run-command: add helper for pp child states There is a recurring pattern of testing parallel process child states and file descriptors to determine if a child is running, receiving any input or if it's ready for cleanup. Name the pp_child structure and introduce a helper to make the checks more readable. Suggested-by: Junio C Hamano Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/run-command.c b/run-command.c index e3e02475ccec50..39896735691c7b 100644 --- a/run-command.c +++ b/run-command.c @@ -1478,15 +1478,22 @@ enum child_state { GIT_CP_WAIT_CLEANUP, }; +struct parallel_child { + enum child_state state; + struct child_process process; + struct strbuf err; + void *data; +}; + +static int child_is_working(const struct parallel_child *pp_child) +{ + return pp_child->state == GIT_CP_WORKING; +} + struct parallel_processes { size_t nr_processes; - struct { - enum child_state state; - struct child_process process; - struct strbuf err; - void *data; - } *children; + struct parallel_child *children; /* * The struct pollfd is logically part of *children, * but the system call expects it as its own array. @@ -1509,7 +1516,7 @@ static void kill_children(const struct parallel_processes *pp, int signo) { for (size_t i = 0; i < opts->processes; i++) - if (pp->children[i].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[i])) kill(pp->children[i].process.pid, signo); } @@ -1665,7 +1672,7 @@ static void pp_buffer_stderr(struct parallel_processes *pp, /* Buffer output from all pipes. */ for (size_t i = 0; i < opts->processes; i++) { - if (pp->children[i].state == GIT_CP_WORKING && + if (child_is_working(&pp->children[i]) && pp->pfd[i].revents & (POLLIN | POLLHUP)) { int n = strbuf_read_once(&pp->children[i].err, pp->children[i].process.err, 0); @@ -1683,7 +1690,7 @@ static void pp_output(const struct parallel_processes *pp) { size_t i = pp->output_owner; - if (pp->children[i].state == GIT_CP_WORKING && + if (child_is_working(&pp->children[i]) && pp->children[i].err.len) { strbuf_write(&pp->children[i].err, stderr); strbuf_reset(&pp->children[i].err); @@ -1748,7 +1755,7 @@ static int pp_collect_finished(struct parallel_processes *pp, * running process time. */ for (i = 0; i < n; i++) - if (pp->children[(pp->output_owner + i) % n].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[(pp->output_owner + i) % n])) break; pp->output_owner = (pp->output_owner + i) % n; } From ec0becacc9847406f2b0147a81f62e023b006351 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:18 +0200 Subject: [PATCH 03/40] run-command: add stdin callback for parallelization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a user of the run_processes_parallel() API wants to pipe a large amount of information to the stdin of each parallel command, that data could exceed the pipe buffer of the process's stdin and can be too big to store in-memory via strbuf & friends or to slurp to a file. Generally this is solved by repeatedly writing to child_process.in between calls to start_command() and finish_command(). For a specific pre-existing example of this, see transport.c:run_pre_push_hook(). This adds a generic callback API to run_processes_parallel() to do exactly that in a unified manner, similar to the existing callback APIs, which can then be used by hooks.h to convert the remaining hooks to the new, simpler parallel interface. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 87 ++++++++++++++++++++++++++++++++++--- run-command.h | 21 +++++++++ t/helper/test-run-command.c | 52 +++++++++++++++++++++- t/t0061-run-command.sh | 31 +++++++++++++ 4 files changed, 182 insertions(+), 9 deletions(-) diff --git a/run-command.c b/run-command.c index 39896735691c7b..aaf0e4ecee1377 100644 --- a/run-command.c +++ b/run-command.c @@ -1490,6 +1490,16 @@ static int child_is_working(const struct parallel_child *pp_child) return pp_child->state == GIT_CP_WORKING; } +static int child_is_ready_for_cleanup(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && !pp_child->process.in; +} + +static int child_is_receiving_input(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && pp_child->process.in > 0; +} + struct parallel_processes { size_t nr_processes; @@ -1659,6 +1669,44 @@ static int pp_start_one(struct parallel_processes *pp, return 0; } +static void pp_buffer_stdin(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts) +{ + /* Buffer stdin for each pipe. */ + for (size_t i = 0; i < opts->processes; i++) { + struct child_process *proc = &pp->children[i].process; + int ret; + + if (!child_is_receiving_input(&pp->children[i])) + continue; + + /* + * child input is provided via path_to_stdin when the feed_pipe cb is + * missing, so we just signal an EOF. + */ + if (!opts->feed_pipe) { + close(proc->in); + proc->in = 0; + continue; + } + + /** + * Feed the pipe: + * ret < 0 means error + * ret == 0 means there is more data to be fed + * ret > 0 means feeding finished + */ + ret = opts->feed_pipe(proc->in, opts->data, pp->children[i].data); + if (ret < 0) + die_errno("feed_pipe"); + + if (ret) { + close(proc->in); + proc->in = 0; + } + } +} + static void pp_buffer_stderr(struct parallel_processes *pp, const struct run_process_parallel_opts *opts, int output_timeout) @@ -1729,6 +1777,7 @@ static int pp_collect_finished(struct parallel_processes *pp, pp->children[i].state = GIT_CP_FREE; if (pp->pfd) pp->pfd[i].fd = -1; + pp->children[i].process.in = 0; child_process_init(&pp->children[i].process); if (opts->ungroup) { @@ -1763,6 +1812,27 @@ static int pp_collect_finished(struct parallel_processes *pp, return result; } +static void pp_handle_child_IO(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts, + int output_timeout) +{ + /* + * First push input, if any (it might no-op), to child tasks to avoid them blocking + * after input. This also prevents deadlocks when ungrouping below, if a child blocks + * while the parent also waits for them to finish. + */ + pp_buffer_stdin(pp, opts); + + if (opts->ungroup) { + for (size_t i = 0; i < opts->processes; i++) + if (child_is_ready_for_cleanup(&pp->children[i])) + pp->children[i].state = GIT_CP_WAIT_CLEANUP; + } else { + pp_buffer_stderr(pp, opts, output_timeout); + pp_output(pp); + } +} + void run_processes_parallel(const struct run_process_parallel_opts *opts) { int i, code; @@ -1782,6 +1852,13 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) "max:%"PRIuMAX, (uintmax_t)opts->processes); + /* + * Child tasks might receive input via stdin, terminating early (or not), so + * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which + * actually writes the data to children stdin fds. + */ + sigchain_push(SIGPIPE, SIG_IGN); + pp_init(&pp, opts, &pp_sig); while (1) { for (i = 0; @@ -1799,13 +1876,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) } if (!pp.nr_processes) break; - if (opts->ungroup) { - for (size_t i = 0; i < opts->processes; i++) - pp.children[i].state = GIT_CP_WAIT_CLEANUP; - } else { - pp_buffer_stderr(&pp, opts, output_timeout); - pp_output(&pp); - } + pp_handle_child_IO(&pp, opts, output_timeout); code = pp_collect_finished(&pp, opts); if (code) { pp.shutdown = 1; @@ -1816,6 +1887,8 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) pp_cleanup(&pp, opts); + sigchain_pop(SIGPIPE); + if (do_trace2) trace2_region_leave(tr2_category, tr2_label, NULL); } diff --git a/run-command.h b/run-command.h index 0df25e445f001c..e1ca965b5b1988 100644 --- a/run-command.h +++ b/run-command.h @@ -420,6 +420,21 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is repeatedly called on every child process who requests + * start_command() to create a pipe by setting child_process.in < 0. + * + * pp_cb is the callback cookie as passed into run_processes_parallel, and + * pp_task_cb is the callback cookie as passed into get_next_task_fn. + * + * Returns < 0 for error + * Returns == 0 when there is more data to be fed (will be called again) + * Returns > 0 when finished (child closed fd or no more data to be fed) + */ +typedef int (*feed_pipe_fn)(int child_in, + void *pp_cb, + void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -473,6 +488,12 @@ struct run_process_parallel_opts */ start_failure_fn start_failure; + /* + * feed_pipe: see feed_pipe_fn() above. This can be NULL to omit any + * special handling. + */ + feed_pipe_fn feed_pipe; + /** * task_finished: See task_finished_fn() above. This can be * NULL to omit any special handling. diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 3719f23cc2d02f..4a56456894ccff 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -23,19 +23,26 @@ static int number_callbacks; static int parallel_next(struct child_process *cp, struct strbuf *err, void *cb, - void **task_cb UNUSED) + void **task_cb) { struct child_process *d = cb; if (number_callbacks >= 4) return 0; strvec_pushv(&cp->args, d->args.v); + cp->in = d->in; + cp->no_stdin = d->no_stdin; if (err) strbuf_addstr(err, "preloaded output of a child\n"); else fprintf(stderr, "preloaded output of a child\n"); number_callbacks++; + + /* test_stdin callback will use this to count remaining lines */ + *task_cb = xmalloc(sizeof(int)); + *(int*)(*task_cb) = 2; + return 1; } @@ -54,15 +61,48 @@ static int no_job(struct child_process *cp UNUSED, static int task_finished(int result UNUSED, struct strbuf *err, void *pp_cb UNUSED, - void *pp_task_cb UNUSED) + void *pp_task_cb) { if (err) strbuf_addstr(err, "asking for a quick stop\n"); else fprintf(stderr, "asking for a quick stop\n"); + + FREE_AND_NULL(pp_task_cb); + return 1; } +static int task_finished_quiet(int result UNUSED, + struct strbuf *err UNUSED, + void *pp_cb UNUSED, + void *pp_task_cb) +{ + FREE_AND_NULL(pp_task_cb); + return 0; +} + +static int test_stdin_pipe_feed(int hook_stdin_fd, void *cb UNUSED, void *task_cb) +{ + int *lines_remaining = task_cb; + + if (*lines_remaining) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "sample stdin %d\n", --(*lines_remaining)); + if (write_in_full(hook_stdin_fd, buf.buf, buf.len) < 0) { + if (errno == EPIPE) { + /* child closed stdin, nothing more to do */ + strbuf_release(&buf); + return 1; + } + die_errno("write"); + } + strbuf_release(&buf); + } + + return !(*lines_remaining); +} + struct testsuite { struct string_list tests, failed; int next; @@ -157,6 +197,7 @@ static int testsuite(int argc, const char **argv) struct run_process_parallel_opts opts = { .get_next_task = next_test, .start_failure = test_failed, + .feed_pipe = test_stdin_pipe_feed, .task_finished = test_finished, .data = &suite, }; @@ -460,12 +501,19 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command-parallel")) { opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; } else if (!strcmp(argv[1], "run-command-abort")) { opts.get_next_task = parallel_next; opts.task_finished = task_finished; } else if (!strcmp(argv[1], "run-command-no-jobs")) { opts.get_next_task = no_job; opts.task_finished = task_finished; + } else if (!strcmp(argv[1], "run-command-stdin")) { + proc.in = -1; + proc.no_stdin = 0; + opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; + opts.feed_pipe = test_stdin_pipe_feed; } else { ret = 1; fprintf(stderr, "check usage\n"); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 76d4936a879afd..2f77fde0d964c8 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -164,6 +164,37 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than test_line_count = 4 err ' +test_expect_success 'run_command listens to stdin' ' + cat >expect <<-\EOF && + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + EOF + + write_script stdin-script <<-\EOF && + echo "listening for stdin:" + while read line + do + echo "$line" + done + EOF + test-tool run-command run-command-stdin 2 ./stdin-script 2>actual && + test_cmp expect actual +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop From 33e5914863ab232164571c2e259cfa1e9c3f30dd Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:19 +0200 Subject: [PATCH 04/40] hook: provide stdin via callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a callback mechanism for feeding stdin to hooks alongside the existing path_to_stdin (which slurps a file's content to stdin). The advantage of this new callback is that it can feed stdin without going through the FS layer. This helps when feeding large amount of data and uses the run-command parallel stdin callback introduced in the preceding commit. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 23 ++++++++++++++++++++++- hook.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/hook.c b/hook.c index b3de1048bf44b9..5ddd7678d18f0d 100644 --- a/hook.c +++ b/hook.c @@ -55,7 +55,7 @@ int hook_exists(struct repository *r, const char *name) static int pick_next_hook(struct child_process *cp, struct strbuf *out UNUSED, void *pp_cb, - void **pp_task_cb UNUSED) + void **pp_task_cb) { struct hook_cb_data *hook_cb = pp_cb; const char *hook_path = hook_cb->hook_path; @@ -65,11 +65,22 @@ static int pick_next_hook(struct child_process *cp, cp->no_stdin = 1; strvec_pushv(&cp->env, hook_cb->options->env.v); + + if (hook_cb->options->path_to_stdin && hook_cb->options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + /* reopen the file for stdin; run_command closes it. */ if (hook_cb->options->path_to_stdin) { cp->no_stdin = 0; cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); } + + if (hook_cb->options->feed_pipe) { + cp->no_stdin = 0; + /* start_command() will allocate a pipe / stdin fd for us */ + cp->in = -1; + } + cp->stdout_to_stderr = 1; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; @@ -77,6 +88,12 @@ static int pick_next_hook(struct child_process *cp, strvec_push(&cp->args, hook_path); strvec_pushv(&cp->args, hook_cb->options->args.v); + /* + * Provide per-hook internal state via task_cb for easy access, so + * hook callbacks don't have to go through hook_cb->options. + */ + *pp_task_cb = hook_cb->options->feed_pipe_cb_data; + /* * This pick_next_hook() will be called again, we're only * running one hook, so indicate that no more work will be @@ -140,6 +157,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, + .feed_pipe = options->feed_pipe, .task_finished = notify_hook_finished, .data = &cb_data, @@ -148,6 +166,9 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); + if (options->path_to_stdin && options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + if (options->invoked_hook) *options->invoked_hook = 0; diff --git a/hook.h b/hook.h index 11863fa7347e6f..2169d4a6bd3f2e 100644 --- a/hook.h +++ b/hook.h @@ -1,6 +1,7 @@ #ifndef HOOK_H #define HOOK_H #include "strvec.h" +#include "run-command.h" struct repository; @@ -37,6 +38,43 @@ struct run_hooks_opt * Path to file which should be piped to stdin for each hook. */ const char *path_to_stdin; + + /** + * Callback used to incrementally feed a child hook stdin pipe. + * + * Useful especially if a hook consumes large quantities of data + * (e.g. a list of all refs in a client push), so feeding it via + * in-memory strings or slurping to/from files is inefficient. + * While the callback allows piecemeal writing, it can also be + * used for smaller inputs, where it gets called only once. + * + * Add hook callback initalization context to `feed_pipe_ctx`. + * Add hook callback internal state to `feed_pipe_cb_data`. + * + */ + feed_pipe_fn feed_pipe; + + /** + * Opaque data pointer used to pass context to `feed_pipe_fn`. + * + * It can be accessed via the second callback arg 'pp_cb': + * ((struct hook_cb_data *) pp_cb)->hook_cb->options->feed_pipe_ctx; + * + * The caller is responsible for managing the memory for this data. + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_ctx; + + /** + * Opaque data pointer used to keep internal state across callback calls. + * + * It can be accessed directly via the third callback arg 'pp_task_cb': + * struct ... *state = pp_task_cb; + * + * The caller is responsible for managing the memory for this data. + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_cb_data; }; #define RUN_HOOKS_OPT_INIT { \ From e8ee80cfb692139d0ca305995effe253b7a00463 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:20 +0200 Subject: [PATCH 05/40] hook: convert 'post-rewrite' hook in sequencer.c to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the custom run-command calls used by post-rewrite with the newer and simpler hook_run_opt(), which does not need to create a custom 'struct child_process' or call find_hook(). Another benefit of using the hook API is that hook_run_opt() handles the SIGPIPE toggle logic. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- sequencer.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/sequencer.c b/sequencer.c index 5476d39ba9b097..71ed31c7740688 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1292,32 +1292,40 @@ int update_head_with_reflog(const struct commit *old_head, return ret; } +static int pipe_from_strbuf(int hook_stdin_fd, void *pp_cb, void *pp_task_cb UNUSED) +{ + struct hook_cb_data *hook_cb = pp_cb; + struct strbuf *to_pipe = hook_cb->options->feed_pipe_ctx; + int ret; + + if (!to_pipe) + BUG("pipe_from_strbuf called without feed_pipe_ctx"); + + ret = write_in_full(hook_stdin_fd, to_pipe->buf, to_pipe->len); + if (ret < 0 && errno != EPIPE) + return ret; + + return 1; /* done writing */ +} + static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { - struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int code; struct strbuf sb = STRBUF_INIT; - const char *hook_path = find_hook(the_repository, "post-rewrite"); - if (!hook_path) - return 0; + strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); - strvec_pushl(&proc.args, hook_path, "amend", NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "post-rewrite"; + opt.feed_pipe_ctx = &sb; + opt.feed_pipe = pipe_from_strbuf; + + strvec_push(&opt.args, "amend"); + + code = run_hooks_opt(the_repository, "post-rewrite", &opt); - code = start_command(&proc); - if (code) - return code; - strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); - sigchain_push(SIGPIPE, SIG_IGN); - write_in_full(proc.in, sb.buf, sb.len); - close(proc.in); strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); + return code; } void commit_post_rewrite(struct repository *r, From d816637f6c253e3829c4c6e817c7749b3a4f8bf4 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:21 +0200 Subject: [PATCH 06/40] hook: allow separate std[out|err] streams The hook API assumes that all hooks merge stdout to stderr. This assumption is proven wrong by pre-push: some of its users actually expect separate stdout and stderr streams and merging them will cause a regression. Therefore this adds a mechanism to allow pre-push to separate the streams, which will be used in the next commit. The mechanism is generic via struct run_hooks_opt just in case there are any more surprise exceptions like this. Reported-by: Chris Darroch Suggested-by: brian m. carlson Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 2 +- hook.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hook.c b/hook.c index 5ddd7678d18f0d..fde1f88ce87b0a 100644 --- a/hook.c +++ b/hook.c @@ -81,7 +81,7 @@ static int pick_next_hook(struct child_process *cp, cp->in = -1; } - cp->stdout_to_stderr = 1; + cp->stdout_to_stderr = hook_cb->options->stdout_to_stderr; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; diff --git a/hook.h b/hook.h index 2169d4a6bd3f2e..2c8a23a569ae66 100644 --- a/hook.h +++ b/hook.h @@ -34,6 +34,15 @@ struct run_hooks_opt */ int *invoked_hook; + /** + * Send the hook's stdout to stderr. + * + * This is the default behavior for all hooks except pre-push, + * which has separate stdout and stderr streams for backwards + * compatibility reasons. + */ + unsigned int stdout_to_stderr:1; + /** * Path to file which should be piped to stdin for each hook. */ @@ -80,6 +89,7 @@ struct run_hooks_opt #define RUN_HOOKS_OPT_INIT { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ + .stdout_to_stderr = 1, \ } struct hook_cb_data { From 9ac9612c828fc8c28afc833ca0a3511f2f406628 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:22 +0200 Subject: [PATCH 07/40] transport: convert pre-push to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the pre-push hook from custom run-command invocations to the new hook API which doesn't require a custom child_process structure and signal toggling. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- transport.c | 95 ++++++++++++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/transport.c b/transport.c index c7f06a7382e605..e876cc9189d33f 100644 --- a/transport.c +++ b/transport.c @@ -1316,65 +1316,72 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing) die(_("Aborting.")); } -static int run_pre_push_hook(struct transport *transport, - struct ref *remote_refs) -{ - int ret = 0, x; - struct ref *r; - struct child_process proc = CHILD_PROCESS_INIT; +struct feed_pre_push_hook_data { struct strbuf buf; - const char *hook_path = find_hook(the_repository, "pre-push"); + const struct ref *refs; +}; - if (!hook_path) - return 0; +static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) +{ + struct feed_pre_push_hook_data *data = pp_task_cb; + const struct ref *r = data->refs; + int ret = 0; - strvec_push(&proc.args, hook_path); - strvec_push(&proc.args, transport->remote->name); - strvec_push(&proc.args, transport->url); + if (!r) + return 1; /* no more refs */ - proc.in = -1; - proc.trace2_hook_name = "pre-push"; + data->refs = r->next; - if (start_command(&proc)) { - finish_command(&proc); - return -1; + switch (r->status) { + case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_REMOTE_UPDATED: + case REF_STATUS_REJECT_STALE: + case REF_STATUS_UPTODATE: + return 0; /* skip refs which won't be pushed */ + default: + break; } - sigchain_push(SIGPIPE, SIG_IGN); + if (!r->peer_ref) + return 0; - strbuf_init(&buf, 256); + strbuf_reset(&data->buf); + strbuf_addf(&data->buf, "%s %s %s %s\n", + r->peer_ref->name, oid_to_hex(&r->new_oid), + r->name, oid_to_hex(&r->old_oid)); - for (r = remote_refs; r; r = r->next) { - if (!r->peer_ref) continue; - if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue; - if (r->status == REF_STATUS_REJECT_STALE) continue; - if (r->status == REF_STATUS_REJECT_REMOTE_UPDATED) continue; - if (r->status == REF_STATUS_UPTODATE) continue; + ret = write_in_full(hook_stdin_fd, data->buf.buf, data->buf.len); + if (ret < 0 && errno != EPIPE) + return ret; /* We do not mind if a hook does not read all refs. */ - strbuf_reset(&buf); - strbuf_addf( &buf, "%s %s %s %s\n", - r->peer_ref->name, oid_to_hex(&r->new_oid), - r->name, oid_to_hex(&r->old_oid)); + return 0; +} - if (write_in_full(proc.in, buf.buf, buf.len) < 0) { - /* We do not mind if a hook does not read all refs. */ - if (errno != EPIPE) - ret = -1; - break; - } - } +static int run_pre_push_hook(struct transport *transport, + struct ref *remote_refs) +{ + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct feed_pre_push_hook_data data; + int ret = 0; + + strvec_push(&opt.args, transport->remote->name); + strvec_push(&opt.args, transport->url); - strbuf_release(&buf); + strbuf_init(&data.buf, 0); + data.refs = remote_refs; - x = close(proc.in); - if (!ret) - ret = x; + opt.feed_pipe = pre_push_hook_feed_stdin; + opt.feed_pipe_cb_data = &data; + + /* + * pre-push hooks expect stdout & stderr to be separate, so don't merge + * them to keep backwards compatibility with existing hooks. + */ + opt.stdout_to_stderr = 0; - sigchain_pop(SIGPIPE); + ret = run_hooks_opt(the_repository, "pre-push", &opt); - x = finish_command(&proc); - if (!ret) - ret = x; + strbuf_release(&data.buf); return ret; } From b7b2157d0d23f6ed98da6fce548d614c5198713d Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:23 +0200 Subject: [PATCH 08/40] reference-transaction: use hook API instead of run-command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the reference-transaction hook to the new hook API, so it doesn't need to set up a struct child_process, call find_hook or toggle the pipe signals. The stdin feed callback is processing one ref update per call. I haven't noticed any performance degradation due to this, however we can batch as many we want in each call, to ensure a good pipe throughtput (i.e. the child does not wait after stdin). Helped-by: Emily Shaffer Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- refs.c | 102 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 52 insertions(+), 50 deletions(-) diff --git a/refs.c b/refs.c index 046b695bb20f54..965b232a06e9b9 100644 --- a/refs.c +++ b/refs.c @@ -15,7 +15,6 @@ #include "iterator.h" #include "refs.h" #include "refs/refs-internal.h" -#include "run-command.h" #include "hook.h" #include "object-name.h" #include "odb.h" @@ -26,7 +25,6 @@ #include "strvec.h" #include "repo-settings.h" #include "setup.h" -#include "sigchain.h" #include "date.h" #include "commit.h" #include "wildmatch.h" @@ -2422,68 +2420,72 @@ static int ref_update_reject_duplicates(struct string_list *refnames, return 0; } -static int run_transaction_hook(struct ref_transaction *transaction, - const char *state) +struct transaction_feed_cb_data { + size_t index; + struct strbuf buf; +}; + +static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_task_cb) { - struct child_process proc = CHILD_PROCESS_INIT; - struct strbuf buf = STRBUF_INIT; - const char *hook; - int ret = 0; + struct hook_cb_data *hook_cb = pp_cb; + struct ref_transaction *transaction = hook_cb->options->feed_pipe_ctx; + struct transaction_feed_cb_data *feed_cb_data = pp_task_cb; + struct strbuf *buf = &feed_cb_data->buf; + struct ref_update *update; + size_t i = feed_cb_data->index++; + int ret; - hook = find_hook(transaction->ref_store->repo, "reference-transaction"); - if (!hook) - return ret; + if (i >= transaction->nr) + return 1; /* No more refs to process */ - strvec_pushl(&proc.args, hook, state, NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "reference-transaction"; + update = transaction->updates[i]; - ret = start_command(&proc); - if (ret) - return ret; + if (update->flags & REF_LOG_ONLY) + return 0; - sigchain_push(SIGPIPE, SIG_IGN); + strbuf_reset(buf); - for (size_t i = 0; i < transaction->nr; i++) { - struct ref_update *update = transaction->updates[i]; + if (!(update->flags & REF_HAVE_OLD)) + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + else if (update->old_target) + strbuf_addf(buf, "ref:%s ", update->old_target); + else + strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid)); - if (update->flags & REF_LOG_ONLY) - continue; + if (!(update->flags & REF_HAVE_NEW)) + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + else if (update->new_target) + strbuf_addf(buf, "ref:%s ", update->new_target); + else + strbuf_addf(buf, "%s ", oid_to_hex(&update->new_oid)); - strbuf_reset(&buf); + strbuf_addf(buf, "%s\n", update->refname); - if (!(update->flags & REF_HAVE_OLD)) - strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); - else if (update->old_target) - strbuf_addf(&buf, "ref:%s ", update->old_target); - else - strbuf_addf(&buf, "%s ", oid_to_hex(&update->old_oid)); + ret = write_in_full(hook_stdin_fd, buf->buf, buf->len); + if (ret < 0 && errno != EPIPE) + return ret; - if (!(update->flags & REF_HAVE_NEW)) - strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); - else if (update->new_target) - strbuf_addf(&buf, "ref:%s ", update->new_target); - else - strbuf_addf(&buf, "%s ", oid_to_hex(&update->new_oid)); + return 0; /* no more input to feed */ +} + +static int run_transaction_hook(struct ref_transaction *transaction, + const char *state) +{ + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct transaction_feed_cb_data feed_ctx = { 0 }; + int ret = 0; - strbuf_addf(&buf, "%s\n", update->refname); + strvec_push(&opt.args, state); - if (write_in_full(proc.in, buf.buf, buf.len) < 0) { - if (errno != EPIPE) { - /* Don't leak errno outside this API */ - errno = 0; - ret = -1; - } - break; - } - } + opt.feed_pipe = transaction_hook_feed_stdin; + opt.feed_pipe_ctx = transaction; + opt.feed_pipe_cb_data = &feed_ctx; - close(proc.in); - sigchain_pop(SIGPIPE); - strbuf_release(&buf); + strbuf_init(&feed_ctx.buf, 0); + + ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt); - ret |= finish_command(&proc); + strbuf_release(&feed_ctx.buf); return ret; } From c549a40547fb9765a9ea78dbe4a3397dbee715b2 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:24 +0200 Subject: [PATCH 09/40] hook: add jobs option Allow the API callers to specify the number of jobs across which hook execution can be parallelized. It defaults to 1 and no hook currently changes it, so all hooks run sequentially as before. This allows us to both pave the way for parallel hook execution (that will be a follow-up patch series building upon this) and to finish the API conversion of builtin/receive-pack.c, keeping the output async sideband thread ("muxer") design as Peff suggested. When .jobs==1 nothing changes, the "copy_to_sideband" async thread still outputs directly via sideband channel 2, keeping the current (mostly) real-time output characteristics, avoids unnecessary poll delays or deadlock risks. When .jobs > 1, a more complex muxer is needed to buffer the hook output and avoid interleaving. After working on this mux I quickly realized I was re-implementing run-command with ungroup=0 so that idea was dropped in favor of run-command which outputs to stderr. In other words, run-command itself already can buffer/deinterleave pp child outputs (ungroup=0), so we can just connect its stderr to the sideband async task when jobs > 1. Maybe it helps to illustrate how it works with ascii graphics: [ Sequential (jobs = 1) ] [ Parallel (jobs > 1) ] +--------------+ +--------+ +--------+ | Hook Process | | Hook 1 | | Hook 2 | +--------------+ +--------+ +--------+ | | | | stderr (inherited) | stderr pipe | | | (captured) | v v v +-------------------------------------------------------------+ | Parent Process | | | | (direct write) [run-command (buffered)] | | | | | | | | writes | | v v | | +-------------------------------------------+ | | | stderr (FD 2) | | | +-------------------------------------------+ | | | | | | (dup2'd to pipe) | | v | | +-----------------------+ | | | sideband async thread | | | +-----------------------+ | +-------------------------------------------------------------+ When use_sideband == 0, the sideband async thread is missing, so this same architecture just outputs via the parent stderr stream. See the following commits for the hook API conversions doing this, using pre-existing sideband thread logic from `copy_to_sideband`. Suggested-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 7 +++++-- hook.h | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hook.c b/hook.c index fde1f88ce87b0a..cde719841250d1 100644 --- a/hook.c +++ b/hook.c @@ -152,8 +152,8 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .tr2_category = "hook", .tr2_label = hook_name, - .processes = 1, - .ungroup = 1, + .processes = options->jobs, + .ungroup = options->jobs == 1, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, @@ -169,6 +169,9 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (options->path_to_stdin && options->feed_pipe) BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + if (!options->jobs) + BUG("run_hooks_opt must be called with options.jobs >= 1"); + if (options->invoked_hook) *options->invoked_hook = 0; diff --git a/hook.h b/hook.h index 2c8a23a569ae66..20eb56fd634c8d 100644 --- a/hook.h +++ b/hook.h @@ -16,6 +16,14 @@ struct run_hooks_opt /* Emit an error if the hook is missing */ unsigned int error_if_missing:1; + /** + * Number of processes to parallelize across. + * + * If > 1, output will be buffered and de-interleaved (ungroup=0). + * If == 1, output will be real-time (ungroup=1). + */ + unsigned int jobs; + /** * An optional initial working directory for the hook, * translates to "struct child_process"'s "dir" member. @@ -90,6 +98,7 @@ struct run_hooks_opt .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ .stdout_to_stderr = 1, \ + .jobs = 1, \ } struct hook_cb_data { From c45a34e12e8699f656ec3613b6ba158c1a57c5e8 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 28 Jan 2026 23:39:25 +0200 Subject: [PATCH 10/40] run-command: poll child input in addition to output Child input feeding might hit the 100ms output poll timeout as a side-effect of the ungroup=0 design when feeding multiple children in parallel and buffering their outputs. This throttles the write throughput as reported by Kristoffer. Peff also noted that the parent might block if the write pipe is full and cause a deadlock if both parent + child wait for one another. Thus we refactor the run-command I/O loop so it polls on both child input and output fds to eliminate the risk of artificial 100ms latencies and unnecessarily blocking the main process. This ensures that parallel hooks are fed data ASAP while maintaining responsiveness for (sideband) output. It's worth noting that in our current design, sequential execution is not affected by this because it still uses the ungroup=1 behavior, so there are no run-command induced buffering delays since the child sequentially outputs directly to the parent-inherited fds. Reported-by: Kristoffer Haugsbakk Suggested-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 80 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/run-command.c b/run-command.c index aaf0e4ecee1377..3356463d4346c8 100644 --- a/run-command.c +++ b/run-command.c @@ -1499,6 +1499,14 @@ static int child_is_receiving_input(const struct parallel_child *pp_child) { return child_is_working(pp_child) && pp_child->process.in > 0; } +static int child_is_sending_output(const struct parallel_child *pp_child) +{ + /* + * all pp children which buffer output through run_command via ungroup=0 + * redirect stdout to stderr, so we just need to check process.err. + */ + return child_is_working(pp_child) && pp_child->process.err > 0; +} struct parallel_processes { size_t nr_processes; @@ -1562,7 +1570,7 @@ static void pp_init(struct parallel_processes *pp, CALLOC_ARRAY(pp->children, n); if (!opts->ungroup) - CALLOC_ARRAY(pp->pfd, n); + CALLOC_ARRAY(pp->pfd, n * 2); for (size_t i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); @@ -1707,21 +1715,63 @@ static void pp_buffer_stdin(struct parallel_processes *pp, } } -static void pp_buffer_stderr(struct parallel_processes *pp, - const struct run_process_parallel_opts *opts, - int output_timeout) +static void pp_buffer_io(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts, + int timeout) { - while (poll(pp->pfd, opts->processes, output_timeout) < 0) { + /* for each potential child slot, prepare two pollfd entries */ + for (size_t i = 0; i < opts->processes; i++) { + if (child_is_sending_output(&pp->children[i])) { + pp->pfd[2*i].fd = pp->children[i].process.err; + pp->pfd[2*i].events = POLLIN | POLLHUP; + } else { + pp->pfd[2*i].fd = -1; + } + + if (child_is_receiving_input(&pp->children[i])) { + pp->pfd[2*i+1].fd = pp->children[i].process.in; + pp->pfd[2*i+1].events = POLLOUT; + } else { + pp->pfd[2*i+1].fd = -1; + } + } + + while (poll(pp->pfd, opts->processes * 2, timeout) < 0) { if (errno == EINTR) continue; pp_cleanup(pp, opts); die_errno("poll"); } - /* Buffer output from all pipes. */ for (size_t i = 0; i < opts->processes; i++) { + /* Handle input feeding (stdin) */ + if (pp->pfd[2*i+1].revents & (POLLOUT | POLLHUP | POLLERR)) { + if (opts->feed_pipe) { + int ret = opts->feed_pipe(pp->children[i].process.in, + opts->data, + pp->children[i].data); + if (ret < 0) + die_errno("feed_pipe"); + if (ret) { + /* done feeding */ + close(pp->children[i].process.in); + pp->children[i].process.in = 0; + } + } else { + /* + * No feed_pipe means there is nothing to do, so + * close the fd. Child input can be fed by other + * methods, such as opts->path_to_stdin which + * slurps a file via dup2, so clean up here. + */ + close(pp->children[i].process.in); + pp->children[i].process.in = 0; + } + } + + /* Handle output reading (stderr) */ if (child_is_working(&pp->children[i]) && - pp->pfd[i].revents & (POLLIN | POLLHUP)) { + pp->pfd[2*i].revents & (POLLIN | POLLHUP)) { int n = strbuf_read_once(&pp->children[i].err, pp->children[i].process.err, 0); if (n == 0) { @@ -1814,21 +1864,15 @@ static int pp_collect_finished(struct parallel_processes *pp, static void pp_handle_child_IO(struct parallel_processes *pp, const struct run_process_parallel_opts *opts, - int output_timeout) + int timeout) { - /* - * First push input, if any (it might no-op), to child tasks to avoid them blocking - * after input. This also prevents deadlocks when ungrouping below, if a child blocks - * while the parent also waits for them to finish. - */ - pp_buffer_stdin(pp, opts); - if (opts->ungroup) { + pp_buffer_stdin(pp, opts); for (size_t i = 0; i < opts->processes; i++) if (child_is_ready_for_cleanup(&pp->children[i])) pp->children[i].state = GIT_CP_WAIT_CLEANUP; } else { - pp_buffer_stderr(pp, opts, output_timeout); + pp_buffer_io(pp, opts, timeout); pp_output(pp); } } @@ -1836,7 +1880,7 @@ static void pp_handle_child_IO(struct parallel_processes *pp, void run_processes_parallel(const struct run_process_parallel_opts *opts) { int i, code; - int output_timeout = 100; + int timeout = 100; int spawn_cap = 4; struct parallel_processes_for_signal pp_sig; struct parallel_processes pp = { @@ -1876,7 +1920,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) } if (!pp.nr_processes) break; - pp_handle_child_IO(&pp, opts, output_timeout); + pp_handle_child_IO(&pp, opts, timeout); code = pp_collect_finished(&pp, opts); if (code) { pp.shutdown = 1; From fc148b146ad41be71a7852c4867f0773cbfe1ff9 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:26 +0200 Subject: [PATCH 11/40] receive-pack: convert update hooks to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook API avoids creating a custom struct child_process and other internal hook plumbing (e.g. calling find_hook()) and prepares for the specification of hooks via configs or running parallel hooks. Execution is still sequential through the run_hooks_opt .jobs == 1, which is the unchanged default for all hooks. When use_sideband==1, the async thread redirects the hook outputs to sideband 2, otherwise it is not used and the hooks write directly to the fds inherited from the main parent process. When .jobs == 1, run-command's poll loop is avoided entirely via the ungroup=1 option like before (this was Jeff's suggestion), achieving the same real-time output performance. When running in parallel, run-command with ungroup=0 will capture and de-interleave the output of each hook, then write to the parent stderr which is redirected via dup2 to the sideband thread, so that each parallel hook output is presented clearly to the client. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Helped-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 103 ++++++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 37 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c491746168a6f..f554dac1ef9b03 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -561,6 +561,48 @@ static int copy_to_sideband(int in, int out UNUSED, void *arg UNUSED) return 0; } +/* + * Start an async thread which redirects hook stderr over the sideband. + * The original stderr fd is saved to `saved_stderr` and STDERR_FILENO is + * redirected to the async's input pipe. + */ +static void prepare_sideband_async(struct async *sideband_async, int *saved_stderr, int *started) +{ + *started = 0; + + if (!use_sideband) + return; + + memset(sideband_async, 0, sizeof(*sideband_async)); + sideband_async->proc = copy_to_sideband; + sideband_async->in = -1; + + if (!start_async(sideband_async)) { + *started = 1; + *saved_stderr = dup(STDERR_FILENO); + if (*saved_stderr >= 0) + dup2(sideband_async->in, STDERR_FILENO); + close(sideband_async->in); + } +} + +/* + * Restore the original stderr and wait for the async sideband thread to finish. + */ +static void finish_sideband_async(struct async *sideband_async, int saved_stderr, int started) +{ + if (!use_sideband) + return; + + if (saved_stderr >= 0) { + dup2(saved_stderr, STDERR_FILENO); + close(saved_stderr); + } + + if (started) + finish_async(sideband_async); +} + static void hmac_hash(unsigned char *out, const char *key_in, size_t key_len, const char *text, size_t text_len) @@ -941,29 +983,25 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct async sideband_async; + int sideband_async_started = 0; + int saved_stderr = -1; int code; - const char *hook_path = find_hook(the_repository, "update"); - if (!hook_path) - return 0; + strvec_pushl(&opt.args, + cmd->ref_name, + oid_to_hex(&cmd->old_oid), + oid_to_hex(&cmd->new_oid), + NULL); - strvec_push(&proc.args, hook_path); - strvec_push(&proc.args, cmd->ref_name); - strvec_push(&proc.args, oid_to_hex(&cmd->old_oid)); - strvec_push(&proc.args, oid_to_hex(&cmd->new_oid)); + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - proc.err = use_sideband ? -1 : 0; - proc.trace2_hook_name = "update"; + code = run_hooks_opt(the_repository, "update", &opt); - code = start_command(&proc); - if (code) - return code; - if (use_sideband) - copy_to_sideband(proc.err, -1, NULL); - return finish_command(&proc); + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); + + return code; } static struct command *find_command_by_refname(struct command *list, @@ -1639,34 +1677,25 @@ static const char *update(struct command *cmd, struct shallow_info *si) static void run_update_post_hook(struct command *commands) { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct async sideband_async; struct command *cmd; - struct child_process proc = CHILD_PROCESS_INIT; - const char *hook; - - hook = find_hook(the_repository, "post-update"); - if (!hook) - return; + int sideband_async_started = 0; + int saved_stderr = -1; for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) continue; - if (!proc.args.nr) - strvec_push(&proc.args, hook); - strvec_push(&proc.args, cmd->ref_name); + strvec_push(&opt.args, cmd->ref_name); } - if (!proc.args.nr) + if (!opt.args.nr) return; - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - proc.err = use_sideband ? -1 : 0; - proc.trace2_hook_name = "post-update"; + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - if (!start_command(&proc)) { - if (use_sideband) - copy_to_sideband(proc.err, -1, NULL); - finish_command(&proc); - } + run_hooks_opt(the_repository, "post-update", &opt); + + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); } static void check_aliased_update_internal(struct command *cmd, From b5e9ad508c2f340bd2d2547d7370ae7ac6a0d65d Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:27 +0200 Subject: [PATCH 12/40] receive-pack: convert receive hooks to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This converts the last remaining hooks to the new hook API, for the same benefits as the previous conversions (no need to toggle signals, manage custom struct child_process, call find_hook(), prepares for specifying hooks via configs, etc.). See the previous three commits for a more in-depth explanation of how this all works. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Helped-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 178 +++++++++++++++++------------------------ 1 file changed, 75 insertions(+), 103 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index f554dac1ef9b03..b5379a48956994 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -791,7 +791,7 @@ static int check_cert_push_options(const struct string_list *push_options) return retval; } -static void prepare_push_cert_sha1(struct child_process *proc) +static void prepare_push_cert_sha1(struct run_hooks_opt *opt) { static int already_done; @@ -817,23 +817,23 @@ static void prepare_push_cert_sha1(struct child_process *proc) nonce_status = check_nonce(sigcheck.payload); } if (!is_null_oid(&push_cert_oid)) { - strvec_pushf(&proc->env, "GIT_PUSH_CERT=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT=%s", oid_to_hex(&push_cert_oid)); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_SIGNER=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_SIGNER=%s", sigcheck.signer ? sigcheck.signer : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_KEY=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_KEY=%s", sigcheck.key ? sigcheck.key : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_STATUS=%c", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_STATUS=%c", sigcheck.result); if (push_cert_nonce) { - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE=%s", push_cert_nonce); - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_STATUS=%s", nonce_status); if (nonce_status == NONCE_SLOP) - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_SLOP=%ld", nonce_stamp_slop); } @@ -845,94 +845,25 @@ struct receive_hook_feed_state { struct ref_push_report *report; int skip_broken; struct strbuf buf; - const struct string_list *push_options; }; -typedef int (*feed_fn)(void *, const char **, size_t *); -static int run_and_feed_hook(const char *hook_name, feed_fn feed, - struct receive_hook_feed_state *feed_state) +static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) { - struct child_process proc = CHILD_PROCESS_INIT; - struct async muxer; - int code; - const char *hook_path = find_hook(the_repository, hook_name); - - if (!hook_path) - return 0; - - strvec_push(&proc.args, hook_path); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = hook_name; - - if (feed_state->push_options) { - size_t i; - for (i = 0; i < feed_state->push_options->nr; i++) - strvec_pushf(&proc.env, - "GIT_PUSH_OPTION_%"PRIuMAX"=%s", - (uintmax_t)i, - feed_state->push_options->items[i].string); - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", - (uintmax_t)feed_state->push_options->nr); - } else - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT"); - - if (tmp_objdir) - strvec_pushv(&proc.env, tmp_objdir_env(tmp_objdir)); - - if (use_sideband) { - memset(&muxer, 0, sizeof(muxer)); - muxer.proc = copy_to_sideband; - muxer.in = -1; - code = start_async(&muxer); - if (code) - return code; - proc.err = muxer.in; - } - - prepare_push_cert_sha1(&proc); - - code = start_command(&proc); - if (code) { - if (use_sideband) - finish_async(&muxer); - return code; - } - - sigchain_push(SIGPIPE, SIG_IGN); - - while (1) { - const char *buf; - size_t n; - if (feed(feed_state, &buf, &n)) - break; - if (write_in_full(proc.in, buf, n) < 0) - break; - } - close(proc.in); - if (use_sideband) - finish_async(&muxer); - - sigchain_pop(SIGPIPE); - - return finish_command(&proc); -} - -static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) -{ - struct receive_hook_feed_state *state = state_; + struct receive_hook_feed_state *state = pp_task_cb; struct command *cmd = state->cmd; + strbuf_reset(&state->buf); + while (cmd && state->skip_broken && (cmd->error_string || cmd->did_not_exist)) cmd = cmd->next; + if (!cmd) - return -1; /* EOF */ - if (!bufp) - return 0; /* OK, can feed something. */ - strbuf_reset(&state->buf); + return 1; /* no more commands left */ + if (!state->report) state->report = cmd->report; + if (state->report) { struct object_id *old_oid; struct object_id *new_oid; @@ -941,23 +872,33 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid; new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid; ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name; + strbuf_addf(&state->buf, "%s %s %s\n", oid_to_hex(old_oid), oid_to_hex(new_oid), ref_name); + state->report = state->report->next; if (!state->report) - state->cmd = cmd->next; + cmd = cmd->next; } else { strbuf_addf(&state->buf, "%s %s %s\n", oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid), cmd->ref_name); - state->cmd = cmd->next; + cmd = cmd->next; } - if (bufp) { - *bufp = state->buf.buf; - *sizep = state->buf.len; + + state->cmd = cmd; + + if (state->buf.len > 0) { + int ret = write_in_full(hook_stdin_fd, state->buf.buf, state->buf.len); + if (ret < 0) { + if (errno == EPIPE) + return 1; /* child closed pipe */ + return ret; + } } - return 0; + + return state->cmd ? 0 : 1; /* 0 = more to come, 1 = EOF */ } static int run_receive_hook(struct command *commands, @@ -965,20 +906,51 @@ static int run_receive_hook(struct command *commands, int skip_broken, const struct string_list *push_options) { - struct receive_hook_feed_state state; - int status; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct command *iter = commands; + struct receive_hook_feed_state feed_state; + struct async sideband_async; + int sideband_async_started = 0; + int saved_stderr = -1; + int ret; - strbuf_init(&state.buf, 0); - state.cmd = commands; - state.skip_broken = skip_broken; - state.report = NULL; - if (feed_receive_hook(&state, NULL, NULL)) + /* if there are no valid commands, don't invoke the hook at all. */ + while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) + iter = iter->next; + if (!iter) return 0; - state.cmd = commands; - state.push_options = push_options; - status = run_and_feed_hook(hook_name, feed_receive_hook, &state); - strbuf_release(&state.buf); - return status; + + if (push_options) { + for (int i = 0; i < push_options->nr; i++) + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_%d=%s", i, + push_options->items[i].string); + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", + (uintmax_t)push_options->nr); + } else { + strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT"); + } + + if (tmp_objdir) + strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir)); + + prepare_push_cert_sha1(&opt); + + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); + + /* set up stdin callback */ + feed_state.cmd = commands; + feed_state.skip_broken = skip_broken; + feed_state.report = NULL; + strbuf_init(&feed_state.buf, 0); + opt.feed_pipe_cb_data = &feed_state; + opt.feed_pipe = feed_receive_hook_cb; + + ret = run_hooks_opt(the_repository, hook_name, &opt); + + strbuf_release(&feed_state.buf); + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); + + return ret; } static int run_update_hook(struct command *cmd) From 37196d8995ee60e08230c5d05dfd26204d604580 Mon Sep 17 00:00:00 2001 From: "Claus Schneider(Eficode)" Date: Fri, 6 Feb 2026 13:22:56 +0000 Subject: [PATCH 13/40] read-cache: update add_files_to_cache take param ignored_too The ignored_too parameter is added to the function add_files_to_cache for usage of explicit updating the index for the updated submodule using the explicit patchspec to the submodule. Signed-off-by: Claus Schneider(Eficode) Signed-off-by: Junio C Hamano --- builtin/add.c | 2 +- builtin/checkout.c | 2 +- builtin/commit.c | 2 +- read-cache-ll.h | 2 +- read-cache.c | 10 ++++++++-- 5 files changed, 12 insertions(+), 6 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 32709794b3873f..eef4959ee31c3d 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -584,7 +584,7 @@ int cmd_add(int argc, else exit_status |= add_files_to_cache(repo, prefix, &pathspec, ps_matched, - include_sparse, flags); + include_sparse, flags, ignored_too); if (take_worktree_changes && !add_renormalize && !ignore_add_errors && report_path_error(ps_matched, &pathspec)) diff --git a/builtin/checkout.c b/builtin/checkout.c index 261699e2f5fc97..9b664c4bbcb0a3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -899,7 +899,7 @@ static int merge_working_tree(const struct checkout_opts *opts, */ add_files_to_cache(the_repository, NULL, NULL, NULL, 0, - 0); + 0, 0); init_ui_merge_options(&o, the_repository); o.verbosity = 0; work = write_in_core_index_as_tree(the_repository); diff --git a/builtin/commit.c b/builtin/commit.c index 0243f17d53c97c..1a0064209062e3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -455,7 +455,7 @@ static const char *prepare_index(const char **argv, const char *prefix, repo_hold_locked_index(the_repository, &index_lock, LOCK_DIE_ON_ERROR); add_files_to_cache(the_repository, also ? prefix : NULL, - &pathspec, ps_matched, 0, 0); + &pathspec, ps_matched, 0, 0, 0 ); if (!all && report_path_error(ps_matched, &pathspec)) exit(128); diff --git a/read-cache-ll.h b/read-cache-ll.h index 71b49d9af48a9d..2c8b4b21b1c7e9 100644 --- a/read-cache-ll.h +++ b/read-cache-ll.h @@ -481,7 +481,7 @@ int cmp_cache_name_compare(const void *a_, const void *b_); int add_files_to_cache(struct repository *repo, const char *prefix, const struct pathspec *pathspec, char *ps_matched, - int include_sparse, int flags); + int include_sparse, int flags, int ignored_too ); void overlay_tree_on_index(struct index_state *istate, const char *tree_name, const char *prefix); diff --git a/read-cache.c b/read-cache.c index 990d4ead0d8ae4..5349b94e591769 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3878,9 +3878,12 @@ void overlay_tree_on_index(struct index_state *istate, struct update_callback_data { struct index_state *index; + struct repository *repo; + struct pathspec *pathspec; int include_sparse; int flags; int add_errors; + int ignored_too; }; static int fix_unmerged_status(struct diff_filepair *p, @@ -3922,7 +3925,7 @@ static void update_callback(struct diff_queue_struct *q, default: die(_("unexpected diff status %c"), p->status); case DIFF_STATUS_MODIFIED: - case DIFF_STATUS_TYPE_CHANGED: + case DIFF_STATUS_TYPE_CHANGED: { if (add_file_to_index(data->index, path, data->flags)) { if (!(data->flags & ADD_CACHE_IGNORE_ERRORS)) die(_("updating files failed")); @@ -3943,7 +3946,7 @@ static void update_callback(struct diff_queue_struct *q, int add_files_to_cache(struct repository *repo, const char *prefix, const struct pathspec *pathspec, char *ps_matched, - int include_sparse, int flags) + int include_sparse, int flags, int ignored_too ) { struct odb_transaction *transaction; struct update_callback_data data; @@ -3953,6 +3956,9 @@ int add_files_to_cache(struct repository *repo, const char *prefix, data.index = repo->index; data.include_sparse = include_sparse; data.flags = flags; + data.repo = repo; + data.ignored_too = ignored_too; + data.pathspec = (struct pathspec *)pathspec; repo_init_revisions(repo, &rev, prefix); setup_revisions(0, NULL, &rev, NULL); From a16c4a245acb2420bafcbd572ba9fb94b1ba5146 Mon Sep 17 00:00:00 2001 From: "Claus Schneider(Eficode)" Date: Fri, 6 Feb 2026 13:22:57 +0000 Subject: [PATCH 14/40] read-cache: submodule add need --force given ignore=all configuration Submodules configured with ignore=all are now skipped during add operations unless overridden by --force and the submodule path is explicitly specified. A message is printed (like ignored files) guiding the user to use the --force flag if the user explicitly wants to update the submodule reference. The reason for the change is to support branch tracking in submodules with configuration `submdule..branch` or similar workflows where the user is not interested in tracking each update of the sha1 in the submdule. You can additionally set `submodule..ignore=all` and the `git status` will state nothing and, with this patch, the `git add` does not either - as the default behaviour. This patch changes the workflow to a more logical behaviour and similar to workflow for ignored files. The patch gives more scenarios for submodules to be used effectively with less friction similar to the "repo" tool. A submodule can be added for many different reasons than a hard dependency. It can be added as loosely coupled dependencies whereas the user wants the latest based on the configuration `submoule..branch`, but are not interested to track each commit in the `super-repo`. Currently it gives friction of handling conflicts between branches even the sha1's are fast-forward and the user just wants the latest in any way. The user can still add a sha1 explicitly to track updates. Signed-off-by: Claus Schneider(Eficode) Signed-off-by: Junio C Hamano --- read-cache.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index 5349b94e591769..b5ed66f6b79e9e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -47,6 +47,9 @@ #include "csum-file.h" #include "promisor-remote.h" #include "hook.h" +#include "submodule.h" +#include "submodule-config.h" +#include "advice.h" /* Mask for the name length in ce_flags in the on-disk index */ @@ -3907,8 +3910,68 @@ static int fix_unmerged_status(struct diff_filepair *p, return DIFF_STATUS_MODIFIED; } +static int skip_submodule(const char *path, + struct repository *repo, + struct pathspec *pathspec, + int ignored_too) +{ + struct stat st; + const struct submodule *sub; + int pathspec_matches = 0; + int ps_i; + char *norm_pathspec = NULL; + + /* Only consider if path is a directory */ + if (lstat(path, &st) || !S_ISDIR(st.st_mode)) + return 0; + + /* Check if it's a submodule with ignore=all */ + sub = submodule_from_path(repo, null_oid(the_hash_algo), path); + if (!sub || !sub->name || !sub->ignore || strcmp(sub->ignore, "all")) + return 0; + + trace_printf("ignore=all: %s\n", path); + trace_printf("pathspec %s\n", (pathspec && pathspec->nr) + ? "has pathspec" + : "no pathspec"); + + /* Check if submodule path is explicitly mentioned in pathspec */ + if (pathspec) { + for (ps_i = 0; ps_i < pathspec->nr; ps_i++) { + const char *m = pathspec->items[ps_i].match; + if (!m) + continue; + norm_pathspec = xstrdup(m); + strip_dir_trailing_slashes(norm_pathspec); + if (!strcmp(path, norm_pathspec)) { + pathspec_matches = 1; + FREE_AND_NULL(norm_pathspec); + break; + } + FREE_AND_NULL(norm_pathspec); + } + } + + /* If explicitly matched and forced, allow adding */ + if (pathspec_matches) { + if (ignored_too && ignored_too > 0) { + trace_printf("Add submodule due to --force: %s\n", path); + return 0; + } else { + advise_if_enabled(ADVICE_ADD_IGNORED_FILE, + _("Skipping submodule due to ignore=all: %s\n" + "Use --force if you really want to add the submodule."), path); + return 1; + } + } + + /* No explicit pathspec match -> skip silently */ + trace_printf("Pathspec to submodule does not match explicitly: %s\n", path); + return 1; +} + static void update_callback(struct diff_queue_struct *q, - struct diff_options *opt UNUSED, void *cbdata) + struct diff_options *opt UNUSED, void *cbdata) { int i; struct update_callback_data *data = cbdata; @@ -3918,14 +3981,19 @@ static void update_callback(struct diff_queue_struct *q, const char *path = p->one->path; if (!data->include_sparse && - !path_in_sparse_checkout(path, data->index)) + !path_in_sparse_checkout(path, data->index)) continue; switch (fix_unmerged_status(p, data)) { default: die(_("unexpected diff status %c"), p->status); case DIFF_STATUS_MODIFIED: - case DIFF_STATUS_TYPE_CHANGED: { + case DIFF_STATUS_TYPE_CHANGED: + if (skip_submodule(path, data->repo, + data->pathspec, + data->ignored_too)) + continue; + if (add_file_to_index(data->index, path, data->flags)) { if (!(data->flags & ADD_CACHE_IGNORE_ERRORS)) die(_("updating files failed")); From 297a27fdf2f68e95d7e344a22f20d9053b74b3ac Mon Sep 17 00:00:00 2001 From: "Claus Schneider(Eficode)" Date: Fri, 6 Feb 2026 13:22:58 +0000 Subject: [PATCH 15/40] tests: t2206-add-submodule-ignored: ignore=all and add --force tests The tests verify that the submodule behavior is intact and updating the config with ignore=all also behaves as intended with configuration in .gitmodules and configuration given on the command line. The usage of --force is showcased and tested in the test suite. The test file is added to meson.build for execution. Signed-off-by: Claus Schneider(Eficode) Signed-off-by: Junio C Hamano --- t/meson.build | 1 + t/t2206-add-submodule-ignored.sh | 134 +++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100755 t/t2206-add-submodule-ignored.sh diff --git a/t/meson.build b/t/meson.build index 459c52a48972e4..db618a32b854d4 100644 --- a/t/meson.build +++ b/t/meson.build @@ -292,6 +292,7 @@ integration_tests = [ 't2203-add-intent.sh', 't2204-add-ignored.sh', 't2205-add-worktree-config.sh', + 't2206-add-submodule-ignored.sh', 't2300-cd-to-toplevel.sh', 't2400-worktree-add.sh', 't2401-worktree-prune.sh', diff --git a/t/t2206-add-submodule-ignored.sh b/t/t2206-add-submodule-ignored.sh new file mode 100755 index 00000000000000..e581e87ab24004 --- /dev/null +++ b/t/t2206-add-submodule-ignored.sh @@ -0,0 +1,134 @@ +#!/bin/sh +# shellcheck disable=SC2016 + +# shellcheck disable=SC2034 +test_description='git add respects submodule ignore=all and explicit pathspec' + +# This test covers the behavior of "git add", "git status" and "git log" when +# dealing with submodules that have the ignore=all setting in +# .gitmodules. It ensures that changes in such submodules are +# ignored by default, but can be staged with "git add --force". + +# shellcheck disable=SC1091 +. ./test-lib.sh + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +base_path=$(pwd -P) + +#1 +test_expect_success 'setup: create origin repos' ' + cd "${base_path}" && + git config --global protocol.file.allow always && + git init sub && + pwd && + cd sub && + test_commit sub_file1 && + git tag v1.0 && + test_commit sub_file2 && + git tag v2.0 && + test_commit sub_file3 && + git tag v3.0 && + cd "${base_path}" && + git init main && + cd main && + test_commit first && + cd "${base_path}" +' +#2 +# add submodule with default config (ignore=none) and +# check log that is contains a path entry for the submodule 'sub' +# change the commit in the submodule and check that 'git status' shows it as modified +test_expect_success 'main: add submodule with default config' ' + cd "${base_path}" && + cd main && + git submodule add ../sub && + git commit -m "add submodule" && + git log --oneline --name-only | grep "^sub$" && + git -C sub reset --hard v2.0 && + git status --porcelain | grep "^ M sub$" && + echo +' +#3 +# change the submodule config to ignore=all and check that status and log do not show changes +test_expect_success 'main: submodule config ignore=all' ' + cd "${base_path}" && + cd main && + git config -f .gitmodules submodule.sub.ignore all && + GIT_TRACE=1 git add . && + git commit -m "update submodule config sub.ignore all" && + ! git status --porcelain | grep "^.*$" && + ! git log --oneline --name-only | grep "^sub$" && + echo +' +#4 +# change the commit in the submodule and check that 'git status' does not show it as modified +# but 'git status --ignore-submodules=none' does show it as modified +test_expect_success 'sub: change to different sha1 and check status in main' ' + cd "${base_path}" && + cd main && + git -C sub reset --hard v1.0 && + ! git status --porcelain | grep "^ M sub$" && + git status --ignore-submodules=none --porcelain | grep "^ M sub$" && + echo +' + +#5 +# check that normal 'git add' does not stage the change in the submodule +test_expect_success 'main: check normal add and status' ' + cd "${base_path}" && + cd main && + GIT_TRACE=1 git add . && + ! git status --porcelain | grep "^ M sub$" && + echo +' + +#6 +# check that 'git add --force .' does not stage the change in the submodule +# and that 'git status' does not show it as modified +test_expect_success 'main: check --force add . and status' ' + cd "${base_path}" && + cd main && + GIT_TRACE=1 git add --force . && + ! git status --porcelain | grep "^M sub$" && + echo +' + +#7 +# check that 'git add .' does not stage the change in the submodule +# and that 'git status' does not show it as modified +test_expect_success 'main: check _add sub_ and status' ' + cd "${base_path}" && + cd main && + GIT_TRACE=1 git add sub 2>&1 | grep "Skipping submodule due to ignore=all: sub" && + ! git status --porcelain | grep "^M sub$" && + echo +' + +#8 +# check that 'git add --force sub' does stage the change in the submodule +# check that 'git add --force ./sub/' does stage the change in the submodule +# and that 'git status --porcelain' does show it as modified +# commit it.. +# check that 'git log --ignore-submodules=none' shows the submodule change +# in the log +test_expect_success 'main: check force add sub and ./sub/ and status' ' + cd "${base_path}" && + cd main && + echo "Adding with --force should work: git add --force sub" && + GIT_TRACE=1 git add --force sub && + git status --porcelain | grep "^M sub$" && + git restore --staged sub && + ! git status --porcelain | grep "^M sub$" && + echo "Adding with --force should work: git add --force ./sub/" && + GIT_TRACE=1 git add --force ./sub/ && + git status --porcelain | grep "^M sub$" && + git commit -m "update submodule pointer" && + ! git status --porcelain | grep "^ M sub$" && + git log --ignore-submodules=none --name-only --oneline | grep "^sub$" && + echo +' + +test_done +exit 0 From 902013225cb7120c59b6ef8771db5c266041f6d5 Mon Sep 17 00:00:00 2001 From: "Claus Schneider(Eficode)" Date: Fri, 6 Feb 2026 13:22:59 +0000 Subject: [PATCH 16/40] tests: fix existing tests when add an ignore=all submodule There are tests that rely on "git add " to update the in the reference in the parent repository which have been updated to use the --force option. Updated tests: - t1013-read-tree-submodule.sh ( fixed in: t/lib-submodule-update.sh ) - t2013-checkout-submodule.sh ( fixed in: t/lib-submodule-update.sh ) - t7406-submodule-update.sh - t7508-status.sh Signed-off-by: Claus Schneider(Eficode) Signed-off-by: Junio C Hamano --- t/lib-submodule-update.sh | 6 +++--- t/t7508-status.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index 36f767cb7488bf..f591de6120c3d3 100644 --- a/t/lib-submodule-update.sh +++ b/t/lib-submodule-update.sh @@ -95,14 +95,14 @@ create_lib_submodule_repo () { git commit -m "modified file2 and added file3" && git push origin modifications ) && - git add sub1 && + git add --force sub1 && git commit -m "Modify sub1" && git checkout -b add_nested_sub modify_sub1 && git -C sub1 checkout -b "add_nested_sub" && git -C sub1 submodule add --branch no_submodule ../submodule_update_sub2 sub2 && git -C sub1 commit -a -m "add a nested submodule" && - git add sub1 && + git add --force sub1 && git commit -a -m "update submodule, that updates a nested submodule" && git checkout -b modify_sub1_recursively && git -C sub1 checkout -b modify_sub1_recursively && @@ -112,7 +112,7 @@ create_lib_submodule_repo () { git -C sub1/sub2 commit -m "make a change in nested sub" && git -C sub1 add sub2 && git -C sub1 commit -m "update nested sub" && - git add sub1 && + git add --force sub1 && git commit -m "update sub1, that updates nested sub" && git -C sub1 push origin modify_sub1_recursively && git -C sub1/sub2 push origin modify_sub1_recursively && diff --git a/t/t7508-status.sh b/t/t7508-status.sh index abad229e9d9eda..a5e21bf8bffb45 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -1576,7 +1576,7 @@ test_expect_success 'git commit will commit a staged but ignored submodule' ' test_expect_success 'git commit --dry-run will show a staged but ignored submodule' ' git reset HEAD^ && - git add sm && + git add --force sm && cat >expect << EOF && On branch main Your branch and '\''upstream'\'' have diverged, From 6cc6d1b4c699323bc2a76e1a4cfbaede242cbfc8 Mon Sep 17 00:00:00 2001 From: "Claus Schneider(Eficode)" Date: Fri, 6 Feb 2026 13:23:00 +0000 Subject: [PATCH 17/40] Documentation: update add --force option + ignore=all config - git-add.adoc: Update the --force documentation for submodule behaviour to be added even the given configuration ignore=all. - gitmodules.adoc and config/submodule.adoc: The submodule config ignore=all now need --force in order to update the index. Signed-off-by: Claus Schneider(Eficode) Signed-off-by: Junio C Hamano --- Documentation/config/submodule.adoc | 13 +++++++------ Documentation/git-add.adoc | 5 ++++- Documentation/gitmodules.adoc | 5 ++++- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc index 0672d9911724d1..b3db1dc2c85812 100644 --- a/Documentation/config/submodule.adoc +++ b/Documentation/config/submodule.adoc @@ -32,15 +32,16 @@ submodule..fetchRecurseSubmodules:: submodule..ignore:: Defines under what circumstances "git status" and the diff family show - a submodule as modified. When set to "all", it will never be considered - modified (but it will nonetheless show up in the output of status and - commit when it has been staged), "dirty" will ignore all changes - to the submodule's work tree and + a submodule as modified. + When set to "all" will never consider the submodule modified. It can + nevertheless be staged using the option --force and it will then show up + in the output of status. + When set to "dirty" will ignore all changes to the submodule's work tree and takes only differences between the HEAD of the submodule and the commit recorded in the superproject into account. "untracked" will additionally let submodules with modified tracked files in their work tree show up. - Using "none" (the default when this option is not set) also shows - submodules that have untracked files in their work tree as changed. + When set to "none"(default) It also show submodules as changed if they have + untracked files in their work tree. This setting overrides any setting made in .gitmodules for this submodule, both settings can be overridden on the command line by using the "--ignore-submodules" option. The 'git submodule' commands are not diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc index 6192daeb0371cf..941135dc637d90 100644 --- a/Documentation/git-add.adoc +++ b/Documentation/git-add.adoc @@ -75,7 +75,10 @@ in linkgit:gitglossary[7]. `-f`:: `--force`:: - Allow adding otherwise ignored files. + Allow adding otherwise ignored files. The option is also used when + `submodule..ignore=all` is set, but you want to stage an + update of the submodule. The `path` to the submodule must be explicitly + specified. `--sparse`:: Allow updating index entries outside of the sparse-checkout cone. diff --git a/Documentation/gitmodules.adoc b/Documentation/gitmodules.adoc index d9bec8b1875502..3792da96aa2ba7 100644 --- a/Documentation/gitmodules.adoc +++ b/Documentation/gitmodules.adoc @@ -70,7 +70,10 @@ submodule..ignore:: -- all;; The submodule will never be considered modified (but will nonetheless show up in the output of status and commit when it has - been staged). + been staged). Add `(new commits)` can be overruled using the + `git add --force `. + The setting affects `status`, `update-index`, `diff` and `log`(due + to underlaying `diff`). dirty;; All changes to the submodule's work tree will be ignored, only committed differences between the `HEAD` of the submodule and its From 57fa3161a0ba4fb5e17dba8244bc854d91ae98bf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:35 +0100 Subject: [PATCH 18/40] refs: remove unused `refs_for_each_include_root_ref()` Remove the unused `refs_for_each_include_root_ref()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 7 ------- refs.h | 6 ------ 2 files changed, 13 deletions(-) diff --git a/refs.c b/refs.c index 600913b99f3f88..466398494f7e9c 100644 --- a/refs.c +++ b/refs.c @@ -1932,13 +1932,6 @@ int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, DO_FOR_EACH_INCLUDE_BROKEN, cb_data); } -int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn, - void *cb_data) -{ - return do_for_each_ref(refs, "", NULL, fn, 0, - DO_FOR_EACH_INCLUDE_ROOT_REFS, cb_data); -} - static int qsort_strcmp(const void *va, const void *vb) { const char *a = *(const char **)va; diff --git a/refs.h b/refs.h index f16b1b697b2bf8..1fdb809343a3f4 100644 --- a/refs.h +++ b/refs.h @@ -471,12 +471,6 @@ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data); int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, each_ref_fn fn, void *cb_data); -/* - * Iterates over all refs including root refs, i.e. pseudorefs and HEAD. - */ -int refs_for_each_include_root_refs(struct ref_store *refs, each_ref_fn fn, - void *cb_data); - /* * Normalizes partial refs to their fully qualified form. * Will prepend to the if it doesn't start with 'refs/'. From 7543920af2f1a48ed4656e38a442125ef60cc3b0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:36 +0100 Subject: [PATCH 19/40] refs: move `refs_head_ref_namespaced()` The function `refs_head_ref_namespaced()` is somewhat special when compared to most of the other functions that take a callback function: while `refs_for_each_*()` functions yield multiple refs, `refs_heasd_ref_namespaced()` will only yield at most the HEAD ref of the current namespace. As such, the function is related to `refs_head_ref()` and not to the for-each functions. Move the function to be located next to `refs_head_ref()` to clarify. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/refs.h b/refs.h index 1fdb809343a3f4..718212a5d7fb43 100644 --- a/refs.h +++ b/refs.h @@ -413,6 +413,9 @@ typedef int each_ref_fn(const struct reference *ref, void *cb_data); */ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data); +int refs_head_ref_namespaced(struct ref_store *refs, + each_ref_fn fn, void *cb_data); + int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data); int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, @@ -456,8 +459,6 @@ int refs_for_each_glob_ref(struct ref_store *refs, each_ref_fn fn, int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn, const char *pattern, const char *prefix, void *cb_data); -int refs_head_ref_namespaced(struct ref_store *refs, each_ref_fn fn, void *cb_data); - /* * references matching any pattern in "exclude_patterns" are omitted from the * result set on a best-effort basis. From 1c3aff34b88f132ba3d489dbd9ff30f2604bb871 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:37 +0100 Subject: [PATCH 20/40] refs: move `do_for_each_ref_flags` further up Move the `do_for_each_ref_flags` enum further up. This prepares for subsequent changes, where the flags will be used by more functions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 74 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/refs.h b/refs.h index 718212a5d7fb43..6fd7a706b5bdaa 100644 --- a/refs.h +++ b/refs.h @@ -402,6 +402,43 @@ int reference_get_peeled_oid(struct repository *repo, */ typedef int each_ref_fn(const struct reference *ref, void *cb_data); +/* + * These flags are passed to refs_ref_iterator_begin() (and do_for_each_ref(), + * which feeds it). + */ +enum do_for_each_ref_flags { + /* + * Include broken references in a do_for_each_ref*() iteration, which + * would normally be omitted. This includes both refs that point to + * missing objects (a true repository corruption), ones with illegal + * names (which we prefer not to expose to callers), as well as + * dangling symbolic refs (i.e., those that point to a non-existent + * ref; this is not a corruption, but as they have no valid oid, we + * omit them from normal iteration results). + */ + DO_FOR_EACH_INCLUDE_BROKEN = (1 << 0), + + /* + * Only include per-worktree refs in a do_for_each_ref*() iteration. + * Normally this will be used with a files ref_store, since that's + * where all reference backends will presumably store their + * per-worktree refs. + */ + DO_FOR_EACH_PER_WORKTREE_ONLY = (1 << 1), + + /* + * Omit dangling symrefs from output; this only has an effect with + * INCLUDE_BROKEN, since they are otherwise not included at all. + */ + DO_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2), + + /* + * Include root refs i.e. HEAD and pseudorefs along with the regular + * refs. + */ + DO_FOR_EACH_INCLUDE_ROOT_REFS = (1 << 3), +}; + /* * The following functions invoke the specified callback function for * each reference indicated. If the function ever returns a nonzero @@ -1326,43 +1363,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, */ struct ref_iterator; -/* - * These flags are passed to refs_ref_iterator_begin() (and do_for_each_ref(), - * which feeds it). - */ -enum do_for_each_ref_flags { - /* - * Include broken references in a do_for_each_ref*() iteration, which - * would normally be omitted. This includes both refs that point to - * missing objects (a true repository corruption), ones with illegal - * names (which we prefer not to expose to callers), as well as - * dangling symbolic refs (i.e., those that point to a non-existent - * ref; this is not a corruption, but as they have no valid oid, we - * omit them from normal iteration results). - */ - DO_FOR_EACH_INCLUDE_BROKEN = (1 << 0), - - /* - * Only include per-worktree refs in a do_for_each_ref*() iteration. - * Normally this will be used with a files ref_store, since that's - * where all reference backends will presumably store their - * per-worktree refs. - */ - DO_FOR_EACH_PER_WORKTREE_ONLY = (1 << 1), - - /* - * Omit dangling symrefs from output; this only has an effect with - * INCLUDE_BROKEN, since they are otherwise not included at all. - */ - DO_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2), - - /* - * Include root refs i.e. HEAD and pseudorefs along with the regular - * refs. - */ - DO_FOR_EACH_INCLUDE_ROOT_REFS = (1 << 3), -}; - /* * Return an iterator that goes over each reference in `refs` for * which the refname begins with prefix. If trim is non-zero, then From 8f0720a5a781562fb1f750b351e14129fc8930ea Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:38 +0100 Subject: [PATCH 21/40] refs: rename `do_for_each_ref_flags` The enum `do_for_each_ref_flags` and its individual values don't match to our current best practices when it comes to naming things. Rename it to `refs_for_each_flag`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ref-filter.c | 2 +- refs.c | 18 +++++++++--------- refs.h | 12 ++++++------ refs/files-backend.c | 12 ++++++------ refs/packed-backend.c | 8 ++++---- refs/reftable-backend.c | 10 +++++----- 6 files changed, 31 insertions(+), 31 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 3917c4ccd9f73a..4bc54ebd9d97ef 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2810,7 +2810,7 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter, if (filter->kind & FILTER_REFS_ROOT_REFS) { /* In this case, we want to print all refs including root refs. */ return for_each_fullref_with_seek(filter, cb, cb_data, - DO_FOR_EACH_INCLUDE_ROOT_REFS); + REFS_FOR_EACH_INCLUDE_ROOT_REFS); } if (!filter->match_as_path) { diff --git a/refs.c b/refs.c index 466398494f7e9c..52a680797a7ca7 100644 --- a/refs.c +++ b/refs.c @@ -1812,7 +1812,7 @@ struct ref_iterator *refs_ref_iterator_begin( const char *prefix, const char **exclude_patterns, int trim, - enum do_for_each_ref_flags flags) + enum refs_for_each_flag flags) { struct ref_iterator *iter; struct strvec normalized_exclude_patterns = STRVEC_INIT; @@ -1834,14 +1834,14 @@ struct ref_iterator *refs_ref_iterator_begin( exclude_patterns = normalized_exclude_patterns.v; } - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) { + if (!(flags & REFS_FOR_EACH_INCLUDE_BROKEN)) { static int ref_paranoia = -1; if (ref_paranoia < 0) ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 1); if (ref_paranoia) { - flags |= DO_FOR_EACH_INCLUDE_BROKEN; - flags |= DO_FOR_EACH_OMIT_DANGLING_SYMREFS; + flags |= REFS_FOR_EACH_INCLUDE_BROKEN; + flags |= REFS_FOR_EACH_OMIT_DANGLING_SYMREFS; } } @@ -1861,7 +1861,7 @@ struct ref_iterator *refs_ref_iterator_begin( static int do_for_each_ref(struct ref_store *refs, const char *prefix, const char **exclude_patterns, each_ref_fn fn, int trim, - enum do_for_each_ref_flags flags, void *cb_data) + enum refs_for_each_flag flags, void *cb_data) { struct ref_iterator *iter; @@ -1897,7 +1897,7 @@ int refs_for_each_replace_ref(struct ref_store *refs, each_ref_fn fn, void *cb_d const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; return do_for_each_ref(refs, git_replace_ref_base, NULL, fn, strlen(git_replace_ref_base), - DO_FOR_EACH_INCLUDE_BROKEN, cb_data); + REFS_FOR_EACH_INCLUDE_BROKEN, cb_data); } int refs_for_each_namespaced_ref(struct ref_store *refs, @@ -1929,7 +1929,7 @@ int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, each_ref_fn fn, void *cb_data) { return do_for_each_ref(refs, prefix, NULL, fn, 0, - DO_FOR_EACH_INCLUDE_BROKEN, cb_data); + REFS_FOR_EACH_INCLUDE_BROKEN, cb_data); } static int qsort_strcmp(const void *va, const void *vb) @@ -2741,7 +2741,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs if (!iter) iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, - DO_FOR_EACH_INCLUDE_BROKEN); + REFS_FOR_EACH_INCLUDE_BROKEN); else if (ref_iterator_seek(iter, dirname.buf, REF_ITERATOR_SEEK_SET_PREFIX) < 0) goto cleanup; @@ -3281,7 +3281,7 @@ int repo_migrate_ref_storage_format(struct repository *repo, * ensure that there are no concurrent writes. */ ret = do_for_each_ref(old_refs, "", NULL, migrate_one_ref, 0, - DO_FOR_EACH_INCLUDE_ROOT_REFS | DO_FOR_EACH_INCLUDE_BROKEN, + REFS_FOR_EACH_INCLUDE_ROOT_REFS | REFS_FOR_EACH_INCLUDE_BROKEN, &data); if (ret < 0) goto done; diff --git a/refs.h b/refs.h index 6fd7a706b5bdaa..2ae4a6e75b74c4 100644 --- a/refs.h +++ b/refs.h @@ -406,7 +406,7 @@ typedef int each_ref_fn(const struct reference *ref, void *cb_data); * These flags are passed to refs_ref_iterator_begin() (and do_for_each_ref(), * which feeds it). */ -enum do_for_each_ref_flags { +enum refs_for_each_flag { /* * Include broken references in a do_for_each_ref*() iteration, which * would normally be omitted. This includes both refs that point to @@ -416,7 +416,7 @@ enum do_for_each_ref_flags { * ref; this is not a corruption, but as they have no valid oid, we * omit them from normal iteration results). */ - DO_FOR_EACH_INCLUDE_BROKEN = (1 << 0), + REFS_FOR_EACH_INCLUDE_BROKEN = (1 << 0), /* * Only include per-worktree refs in a do_for_each_ref*() iteration. @@ -424,19 +424,19 @@ enum do_for_each_ref_flags { * where all reference backends will presumably store their * per-worktree refs. */ - DO_FOR_EACH_PER_WORKTREE_ONLY = (1 << 1), + REFS_FOR_EACH_PER_WORKTREE_ONLY = (1 << 1), /* * Omit dangling symrefs from output; this only has an effect with * INCLUDE_BROKEN, since they are otherwise not included at all. */ - DO_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2), + REFS_FOR_EACH_OMIT_DANGLING_SYMREFS = (1 << 2), /* * Include root refs i.e. HEAD and pseudorefs along with the regular * refs. */ - DO_FOR_EACH_INCLUDE_ROOT_REFS = (1 << 3), + REFS_FOR_EACH_INCLUDE_ROOT_REFS = (1 << 3), }; /* @@ -1372,7 +1372,7 @@ struct ref_iterator; struct ref_iterator *refs_ref_iterator_begin( struct ref_store *refs, const char *prefix, const char **exclude_patterns, - int trim, enum do_for_each_ref_flags flags); + int trim, enum refs_for_each_flag flags); /* * Advance the iterator to the first or next item and return ITER_OK. diff --git a/refs/files-backend.c b/refs/files-backend.c index b1b13b41f6c7d1..6c98e14414197b 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -439,7 +439,7 @@ static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs, dir = get_ref_dir(refs->loose->root); - if (flags & DO_FOR_EACH_INCLUDE_ROOT_REFS) + if (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) add_root_refs(refs, dir); /* @@ -955,17 +955,17 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { - if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && + if (iter->flags & REFS_FOR_EACH_PER_WORKTREE_ONLY && parse_worktree_ref(iter->iter0->ref.name, NULL, NULL, NULL) != REF_WORKTREE_CURRENT) continue; - if ((iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS) && + if ((iter->flags & REFS_FOR_EACH_OMIT_DANGLING_SYMREFS) && (iter->iter0->ref.flags & REF_ISSYMREF) && (iter->iter0->ref.flags & REF_ISBROKEN)) continue; - if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && + if (!(iter->flags & REFS_FOR_EACH_INCLUDE_BROKEN) && !ref_resolves_to_object(iter->iter0->ref.name, iter->repo, iter->iter0->ref.oid, @@ -1012,7 +1012,7 @@ static struct ref_iterator *files_ref_iterator_begin( struct ref_iterator *ref_iterator; unsigned int required_flags = REF_STORE_READ; - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) + if (!(flags & REFS_FOR_EACH_INCLUDE_BROKEN)) required_flags |= REF_STORE_ODB; refs = files_downcast(ref_store, required_flags, "ref_iterator_begin"); @@ -1050,7 +1050,7 @@ static struct ref_iterator *files_ref_iterator_begin( */ packed_iter = refs_ref_iterator_begin( refs->packed_ref_store, prefix, exclude_patterns, 0, - DO_FOR_EACH_INCLUDE_BROKEN); + REFS_FOR_EACH_INCLUDE_BROKEN); overlay_iter = overlay_ref_iterator_begin(loose_iter, packed_iter); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 59b3ecb9d64716..5ef4ae32b83f37 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -982,11 +982,11 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) const char *refname = iter->base.ref.name; const char *prefix = iter->prefix; - if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && + if (iter->flags & REFS_FOR_EACH_PER_WORKTREE_ONLY && !is_per_worktree_ref(iter->base.ref.name)) continue; - if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && + if (!(iter->flags & REFS_FOR_EACH_INCLUDE_BROKEN) && !ref_resolves_to_object(iter->base.ref.name, iter->repo, &iter->oid, iter->flags)) continue; @@ -1159,7 +1159,7 @@ static struct ref_iterator *packed_ref_iterator_begin( struct ref_iterator *ref_iterator; unsigned int required_flags = REF_STORE_READ; - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) + if (!(flags & REFS_FOR_EACH_INCLUDE_BROKEN)) required_flags |= REF_STORE_ODB; refs = packed_downcast(ref_store, required_flags, "ref_iterator_begin"); @@ -1401,7 +1401,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re * of updates is exhausted, leave i set to updates->nr. */ iter = packed_ref_iterator_begin(&refs->base, "", NULL, - DO_FOR_EACH_INCLUDE_BROKEN); + REFS_FOR_EACH_INCLUDE_BROKEN); if ((ok = ref_iterator_advance(iter)) != ITER_OK) { ref_iterator_free(iter); iter = NULL; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 5611808ad75a8b..34bc074dd37a9b 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -662,7 +662,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) * the root refs are to be included. We emulate the same behaviour here. */ if (!starts_with(iter->ref.refname, "refs/") && - !(iter->flags & DO_FOR_EACH_INCLUDE_ROOT_REFS && + !(iter->flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS && is_root_ref(iter->ref.refname))) { continue; } @@ -676,7 +676,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) if (iter->exclude_patterns && should_exclude_current_ref(iter)) continue; - if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && + if (iter->flags & REFS_FOR_EACH_PER_WORKTREE_ONLY && parse_worktree_ref(iter->ref.refname, NULL, NULL, NULL) != REF_WORKTREE_CURRENT) continue; @@ -714,12 +714,12 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) flags |= REF_BAD_NAME | REF_ISBROKEN; } - if (iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS && + if (iter->flags & REFS_FOR_EACH_OMIT_DANGLING_SYMREFS && flags & REF_ISSYMREF && flags & REF_ISBROKEN) continue; - if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && + if (!(iter->flags & REFS_FOR_EACH_INCLUDE_BROKEN) && !ref_resolves_to_object(iter->ref.refname, refs->base.repo, &iter->oid, flags)) continue; @@ -871,7 +871,7 @@ static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_sto struct reftable_ref_store *refs; unsigned int required_flags = REF_STORE_READ; - if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) + if (!(flags & REFS_FOR_EACH_INCLUDE_BROKEN)) required_flags |= REF_STORE_ODB; refs = reftable_be_downcast(ref_store, required_flags, "ref_iterator_begin"); From 635f08b7394b9dda013a0b78f4db11348dc7717b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:39 +0100 Subject: [PATCH 22/40] refs: rename `each_ref_fn` Similar to the preceding commit, rename `each_ref_fn` to better match our current best practices around how we name things. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- pack-bitmap.c | 2 +- pack-bitmap.h | 2 +- ref-filter.c | 6 +++--- refs.c | 34 +++++++++++++++++----------------- refs.h | 38 +++++++++++++++++++------------------- refs/iterator.c | 2 +- revision.c | 8 ++++---- submodule.c | 2 +- upload-pack.c | 2 +- worktree.c | 2 +- worktree.h | 2 +- 11 files changed, 50 insertions(+), 50 deletions(-) diff --git a/pack-bitmap.c b/pack-bitmap.c index 1c93871484a24f..efef7081e6a004 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -3324,7 +3324,7 @@ static const struct string_list *bitmap_preferred_tips(struct repository *r) } void for_each_preferred_bitmap_tip(struct repository *repo, - each_ref_fn cb, void *cb_data) + refs_for_each_cb cb, void *cb_data) { struct string_list_item *item; const struct string_list *preferred_tips; diff --git a/pack-bitmap.h b/pack-bitmap.h index d0611d04813010..a95e1c2d115a31 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -105,7 +105,7 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git, * "pack.preferBitmapTips" and invoke the callback on each function. */ void for_each_preferred_bitmap_tip(struct repository *repo, - each_ref_fn cb, void *cb_data); + refs_for_each_cb cb, void *cb_data); #define GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL \ "GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL" diff --git a/ref-filter.c b/ref-filter.c index 4bc54ebd9d97ef..049e845a1909fe 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2781,7 +2781,7 @@ static int start_ref_iterator_after(struct ref_iterator *iter, const char *marke return ret; } -static int for_each_fullref_with_seek(struct ref_filter *filter, each_ref_fn cb, +static int for_each_fullref_with_seek(struct ref_filter *filter, refs_for_each_cb cb, void *cb_data, unsigned int flags) { struct ref_iterator *iter; @@ -2804,7 +2804,7 @@ static int for_each_fullref_with_seek(struct ref_filter *filter, each_ref_fn cb, * pattern match, so the callback still has to match each ref individually. */ static int for_each_fullref_in_pattern(struct ref_filter *filter, - each_ref_fn cb, + refs_for_each_cb cb, void *cb_data) { if (filter->kind & FILTER_REFS_ROOT_REFS) { @@ -3303,7 +3303,7 @@ void filter_is_base(struct repository *r, free(bases); } -static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref_fn fn, void *cb_data) +static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for_each_cb fn, void *cb_data) { const char *prefix = NULL; int ret = 0; diff --git a/refs.c b/refs.c index 52a680797a7ca7..a45cc612111840 100644 --- a/refs.c +++ b/refs.c @@ -445,7 +445,7 @@ char *refs_resolve_refdup(struct ref_store *refs, struct for_each_ref_filter { const char *pattern; const char *prefix; - each_ref_fn *fn; + refs_for_each_cb *fn; void *cb_data; }; @@ -527,22 +527,22 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, refs_for_each_rawref(refs, warn_if_dangling_symref, &data); } -int refs_for_each_tag_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_tag_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return refs_for_each_ref_in(refs, "refs/tags/", fn, cb_data); } -int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_branch_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data); } -int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_remote_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data); } -int refs_head_ref_namespaced(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_head_ref_namespaced(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { struct strbuf buf = STRBUF_INIT; int ret = 0; @@ -590,7 +590,7 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix, strbuf_release(&normalized_pattern); } -int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn, +int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb fn, const char *pattern, const char *prefix, void *cb_data) { struct strbuf real_pattern = STRBUF_INIT; @@ -620,7 +620,7 @@ int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn, return ret; } -int refs_for_each_glob_ref(struct ref_store *refs, each_ref_fn fn, +int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, const char *pattern, void *cb_data) { return refs_for_each_glob_ref_in(refs, fn, pattern, NULL, cb_data); @@ -1788,7 +1788,7 @@ const char *find_descendant_ref(const char *dirname, return NULL; } -int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_head_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { struct object_id oid; int flag; @@ -1860,7 +1860,7 @@ struct ref_iterator *refs_ref_iterator_begin( static int do_for_each_ref(struct ref_store *refs, const char *prefix, const char **exclude_patterns, - each_ref_fn fn, int trim, + refs_for_each_cb fn, int trim, enum refs_for_each_flag flags, void *cb_data) { struct ref_iterator *iter; @@ -1874,25 +1874,25 @@ static int do_for_each_ref(struct ref_store *refs, const char *prefix, return do_for_each_ref_iterator(iter, fn, cb_data); } -int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return do_for_each_ref(refs, "", NULL, fn, 0, 0, cb_data); } int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { return do_for_each_ref(refs, prefix, NULL, fn, strlen(prefix), 0, cb_data); } int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, const char **exclude_patterns, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { return do_for_each_ref(refs, prefix, exclude_patterns, fn, 0, 0, cb_data); } -int refs_for_each_replace_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; return do_for_each_ref(refs, git_replace_ref_base, NULL, fn, @@ -1902,7 +1902,7 @@ int refs_for_each_replace_ref(struct ref_store *refs, each_ref_fn fn, void *cb_d int refs_for_each_namespaced_ref(struct ref_store *refs, const char **exclude_patterns, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { struct strvec namespaced_exclude_patterns = STRVEC_INIT; struct strbuf prefix = STRBUF_INIT; @@ -1920,13 +1920,13 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, return ret; } -int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return refs_for_each_rawref_in(refs, "", fn, cb_data); } int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { return do_for_each_ref(refs, prefix, NULL, fn, 0, REFS_FOR_EACH_INCLUDE_BROKEN, cb_data); @@ -1994,7 +1994,7 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store, const char *namespace, const char **patterns, const char **exclude_patterns, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { struct strvec namespaced_exclude_patterns = STRVEC_INIT; struct string_list prefixes = STRING_LIST_INIT_DUP; diff --git a/refs.h b/refs.h index 2ae4a6e75b74c4..5190e98b2c6e1d 100644 --- a/refs.h +++ b/refs.h @@ -170,7 +170,7 @@ int ref_store_remove_on_disk(struct ref_store *refs, struct strbuf *err); * * peel_object(r, oid, &peeled); * - * with the "oid" value given to the each_ref_fn callback, except + * with the "oid" value given to the refs_for_each_cb callback, except * that some ref storage may be able to answer the query without * actually loading the object in memory. */ @@ -329,7 +329,7 @@ int check_tag_ref(struct strbuf *sb, const char *name); struct ref_transaction; /* - * Bit values set in the flags argument passed to each_ref_fn() and + * Bit values set in the flags argument passed to refs_for_each_cb() and * stored in ref_iterator::flags. Other bits are for internal use * only: */ @@ -400,7 +400,7 @@ int reference_get_peeled_oid(struct repository *repo, * argument is only guaranteed to be valid for the duration of a * single callback invocation. */ -typedef int each_ref_fn(const struct reference *ref, void *cb_data); +typedef int refs_for_each_cb(const struct reference *ref, void *cb_data); /* * These flags are passed to refs_ref_iterator_begin() (and do_for_each_ref(), @@ -449,22 +449,22 @@ enum refs_for_each_flag { * stop the iteration. Returned references are sorted. */ int refs_head_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_head_ref_namespaced(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_tag_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_branch_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_remote_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); int refs_for_each_replace_ref(struct ref_store *refs, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); /* * references matching any pattern in "exclude_patterns" are omitted from the @@ -472,7 +472,7 @@ int refs_for_each_replace_ref(struct ref_store *refs, */ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, const char **exclude_patterns, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); /** * iterate all refs in "patterns" by partitioning patterns into disjoint sets @@ -487,13 +487,13 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *refs, const char *namespace, const char **patterns, const char **exclude_patterns, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); /* iterates all refs that match the specified glob pattern. */ -int refs_for_each_glob_ref(struct ref_store *refs, each_ref_fn fn, +int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, const char *pattern, void *cb_data); -int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn, +int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb fn, const char *pattern, const char *prefix, void *cb_data); /* @@ -502,12 +502,12 @@ int refs_for_each_glob_ref_in(struct ref_store *refs, each_ref_fn fn, */ int refs_for_each_namespaced_ref(struct ref_store *refs, const char **exclude_patterns, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); /* can be used to learn about broken ref and symref */ -int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data); +int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); /* * Normalizes partial refs to their fully qualified form. @@ -1421,6 +1421,6 @@ void ref_iterator_free(struct ref_iterator *ref_iterator); * iterator style. */ int do_for_each_ref_iterator(struct ref_iterator *iter, - each_ref_fn fn, void *cb_data); + refs_for_each_cb fn, void *cb_data); #endif /* REFS_H */ diff --git a/refs/iterator.c b/refs/iterator.c index d79aa5ec82dc6f..d5cacde51bf87a 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -423,7 +423,7 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, } int do_for_each_ref_iterator(struct ref_iterator *iter, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { int retval = 0, ok; diff --git a/revision.c b/revision.c index 29972c3a19887a..8c206830d5e4a6 100644 --- a/revision.c +++ b/revision.c @@ -1646,7 +1646,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs, static void handle_refs(struct ref_store *refs, struct rev_info *revs, unsigned flags, - int (*for_each)(struct ref_store *, each_ref_fn, void *)) + int (*for_each)(struct ref_store *, refs_for_each_cb, void *)) { struct all_refs_cb cb; @@ -2728,7 +2728,7 @@ void revision_opts_finish(struct rev_info *revs) } } -static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn, +static int for_each_bisect_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data, const char *term) { struct strbuf bisect_refs = STRBUF_INIT; @@ -2739,12 +2739,12 @@ static int for_each_bisect_ref(struct ref_store *refs, each_ref_fn fn, return status; } -static int for_each_bad_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +static int for_each_bad_bisect_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return for_each_bisect_ref(refs, fn, cb_data, term_bad); } -static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) +static int for_each_good_bisect_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) { return for_each_bisect_ref(refs, fn, cb_data, term_good); } diff --git a/submodule.c b/submodule.c index 508938e4da5058..4f9aaa2c75679b 100644 --- a/submodule.c +++ b/submodule.c @@ -101,7 +101,7 @@ int is_staging_gitmodules_ok(struct index_state *istate) } static int for_each_remote_ref_submodule(const char *submodule, - each_ref_fn fn, void *cb_data) + refs_for_each_cb fn, void *cb_data) { return refs_for_each_remote_ref(repo_get_submodule_ref_store(the_repository, submodule), diff --git a/upload-pack.c b/upload-pack.c index 2d2b70cbf2dd0b..7fe397b0d09bb3 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -607,7 +607,7 @@ static int allow_hidden_refs(enum allow_uor allow_uor) return !(allow_uor & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1)); } -static void for_each_namespaced_ref_1(each_ref_fn fn, +static void for_each_namespaced_ref_1(refs_for_each_cb fn, struct upload_pack_data *data) { const char **excludes = NULL; diff --git a/worktree.c b/worktree.c index 9308389cb6f029..bf8c54c04d530d 100644 --- a/worktree.c +++ b/worktree.c @@ -575,7 +575,7 @@ void strbuf_worktree_ref(const struct worktree *wt, strbuf_addstr(sb, refname); } -int other_head_refs(each_ref_fn fn, void *cb_data) +int other_head_refs(refs_for_each_cb fn, void *cb_data) { struct worktree **worktrees, **p; struct strbuf refname = STRBUF_INIT; diff --git a/worktree.h b/worktree.h index e4bcccdc0aef5a..12484a91a7f835 100644 --- a/worktree.h +++ b/worktree.h @@ -191,7 +191,7 @@ int is_shared_symref(const struct worktree *wt, * Similar to head_ref() for all HEADs _except_ one from the current * worktree, which is covered by head_ref(). */ -int other_head_refs(each_ref_fn fn, void *cb_data); +int other_head_refs(refs_for_each_cb fn, void *cb_data); int is_worktree_being_rebased(const struct worktree *wt, const char *target); int is_worktree_being_bisected(const struct worktree *wt, const char *target); From aefcc9b367016581743e57adf667ee4d56691bb1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:40 +0100 Subject: [PATCH 23/40] refs: introduce `refs_for_each_ref_ext` In the refs subsystem we have a proliferation of functions that all iterate through references. (Almost) all of these functions internally call `do_for_each_ref()` and provide slightly different arguments so that one can control different aspects of its behaviour. This approach doesn't really scale: every time there is a slightly different use case for iterating through refs we create another new function. This combinatorial explosion doesn't make a lot of sense: it leads to confusing interfaces and heightens the maintenance burden. Refactor the code to become more composable by: - Exposing `do_for_each_ref()` as `refs_for_each_ref_ext()`. - Introducing an options structure that lets the caller control individual options. This gives us a much better foundation to build on going forward. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 78 ++++++++++++++++++++++++++++++++++++---------------------- refs.h | 29 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 30 deletions(-) diff --git a/refs.c b/refs.c index a45cc612111840..ec9e4663810c39 100644 --- a/refs.c +++ b/refs.c @@ -1858,62 +1858,76 @@ struct ref_iterator *refs_ref_iterator_begin( return iter; } -static int do_for_each_ref(struct ref_store *refs, const char *prefix, - const char **exclude_patterns, - refs_for_each_cb fn, int trim, - enum refs_for_each_flag flags, void *cb_data) +int refs_for_each_ref_ext(struct ref_store *refs, + refs_for_each_cb cb, void *cb_data, + const struct refs_for_each_ref_options *opts) { struct ref_iterator *iter; if (!refs) return 0; - iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns, trim, - flags); + iter = refs_ref_iterator_begin(refs, opts->prefix ? opts->prefix : "", + opts->exclude_patterns, + opts->trim_prefix, opts->flags); - return do_for_each_ref_iterator(iter, fn, cb_data); + return do_for_each_ref_iterator(iter, cb, cb_data); } -int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) +int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { - return do_for_each_ref(refs, "", NULL, fn, 0, 0, cb_data); + struct refs_for_each_ref_options opts = { 0 }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb fn, void *cb_data) + refs_for_each_cb cb, void *cb_data) { - return do_for_each_ref(refs, prefix, NULL, fn, strlen(prefix), 0, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .trim_prefix = strlen(prefix), + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data) + refs_for_each_cb cb, void *cb_data) { - return do_for_each_ref(refs, prefix, exclude_patterns, fn, 0, 0, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .exclude_patterns = exclude_patterns, + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) +int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; - return do_for_each_ref(refs, git_replace_ref_base, NULL, fn, - strlen(git_replace_ref_base), - REFS_FOR_EACH_INCLUDE_BROKEN, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = git_replace_ref_base, + .trim_prefix = strlen(git_replace_ref_base), + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } int refs_for_each_namespaced_ref(struct ref_store *refs, const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data) + refs_for_each_cb cb, void *cb_data) { + struct refs_for_each_ref_options opts = { 0 }; struct strvec namespaced_exclude_patterns = STRVEC_INIT; struct strbuf prefix = STRBUF_INIT; int ret; - exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns, - get_git_namespace(), - &namespaced_exclude_patterns); - + opts.exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns, + get_git_namespace(), + &namespaced_exclude_patterns); strbuf_addf(&prefix, "%srefs/", get_git_namespace()); - ret = do_for_each_ref(refs, prefix.buf, exclude_patterns, fn, 0, 0, cb_data); + opts.prefix = prefix.buf; + + ret = refs_for_each_ref_ext(refs, cb, cb_data, &opts); strvec_clear(&namespaced_exclude_patterns); strbuf_release(&prefix); @@ -1926,10 +1940,13 @@ int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_d } int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb fn, void *cb_data) + refs_for_each_cb cb, void *cb_data) { - return do_for_each_ref(refs, prefix, NULL, fn, 0, - REFS_FOR_EACH_INCLUDE_BROKEN, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } static int qsort_strcmp(const void *va, const void *vb) @@ -3187,6 +3204,9 @@ int repo_migrate_ref_storage_format(struct repository *repo, struct strbuf *errbuf) { struct ref_store *old_refs = NULL, *new_refs = NULL; + struct refs_for_each_ref_options for_each_ref_opts = { + .flags = REFS_FOR_EACH_INCLUDE_ROOT_REFS | REFS_FOR_EACH_INCLUDE_BROKEN, + }; struct ref_transaction *transaction = NULL; struct strbuf new_gitdir = STRBUF_INIT; struct migration_data data = { @@ -3270,7 +3290,7 @@ int repo_migrate_ref_storage_format(struct repository *repo, data.errbuf = errbuf; /* - * We need to use the internal `do_for_each_ref()` here so that we can + * We need to use `refs_for_each_ref_ext()` here so that we can * also include broken refs and symrefs. These would otherwise be * skipped silently. * @@ -3280,9 +3300,7 @@ int repo_migrate_ref_storage_format(struct repository *repo, * allow for a central lock due to its design. It's thus on the user to * ensure that there are no concurrent writes. */ - ret = do_for_each_ref(old_refs, "", NULL, migrate_one_ref, 0, - REFS_FOR_EACH_INCLUDE_ROOT_REFS | REFS_FOR_EACH_INCLUDE_BROKEN, - &data); + ret = refs_for_each_ref_ext(old_refs, migrate_one_ref, &data, &for_each_ref_opts); if (ret < 0) goto done; diff --git a/refs.h b/refs.h index 5190e98b2c6e1d..bb9c64a51c0c75 100644 --- a/refs.h +++ b/refs.h @@ -453,8 +453,37 @@ int refs_head_ref(struct ref_store *refs, int refs_head_ref_namespaced(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); + +struct refs_for_each_ref_options { + /* Only iterate over references that have this given prefix. */ + const char *prefix; + + /* + * Exclude any references that match any of these patterns on a + * best-effort basis. The caller needs to be prepared for the exclude + * patterns to be ignored. + * + * The array must be terminated with a NULL sentinel value. + */ + const char **exclude_patterns; + + /* + * The number of bytes to trim from the refname. Note that the trimmed + * bytes must not cause the reference to become empty. As such, this + * field should typically only be set when one uses a `prefix` ending + * in a slash. + */ + size_t trim_prefix; + + /* Flags that change which refs will be included. */ + enum refs_for_each_flag flags; +}; + int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); +int refs_for_each_ref_ext(struct ref_store *refs, + refs_for_each_cb cb, void *cb_data, + const struct refs_for_each_ref_options *opts); int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, refs_for_each_cb fn, void *cb_data); int refs_for_each_tag_ref(struct ref_store *refs, From daf01b1366ca644d45374451560aeeb4fc8a7765 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:41 +0100 Subject: [PATCH 24/40] refs: speed up `refs_for_each_glob_ref_in()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function `refs_for_each_glob_ref_in()` can be used to iterate through all refs in a specific prefix with globbing. The logic to handle this is currently hosted by `refs_for_each_glob_ref_in()`, which sets up a callback function that knows to filter out refs that _don't_ match the given globbing pattern. The way we do this is somewhat inefficient though: even though the function is expected to only yield refs in the given prefix, we still end up iterating through _all_ references, regardless of whether or not their name matches the given prefix. Extend `refs_for_each_ref_ext()` so that it can handle patterns and adapt `refs_for_each_glob_ref_in()` to use it. This means we continue to use the same callback-based infrastructure to filter individual refs via the globbing pattern, but we can now also use the other functionality of the `_ext()` variant. Most importantly, this means that we now properly handle the prefix. This results in a performance improvement when using a prefix where a significant majority of refs exists outside of the prefix. The following benchmark is an extreme case, with 1 million refs that exist outside the prefix and a single ref that exists inside it: Benchmark 1: git rev-parse --branches=refs/heads/* (rev = HEAD~) Time (mean ± σ): 115.9 ms ± 0.7 ms [User: 113.0 ms, System: 2.4 ms] Range (min … max): 114.9 ms … 117.8 ms 25 runs Benchmark 2: git rev-parse --branches=refs/heads/* (rev = HEAD) Time (mean ± σ): 1.1 ms ± 0.1 ms [User: 0.3 ms, System: 0.7 ms] Range (min … max): 1.0 ms … 2.3 ms 2092 runs Summary git rev-parse --branches=refs/heads/* (rev = HEAD) ran 107.01 ± 6.49 times faster than git rev-parse --branches=refs/heads/* (rev = HEAD~) Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 87 ++++++++++++++++++++++++++++++++++++---------------------- refs.h | 10 +++++++ 2 files changed, 64 insertions(+), 33 deletions(-) diff --git a/refs.c b/refs.c index ec9e4663810c39..e4402d787f51eb 100644 --- a/refs.c +++ b/refs.c @@ -444,7 +444,7 @@ char *refs_resolve_refdup(struct ref_store *refs, /* The argument to for_each_filter_refs */ struct for_each_ref_filter { const char *pattern; - const char *prefix; + size_t trim_prefix; refs_for_each_cb *fn; void *cb_data; }; @@ -475,9 +475,11 @@ static int for_each_filter_refs(const struct reference *ref, void *data) if (wildmatch(filter->pattern, ref->name, 0)) return 0; - if (filter->prefix) { + if (filter->trim_prefix) { struct reference skipped = *ref; - skip_prefix(skipped.name, filter->prefix, &skipped.name); + if (strlen(skipped.name) <= filter->trim_prefix) + BUG("attempt to trim too many characters"); + skipped.name += filter->trim_prefix; return filter->fn(&skipped, filter->cb_data); } else { return filter->fn(ref, filter->cb_data); @@ -590,40 +592,24 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix, strbuf_release(&normalized_pattern); } -int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb fn, +int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb cb, const char *pattern, const char *prefix, void *cb_data) { - struct strbuf real_pattern = STRBUF_INIT; - struct for_each_ref_filter filter; - int ret; - - if (!prefix && !starts_with(pattern, "refs/")) - strbuf_addstr(&real_pattern, "refs/"); - else if (prefix) - strbuf_addstr(&real_pattern, prefix); - strbuf_addstr(&real_pattern, pattern); - - if (!has_glob_specials(pattern)) { - /* Append implied '/' '*' if not present. */ - strbuf_complete(&real_pattern, '/'); - /* No need to check for '*', there is none. */ - strbuf_addch(&real_pattern, '*'); - } - - filter.pattern = real_pattern.buf; - filter.prefix = prefix; - filter.fn = fn; - filter.cb_data = cb_data; - ret = refs_for_each_ref(refs, for_each_filter_refs, &filter); - - strbuf_release(&real_pattern); - return ret; + struct refs_for_each_ref_options opts = { + .pattern = pattern, + .prefix = prefix, + .trim_prefix = prefix ? strlen(prefix) : 0, + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, +int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb cb, const char *pattern, void *cb_data) { - return refs_for_each_glob_ref_in(refs, fn, pattern, NULL, cb_data); + struct refs_for_each_ref_options opts = { + .pattern = pattern, + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } const char *prettify_refname(const char *name) @@ -1862,16 +1848,51 @@ int refs_for_each_ref_ext(struct ref_store *refs, refs_for_each_cb cb, void *cb_data, const struct refs_for_each_ref_options *opts) { + struct strbuf real_pattern = STRBUF_INIT; + struct for_each_ref_filter filter; struct ref_iterator *iter; + size_t trim_prefix = opts->trim_prefix; + int ret; if (!refs) return 0; + if (opts->pattern) { + if (!opts->prefix && !starts_with(opts->pattern, "refs/")) + strbuf_addstr(&real_pattern, "refs/"); + else if (opts->prefix) + strbuf_addstr(&real_pattern, opts->prefix); + strbuf_addstr(&real_pattern, opts->pattern); + + if (!has_glob_specials(opts->pattern)) { + /* Append implied '/' '*' if not present. */ + strbuf_complete(&real_pattern, '/'); + /* No need to check for '*', there is none. */ + strbuf_addch(&real_pattern, '*'); + } + + filter.pattern = real_pattern.buf; + filter.trim_prefix = opts->trim_prefix; + filter.fn = cb; + filter.cb_data = cb_data; + + /* + * We need to trim the prefix in the callback function as the + * pattern is expected to match on the full refname. + */ + trim_prefix = 0; + + cb = for_each_filter_refs; + cb_data = &filter; + } + iter = refs_ref_iterator_begin(refs, opts->prefix ? opts->prefix : "", opts->exclude_patterns, - opts->trim_prefix, opts->flags); + trim_prefix, opts->flags); - return do_for_each_ref_iterator(iter, cb, cb_data); + ret = do_for_each_ref_iterator(iter, cb, cb_data); + strbuf_release(&real_pattern); + return ret; } int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) diff --git a/refs.h b/refs.h index bb9c64a51c0c75..a66dbf38652def 100644 --- a/refs.h +++ b/refs.h @@ -458,6 +458,16 @@ struct refs_for_each_ref_options { /* Only iterate over references that have this given prefix. */ const char *prefix; + /* + * A globbing pattern that can be used to only yield refs that match. + * If given, refs will be matched against the pattern with + * `wildmatch()`. + * + * If the pattern doesn't contain any globbing characters then it is + * treated as if it was ending with "/" and "*". + */ + const char *pattern; + /* * Exclude any references that match any of these patterns on a * best-effort basis. The caller needs to be prepared for the exclude From 5387919327574b5067f7efd986fca8793c95c71a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:42 +0100 Subject: [PATCH 25/40] refs: generalize `refs_for_each_namespaced_ref()` The function `refs_for_each_namespaced_ref()` iterates through all references that are part of the current ref namespace. This namespace can be configured by setting the `GIT_NAMESPACE` environment variable and is then retrieved by calling `get_git_namespace()`. If a namespace is configured, then we: - Obviously only yield refs that exist in this namespace. - Rewrite exclude patterns so that they work for the given namespace, if any namespace is currently configured. Port this logic to `refs_for_each_ref_ext()` by adding a new `namespace` field to the options structure. This gives callers more flexibility as they can decide by themselves whether they want to use the globally configured or an arbitrary other namespace. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 47 +++++++++++++++++++++++++++++------------------ refs.h | 7 +++++++ 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index e4402d787f51eb..0d0f0edbfb1140 100644 --- a/refs.c +++ b/refs.c @@ -1848,10 +1848,14 @@ int refs_for_each_ref_ext(struct ref_store *refs, refs_for_each_cb cb, void *cb_data, const struct refs_for_each_ref_options *opts) { + struct strvec namespaced_exclude_patterns = STRVEC_INIT; + struct strbuf namespaced_prefix = STRBUF_INIT; struct strbuf real_pattern = STRBUF_INIT; struct for_each_ref_filter filter; struct ref_iterator *iter; size_t trim_prefix = opts->trim_prefix; + const char **exclude_patterns; + const char *prefix; int ret; if (!refs) @@ -1886,11 +1890,29 @@ int refs_for_each_ref_ext(struct ref_store *refs, cb_data = &filter; } - iter = refs_ref_iterator_begin(refs, opts->prefix ? opts->prefix : "", - opts->exclude_patterns, + if (opts->namespace) { + strbuf_addstr(&namespaced_prefix, opts->namespace); + if (opts->prefix) + strbuf_addstr(&namespaced_prefix, opts->prefix); + else + strbuf_addstr(&namespaced_prefix, "refs/"); + + prefix = namespaced_prefix.buf; + exclude_patterns = get_namespaced_exclude_patterns(opts->exclude_patterns, + opts->namespace, + &namespaced_exclude_patterns); + } else { + prefix = opts->prefix ? opts->prefix : ""; + exclude_patterns = opts->exclude_patterns; + } + + iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns, trim_prefix, opts->flags); ret = do_for_each_ref_iterator(iter, cb, cb_data); + + strvec_clear(&namespaced_exclude_patterns); + strbuf_release(&namespaced_prefix); strbuf_release(&real_pattern); return ret; } @@ -1937,22 +1959,11 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, const char **exclude_patterns, refs_for_each_cb cb, void *cb_data) { - struct refs_for_each_ref_options opts = { 0 }; - struct strvec namespaced_exclude_patterns = STRVEC_INIT; - struct strbuf prefix = STRBUF_INIT; - int ret; - - opts.exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns, - get_git_namespace(), - &namespaced_exclude_patterns); - strbuf_addf(&prefix, "%srefs/", get_git_namespace()); - opts.prefix = prefix.buf; - - ret = refs_for_each_ref_ext(refs, cb, cb_data, &opts); - - strvec_clear(&namespaced_exclude_patterns); - strbuf_release(&prefix); - return ret; + struct refs_for_each_ref_options opts = { + .exclude_patterns = exclude_patterns, + .namespace = get_git_namespace(), + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) diff --git a/refs.h b/refs.h index a66dbf38652def..5a5fb4e1e426b8 100644 --- a/refs.h +++ b/refs.h @@ -468,6 +468,13 @@ struct refs_for_each_ref_options { */ const char *pattern; + /* + * If set, only yield refs part of the configured namespace. Exclude + * patterns will be rewritten to apply to the namespace, and the prefix + * will be considered relative to the namespace. + */ + const char *namespace; + /* * Exclude any references that match any of these patterns on a * best-effort basis. The caller needs to be prepared for the exclude From f503bb7dc96ee92623ade8d60eed401ecfddae0f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:43 +0100 Subject: [PATCH 26/40] refs: generalize `refs_for_each_fullref_in_prefixes()` The function `refs_for_each_fullref_in_prefixes()` can be used to iterate over all references part of any of the user-provided prefixes. In contrast to the `prefix` parameter of `refs_for_each_ref_ext()` it knows to handle the case well where multiple of the passed-in prefixes start with a common prefix by computing longest common prefixes and then iterating over those. While we could move this logic into `refs_for_each_ref_ext()`, this one feels somewhat special as we perform multiple iterations. But what we _can_ do is to generalize how this function works: instead of accepting only a small handful of parameters, we can have it accept the full options structure. One obvious exception is that the caller must not provide a prefix via the options. But this case can be easily detected. Refactor the code accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ls-refs.c | 11 +++++++---- ref-filter.c | 11 +++++++---- refs.c | 39 +++++++++++++++------------------------ refs.h | 16 +++++----------- 4 files changed, 34 insertions(+), 43 deletions(-) diff --git a/ls-refs.c b/ls-refs.c index 8641281b86c55a..9759826ca70bd5 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -160,6 +160,7 @@ static int ls_refs_config(const char *var, const char *value, int ls_refs(struct repository *r, struct packet_reader *request) { + struct refs_for_each_ref_options opts = { 0 }; struct ls_refs_data data; memset(&data, 0, sizeof(data)); @@ -201,10 +202,12 @@ int ls_refs(struct repository *r, struct packet_reader *request) send_possibly_unborn_head(&data); if (!data.prefixes.nr) strvec_push(&data.prefixes, ""); - refs_for_each_fullref_in_prefixes(get_main_ref_store(r), - get_git_namespace(), data.prefixes.v, - hidden_refs_to_excludes(&data.hidden_refs), - send_ref, &data); + + opts.exclude_patterns = hidden_refs_to_excludes(&data.hidden_refs); + opts.namespace = get_git_namespace(); + + refs_for_each_ref_in_prefixes(get_main_ref_store(r), data.prefixes.v, + &opts, send_ref, &data); packet_fflush(stdout); strvec_clear(&data.prefixes); strbuf_release(&data.buf); diff --git a/ref-filter.c b/ref-filter.c index 049e845a1909fe..7c682e0a3395ba 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2807,6 +2807,10 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter, refs_for_each_cb cb, void *cb_data) { + struct refs_for_each_ref_options opts = { + .exclude_patterns = filter->exclude.v, + }; + if (filter->kind & FILTER_REFS_ROOT_REFS) { /* In this case, we want to print all refs including root refs. */ return for_each_fullref_with_seek(filter, cb, cb_data, @@ -2836,10 +2840,9 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter, return for_each_fullref_with_seek(filter, cb, cb_data, 0); } - return refs_for_each_fullref_in_prefixes(get_main_ref_store(the_repository), - NULL, filter->name_patterns, - filter->exclude.v, - cb, cb_data); + return refs_for_each_ref_in_prefixes(get_main_ref_store(the_repository), + filter->name_patterns, &opts, + cb, cb_data); } /* diff --git a/refs.c b/refs.c index 0d0f0edbfb1140..0aa3b68dd90db8 100644 --- a/refs.c +++ b/refs.c @@ -2039,40 +2039,31 @@ static void find_longest_prefixes(struct string_list *out, strbuf_release(&prefix); } -int refs_for_each_fullref_in_prefixes(struct ref_store *ref_store, - const char *namespace, - const char **patterns, - const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data) +int refs_for_each_ref_in_prefixes(struct ref_store *ref_store, + const char **prefixes, + const struct refs_for_each_ref_options *opts, + refs_for_each_cb cb, void *cb_data) { - struct strvec namespaced_exclude_patterns = STRVEC_INIT; - struct string_list prefixes = STRING_LIST_INIT_DUP; + struct string_list longest_prefixes = STRING_LIST_INIT_DUP; struct string_list_item *prefix; - struct strbuf buf = STRBUF_INIT; - int ret = 0, namespace_len; + int ret = 0; - find_longest_prefixes(&prefixes, patterns); + if (opts->prefix) + BUG("refs_for_each_ref_in_prefixes called with specific prefix"); - if (namespace) - strbuf_addstr(&buf, namespace); - namespace_len = buf.len; + find_longest_prefixes(&longest_prefixes, prefixes); - exclude_patterns = get_namespaced_exclude_patterns(exclude_patterns, - namespace, - &namespaced_exclude_patterns); + for_each_string_list_item(prefix, &longest_prefixes) { + struct refs_for_each_ref_options prefix_opts = *opts; + prefix_opts.prefix = prefix->string; - for_each_string_list_item(prefix, &prefixes) { - strbuf_addstr(&buf, prefix->string); - ret = refs_for_each_fullref_in(ref_store, buf.buf, - exclude_patterns, fn, cb_data); + ret = refs_for_each_ref_ext(ref_store, cb, cb_data, + &prefix_opts); if (ret) break; - strbuf_setlen(&buf, namespace_len); } - strvec_clear(&namespaced_exclude_patterns); - string_list_clear(&prefixes, 0); - strbuf_release(&buf); + string_list_clear(&longest_prefixes, 0); return ret; } diff --git a/refs.h b/refs.h index 5a5fb4e1e426b8..faed63aa812718 100644 --- a/refs.h +++ b/refs.h @@ -521,19 +521,13 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, refs_for_each_cb fn, void *cb_data); /** - * iterate all refs in "patterns" by partitioning patterns into disjoint sets + * Iterate all refs in "prefixes" by partitioning prefixes into disjoint sets * and iterating the longest-common prefix of each set. - * - * references matching any pattern in "exclude_patterns" are omitted from the - * result set on a best-effort basis. - * - * callers should be prepared to ignore references that they did not ask for. */ -int refs_for_each_fullref_in_prefixes(struct ref_store *refs, - const char *namespace, - const char **patterns, - const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data); +int refs_for_each_ref_in_prefixes(struct ref_store *refs, + const char **prefixes, + const struct refs_for_each_ref_options *opts, + refs_for_each_cb cb, void *cb_data); /* iterates all refs that match the specified glob pattern. */ int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, From 5507200b504f478516bf93767ac3ed3bebed7226 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:44 +0100 Subject: [PATCH 27/40] refs: improve verification for-each-ref options Improve verification of the passed-in for-each-ref options: - Require that the `refs` store must be given. It's arguably very surprising that we simply return successfully in case the ref store is a `NULL` pointer. - When expected to trim ref prefixes we will `BUG()` in case the refname would become empty or in case we're expected to trim a longer prefix than the refname is long. As such, this case is only guaranteed to _not_ `BUG()` in case the caller also specified a prefix. And furthermore, that prefix must end in a trailing slash, as otherwise it may produce an exact match that could lead us to trim to the empty string. An audit shows that there are no callsites that rely on either of these behaviours, so this should not result in a functional change. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 0aa3b68dd90db8..a57eafd6de607c 100644 --- a/refs.c +++ b/refs.c @@ -1859,7 +1859,18 @@ int refs_for_each_ref_ext(struct ref_store *refs, int ret; if (!refs) - return 0; + BUG("no ref store passed"); + + if (opts->trim_prefix) { + size_t prefix_len; + + if (!opts->prefix) + BUG("trimming only allowed with a prefix"); + + prefix_len = strlen(opts->prefix); + if (prefix_len == opts->trim_prefix && opts->prefix[prefix_len - 1] != '/') + BUG("ref pattern must end in a trailing slash when trimming"); + } if (opts->pattern) { if (!opts->prefix && !starts_with(opts->pattern, "refs/")) From 00be226f1f2a1036ea3920f8700b23b7cc55bf57 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:45 +0100 Subject: [PATCH 28/40] refs: replace `refs_for_each_ref_in()` Replace calls to `refs_for_each_ref_in()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- bisect.c | 8 ++++++-- builtin/rev-parse.c | 13 +++++++++---- pack-bitmap.c | 13 +++++++------ refs.c | 34 ++++++++++++++++++---------------- refs.h | 2 -- t/helper/test-ref-store.c | 7 +++++-- 6 files changed, 45 insertions(+), 32 deletions(-) diff --git a/bisect.c b/bisect.c index 2bdad4ee42e937..296836c15450c9 100644 --- a/bisect.c +++ b/bisect.c @@ -473,8 +473,12 @@ static int register_ref(const struct reference *ref, void *cb_data UNUSED) static int read_bisect_refs(void) { - return refs_for_each_ref_in(get_main_ref_store(the_repository), - "refs/bisect/", register_ref, NULL); + struct refs_for_each_ref_options opts = { + .prefix = "refs/bisect/", + .trim_prefix = strlen("refs/bisect/"), + }; + return refs_for_each_ref_ext(get_main_ref_store(the_repository), + register_ref, NULL, &opts); } static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 9032cc6327c004..02703f2fb88987 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -613,13 +613,18 @@ static int opt_with_value(const char *arg, const char *opt, const char **value) static void handle_ref_opt(const char *pattern, const char *prefix) { - if (pattern) + if (pattern) { refs_for_each_glob_ref_in(get_main_ref_store(the_repository), show_reference, pattern, prefix, NULL); - else - refs_for_each_ref_in(get_main_ref_store(the_repository), - prefix, show_reference, NULL); + } else { + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .trim_prefix = strlen(prefix), + }; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_reference, NULL, &opts); + } clear_ref_exclusions(&ref_excludes); } diff --git a/pack-bitmap.c b/pack-bitmap.c index efef7081e6a004..22419bfb33dd4e 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -3326,6 +3326,7 @@ static const struct string_list *bitmap_preferred_tips(struct repository *r) void for_each_preferred_bitmap_tip(struct repository *repo, refs_for_each_cb cb, void *cb_data) { + struct refs_for_each_ref_options opts = { 0 }; struct string_list_item *item; const struct string_list *preferred_tips; struct strbuf buf = STRBUF_INIT; @@ -3335,16 +3336,16 @@ void for_each_preferred_bitmap_tip(struct repository *repo, return; for_each_string_list_item(item, preferred_tips) { - const char *pattern = item->string; + opts.prefix = item->string; - if (!ends_with(pattern, "/")) { + if (!ends_with(opts.prefix, "/")) { strbuf_reset(&buf); - strbuf_addf(&buf, "%s/", pattern); - pattern = buf.buf; + strbuf_addf(&buf, "%s/", opts.prefix); + opts.prefix = buf.buf; } - refs_for_each_ref_in(get_main_ref_store(repo), - pattern, cb, cb_data); + refs_for_each_ref_ext(get_main_ref_store(repo), + cb, cb_data, &opts); } strbuf_release(&buf); diff --git a/refs.c b/refs.c index a57eafd6de607c..7b1ef769c0ee9d 100644 --- a/refs.c +++ b/refs.c @@ -529,19 +529,31 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, refs_for_each_rawref(refs, warn_if_dangling_symref, &data); } -int refs_for_each_tag_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) +int refs_for_each_tag_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { - return refs_for_each_ref_in(refs, "refs/tags/", fn, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = "refs/tags/", + .trim_prefix = strlen("refs/tags/"), + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_branch_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) +int refs_for_each_branch_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { - return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = "refs/heads/", + .trim_prefix = strlen("refs/heads/"), + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_remote_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) +int refs_for_each_remote_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { - return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data); + struct refs_for_each_ref_options opts = { + .prefix = "refs/remotes/", + .trim_prefix = strlen("refs/remotes/"), + }; + return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } int refs_head_ref_namespaced(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) @@ -1934,16 +1946,6 @@ int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb cb, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .prefix = prefix, - .trim_prefix = strlen(prefix), - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, const char **exclude_patterns, refs_for_each_cb cb, void *cb_data) diff --git a/refs.h b/refs.h index faed63aa812718..7a3bc9e5b70200 100644 --- a/refs.h +++ b/refs.h @@ -501,8 +501,6 @@ int refs_for_each_ref(struct ref_store *refs, int refs_for_each_ref_ext(struct ref_store *refs, refs_for_each_cb cb, void *cb_data, const struct refs_for_each_ref_options *opts); -int refs_for_each_ref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb fn, void *cb_data); int refs_for_each_tag_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); int refs_for_each_branch_ref(struct ref_store *refs, diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index b1215947c5e67a..a2ef1b69495abf 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -163,8 +163,11 @@ static int each_ref(const struct reference *ref, void *cb_data UNUSED) static int cmd_for_each_ref(struct ref_store *refs, const char **argv) { const char *prefix = notnull(*argv++, "prefix"); - - return refs_for_each_ref_in(refs, prefix, each_ref, NULL); + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .trim_prefix = strlen(prefix), + }; + return refs_for_each_ref_ext(refs, each_ref, NULL, &opts); } static int cmd_for_each_ref__exclude(struct ref_store *refs, const char **argv) From 67034081adc956c4dc8b51f59a9b70b8861c4642 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:46 +0100 Subject: [PATCH 29/40] refs: replace `refs_for_each_rawref()` Replace calls to `refs_for_each_rawref()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/describe.c | 7 +++++-- builtin/fsck.c | 7 +++++-- fetch-pack.c | 15 +++++++++++---- refs.c | 10 ++++------ refs.h | 1 - refs/files-backend.c | 7 +++++-- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index abfe3525a5385b..bffeed13a3cb14 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -641,6 +641,9 @@ int cmd_describe(int argc, const char *prefix, struct repository *repo UNUSED ) { + struct refs_for_each_ref_options for_each_ref_opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; int contains = 0; struct option options[] = { OPT_BOOL(0, "contains", &contains, N_("find the tag that comes after the commit")), @@ -738,8 +741,8 @@ int cmd_describe(int argc, } hashmap_init(&names, commit_name_neq, NULL, 0); - refs_for_each_rawref(get_main_ref_store(the_repository), get_name, - NULL); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + get_name, NULL, &for_each_ref_opts); if (!hashmap_get_size(&names) && !always) die(_("No names found, cannot describe anything.")); diff --git a/builtin/fsck.c b/builtin/fsck.c index 0512f78a87fe1d..24cdb657f5f2f0 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -598,6 +598,9 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED) static void snapshot_refs(struct snapshot *snap, int argc, const char **argv) { + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; struct worktree **worktrees, **p; const char *head_points_at; struct object_id head_oid; @@ -623,8 +626,8 @@ static void snapshot_refs(struct snapshot *snap, int argc, const char **argv) return; } - refs_for_each_rawref(get_main_ref_store(the_repository), - snapshot_ref, snap); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + snapshot_ref, snap, &opts); worktrees = get_worktrees(); for (p = worktrees; *p; p++) { diff --git a/fetch-pack.c b/fetch-pack.c index 40316c9a348f23..570caa03fab33d 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -292,11 +292,14 @@ static int next_flush(int stateless_rpc, int count) static void mark_tips(struct fetch_negotiator *negotiator, const struct oid_array *negotiation_tips) { + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; int i; if (!negotiation_tips) { - refs_for_each_rawref(get_main_ref_store(the_repository), - rev_list_insert_ref_oid, negotiator); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + rev_list_insert_ref_oid, negotiator, &opts); return; } @@ -792,8 +795,12 @@ static void mark_complete_and_common_ref(struct fetch_negotiator *negotiator, */ trace2_region_enter("fetch-pack", "mark_complete_local_refs", NULL); if (!args->deepen) { - refs_for_each_rawref(get_main_ref_store(the_repository), - mark_complete_oid, NULL); + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; + + refs_for_each_ref_ext(get_main_ref_store(the_repository), + mark_complete_oid, NULL, &opts); for_each_cached_alternate(NULL, mark_alternate_complete); if (cutoff) mark_recent_complete_commits(args, cutoff); diff --git a/refs.c b/refs.c index 7b1ef769c0ee9d..791654a0f6c4c5 100644 --- a/refs.c +++ b/refs.c @@ -526,7 +526,10 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, .indent = indent, .dry_run = dry_run, }; - refs_for_each_rawref(refs, warn_if_dangling_symref, &data); + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; + refs_for_each_ref_ext(refs, warn_if_dangling_symref, &data, &opts); } int refs_for_each_tag_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) @@ -1979,11 +1982,6 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data) -{ - return refs_for_each_rawref_in(refs, "", fn, cb_data); -} - int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, refs_for_each_cb cb, void *cb_data) { diff --git a/refs.h b/refs.h index 7a3bc9e5b70200..01dc3c2fd40ec9 100644 --- a/refs.h +++ b/refs.h @@ -543,7 +543,6 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); /* can be used to learn about broken ref and symref */ -int refs_for_each_rawref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, refs_for_each_cb fn, void *cb_data); diff --git a/refs/files-backend.c b/refs/files-backend.c index 6c98e14414197b..ab967607811142 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3149,6 +3149,9 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, struct ref_transaction *transaction, struct strbuf *err) { + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; size_t i; int ret = 0; struct string_list affected_refnames = STRING_LIST_INIT_NODUP; @@ -3173,8 +3176,8 @@ static int files_transaction_finish_initial(struct files_ref_store *refs, * so here we really only check that none of the references * that we are creating already exists. */ - if (refs_for_each_rawref(&refs->base, ref_present, - &transaction->refnames)) + if (refs_for_each_ref_ext(&refs->base, ref_present, + &transaction->refnames, &opts)) BUG("initial ref transaction called with existing refs"); packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, From 5ef6d593f18ac6b1e7540eed45f5d795d5a82faa Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:47 +0100 Subject: [PATCH 30/40] refs: replace `refs_for_each_rawref_in()` Replace calls to `refs_for_each_rawref_in()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/remote.c | 8 ++++++-- refs.c | 10 ---------- refs.h | 4 ---- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index ace390c671d61c..0fddaa177331f6 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -912,6 +912,9 @@ static int mv(int argc, const char **argv, const char *prefix, old_remote_context.buf); if (refspecs_need_update) { + struct refs_for_each_ref_options opts = { + .flags = REFS_FOR_EACH_INCLUDE_BROKEN, + }; rename.transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), 0, &err); if (!rename.transaction) @@ -923,9 +926,10 @@ static int mv(int argc, const char **argv, const char *prefix, strbuf_reset(&buf); strbuf_addf(&buf, "refs/remotes/%s/", rename.old_name); + opts.prefix = buf.buf; - result = refs_for_each_rawref_in(get_main_ref_store(the_repository), buf.buf, - rename_one_ref, &rename); + result = refs_for_each_ref_ext(get_main_ref_store(the_repository), + rename_one_ref, &rename, &opts); if (result < 0) die(_("queueing remote ref renames failed: %s"), rename.err->buf); diff --git a/refs.c b/refs.c index 791654a0f6c4c5..172d4cf941f464 100644 --- a/refs.c +++ b/refs.c @@ -1982,16 +1982,6 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb cb, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .prefix = prefix, - .flags = REFS_FOR_EACH_INCLUDE_BROKEN, - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - static int qsort_strcmp(const void *va, const void *vb) { const char *a = *(const char **)va; diff --git a/refs.h b/refs.h index 01dc3c2fd40ec9..673d4ccce50d61 100644 --- a/refs.h +++ b/refs.h @@ -542,10 +542,6 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, const char **exclude_patterns, refs_for_each_cb fn, void *cb_data); -/* can be used to learn about broken ref and symref */ -int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, - refs_for_each_cb fn, void *cb_data); - /* * Normalizes partial refs to their fully qualified form. * Will prepend to the if it doesn't start with 'refs/'. From 4091d2989353aaf14080ee64ee2e94b60ceaf18d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:48 +0100 Subject: [PATCH 31/40] refs: replace `refs_for_each_glob_ref_in()` Replace calls to `refs_for_each_glob_ref_in()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/bisect.c | 37 +++++++++++++++++++++++++++---------- builtin/rev-parse.c | 10 +++++++--- refs.c | 11 ----------- refs.h | 3 --- revision.c | 30 +++++++++++++++++++++--------- 5 files changed, 55 insertions(+), 36 deletions(-) diff --git a/builtin/bisect.c b/builtin/bisect.c index 4cc118fb57df59..4520e585d0677f 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -422,13 +422,17 @@ static void bisect_status(struct bisect_state *state, { char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad); char *good_glob = xstrfmt("%s-*", terms->term_good); + struct refs_for_each_ref_options opts = { + .pattern = good_glob, + .prefix = "refs/bisect/", + .trim_prefix = strlen("refs/bisect/"), + }; if (refs_ref_exists(get_main_ref_store(the_repository), bad_ref)) state->nr_bad = 1; - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), inc_nr, - good_glob, "refs/bisect/", - (void *) &state->nr_good); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + inc_nr, &state->nr_good, &opts); free(good_glob); free(bad_ref); @@ -562,6 +566,10 @@ static int add_bisect_ref(const struct reference *ref, void *cb) static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs) { + struct refs_for_each_ref_options opts = { + .prefix = "refs/bisect/", + .trim_prefix = strlen("refs/bisect/"), + }; int res = 0; struct add_bisect_ref_data cb = { revs }; char *good = xstrfmt("%s-*", terms->term_good); @@ -581,11 +589,16 @@ static int prepare_revs(struct bisect_terms *terms, struct rev_info *revs) reset_revision_walk(); repo_init_revisions(the_repository, revs, NULL); setup_revisions(0, NULL, revs, NULL); - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - add_bisect_ref, bad, "refs/bisect/", &cb); + + opts.pattern = bad; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + add_bisect_ref, &cb, &opts); + cb.object_flags = UNINTERESTING; - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - add_bisect_ref, good, "refs/bisect/", &cb); + opts.pattern = good; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + add_bisect_ref, &cb, &opts); + if (prepare_revision_walk(revs)) res = error(_("revision walk setup failed")); @@ -1191,10 +1204,14 @@ static int verify_good(const struct bisect_terms *terms, const char *command) char *good_glob = xstrfmt("%s-*", terms->term_good); int no_checkout = refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"); + struct refs_for_each_ref_options opts = { + .pattern = good_glob, + .prefix = "refs/bisect/", + .trim_prefix = strlen("refs/bisect/"), + }; - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - get_first_good, good_glob, "refs/bisect/", - &good_rev); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + get_first_good, &good_rev, &opts); free(good_glob); if (refs_read_ref(get_main_ref_store(the_repository), no_checkout ? "BISECT_HEAD" : "HEAD", ¤t_rev)) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 02703f2fb88987..61a3f0fdb99b36 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -614,9 +614,13 @@ static int opt_with_value(const char *arg, const char *opt, const char **value) static void handle_ref_opt(const char *pattern, const char *prefix) { if (pattern) { - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - show_reference, pattern, prefix, - NULL); + struct refs_for_each_ref_options opts = { + .pattern = pattern, + .prefix = prefix, + .trim_prefix = prefix ? strlen(prefix) : 0, + }; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_reference, NULL, &opts); } else { struct refs_for_each_ref_options opts = { .prefix = prefix, diff --git a/refs.c b/refs.c index 172d4cf941f464..b4ef4ffff05729 100644 --- a/refs.c +++ b/refs.c @@ -607,17 +607,6 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix, strbuf_release(&normalized_pattern); } -int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb cb, - const char *pattern, const char *prefix, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .pattern = pattern, - .prefix = prefix, - .trim_prefix = prefix ? strlen(prefix) : 0, - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb cb, const char *pattern, void *cb_data) { diff --git a/refs.h b/refs.h index 673d4ccce50d61..3fa2c11c1f1d25 100644 --- a/refs.h +++ b/refs.h @@ -531,9 +531,6 @@ int refs_for_each_ref_in_prefixes(struct ref_store *refs, int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, const char *pattern, void *cb_data); -int refs_for_each_glob_ref_in(struct ref_store *refs, refs_for_each_cb fn, - const char *pattern, const char *prefix, void *cb_data); - /* * references matching any pattern in "exclude_patterns" are omitted from the * result set on a best-effort basis. diff --git a/revision.c b/revision.c index 8c206830d5e4a6..074a75b859c592 100644 --- a/revision.c +++ b/revision.c @@ -2827,34 +2827,46 @@ static int handle_revision_pseudo_opt(struct rev_info *revs, exclude_hidden_refs(&revs->ref_excludes, optarg); return argcount; } else if (skip_prefix(arg, "--branches=", &optarg)) { + struct refs_for_each_ref_options opts = { + .prefix = "refs/heads/", + .trim_prefix = strlen("refs/heads/"), + .pattern = optarg, + }; struct all_refs_cb cb; if (revs->ref_excludes.hidden_refs_configured) return error(_("options '%s' and '%s' cannot be used together"), "--exclude-hidden", "--branches"); init_all_refs_cb(&cb, revs, *flags); - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - handle_one_ref, optarg, - "refs/heads/", &cb); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + handle_one_ref, &cb, &opts); clear_ref_exclusions(&revs->ref_excludes); } else if (skip_prefix(arg, "--tags=", &optarg)) { + struct refs_for_each_ref_options opts = { + .prefix = "refs/tags/", + .trim_prefix = strlen("refs/tags/"), + .pattern = optarg, + }; struct all_refs_cb cb; if (revs->ref_excludes.hidden_refs_configured) return error(_("options '%s' and '%s' cannot be used together"), "--exclude-hidden", "--tags"); init_all_refs_cb(&cb, revs, *flags); - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - handle_one_ref, optarg, - "refs/tags/", &cb); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + handle_one_ref, &cb, &opts); clear_ref_exclusions(&revs->ref_excludes); } else if (skip_prefix(arg, "--remotes=", &optarg)) { + struct refs_for_each_ref_options opts = { + .prefix = "refs/remotes/", + .trim_prefix = strlen("refs/remotes/"), + .pattern = optarg, + }; struct all_refs_cb cb; if (revs->ref_excludes.hidden_refs_configured) return error(_("options '%s' and '%s' cannot be used together"), "--exclude-hidden", "--remotes"); init_all_refs_cb(&cb, revs, *flags); - refs_for_each_glob_ref_in(get_main_ref_store(the_repository), - handle_one_ref, optarg, - "refs/remotes/", &cb); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + handle_one_ref, &cb, &opts); clear_ref_exclusions(&revs->ref_excludes); } else if (!strcmp(arg, "--reflog")) { add_reflogs_to_pending(revs, *flags); From 3fc1ad03c6243b44c2dcab480acaced44921b1c5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:49 +0100 Subject: [PATCH 32/40] refs: replace `refs_for_each_glob_ref()` Replace calls to `refs_for_each_glob_ref()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 7 +++++-- notes.c | 7 +++++-- refs.c | 9 --------- refs.h | 4 ---- revision.c | 7 +++++-- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index a3bc7e9380b9b6..a3323fbfd7387c 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1542,6 +1542,9 @@ static void add_negotiation_tips(struct git_transport_options *smart_options) for (i = 0; i < negotiation_tip.nr; i++) { const char *s = negotiation_tip.items[i].string; + struct refs_for_each_ref_options opts = { + .pattern = s, + }; int old_nr; if (!has_glob_specials(s)) { struct object_id oid; @@ -1553,8 +1556,8 @@ static void add_negotiation_tips(struct git_transport_options *smart_options) continue; } old_nr = oids->nr; - refs_for_each_glob_ref(get_main_ref_store(the_repository), - add_oid, s, oids); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + add_oid, oids, &opts); if (old_nr == oids->nr) warning("ignoring --negotiation-tip=%s because it does not match any refs", s); diff --git a/notes.c b/notes.c index 090c48bbd52303..51a7ef9f830a13 100644 --- a/notes.c +++ b/notes.c @@ -952,8 +952,11 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob) { assert(list->strdup_strings); if (has_glob_specials(glob)) { - refs_for_each_glob_ref(get_main_ref_store(the_repository), - string_list_add_one_ref, glob, list); + struct refs_for_each_ref_options opts = { + .pattern = glob, + }; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + string_list_add_one_ref, list, &opts); } else { struct object_id oid; if (repo_get_oid(the_repository, glob, &oid)) diff --git a/refs.c b/refs.c index b4ef4ffff05729..ca7fc7289bc505 100644 --- a/refs.c +++ b/refs.c @@ -607,15 +607,6 @@ void normalize_glob_ref(struct string_list_item *item, const char *prefix, strbuf_release(&normalized_pattern); } -int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb cb, - const char *pattern, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .pattern = pattern, - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - const char *prettify_refname(const char *name) { if (skip_prefix(name, "refs/heads/", &name) || diff --git a/refs.h b/refs.h index 3fa2c11c1f1d25..b63775fa352d2e 100644 --- a/refs.h +++ b/refs.h @@ -527,10 +527,6 @@ int refs_for_each_ref_in_prefixes(struct ref_store *refs, const struct refs_for_each_ref_options *opts, refs_for_each_cb cb, void *cb_data); -/* iterates all refs that match the specified glob pattern. */ -int refs_for_each_glob_ref(struct ref_store *refs, refs_for_each_cb fn, - const char *pattern, void *cb_data); - /* * references matching any pattern in "exclude_patterns" are omitted from the * result set on a best-effort basis. diff --git a/revision.c b/revision.c index 074a75b859c592..4ddb3370c6cb10 100644 --- a/revision.c +++ b/revision.c @@ -2814,10 +2814,13 @@ static int handle_revision_pseudo_opt(struct rev_info *revs, handle_refs(refs, revs, *flags, refs_for_each_remote_ref); clear_ref_exclusions(&revs->ref_excludes); } else if ((argcount = parse_long_opt("glob", argv, &optarg))) { + struct refs_for_each_ref_options opts = { + .pattern = optarg, + }; struct all_refs_cb cb; init_all_refs_cb(&cb, revs, *flags); - refs_for_each_glob_ref(get_main_ref_store(the_repository), - handle_one_ref, optarg, &cb); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + handle_one_ref, &cb, &opts); clear_ref_exclusions(&revs->ref_excludes); return argcount; } else if ((argcount = parse_long_opt("exclude", argv, &optarg))) { From 96c35a9ba5f1b5afac1e30ca93815dcd540d8b16 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:50 +0100 Subject: [PATCH 33/40] refs: replace `refs_for_each_namespaced_ref()` Replace calls to `refs_for_each_namespaced_ref()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- http-backend.c | 8 ++++++-- refs.c | 11 ----------- refs.h | 8 -------- upload-pack.c | 11 +++++++---- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/http-backend.c b/http-backend.c index 0122146df607b2..1a171c5c5a0b02 100644 --- a/http-backend.c +++ b/http-backend.c @@ -565,9 +565,13 @@ static void get_info_refs(struct strbuf *hdr, char *arg UNUSED) run_service(argv, 0); } else { + struct refs_for_each_ref_options opts = { + .namespace = get_git_namespace(), + }; + select_getanyfile(hdr); - refs_for_each_namespaced_ref(get_main_ref_store(the_repository), - NULL, show_text_ref, &buf); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_text_ref, &buf, &opts); send_strbuf(hdr, "text/plain", &buf); } strbuf_release(&buf); diff --git a/refs.c b/refs.c index ca7fc7289bc505..35a4925ac4e50d 100644 --- a/refs.c +++ b/refs.c @@ -1951,17 +1951,6 @@ int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb cb, void return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_namespaced_ref(struct ref_store *refs, - const char **exclude_patterns, - refs_for_each_cb cb, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .exclude_patterns = exclude_patterns, - .namespace = get_git_namespace(), - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - static int qsort_strcmp(const void *va, const void *vb) { const char *a = *(const char **)va; diff --git a/refs.h b/refs.h index b63775fa352d2e..1b468c4ffb7166 100644 --- a/refs.h +++ b/refs.h @@ -527,14 +527,6 @@ int refs_for_each_ref_in_prefixes(struct ref_store *refs, const struct refs_for_each_ref_options *opts, refs_for_each_cb cb, void *cb_data); -/* - * references matching any pattern in "exclude_patterns" are omitted from the - * result set on a best-effort basis. - */ -int refs_for_each_namespaced_ref(struct ref_store *refs, - const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data); - /* * Normalizes partial refs to their fully qualified form. * Will prepend to the if it doesn't start with 'refs/'. diff --git a/upload-pack.c b/upload-pack.c index 7fe397b0d09bb3..d21f0577f982fa 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -610,7 +610,10 @@ static int allow_hidden_refs(enum allow_uor allow_uor) static void for_each_namespaced_ref_1(refs_for_each_cb fn, struct upload_pack_data *data) { - const char **excludes = NULL; + struct refs_for_each_ref_options opts = { + .namespace = get_git_namespace(), + }; + /* * If `data->allow_uor` allows fetching hidden refs, we need to * mark all references (including hidden ones), to check in @@ -621,10 +624,10 @@ static void for_each_namespaced_ref_1(refs_for_each_cb fn, * hidden references. */ if (allow_hidden_refs(data->allow_uor)) - excludes = hidden_refs_to_excludes(&data->hidden_refs); + opts.exclude_patterns = hidden_refs_to_excludes(&data->hidden_refs); - refs_for_each_namespaced_ref(get_main_ref_store(the_repository), - excludes, fn, data); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + fn, data, &opts); } From 1dd4f1e43f8f11ebb13c1b9edbd91219a134443d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 23 Feb 2026 12:59:51 +0100 Subject: [PATCH 34/40] refs: replace `refs_for_each_fullref_in()` Replace calls to `refs_for_each_fullref_in()` with the newly introduced `refs_for_each_ref_ext()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- bisect.c | 8 +++++--- builtin/receive-pack.c | 8 ++++---- builtin/rev-parse.c | 15 +++++++-------- builtin/show-ref.c | 21 +++++++++++++-------- refs.c | 11 ----------- refs.h | 8 -------- revision.c | 4 +++- t/helper/test-ref-store.c | 8 +++++--- 8 files changed, 37 insertions(+), 46 deletions(-) diff --git a/bisect.c b/bisect.c index 296836c15450c9..ef17a442e55d2c 100644 --- a/bisect.c +++ b/bisect.c @@ -1190,13 +1190,15 @@ static int mark_for_removal(const struct reference *ref, void *cb_data) int bisect_clean_state(void) { + struct refs_for_each_ref_options opts = { + .prefix = "refs/bisect/", + }; int result = 0; /* There may be some refs packed during bisection */ struct string_list refs_for_removal = STRING_LIST_INIT_DUP; - refs_for_each_fullref_in(get_main_ref_store(the_repository), - "refs/bisect/", NULL, mark_for_removal, - &refs_for_removal); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + mark_for_removal, &refs_for_removal, &opts); string_list_append(&refs_for_removal, "BISECT_HEAD"); string_list_append(&refs_for_removal, "BISECT_EXPECTED_REV"); result = refs_delete_refs(get_main_ref_store(the_repository), diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 4c0112b4bc2189..8c5ad5b81e9bd6 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -343,9 +343,9 @@ static void show_one_alternate_ref(const struct object_id *oid, static void write_head_info(void) { + struct refs_for_each_ref_options opts = { 0 }; static struct oidset seen = OIDSET_INIT; struct strvec excludes_vector = STRVEC_INIT; - const char **exclude_patterns; /* * We need access to the reference names both with and without their @@ -353,12 +353,12 @@ static void write_head_info(void) * thus have to adapt exclude patterns to carry the namespace prefix * ourselves. */ - exclude_patterns = get_namespaced_exclude_patterns( + opts.exclude_patterns = get_namespaced_exclude_patterns( hidden_refs_to_excludes(&hidden_refs), get_git_namespace(), &excludes_vector); - refs_for_each_fullref_in(get_main_ref_store(the_repository), "", - exclude_patterns, show_ref_cb, &seen); + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_ref_cb, &seen, &opts); odb_for_each_alternate_ref(the_repository->objects, show_one_alternate_ref, &seen); diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 61a3f0fdb99b36..01a62800e87938 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -940,14 +940,13 @@ int cmd_rev_parse(int argc, continue; } if (!strcmp(arg, "--bisect")) { - refs_for_each_fullref_in(get_main_ref_store(the_repository), - "refs/bisect/bad", - NULL, show_reference, - NULL); - refs_for_each_fullref_in(get_main_ref_store(the_repository), - "refs/bisect/good", - NULL, anti_reference, - NULL); + struct refs_for_each_ref_options opts = { 0 }; + opts.prefix = "refs/bisect/bad"; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_reference, NULL, &opts); + opts.prefix = "refs/bisect/good"; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + anti_reference, NULL, &opts); continue; } if (opt_with_value(arg, "--branches", &arg)) { diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 4d4984e4e0c244..5d31acea7c779a 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -215,14 +215,19 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts, refs_head_ref(get_main_ref_store(the_repository), show_ref, &show_ref_data); if (opts->branches_only || opts->tags_only) { - if (opts->branches_only) - refs_for_each_fullref_in(get_main_ref_store(the_repository), - "refs/heads/", NULL, - show_ref, &show_ref_data); - if (opts->tags_only) - refs_for_each_fullref_in(get_main_ref_store(the_repository), - "refs/tags/", NULL, show_ref, - &show_ref_data); + struct refs_for_each_ref_options for_each_ref_opts = { 0 }; + + if (opts->branches_only) { + for_each_ref_opts.prefix = "refs/heads/"; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_ref, &show_ref_data, &for_each_ref_opts); + } + + if (opts->tags_only) { + for_each_ref_opts.prefix = "refs/tags/"; + refs_for_each_ref_ext(get_main_ref_store(the_repository), + show_ref, &show_ref_data, &for_each_ref_opts); + } } else { refs_for_each_ref(get_main_ref_store(the_repository), show_ref, &show_ref_data); diff --git a/refs.c b/refs.c index 35a4925ac4e50d..af51a648d59d49 100644 --- a/refs.c +++ b/refs.c @@ -1929,17 +1929,6 @@ int refs_for_each_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data return refs_for_each_ref_ext(refs, cb, cb_data, &opts); } -int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, - const char **exclude_patterns, - refs_for_each_cb cb, void *cb_data) -{ - struct refs_for_each_ref_options opts = { - .prefix = prefix, - .exclude_patterns = exclude_patterns, - }; - return refs_for_each_ref_ext(refs, cb, cb_data, &opts); -} - int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb cb, void *cb_data) { const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; diff --git a/refs.h b/refs.h index 1b468c4ffb7166..9b5d57a9b7994f 100644 --- a/refs.h +++ b/refs.h @@ -510,14 +510,6 @@ int refs_for_each_remote_ref(struct ref_store *refs, int refs_for_each_replace_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data); -/* - * references matching any pattern in "exclude_patterns" are omitted from the - * result set on a best-effort basis. - */ -int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix, - const char **exclude_patterns, - refs_for_each_cb fn, void *cb_data); - /** * Iterate all refs in "prefixes" by partitioning prefixes into disjoint sets * and iterating the longest-common prefix of each set. diff --git a/revision.c b/revision.c index 4ddb3370c6cb10..0136ef64f53358 100644 --- a/revision.c +++ b/revision.c @@ -2731,10 +2731,12 @@ void revision_opts_finish(struct rev_info *revs) static int for_each_bisect_ref(struct ref_store *refs, refs_for_each_cb fn, void *cb_data, const char *term) { + struct refs_for_each_ref_options opts = { 0 }; struct strbuf bisect_refs = STRBUF_INIT; int status; strbuf_addf(&bisect_refs, "refs/bisect/%s", term); - status = refs_for_each_fullref_in(refs, bisect_refs.buf, NULL, fn, cb_data); + opts.prefix = bisect_refs.buf; + status = refs_for_each_ref_ext(refs, fn, cb_data, &opts); strbuf_release(&bisect_refs); return status; } diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index a2ef1b69495abf..74edf2029a28fc 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -173,10 +173,12 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv) static int cmd_for_each_ref__exclude(struct ref_store *refs, const char **argv) { const char *prefix = notnull(*argv++, "prefix"); - const char **exclude_patterns = argv; + struct refs_for_each_ref_options opts = { + .prefix = prefix, + .exclude_patterns = argv, + }; - return refs_for_each_fullref_in(refs, prefix, exclude_patterns, each_ref, - NULL); + return refs_for_each_ref_ext(refs, each_ref, NULL, &opts); } static int cmd_resolve_ref(struct ref_store *refs, const char **argv) From a66c8c7f91b13ba56e734fe95ab8ad5e61ad6b45 Mon Sep 17 00:00:00 2001 From: K Jayatheerth Date: Mon, 23 Feb 2026 19:22:48 +0530 Subject: [PATCH 35/40] repo: remove unnecessary variable shadow Avoid redeclaring `entry` inside the conditional block, removing unnecessary variable shadowing and improving code clarity without changing behavior. Signed-off-by: K Jayatheerth Acked-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/repo.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/repo.c b/builtin/repo.c index e77e8db563d410..e952778bbf8dd6 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -276,7 +276,6 @@ static void stats_table_print_structure(const struct stats_table *table) const char *value = ""; if (entry) { - struct stats_table_entry *entry = item->util; value = entry->value; } From 1a9df8de368cbebd881336f64e424422f3dbb993 Mon Sep 17 00:00:00 2001 From: LorenzoPegorari Date: Fri, 27 Feb 2026 22:45:19 +0100 Subject: [PATCH 36/40] diff: handle ANSI escape codes in prefix when calculating diffstat width The diffstat width is calculated by taking the terminal width and incorrectly subtracting the `strlen()` of `line_prefix`, instead of the actual display width of `line_prefix`, which may contain ANSI escape codes (e.g., ANSI-colored strings in `log --graph --stat`). Utilize the display width instead, obtained via `utf8_strnwidth()` with the flag `skip_ansi`. Signed-off-by: LorenzoPegorari Signed-off-by: Junio C Hamano --- diff.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index a1961526c0dab1..9c848ec5db54b2 100644 --- a/diff.c +++ b/diff.c @@ -2713,7 +2713,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) count = i; /* where we can stop scanning in data->files[] */ /* - * We have width = stat_width or term_columns() columns total. + * We have width = stat_width or term_columns() columns total minus the + * length of line_prefix skipping ANSI escape codes to get the display + * width (e.g., skip ANSI-colored strings in "log --graph --stat"). * We want a maximum of min(max_len, stat_name_width) for the name part. * We want a maximum of min(max_change, stat_graph_width) for the +- part. * We also need 1 for " " and 4 + decimal_width(max_change) @@ -2740,14 +2742,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) * separators and this message, this message will "overflow" * making the line longer than the maximum width. */ - - /* - * NEEDSWORK: line_prefix is often used for "log --graph" output - * and contains ANSI-colored string. utf8_strnwidth() should be - * used to correctly count the display width instead of strlen(). - */ if (options->stat_width == -1) - width = term_columns() - strlen(line_prefix); + width = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1); else width = options->stat_width ? options->stat_width : 80; number_width = decimal_width(max_change) > number_width ? From 064b869efc50be2557015665bc3dc8ac9008a6e4 Mon Sep 17 00:00:00 2001 From: LorenzoPegorari Date: Fri, 27 Feb 2026 22:48:35 +0100 Subject: [PATCH 37/40] t4052: test for diffstat width when prefix contains ANSI escape codes Add test checking the calculation of the diffstat display width when the `line_prefix`, which is text that goes before the diffstat, contains ANSI escape codes. This situation happens, for example, when `git log --stat --graph` is executed: * `--stat` will create a diffstat for each commit * `--graph` will stuff `line_prefix` with the graph portion of the log, which contains ANSI escape codes to color the text Signed-off-by: LorenzoPegorari Signed-off-by: Junio C Hamano --- t/t4052-stat-output.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 740bb9709181ab..7c749062e2b8d1 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -413,4 +413,36 @@ test_expect_success 'merge --stat respects COLUMNS with long name' ' test_cmp expect actual ' +# We want git-log to print only 1 commit containing a single branch graph and a +# diffstat (the diffstat display width, when not manually set through the +# option "--stat-width", will be automatically calculated). +# The diffstat will be only one file, with a placeholder FILENAME, that, with +# enough terminal display width, will contain the following line: +# "| ${FILENAME} | 0" +# where "" and "" are ANSI escape codes to color the text. +# To calculate the minimium terminal display width MIN_TERM_WIDTH so that the +# FILENAME in the diffstat will not be shortened, we take the FILENAME length +# and add 9 to it. +# To check if the diffstat width, when the line_prefix (the "|" of +# the graph) contains ANSI escape codes (the ANSI escape codes to color the +# text), is calculated correctly, we: +# 1. check if it contains the line defined before when using MIN_TERM_WIDTH +# 2. check if it contains the line defined before, but with the FILENAME +# shortened by only one character, when using MIN_TERM_WIDTH - 1 + +test_expect_success 'diffstat where line_prefix contains ANSI escape codes is correct width' ' + FILENAME="placeholder-text-placeholder-text" && + FILENAME_TRIMMED="...eholder-text-placeholder-text" && + MIN_TERM_WIDTH=$((${#FILENAME} + 9)) && + test_config color.diff always && + git commit --allow-empty --allow-empty-message && + >${FILENAME} && + git add ${FILENAME} && + git commit --allow-empty-message && + COLUMNS=$((MIN_TERM_WIDTH)) git log --graph --stat -n1 | test_decode_color >out && + test_grep "| ${FILENAME} | 0" out && + COLUMNS=$((MIN_TERM_WIDTH - 1)) git log --graph --stat -n1 | test_decode_color >out && + test_grep "| ${FILENAME_TRIMMED} | 0" out +' + test_done From 005f3fbe07a20dd5f7dea57f6f46cd797387e56a Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 2 Mar 2026 21:17:04 +0200 Subject: [PATCH 38/40] builtin/receive-pack: avoid spinning no-op sideband async threads Exit early if the hooks do not exist, to avoid spinning up/down sideband async threads which no-op. It is important to call the hook_exists() API provided by hook.[ch] because it covers both config-defined hooks and the "traditional" hooks from the hookdir. find_hook() only covers the hookdir hooks. The regression happened because the no-op async threads add some additional overhead which can be measured with the receive-refs test of the benchmarks suite [1]. Reproduced using: cd benchmarks/receive-refs && \ ./run --revisions /path/to/git \ fc148b146ad41be71a7852c4867f0773cbfe1ff9~,fc148b146ad41be71a7852c4867f0773cbfe1ff9 \ --parameter-list refformat reftable --parameter-list refcount 10000 1: https://gitlab.com/gitlab-org/data-access/git/benchmarks Fixes: fc148b146ad4 ("receive-pack: convert update hooks to new API") Reported-by: Patrick Steinhardt Helped-by: Jeff King Signed-off-by: Adrian Ratiu [jc: avoid duplicated hardcoded hook names] Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index b5379a48956994..e53aaf51041948 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -914,6 +914,9 @@ static int run_receive_hook(struct command *commands, int saved_stderr = -1; int ret; + if (!hook_exists(the_repository, hook_name)) + return 0; + /* if there are no valid commands, don't invoke the hook at all. */ while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) iter = iter->next; @@ -955,12 +958,16 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { + static const char hook_name[] = "update"; struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; struct async sideband_async; int sideband_async_started = 0; int saved_stderr = -1; int code; + if (!hook_exists(the_repository, hook_name)) + return 0; + strvec_pushl(&opt.args, cmd->ref_name, oid_to_hex(&cmd->old_oid), @@ -969,7 +976,7 @@ static int run_update_hook(struct command *cmd) prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - code = run_hooks_opt(the_repository, "update", &opt); + code = run_hooks_opt(the_repository, hook_name, &opt); finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); @@ -1649,12 +1656,16 @@ static const char *update(struct command *cmd, struct shallow_info *si) static void run_update_post_hook(struct command *commands) { + static const char hook_name[] = "post-update"; struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; struct async sideband_async; struct command *cmd; int sideband_async_started = 0; int saved_stderr = -1; + if (!hook_exists(the_repository, hook_name)) + return; + for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) continue; @@ -1665,7 +1676,7 @@ static void run_update_post_hook(struct command *commands) prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - run_hooks_opt(the_repository, "post-update", &opt); + run_hooks_opt(the_repository, hook_name, &opt); finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); } From 4aa72ea1f64e8ddcd1865c76b24591c0916c0b5d Mon Sep 17 00:00:00 2001 From: Tian Yuchen Date: Sun, 8 Mar 2026 12:33:44 +0800 Subject: [PATCH 39/40] .mailmap: update email address for Tian Yuchen Map my old Gmail address to my new custom address in .mailmap. Signed-off-by: Tian Yuchen Signed-off-by: Junio C Hamano --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 8a39e93bf85c56..c2e3939beb286d 100644 --- a/.mailmap +++ b/.mailmap @@ -283,6 +283,7 @@ Thomas Ackermann Thomas Rast Thomas Rast Thomas Rast +Tian Yuchen Timo Hirvonen Toby Allsopp Tom Grennan From d181b9354cf85b44455ce3ca9e6af0b9559e0ae2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 Mar 2026 14:35:46 -0700 Subject: [PATCH 40/40] The 13th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index fa6e42f3bb100a..ed5231f82cee91 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -60,6 +60,10 @@ UI, Workflows & Features * "git config list" is taught to show the values interpreted for specific type with "--type=" option. + * "git add " has been taught to honor + submodule..ignore that is set to "all" (and requires "git add + -f" to override it). + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -143,6 +147,11 @@ Performance, Internal Implementation, Development Support etc. were kept track of by a single global variable in-core, which has been corrected by moving it to per-repository data structure. + * Use the hook API to replace ad-hoc invocation of hook scripts via + the run_command() API. + + * Code refactoring around refs-for-each-* API functions. + Fixes since v2.53 ----------------- @@ -234,6 +243,10 @@ Fixes since v2.53 to access pack data by "fsck" has been updated to avoid this. (merge 13eb65d366 ps/fsck-stream-from-the-right-object-instance later to maint). + * "git log --graph --stat" did not count the display width of colored + graph part of its own output correctly, which has been corrected. + (merge 064b869efc lp/diff-stat-utf8-display-width-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint). @@ -264,3 +277,4 @@ Fixes since v2.53 (merge ed84bc1c0d kh/doc-patch-id-4 later to maint). (merge 7451864bfa sc/pack-redundant-leakfix later to maint). (merge f87593ab1a cx/fetch-display-ubfix later to maint). + (merge a66c8c7f91 jk/repo-structure-cleanup later to maint).