From 06c81a1118a6c9a77505b0ae3092576fa4c01763 Mon Sep 17 00:00:00 2001 From: Abraham Samuel Adekunle Date: Sat, 14 Feb 2026 12:03:54 +0100 Subject: [PATCH 01/12] interactive -p: add new `--auto-advance` flag When using the interactive add, reset, stash or checkout machinery, we do not have the option of reworking with a file when selecting hunks, because the session automatically advances to the next file or ends if we have just one file. Introduce the flag `--auto-advance` which auto advances by default, when interactively selecting patches with the '--patch' option. However, the `--no-auto-advance` option does not auto advance, thereby allowing users the option to rework with files. Signed-off-by: Abraham Samuel Adekunle Signed-off-by: Junio C Hamano --- add-interactive.c | 2 ++ add-interactive.h | 4 +++- builtin/add.c | 4 ++++ builtin/checkout.c | 7 +++++++ builtin/reset.c | 4 ++++ builtin/stash.c | 8 ++++++++ t/t9902-completion.sh | 1 + 7 files changed, 29 insertions(+), 1 deletion(-) diff --git a/add-interactive.c b/add-interactive.c index 68fc09547dd02e..ce9e737e0f5aff 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -64,6 +64,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r, s->r = r; s->context = -1; s->interhunkcontext = -1; + s->auto_advance = add_p_opt->auto_advance; s->use_color_interactive = check_color_config(r, "color.interactive"); @@ -1017,6 +1018,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, struct add_p_opt add_p_opt = { .context = s->context, .interhunkcontext = s->interhunkcontext, + .auto_advance = s->auto_advance }; struct strvec args = STRVEC_INIT; struct pathspec ps_selected = { 0 }; diff --git a/add-interactive.h b/add-interactive.h index da49502b7656f4..784339777509f7 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -6,9 +6,10 @@ struct add_p_opt { int context; int interhunkcontext; + int auto_advance; }; -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1 } +#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1, .auto_advance = 1 } struct add_i_state { struct repository *r; @@ -29,6 +30,7 @@ struct add_i_state { int use_single_key; char *interactive_diff_filter, *interactive_diff_algorithm; int context, interhunkcontext; + int auto_advance; }; void init_add_i_state(struct add_i_state *s, struct repository *r, diff --git a/builtin/add.c b/builtin/add.c index 32709794b3873f..4357f87b7f807b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -256,6 +256,8 @@ static struct option builtin_add_options[] = { OPT_GROUP(""), OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")), OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")), + OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + N_("auto advance to the next file when selecting hunks interactively")), OPT_DIFF_UNIFIED(&add_p_opt.context), OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")), @@ -418,6 +420,8 @@ int cmd_add(int argc, die(_("the option '%s' requires '%s'"), "--unified", "--interactive/--patch"); if (add_p_opt.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--interactive/--patch"); + if (!add_p_opt.auto_advance) + die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--interactive/--patch"); } if (edit_interactive) { diff --git a/builtin/checkout.c b/builtin/checkout.c index 66b69df6e67234..6710b53fd08b28 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -63,6 +63,7 @@ struct checkout_opts { int patch_mode; int patch_context; int patch_interhunk_context; + int auto_advance; int quiet; int merge; int force; @@ -111,6 +112,7 @@ struct checkout_opts { .merge = -1, \ .patch_context = -1, \ .patch_interhunk_context = -1, \ + .auto_advance = 1, \ } struct branch_info { @@ -549,6 +551,7 @@ static int checkout_paths(const struct checkout_opts *opts, struct add_p_opt add_p_opt = { .context = opts->patch_context, .interhunkcontext = opts->patch_interhunk_context, + .auto_advance = opts->auto_advance }; const char *rev = new_branch_info->name; char rev_oid[GIT_MAX_HEXSZ + 1]; @@ -1801,6 +1804,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix, die(_("the option '%s' requires '%s'"), "--unified", "--patch"); if (opts->patch_interhunk_context != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); + if (!opts->auto_advance) + die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } if (opts->show_progress < 0) { @@ -1999,6 +2004,8 @@ int cmd_checkout(int argc, OPT_BOOL(0, "guess", &opts.dwim_new_local_branch, N_("second guess 'git checkout ' (default)")), OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")), + OPT_BOOL(0, "auto-advance", &opts.auto_advance, + N_("auto advance to the next file when selecting hunks interactively")), OPT_END() }; diff --git a/builtin/reset.c b/builtin/reset.c index ed35802af15c94..7535591530d2e5 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -371,6 +371,8 @@ int cmd_reset(int argc, PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater), OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")), + OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + N_("auto advance to the next file when selecting hunks interactively")), OPT_DIFF_UNIFIED(&add_p_opt.context), OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), OPT_BOOL('N', "intent-to-add", &intent_to_add, @@ -443,6 +445,8 @@ int cmd_reset(int argc, die(_("the option '%s' requires '%s'"), "--unified", "--patch"); if (add_p_opt.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); + if (!add_p_opt.auto_advance) + die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } /* git reset tree [--] paths... can be used to diff --git a/builtin/stash.c b/builtin/stash.c index 948eba06fbccb3..1a98358fa6a838 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1849,6 +1849,8 @@ static int push_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), + OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + N_("auto advance to the next file when selecting hunks interactively")), OPT_DIFF_UNIFIED(&add_p_opt.context), OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), @@ -1911,6 +1913,8 @@ static int push_stash(int argc, const char **argv, const char *prefix, die(_("the option '%s' requires '%s'"), "--unified", "--patch"); if (add_p_opt.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); + if (!add_p_opt.auto_advance) + die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } if (add_p_opt.context < -1) @@ -1952,6 +1956,8 @@ static int save_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), + OPT_BOOL(0, "auto-advance", &add_p_opt.auto_advance, + N_("auto advance to the next file when selecting hunks interactively")), OPT_DIFF_UNIFIED(&add_p_opt.context), OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), @@ -1983,6 +1989,8 @@ static int save_stash(int argc, const char **argv, const char *prefix, die(_("the option '%s' requires '%s'"), "--unified", "--patch"); if (add_p_opt.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); + if (!add_p_opt.auto_advance) + die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch"); } ret = do_push_stash(&ps, stash_msg, quiet, keep_index, diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 964e1f156932c6..3da5527ae5c9af 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -2601,6 +2601,7 @@ test_expect_success 'double dash "git checkout"' ' --ignore-skip-worktree-bits Z --ignore-other-worktrees Z --recurse-submodules Z + --auto-advance Z --progress Z --guess Z --no-guess Z From 57088095e90a940d232e36594c6fa761f283d35d Mon Sep 17 00:00:00 2001 From: Abraham Samuel Adekunle Date: Sat, 14 Feb 2026 12:04:44 +0100 Subject: [PATCH 02/12] add-patch: modify patch_update_file() signature The function `patch_update_file()` takes the add_p_state struct pointer and the current `struct file_diff` pointer and returns an int. When using the `--no-auto-advance` flag, we want to be able to request the next or previous file from the caller. Modify the function signature to instead take the index of the current `file_diff` and the `add_p_state` struct pointer so that we can compute the `file_diff` from the index while also having access to the file index. This will help us request the next or previous file from the caller. Signed-off-by: Abraham Samuel Adekunle Signed-off-by: Junio C Hamano --- add-patch.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/add-patch.c b/add-patch.c index 173a53241ebf07..49eb8957453f94 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1441,20 +1441,21 @@ static bool get_first_undecided(const struct file_diff *file_diff, size_t *idx) return false; } -static int patch_update_file(struct add_p_state *s, - struct file_diff *file_diff) +static size_t patch_update_file(struct add_p_state *s, size_t idx) { size_t hunk_index = 0; ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1; struct hunk *hunk; char ch; struct child_process cp = CHILD_PROCESS_INIT; - int colored = !!s->colored.len, quit = 0, use_pager = 0; + int colored = !!s->colored.len, use_pager = 0; enum prompt_mode_type prompt_mode_type; + struct file_diff *file_diff = s->file_diff + idx; + size_t patch_update_resp = idx; /* Empty added files have no hunks */ if (!file_diff->hunk_nr && !file_diff->added) - return 0; + return patch_update_resp + 1; strbuf_reset(&s->buf); render_diff_header(s, file_diff, colored, &s->buf); @@ -1498,9 +1499,10 @@ static int patch_update_file(struct add_p_state *s, /* Everything decided? */ if (undecided_previous < 0 && undecided_next < 0 && - hunk->use != UNDECIDED_HUNK) - break; - + hunk->use != UNDECIDED_HUNK) { + patch_update_resp++; + break; + } strbuf_reset(&s->buf); if (file_diff->hunk_nr) { if (rendered_hunk_index != hunk_index) { @@ -1570,7 +1572,7 @@ static int patch_update_file(struct add_p_state *s, fputs(s->s.reset_color_interactive, stdout); fflush(stdout); if (read_single_character(s) == EOF) { - quit = 1; + patch_update_resp = s->file_diff_nr; break; } @@ -1616,7 +1618,7 @@ static int patch_update_file(struct add_p_state *s, hunk->use = SKIP_HUNK; } } else if (ch == 'q') { - quit = 1; + patch_update_resp = s->file_diff_nr; break; } else if (s->answer.buf[0] == 'K') { if (permitted & ALLOW_GOTO_PREVIOUS_HUNK) @@ -1803,7 +1805,7 @@ static int patch_update_file(struct add_p_state *s, } putchar('\n'); - return quit; + return patch_update_resp; } int run_add_p(struct repository *r, enum add_p_mode mode, @@ -1852,11 +1854,15 @@ int run_add_p(struct repository *r, enum add_p_mode mode, return -1; } - for (i = 0; i < s.file_diff_nr; i++) - if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) + for (i = 0; i < s.file_diff_nr;) { + if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) { binary_count++; - else if (patch_update_file(&s, s.file_diff + i)) + i++; + continue; + } + if ((i = patch_update_file(&s, i)) == s.file_diff_nr) break; + } if (s.file_diff_nr == 0) err(&s, _("No changes.")); From d3cce5e76fcbce00d33013424bece954f11fa2f3 Mon Sep 17 00:00:00 2001 From: Abraham Samuel Adekunle Date: Sat, 14 Feb 2026 12:06:06 +0100 Subject: [PATCH 03/12] add-patch: allow all-or-none application of patches When the flag `--no-auto-advance` is used with `--patch`, if the user has decided `USE` on a hunk in a file, goes to another file, and then returns to this file and changes the previous decision on the hunk to `SKIP`, because the patch has already been applied, the last decision is not registered and the now SKIPPED hunk is still applied. Move the logic for applying patches into a function so that we can reuse this logic to implement the all or non application of the patches after the user is done with the hunk selection. Signed-off-by: Abraham Samuel Adekunle Signed-off-by: Junio C Hamano --- add-patch.c | 62 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/add-patch.c b/add-patch.c index 49eb8957453f94..d6528d48185648 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1420,6 +1420,40 @@ N_("j - go to the next undecided hunk, roll over at the bottom\n" "P - print the current hunk using the pager\n" "? - print help\n"); +static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) +{ + struct child_process cp = CHILD_PROCESS_INIT; + size_t j; + + /* Any hunk to be used? */ + for (j = 0; j < file_diff->hunk_nr; j++) + if (file_diff->hunk[j].use == USE_HUNK) + break; + + if (j < file_diff->hunk_nr || + (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) { + /* At least one hunk selected: apply */ + strbuf_reset(&s->buf); + reassemble_patch(s, file_diff, 0, &s->buf); + + discard_index(s->s.r->index); + if (s->mode->apply_for_checkout) + apply_for_checkout(s, &s->buf, + s->mode->is_reverse); + else { + setup_child_process(s, &cp, "apply", NULL); + strvec_pushv(&cp.args, s->mode->apply_args); + if (pipe_command(&cp, s->buf.buf, s->buf.len, + NULL, 0, NULL, 0)) + error(_("'git apply' failed")); + } + if (repo_read_index(s->s.r) >= 0) + repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, + 1, NULL, NULL, NULL); + } + +} + static size_t dec_mod(size_t a, size_t m) { return a > 0 ? a - 1 : m - 1; @@ -1447,7 +1481,6 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1; struct hunk *hunk; char ch; - struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len, use_pager = 0; enum prompt_mode_type prompt_mode_type; struct file_diff *file_diff = s->file_diff + idx; @@ -1777,32 +1810,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } } - /* Any hunk to be used? */ - for (i = 0; i < file_diff->hunk_nr; i++) - if (file_diff->hunk[i].use == USE_HUNK) - break; - - if (i < file_diff->hunk_nr || - (!file_diff->hunk_nr && file_diff->head.use == USE_HUNK)) { - /* At least one hunk selected: apply */ - strbuf_reset(&s->buf); - reassemble_patch(s, file_diff, 0, &s->buf); - - discard_index(s->s.r->index); - if (s->mode->apply_for_checkout) - apply_for_checkout(s, &s->buf, - s->mode->is_reverse); - else { - setup_child_process(s, &cp, "apply", NULL); - strvec_pushv(&cp.args, s->mode->apply_args); - if (pipe_command(&cp, s->buf.buf, s->buf.len, - NULL, 0, NULL, 0)) - error(_("'git apply' failed")); - } - if (repo_read_index(s->s.r) >= 0) - repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, - 1, NULL, NULL, NULL); - } + apply_patch(s, file_diff); putchar('\n'); return patch_update_resp; From 417b181f99ce53f50dea6541430cfe1f1f359a6a Mon Sep 17 00:00:00 2001 From: Abraham Samuel Adekunle Date: Sat, 14 Feb 2026 12:06:55 +0100 Subject: [PATCH 04/12] add-patch: allow interfile navigation when selecting hunks After deciding on all hunks in a file, the interactive session advances automatically to the next file if there is another, or the process ends. Now using the `--no-auto-advance` flag with `--patch`, the process does not advance automatically. A user can choose to go to the next file by pressing '>' or the previous file by pressing '<', before or after deciding on all hunks in the current file. After all hunks have been decided in a file, the user can still rework with the file by applying the options available in the permit set for that hunk, and after all the decisions, the user presses 'q' to submit. After all hunks have been decided, the user can press '?' which will show the hunk selection summary in the help patch remainder text including the total hunks, number of hunks marked for use and number of hunks marked for skip. This feature is enabled by passing the `--no-auto-advance` flag to `--patch` option of the subcommands add, stash, reset, and checkout. Signed-off-by: Abraham Samuel Adekunle Signed-off-by: Junio C Hamano --- add-patch.c | 66 ++++++++++++++++++++++-- t/t3701-add-interactive.sh | 100 +++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 5 deletions(-) diff --git a/add-patch.c b/add-patch.c index d6528d48185648..d9900d58d55fd3 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1418,7 +1418,10 @@ N_("j - go to the next undecided hunk, roll over at the bottom\n" "e - manually edit the current hunk\n" "p - print the current hunk\n" "P - print the current hunk using the pager\n" - "? - print help\n"); + "> - go to the next file, roll over at the bottom\n" + "< - go to the previous file, roll over at the top\n" + "? - print help\n" + "HUNKS SUMMARY - Hunks: %d, USE: %d, SKIP: %d\n"); static void apply_patch(struct add_p_state *s, struct file_diff *file_diff) { @@ -1483,6 +1486,7 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) char ch; int colored = !!s->colored.len, use_pager = 0; enum prompt_mode_type prompt_mode_type; + int all_decided = 0; struct file_diff *file_diff = s->file_diff + idx; size_t patch_update_resp = idx; @@ -1501,7 +1505,9 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3, ALLOW_SEARCH_AND_GOTO = 1 << 4, ALLOW_SPLIT = 1 << 5, - ALLOW_EDIT = 1 << 6 + ALLOW_EDIT = 1 << 6, + ALLOW_GOTO_PREVIOUS_FILE = 1 << 7, + ALLOW_GOTO_NEXT_FILE = 1 << 8 } permitted = 0; if (hunk_index >= file_diff->hunk_nr) @@ -1533,8 +1539,12 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) /* Everything decided? */ if (undecided_previous < 0 && undecided_next < 0 && hunk->use != UNDECIDED_HUNK) { - patch_update_resp++; - break; + if (!s->s.auto_advance) + all_decided = 1; + else { + patch_update_resp++; + break; + } } strbuf_reset(&s->buf); if (file_diff->hunk_nr) { @@ -1583,6 +1593,14 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); } + if (!s->s.auto_advance && s->file_diff_nr > 1) { + permitted |= ALLOW_GOTO_NEXT_FILE; + strbuf_addstr(&s->buf, ",>"); + } + if (!s->s.auto_advance && s->file_diff_nr > 1) { + permitted |= ALLOW_GOTO_PREVIOUS_FILE; + strbuf_addstr(&s->buf, ",<"); + } strbuf_addstr(&s->buf, ",p,P"); } if (file_diff->deleted) @@ -1653,6 +1671,28 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } else if (ch == 'q') { patch_update_resp = s->file_diff_nr; break; + } else if (!s->s.auto_advance && s->answer.buf[0] == '>') { + if (permitted & ALLOW_GOTO_NEXT_FILE) { + if (patch_update_resp == s->file_diff_nr - 1) + patch_update_resp = 0; + else + patch_update_resp++; + break; + } else { + err(s, _("No next file")); + continue; + } + } else if (!s->s.auto_advance && s->answer.buf[0] == '<') { + if (permitted & ALLOW_GOTO_PREVIOUS_FILE) { + if (patch_update_resp == 0) + patch_update_resp = s->file_diff_nr - 1; + else + patch_update_resp--; + break; + } else { + err(s, _("No previous file")); + continue; + } } else if (s->answer.buf[0] == 'K') { if (permitted & ALLOW_GOTO_PREVIOUS_HUNK) hunk_index = dec_mod(hunk_index, @@ -1798,6 +1838,18 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) * commands shown in the prompt that are not * always available. */ + if (all_decided && !strncmp(p, "HUNKS SUMMARY", 13)) { + int total = file_diff->hunk_nr, used = 0, skipped = 0; + + for (i = 0; i < file_diff->hunk_nr; i++) { + if (file_diff->hunk[i].use == USE_HUNK) + used += 1; + if (file_diff->hunk[i].use == SKIP_HUNK) + skipped += 1; + } + color_fprintf_ln(stdout, s->s.help_color, _(p), + total, used, skipped); + } if (*p != '?' && !strchr(s->buf.buf, *p)) continue; @@ -1810,7 +1862,8 @@ static size_t patch_update_file(struct add_p_state *s, size_t idx) } } - apply_patch(s, file_diff); + if (s->s.auto_advance) + apply_patch(s, file_diff); putchar('\n'); return patch_update_resp; @@ -1871,6 +1924,9 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if ((i = patch_update_file(&s, i)) == s.file_diff_nr) break; } + if (!s.s.auto_advance) + for (i = 0; i < s.file_diff_nr; i++) + apply_patch(&s, s.file_diff + i); if (s.file_diff_nr == 0) err(&s, _("No changes.")); diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 4285314f35f8f2..7924c9a28b6e92 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1441,5 +1441,105 @@ test_expect_success 'EOF quits' ' test_grep file out && test_grep ! file2 out ' +for cmd in add checkout reset "stash save" "stash push" +do + test_expect_success "$cmd rejects invalid --no-auto-advance options" ' + test_must_fail git $cmd --no-auto-advance 2>actual && + test_grep -E "requires .*--(interactive|patch)" actual + ' +done + +test_expect_success 'manual advance (">") moves to next file with --no-auto-advance' ' + git reset --hard && + echo line1 >first-file && + echo line2 >second-file && + git add -A && + git commit -m initial >/dev/null 2>&1 && + echo change_first >>first-file && + echo change_second >>second-file && + + printf ">\nq\n" | git add -p --no-auto-advance >output.test 2>&1 && + test_grep -E "(a|b)/second-file" output.test +' + +test_expect_success 'select n on a hunk, go to another file, come back and change to y stages' ' + git reset --hard && + echo one >f1 && + echo one >f2 && + git add -A && + git commit -m initial >/dev/null 2>&1 && + echo change1 >>f1 && + echo change2 >>f2 && + + printf "n\n>\n<\ny\nq\n" | git add -p --no-auto-advance >output.staged 2>&1 && + git diff --cached --name-only >staged && + test_grep -E "(a/f1)" output.staged +' + +test_expect_success 'select y on a hunk, go to another file, come back and change to n does not stage' ' + git reset --hard && + echo one >f1 && + echo one >f2 && + git add -A && + git commit -m initial >/dev/null 2>&1 && + echo change1 >>f1 && + echo change2 >>f2 && + + printf "y\n>\n<\nn\nq\n" | git add -p --no-auto-advance >output.unstaged 2>&1 && + git diff --cached --name-only >staged && + test_must_be_empty staged +' + +test_expect_success 'deciding all hunks in a file does not auto advance' ' + git reset --hard && + echo line >stay && + echo line >other && + git add -A && + git commit -m initial >/dev/null 2>&1 && + echo change >>stay && + echo change >>other && + test_write_lines y | git add -p --no-auto-advance >raw-output 2>&1 && + test_grep "(1/1) Stage this hunk (was: y)" raw-output && + test_grep ! "diff --git a/stay b/stay" raw-output +' +test_expect_success 'HUNKS SUMMARY does not show in help text when there are undecided hunks' ' + git reset --hard && + test_write_lines 1 2 3 4 5 6 7 8 9 >f && + git add f && + git commit -m initial >/dev/null 2>&1 && + test_write_lines 1 X 3 4 Y 6 7 Z 9 >f && + test_write_lines s y n | git add -p --no-auto-advance >raw-nostat 2>&1 && + test_grep ! "HUNKS SUMMARY - Hunks: " raw-nostat +' + +test_expect_success 'help text shows HUNK SUMMARY when all hunks have been decided' ' + git reset --hard && + test_write_lines 1 2 3 4 5 6 7 8 9 >f2 && + git add f2 && + git commit -m initial >/dev/null 2>&1 && + test_write_lines 1 X 3 4 Y 6 7 Z 9 >f2 && + printf "s\ny\nn\ny\n?\n" | git add -p --no-auto-advance >raw-stat 2>&1 && + test_grep "HUNKS SUMMARY - Hunks: 3, USE: 2, SKIP: 1" raw-stat +' + +test_expect_success 'selective staging across multiple files with --no-advance' ' + git reset --hard && + test_write_lines 1 2 3 4 5 6 7 8 9 >a.file && + test_write_lines 1 2 3 4 5 6 7 8 9 >b.file && + test_write_lines 1 2 3 4 5 6 7 8 9 >c.file && + git add -A && + git commit -m initial >/dev/null 2>&1 && + test_write_lines 1 A2 3 4 A5 6 7 8 9 >a.file && + test_write_lines 1 2 B3 4 5 6 7 B8 9 >b.file && + test_write_lines C1 2 3 4 5 C6 7 8 9 >c.file && + printf "s\ny\nn\n>\ns\nn\ny\n>\ns\ny\ny\nq\n" | git add -p --no-auto-advance >output.index 2>&1 && + git diff --cached >staged.diff && + test_grep "+A2" staged.diff && + test_grep ! "+A5" staged.diff && + test_grep "+B8" staged.diff && + test_grep ! "+B3" staged.diff && + test_grep "+C1" staged.diff && + test_grep "+C6" staged.diff +' test_done From 767ee993b7ea3175691444af67550817de7f6c73 Mon Sep 17 00:00:00 2001 From: Colin Stagner Date: Tue, 17 Feb 2026 20:31:30 -0600 Subject: [PATCH 05/12] contrib/subtree: capture additional test-cases Patch series e7b07376e5 (Merge branch 'rs/subtree-fixes', 2018-10-26) corrects several defects in `git subtree split`. The defects affect `split --rejoin` and merge commit processing. There is no test coverage for this, and e7b07376e5 did not introduce any. Convert the minimum working example [1] from the original patch submission [2] into test cases. [1]: https://gist.github.com/FoxFireX/1b794384612b7fd5e7cd157cff96269e [2]: <20180928183540.48968-1-roger.strain@swri.org> Signed-off-by: Colin Stagner Signed-off-by: Junio C Hamano --- contrib/subtree/t/t7900-subtree.sh | 110 +++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index e7040718f221f0..3ee2f95d86d180 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -1575,6 +1575,116 @@ test_expect_success 'push split to subproj' ' ) ' +# --ignore-joins must ignore mainline content outside of the +# subtree. This test verifies that the logic in +# `find_existing_splits()` correctly handles a `git subtree add` +# In this test, the split history must not contain a commit titled +# +# Add 'sub/' from commit ... +# +# see: dd21d43b58 (subtree: make --ignore-joins pay +# attention to adds, 2018-09-28) +test_expect_success 'split --ignore-joins respects subtree add' ' + subtree_test_create_repo "$test_count" && + ( + cd "$test_count" && + test_commit main_must_not_be_in_subtree && + test_create_subtree_add . mksubtree sub sub1 && + test_commit sub/sub2 && + test_commit main_must_not_be_in_subtree2 && + git subtree split --prefix sub -b first_split --rejoin && + test_commit sub/sub3 && + no_ignore_joins="$(git subtree split --prefix sub -b no_ignore_joins)" && + ignore_joins="$(git subtree split --prefix sub --ignore-joins -b ignore_joins)" && + git checkout ignore_joins && + test_path_is_file sub1.t && + test_path_is_file sub2.t && + test_path_is_file sub3.t && + ! test_path_is_file main_must_not_be_in_subtree.t && + ! test_path_is_file main_must_not_be_in_subtree2.t && + test -z "$(git log -1 --grep "Add '''sub/''' from commit" ignore_joins)" && + test "$no_ignore_joins" = "$ignore_joins" && + test "$(git rev-list --count ignore_joins)" -eq 3; + ) +' + +# split excludes commits reachable from any previous --rejoin. +# These ignored commits can still be the basis for new work +# after the --rejoin. These commits must be processed, even +# if they are excluded. Otherwise, the split history will be +# incorrect. +# +# here, the merge +# +# git merge --no-ff new_work_based_on_prejoin +# +# doesn't contain any subtree changes and so should not end +# up in the split history. this subtree should be flat, +# with no merges. +# +# see: 315a84f9aa (subtree: use commits before rejoins for +# splits, 2018-09-28) +test_expect_success 'split links out-of-tree pre --rejoin commits with post --rejoin commits' ' + subtree_test_create_repo "$test_count" && + ( + cd "$test_count" && + test_commit main_must_not_be_in_subtree && + mkdir sub && + test_commit sub/sub1 && + test_commit sub/sub2 && + git subtree split --prefix sub --rejoin && + test "$(git rev-list --count HEAD)" -eq 6 && + git checkout sub/sub1 && + git checkout -b new_work_based_on_prejoin && + test_commit main_must_not_be_in_subtree2 && + git checkout main && + git merge --no-ff new_work_based_on_prejoin && + test_commit sub/sub3 && + git subtree split -d --prefix sub -b second_split && + git checkout second_split && + test_path_is_file sub1.t && + test_path_is_file sub2.t && + test_path_is_file sub3.t && + ! test_path_is_file main_must_not_be_in_subtree.t && + ! test_path_is_file main_must_not_be_in_subtree2.t && + test "$(git rev-list --count --merges second_split)" -eq 0 && + test "$(git rev-list --count second_split)" -eq 3; + ) +' + +# split must keep merge commits with unrelated histories, even +# if both parents are treesame. When deciding whether or not +# to eliminate a parent, copy_or_skip compares the merge-base +# of each parent. +# +# in the split_of_merges branch: +# +# * expect 4 commits +# * HEAD~ must be a merge +# +# see: 68f8ff8151 (subtree: improve decision on merges kept +# in split, 2018-09-28) +test_expect_success 'split preserves merges with unrelated history' ' + subtree_test_create_repo "$test_count" && + ( + cd "$test_count" && + test_commit main_must_not_be_in_subtree && + mkdir sub && + test_commit sub/sub1 && + git checkout --orphan new_history && + git checkout sub/sub1 -- . && + git add . && + git commit -m "treesame history but not a merge-base" && + git checkout main && + git merge --allow-unrelated-histories --no-ff new_history && + test "$(git rev-parse "HEAD^1^{tree}")" = "$(git rev-parse "HEAD^2^{tree}")" && + test_commit sub/sub2 && + git subtree split -d --prefix sub -b split_of_merges && + test "$(git rev-list --count split_of_merges)" -eq 4 && + test -n "$(git rev-list --merges HEAD~)"; + ) +' + # # This test covers 2 cases in subtree split copy_or_skip code # 1) Merges where one parent is a superset of the changes of the other From 715b406e47d51a6f4f6be3b0ed42cfbd59217258 Mon Sep 17 00:00:00 2001 From: Colin Stagner Date: Tue, 17 Feb 2026 20:31:31 -0600 Subject: [PATCH 06/12] contrib/subtree: test history depth Add history depth checks to some of the subtree unit tests. These checks were previously introduced as part of 28a7e27cff (contrib/subtree: detect rewritten subtree commits, 2026-01-09), which has since been reverted. Signed-off-by: Colin Stagner Signed-off-by: Junio C Hamano --- contrib/subtree/t/t7900-subtree.sh | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 3ee2f95d86d180..dad8dea63a0b38 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -411,8 +411,9 @@ test_expect_success 'split sub dir/ with --rejoin' ' git fetch ./"sub proj" HEAD && git subtree merge --prefix="sub dir" FETCH_HEAD && split_hash=$(git subtree split --prefix="sub dir" --annotate="*") && - git subtree split --prefix="sub dir" --annotate="*" --rejoin && - test "$(last_commit_subject)" = "Split '\''sub dir/'\'' into commit '\''$split_hash'\''" + git subtree split --prefix="sub dir" --annotate="*" -b spl --rejoin && + test "$(last_commit_subject)" = "Split '\''sub dir/'\'' into commit '\''$split_hash'\''" && + test "$(git rev-list --count spl)" -eq 5 ) ' @@ -442,18 +443,25 @@ test_expect_success 'split with multiple subtrees' ' git -C "$test_count" subtree add --prefix=subADir FETCH_HEAD && git -C "$test_count" fetch ./subB HEAD && git -C "$test_count" subtree add --prefix=subBDir FETCH_HEAD && + test "$(git -C "$test_count" rev-list --count main)" -eq 7 && test_create_commit "$test_count" subADir/main-subA1 && test_create_commit "$test_count" subBDir/main-subB1 && git -C "$test_count" subtree split --prefix=subADir \ - --squash --rejoin -m "Sub A Split 1" && + --squash --rejoin -m "Sub A Split 1" -b a1 && + test "$(git -C "$test_count" rev-list --count main..a1)" -eq 1 && git -C "$test_count" subtree split --prefix=subBDir \ - --squash --rejoin -m "Sub B Split 1" && + --squash --rejoin -m "Sub B Split 1" -b b1 && + test "$(git -C "$test_count" rev-list --count main..b1)" -eq 1 && test_create_commit "$test_count" subADir/main-subA2 && test_create_commit "$test_count" subBDir/main-subB2 && git -C "$test_count" subtree split --prefix=subADir \ - --squash --rejoin -m "Sub A Split 2" && + --squash --rejoin -m "Sub A Split 2" -b a2 && + test "$(git -C "$test_count" rev-list --count main..a2)" -eq 2 && + test "$(git -C "$test_count" rev-list --count a1..a2)" -eq 1 && test "$(git -C "$test_count" subtree split --prefix=subBDir \ - --squash --rejoin -d -m "Sub B Split 1" 2>&1 | grep -w "\[1\]")" = "" + --squash --rejoin -d -m "Sub B Split 1" -b b2 2>&1 | grep -w "\[1\]")" = "" && + test "$(git -C "$test_count" rev-list --count main..b2)" -eq 2 && + test "$(git -C "$test_count" rev-list --count b1..b2)" -eq 1 ' # When subtree split-ing a directory that has other subtree @@ -477,6 +485,7 @@ do test_path_is_file subA/file1.t && test_path_is_file subA/subB/file2.t && git subtree split --prefix=subA --branch=bsplit && + test "$(git rev-list --count bsplit)" -eq 2 && git checkout bsplit && test_path_is_file file1.t && test_path_is_file subB/file2.t && @@ -489,6 +498,7 @@ do --prefix=subA/subB mksubtree && test_path_is_file subA/subB/file3.t && git subtree split --prefix=subA --branch=bsplit && + test "$(git rev-list --count bsplit)" -eq 3 && git checkout bsplit && test_path_is_file file1.t && test_path_is_file subB/file2.t && From 1f70684b517f4fcfeb7b998b0b7f3146ee8a8c75 Mon Sep 17 00:00:00 2001 From: Colin Stagner Date: Tue, 17 Feb 2026 20:31:32 -0600 Subject: [PATCH 07/12] contrib/subtree: process out-of-prefix subtrees `should_ignore_subtree_split_commit` detects subtrees which are outside of the current path --prefix and ignores them. This can speed up splits of repositories that have many subtrees. Since its inception [1], every iteration of this logic [2], [3] incorrectly excludes commits. This alters the split history. The split history and its commit hashes are API contract, so this is not permissible. While a commit from a different subtree may look like it doesn't contribute anything to a split, sometimes it does. Merge commits are a particular hot spot. For these, the pruning logic in `copy_or_skip` performs: 1. a check for "treesame" parents 2. two different common ancestry checks These checks operate on the **split history**, not the input history. The split history omits commits that do not affect the --prefix. This can significantly alter the ancestry of a merge. In order to determine if `copy_or_skip` will skip a merge, it is likely necessary to compute all the split history... which is what `should_ignore_subtree_split_commit` tries to avoid. To make this logic API-preserving, we could gate it behind a new CLI argument. The present implementation is actually a speed penalty in many cases, however, so this is not done here. Remove the `should_ignore_subtree_split_commit` logic. This fixes the regression reported in [4]. [1]: 98ba49ccc2 (subtree: fix split processing with multiple subtrees present, 2023-12-01) [2]: 83f9dad7d6 (contrib/subtree: fix split with squashed subtrees, 2025-09-09) [3]: 28a7e27cff (contrib/subtree: detect rewritten subtree commits, 2026-01-09) [4]: <20251230170719.845029-1-george@mail.dietrich.pub> Reported-by: George Reported-by: Christian Heusel Signed-off-by: Colin Stagner Signed-off-by: Junio C Hamano --- contrib/subtree/git-subtree.sh | 50 +--------------------- contrib/subtree/t/t7900-subtree.sh | 68 ++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 17106d1a721519..ba9fb2ee5dac37 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -785,42 +785,6 @@ ensure_valid_ref_format () { die "fatal: '$1' does not look like a ref" } -# Usage: should_ignore_subtree_split_commit REV -# -# Check if REV is a commit from another subtree and should be -# ignored from processing for splits -should_ignore_subtree_split_commit () { - assert test $# = 1 - - git show \ - --no-patch \ - --no-show-signature \ - --format='%(trailers:key=git-subtree-dir,key=git-subtree-mainline)' \ - "$1" | - ( - have_mainline= - subtree_dir= - - while read -r trailer val - do - case "$trailer" in - git-subtree-dir:) - subtree_dir="${val%/}" ;; - git-subtree-mainline:) - have_mainline=y ;; - esac - done - - if test -n "${subtree_dir}" && - test -z "${have_mainline}" && - test "${subtree_dir}" != "$arg_prefix" - then - return 0 - fi - return 1 - ) -} - # Usage: process_split_commit REV PARENTS process_split_commit () { assert test $# = 2 @@ -1006,19 +970,7 @@ cmd_split () { eval "$grl" | while read rev parents do - if should_ignore_subtree_split_commit "$rev" - then - continue - fi - parsedparents='' - for parent in $parents - do - if ! should_ignore_subtree_split_commit "$parent" - then - parsedparents="$parsedparents$parent " - fi - done - process_split_commit "$rev" "$parsedparents" + process_split_commit "$rev" "$parents" done || exit $? latest_new=$(cache_get latest_new) || exit $? diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index dad8dea63a0b38..05a774ad47f219 100755 --- a/contrib/subtree/t/t7900-subtree.sh +++ b/contrib/subtree/t/t7900-subtree.sh @@ -428,8 +428,7 @@ test_expect_success 'split sub dir/ with --rejoin' ' # - Perform 'split' on subtree B # - Create new commits with changes to subtree A and B # - Perform split on subtree A -# - Check that the commits in subtree B are not processed -# as part of the subtree A split +# - Check for expected history test_expect_success 'split with multiple subtrees' ' subtree_test_create_repo "$test_count" && subtree_test_create_repo "$test_count/subA" && @@ -458,8 +457,8 @@ test_expect_success 'split with multiple subtrees' ' --squash --rejoin -m "Sub A Split 2" -b a2 && test "$(git -C "$test_count" rev-list --count main..a2)" -eq 2 && test "$(git -C "$test_count" rev-list --count a1..a2)" -eq 1 && - test "$(git -C "$test_count" subtree split --prefix=subBDir \ - --squash --rejoin -d -m "Sub B Split 1" -b b2 2>&1 | grep -w "\[1\]")" = "" && + git -C "$test_count" subtree split --prefix=subBDir \ + --squash --rejoin -d -m "Sub B Split 1" -b b2 && test "$(git -C "$test_count" rev-list --count main..b2)" -eq 2 && test "$(git -C "$test_count" rev-list --count b1..b2)" -eq 1 ' @@ -507,6 +506,67 @@ do ' done +# Usually, +# +# git subtree merge -P subA --squash f00... +# +# makes two commits, in this order: +# +# 1. Squashed 'subA/' content from commit f00... +# 2. Merge commit (1) as 'subA' +# +# Commit 1 updates the subtree but does *not* rewrite paths. +# Commit 2 rewrites all trees to start with `subA/` +# +# Commit 1 either has no parents or depends only on other +# "Squashed 'subA/' content" commits. +# +# For merge without --squash, subtree produces just one commit: +# a merge commit with git-subtree trailers. +# +# In either case, if the user rebases these commits, they will +# still have the git-subtree-* trailers… but will NOT have +# the layout described above. +# +# Test that subsequent `git subtree split` are not confused by this. +test_expect_success 'split with rebased subtree commit' ' + subtree_test_create_repo "$test_count" && + ( + cd "$test_count" && + test_commit file0 && + test_create_subtree_add \ + . mksubtree subA file1 --squash && + test_path_is_file subA/file1.t && + mkdir subB && + test_commit subB/bfile && + git commit --amend -F - <<'EOF' && +Squashed '\''subB/'\'' content from commit '\''badf00da911bbe895347b4b236f5461d55dc9877'\'' + +Simulate a cherry-picked or rebased subtree commit. + +git-subtree-dir: subB +git-subtree-split: badf00da911bbe895347b4b236f5461d55dc9877 +EOF + test_commit subA/file2 && + test_commit subB/bfile2 && + git commit --amend -F - <<'EOF' && +Split '\''subB/'\'' into commit '\''badf00da911bbe895347b4b236f5461d55dc9877'\'' + +Simulate a cherry-picked or rebased subtree commit. + +git-subtree-dir: subB +git-subtree-mainline: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +git-subtree-split: badf00da911bbe895347b4b236f5461d55dc9877 +EOF + git subtree split --prefix=subA --branch=bsplit && + git checkout bsplit && + test_path_is_file file1.t && + test_path_is_file file2.t && + test "$(last_commit_subject)" = "subA/file2" && + test "$(git rev-list --count bsplit)" -eq 2 + ) +' + test_expect_success 'split sub dir/ with --rejoin from scratch' ' subtree_test_create_repo "$test_count" && test_create_commit "$test_count" main1 && From a4b61a71c4060748e4d6e8d970e52989c04f1f88 Mon Sep 17 00:00:00 2001 From: Lambert Duclos-de Guise Date: Sat, 21 Feb 2026 17:28:13 +0000 Subject: [PATCH 08/12] t2004: use test_path_is_file instead of test -f Replace 'test -f' with the helper function 'test_path_is_file' to provide better error messages upon failure. Signed-off-by: Lambert Duclos-de Guise Signed-off-by: Junio C Hamano --- t/t2004-checkout-cache-temp.sh | 42 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh index b92d96fdc4ed5f..0afe0ff7ca1f31 100755 --- a/t/t2004-checkout-cache-temp.sh +++ b/t/t2004-checkout-cache-temp.sh @@ -42,7 +42,7 @@ test_expect_success 'checkout one stage 0 to temporary file' ' test_line_count = 1 actual && test $(cut "-d " -f2 actual) = path1 && p=$(cut "-d " -f1 actual) && - test -f $p && + test_path_is_file $p && test $(cat $p) = tree1path1 ' @@ -55,7 +55,7 @@ test_expect_success 'checkout all stage 0 to temporary files' ' do test $(grep $f actual | cut "-d " -f2) = $f && p=$(grep $f actual | cut "-d " -f1) && - test -f $p && + test_path_is_file $p && test $(cat $p) = tree1$f || return 1 done ' @@ -71,7 +71,7 @@ test_expect_success 'checkout one stage 2 to temporary file' ' test_line_count = 1 actual && test $(cut "-d " -f2 actual) = path1 && p=$(cut "-d " -f1 actual) && - test -f $p && + test_path_is_file $p && test $(cat $p) = tree2path1 ' @@ -83,7 +83,7 @@ test_expect_success 'checkout all stage 2 to temporary files' ' do test $(grep $f actual | cut "-d " -f2) = $f && p=$(grep $f actual | cut "-d " -f1) && - test -f $p && + test_path_is_file $p && test $(cat $p) = tree2$f || return 1 done ' @@ -108,9 +108,9 @@ test_expect_success 'checkout all stages/one file to temporary files' ' test_line_count = 1 actual && test $(cut "-d " -f2 actual) = path1 && cut "-d " -f1 actual | (read s1 s2 s3 && - test -f $s1 && - test -f $s2 && - test -f $s3 && + test_path_is_file $s1 && + test_path_is_file $s2 && + test_path_is_file $s3 && test $(cat $s1) = tree1path1 && test $(cat $s2) = tree2path1 && test $(cat $s3) = tree3path1) @@ -143,8 +143,8 @@ test_expect_success 'checkout some stages/one file to temporary files' ' test $(cut "-d " -f2 actual) = path2 && cut "-d " -f1 actual | (read s1 s2 s3 && test $s1 = . && - test -f $s2 && - test -f $s3 && + test_path_is_file $s2 && + test_path_is_file $s3 && test $(cat $s2) = tree2path2 && test $(cat $s3) = tree3path2) ' @@ -162,9 +162,9 @@ test_expect_success '-- path0: no entry' ' test_expect_success '-- path1: all 3 stages' ' test $(grep path1 actual | cut "-d " -f2) = path1 && grep path1 actual | cut "-d " -f1 | (read s1 s2 s3 && - test -f $s1 && - test -f $s2 && - test -f $s3 && + test_path_is_file $s1 && + test_path_is_file $s2 && + test_path_is_file $s3 && test $(cat $s1) = tree1path1 && test $(cat $s2) = tree2path1 && test $(cat $s3) = tree3path1) @@ -174,8 +174,8 @@ test_expect_success '-- path2: no stage 1, have stage 2 and 3' ' test $(grep path2 actual | cut "-d " -f2) = path2 && grep path2 actual | cut "-d " -f1 | (read s1 s2 s3 && test $s1 = . && - test -f $s2 && - test -f $s3 && + test_path_is_file $s2 && + test_path_is_file $s3 && test $(cat $s2) = tree2path2 && test $(cat $s3) = tree3path2) ' @@ -183,9 +183,9 @@ test_expect_success '-- path2: no stage 1, have stage 2 and 3' ' test_expect_success '-- path3: no stage 2, have stage 1 and 3' ' test $(grep path3 actual | cut "-d " -f2) = path3 && grep path3 actual | cut "-d " -f1 | (read s1 s2 s3 && - test -f $s1 && + test_path_is_file $s1 && test $s2 = . && - test -f $s3 && + test_path_is_file $s3 && test $(cat $s1) = tree1path3 && test $(cat $s3) = tree3path3) ' @@ -193,8 +193,8 @@ test_expect_success '-- path3: no stage 2, have stage 1 and 3' ' test_expect_success '-- path4: no stage 3, have stage 1 and 3' ' test $(grep path4 actual | cut "-d " -f2) = path4 && grep path4 actual | cut "-d " -f1 | (read s1 s2 s3 && - test -f $s1 && - test -f $s2 && + test_path_is_file $s1 && + test_path_is_file $s2 && test $s3 = . && test $(cat $s1) = tree1path4 && test $(cat $s2) = tree2path4) @@ -203,7 +203,7 @@ test_expect_success '-- path4: no stage 3, have stage 1 and 3' ' test_expect_success '-- asubdir/path5: no stage 2 and 3 have stage 1' ' test $(grep asubdir/path5 actual | cut "-d " -f2) = asubdir/path5 && grep asubdir/path5 actual | cut "-d " -f1 | (read s1 s2 s3 && - test -f $s1 && + test_path_is_file $s1 && test $s2 = . && test $s3 = . && test $(cat $s1) = tree1asubdir/path5) @@ -216,7 +216,7 @@ test_expect_success 'checkout --temp within subdir' ' test_line_count = 1 actual && test $(grep path5 actual | cut "-d " -f2) = path5 && grep path5 actual | cut "-d " -f1 | (read s1 s2 s3 && - test -f ../$s1 && + test_path_is_file ../$s1 && test $s2 = . && test $s3 = . && test $(cat ../$s1) = tree1asubdir/path5) @@ -230,7 +230,7 @@ test_expect_success 'checkout --temp symlink' ' test_line_count = 1 actual && test $(cut "-d " -f2 actual) = path6 && p=$(cut "-d " -f1 actual) && - test -f $p && + test_path_is_file $p && test $(cat $p) = path7 ' From 40b30f245d57f89215f89c29b72e8b557f09b82a Mon Sep 17 00:00:00 2001 From: Pushkar Singh Date: Sat, 21 Feb 2026 11:05:12 +0000 Subject: [PATCH 09/12] path: factor out skip_slashes() in normalize_path_copy_len() Extract skip_slashes() to avoid repeating the same is_dir_sep() loop in multiple places inside normalize_path_copy_len(). Keep the dot-component handling inline to preserve the original control flow and readability, as suggested in review. No functional changes. Behavior verified with t0060-path-utils.sh. Signed-off-by: Pushkar Singh Signed-off-by: Junio C Hamano --- path.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/path.c b/path.c index d726537622cda6..1772fcb21c15cd 100644 --- a/path.c +++ b/path.c @@ -1112,6 +1112,14 @@ const char *remove_leading_path(const char *in, const char *prefix) * end with a '/', then the callers need to be fixed up accordingly. * */ + +static const char *skip_slashes(const char *p) +{ + while (is_dir_sep(*p)) + p++; + return p; +} + int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) { char *dst0; @@ -1129,8 +1137,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) } dst0 = dst; - while (is_dir_sep(*src)) - src++; + src = skip_slashes(src); for (;;) { char c = *src; @@ -1150,8 +1157,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) } else if (is_dir_sep(src[1])) { /* (2) */ src += 2; - while (is_dir_sep(*src)) - src++; + src = skip_slashes(src); continue; } else if (src[1] == '.') { if (!src[2]) { @@ -1161,8 +1167,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) } else if (is_dir_sep(src[2])) { /* (4) */ src += 3; - while (is_dir_sep(*src)) - src++; + src = skip_slashes(src); goto up_one; } } @@ -1182,6 +1187,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) up_one: /* + * strip the last component + * * dst0..dst is prefix portion, and dst[-1] is '/'; * go up one level. */ From 7451864bfac0fc7ac829aceecd7f339b80dac732 Mon Sep 17 00:00:00 2001 From: Sahitya Chandra Date: Sat, 21 Feb 2026 16:08:59 +0530 Subject: [PATCH 10/12] pack-redundant: fix memory leak when open_pack_index() fails In add_pack(), we allocate l.remaining_objects with llist_init() before calling open_pack_index(). If open_pack_index() fails we return NULL without freeing the allocated list, leaking the memory. Fix by calling llist_free(l.remaining_objects) on the error path before returning. Signed-off-by: Sahitya Chandra Signed-off-by: Junio C Hamano --- builtin/pack-redundant.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index e4ecf774ca6405..86749bb7e77ae7 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -546,8 +546,10 @@ static struct pack_list * add_pack(struct packed_git *p) l.pack = p; llist_init(&l.remaining_objects); - if (open_pack_index(p)) + if (open_pack_index(p)) { + llist_free(l.remaining_objects); return NULL; + } base = p->index_data; base += 256 * 4 + ((p->index_version < 2) ? 4 : 8); From 2d88ab078db03b6a608d30b8ef49cc7afb4b2f1c Mon Sep 17 00:00:00 2001 From: Han Young Date: Tue, 24 Feb 2026 14:13:29 +0800 Subject: [PATCH 11/12] diffcore-break: avoid segfault with freed entries After we have freed the file pair, we should set the queue reference to null. When computing a diff in a partial clone, there is a chance that we could trigger a prefetch of missing objects when there are freed entries in the global diff queue due to break-rewrites detection. The segfault only occurs if an entry has been freed by break-rewrites and there is an entry to be prefetched. There is a new test in t4067 that trigger the segmentation fault that results in this case. The test explicitly fetch the necessary blobs to trigger the break rewrites, some blobs are left to be prefetched. The fix is to set the queue pointer to NULL after it is freed, the prefetch will skip NULL entries. Signed-off-by: Han Young Signed-off-by: Junio C Hamano --- diffcore-break.c | 1 + t/t4067-diff-partial-clone.sh | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/diffcore-break.c b/diffcore-break.c index c4c2173f3096bc..9b11fe2fa0e622 100644 --- a/diffcore-break.c +++ b/diffcore-break.c @@ -222,6 +222,7 @@ void diffcore_break(struct repository *r, int break_score) free(p); /* not diff_free_filepair(), we are * reusing one and two here. */ + q->queue[i] = NULL; continue; } } diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh index 72f25de44950ae..30813109ac044e 100755 --- a/t/t4067-diff-partial-clone.sh +++ b/t/t4067-diff-partial-clone.sh @@ -132,6 +132,37 @@ test_expect_success 'diff with rename detection batches blobs' ' test_line_count = 1 done_lines ' +test_expect_success 'diff succeeds even if prefetch triggered by break-rewrites' ' + test_when_finished "rm -rf server client trace" && + + test_create_repo server && + echo xyz >server/foo && + mkdir server/bar && + test_seq -f "line %d" 1 100 >server/bar/baz && + git -C server add -A && + git -C server commit -m x && + + echo xyzz >server/foo && + test_seq -f "line %d" 90 190 >server/bar/baz && + git -C server add -A && + git -C server commit -m x && + + test_config -C server uploadpack.allowfilter 1 && + test_config -C server uploadpack.allowanysha1inwant 1 && + git clone --filter=blob:limit=0 "file://$(pwd)/server" client && + + # Fetch bar/baz without fetching foo. + # Foo will be lazily fetched during break rewrites detection. + git -C client checkout HEAD~1 bar && + + # Ensure baz in the working tree is different from baz in HEAD~1. + # We need baz to trigger break-rewrites detection. + git -C client reset --hard HEAD && + + # break-rewrites detction in reset. + git -C client reset HEAD~1 +' + test_expect_success 'diff succeeds even if entries are removed from queue' ' test_when_finished "rm -rf server client trace" && From 50d063e335afd5828fbb9de2f2b2fb44fd884d2b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 3 Mar 2026 11:08:01 -0800 Subject: [PATCH 12/12] The 10th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index 207db16b80d7a7..6425d47e00c165 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -44,6 +44,9 @@ UI, Workflows & Features against the tree of the (potentially quite different from the current working tree) given commit. + * "git add -p" learned a new mode that allows the user to revisit a + file that was already dealt with. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -179,6 +182,9 @@ Fixes since v2.53 * Update build precedure for mergetool documentation in meson-based builds. (merge 58e4eeeeb5 pw/meson-doc-mergetool later to maint). + * An earlier attempt to optimize "git subtree" discarded too much + relevant histories, which has been corrected. + * 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). @@ -207,3 +213,10 @@ Fixes since v2.53 (merge 96286f14b0 ty/symlinks-use-unsigned-for-bitset later to maint). (merge b10e0cb1f3 kh/doc-am-xref later to maint). (merge ed84bc1c0d kh/doc-patch-id-4 later to maint). + (merge 7451864bfa sc/pack-redundant-leakfix later to maint). + + * A prefetch call can be triggered to access a stale diff_queue entry + after diffcore-break breaks a filepair into two and freed the + original entry that is no longer used, leading to a segfault, which + has been corrected. + (merge 2d88ab078d hy/diff-lazy-fetch-with-break-fix later to maint).