* [PATCH v3 7/9] sequencer: simplify pick_one_commit()
From: Phillip Wood @ 2026-07-15 15:22 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Unless we're rebasing, all we do in pick_one_commit() is call
do_pick_commit() and return its result. Simplify the code by returning
early if we're not rebasing so that we don't have to repeatedly call
is_rebase_i() in the rest of the function. Note that there are a couple
of conditions that do not call is_rebase_i() but they check for either
an "edit" or a "fixup" command, both of which imply we're rebasing.
The only block that does not return early is the one guarded by
"!res". Move the return into that block to make it clear that after
recording the commit as rewritten, all we do is return from the
function.
As the conditional blocks are all mutually exclusive (either the
conditions are mutually exclusive, or an earlier conditional block
that would match a later one contains a "return" statement) chain
them together with "else if" to make that clear.
While we could remove "res" from the conditions below "if (!res)"
they are left alone because, when we start using an enum in the next
commit, it makes it clear that these clauses are handling cases where
there are conflicts.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8f3eed205e7..9016af9b5d7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4966,12 +4966,14 @@ static int pick_one_commit(struct repository *r,
res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
check_todo);
- if (is_rebase_i(opts) && res < 0) {
+ if (!is_rebase_i(opts))
+ return res;
+
+ if (res < 0) {
/* Reschedule */
*reschedule = 1;
return -1;
- }
- if (item->command == TODO_EDIT) {
+ } else if (item->command == TODO_EDIT) {
struct commit *commit = item->commit;
if (!res) {
if (!opts->verbose)
@@ -4981,14 +4983,14 @@ static int pick_one_commit(struct repository *r,
}
return error_with_patch(r, commit,
arg, item->arg_len, opts, res, !res);
- }
- if (is_rebase_i(opts) && !res)
+ } else if (!res) {
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
- if (res && is_fixup(item->command)) {
+ return 0;
+ } else if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res && is_rebase_i(opts)) {
+ } else if (res) {
int to_amend = 0;
struct object_id oid;
@@ -5008,7 +5010,8 @@ static int pick_one_commit(struct repository *r,
return error_with_patch(r, item->commit, arg, item->arg_len,
opts, res, to_amend);
}
- return res;
+
+ BUG("Unhandled return value from do_pick_commit()");
}
static int pick_commits(struct repository *r,
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 6/9] sequencer: remove unnecessary condition in pick_one_commit()
From: Phillip Wood @ 2026-07-15 15:22 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
item->commit holds the commit to be picked and so it must be non-NULL
otherwise pick_one_commit() would not know which commit to pick.
It is also unconditionally dereferenced in do_pick_commit() which is
called at the top of this function. Therefore the check to see if it
is non-NULL is superfluous.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index a00e3622c87..8f3eed205e7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4988,7 +4988,7 @@ static int pick_one_commit(struct repository *r,
if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res && is_rebase_i(opts) && item->commit) {
+ } else if (res && is_rebase_i(opts)) {
int to_amend = 0;
struct object_id oid;
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 5/9] sequencer: simplify handling of fixup with conflicts
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Commit e032abd5a0 (rebase: fix rewritten list for failed pick,
2023-09-06) introduced an early return when res == -1, so if
we enter this conditional block then res is positive. After the
last couple of commits the only possible positive value is 1. That
means we can simplify the code by removing the conditional call to
intend_to_amend() and have error_failed_squash() request that it is
called in error_with_patch() instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 70e12eab0ec..a00e3622c87 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3874,7 +3874,7 @@ static int error_failed_squash(struct repository *r,
return error(_("could not copy '%s' to '%s'"),
rebase_path_message(),
git_path_merge_msg(r));
- return error_with_patch(r, commit, subject, subject_len, opts, 1, 0);
+ return error_with_patch(r, commit, subject, subject_len, opts, 1, 1);
}
static int do_exec(struct repository *r, const char *command_line, int quiet)
@@ -4986,8 +4986,6 @@ static int pick_one_commit(struct repository *r,
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
if (res && is_fixup(item->command)) {
- if (res == 1)
- intend_to_amend();
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
} else if (res && is_rebase_i(opts) && item->commit) {
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 4/9] sequencer: remove unnecessary "or" in pick_one_commit()
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If error_with_patch(..., res, ...) succeeds then it returns "res", if
it fails then it returns -1. This means that or-ing the return value
with "res" is pointless as the result is the same as the return value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1db844100ad..70e12eab0ec 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5007,9 +5007,8 @@ static int pick_one_commit(struct repository *r,
oideq(&opts->squash_onto, &oid))))
to_amend = 1;
- return res | error_with_patch(r, item->commit,
- arg, item->arg_len, opts,
- res, to_amend);
+ return error_with_patch(r, item->commit, arg, item->arg_len,
+ opts, res, to_amend);
}
return res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 2/9] sequencer: be more careful with external merge
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If an external merge strategy cannot merge (for example because it
would overwrite an untracked file) it exits with a non-zero exit
code other than 1. This should be treated differently from a merge
with conflicts, which is signaled by an exit code of 1, because, as
the merge failed, we need to reschedule the last pick. The caller
expects us to return -1 in this case. Also reschedule without trying
to merge if the commit message cannot be written as that prevents us
from successfully picking the commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 19 +++++++++++++++----
t/t3404-rebase-interactive.sh | 11 +++++++++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 57855b0066a..eaffa8ebb84 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2453,14 +2453,25 @@ static int do_pick_commit(struct repository *r,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
- res = write_message(ctx->message.buf, ctx->message.len,
- git_path_merge_msg(r), 0);
+ if (write_message(ctx->message.buf, ctx->message.len,
+ git_path_merge_msg(r), 0)) {
+ res = -1;
+ goto leave;
+ }
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(r, opts->strategy,
- opts->xopts.nr, opts->xopts.v,
+ res = try_merge_command(r, opts->strategy,
+ opts->xopts.nr, opts->xopts.v,
common, oid_to_hex(&head), remotes);
+ /*
+ * If there were conflicts, try_merge_command() returns 1,
+ * any other no-zero return code means that either the merge
+ * command could not be run, or it failed to merge.
+ */
+ if (res && res != 1)
+ res = -1;
+
commit_list_free(common);
commit_list_free(remotes);
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c271..297b84e60d5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1249,6 +1249,17 @@ test_expect_success 'interrupted rebase -i with --strategy and -X' '
git rebase --continue &&
test $(git show conflict-branch:conflict) = $(cat conflict) &&
test $(cat file1) = Z
+'
+
+test_expect_success 'failing pick with --strategy is rescheduled' '
+ test_when_finished "rm -rf bin; test_might_fail git rebase --abort" &&
+ mkdir bin &&
+ echo exit 2 | write_script bin/git-merge-fail &&
+ git log -1 --format="pick %H # %s" HEAD >expect &&
+ test_must_fail env PATH="$PWD/bin:$PATH" \
+ git rebase --no-ff --strategy fail HEAD^ &&
+ test_cmp expect .git/rebase-merge/git-rebase-todo &&
+ test_cmp expect .git/rebase-merge/done
'
test_expect_success 'rebase -i error on commits with \ in message' '
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 3/9] sequencer: never reschedule on failed commit
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If "git commit" fails to run then run_git_commit() returns -1 which
causes the current command to be rescheduled. This is incorrect as
we have successfully picked the commit and have written all the state
files we need to successfully commit when the user continues. Fix this
by converting -1 to 1 which matches what do_merge() does.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index eaffa8ebb84..1db844100ad 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2542,6 +2542,12 @@ static int do_pick_commit(struct repository *r,
res = run_git_commit(NULL, reflog_action, opts, flags);
*check_todo = 1;
}
+ /*
+ * If "git commit" failed to run then res == -1, but we don't
+ * want reschedule the last command because the picking the
+ * commit was successful.
+ */
+ res = !!res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH v3 0/9] sequencer: do not record dropped commits as rewritten
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
Thanks to everyone who commented on v2. I've dropped patch 2 which
Andrei pointed out was pointless and tried to make the remaining
commit messages clearer as requested by Oswald.
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
post-rewrite hooks this means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
This series is structured as follows:
Patch 1 restores some test coverage that was lost when the default
rebase backend was changed.
Patches 2 & 3 fix the return value of do_pick_commit() when an external
command fails (this is in preparation for patch 8).
Patches 4-7 try and simplify the control flow in pick_one_commit()
in preparation for patch 8.
Patch 8 changes the return type of do_pick_commit() to an enum.
Patch 9 adds a new member to the enum from patch 8 for commits that
are dropped when they become empty and uses that to stop them from
being recorded as rewritten.
Cover letter for v2:
Thanks to everyone who commented on v1. I've squashed the fixups that
Junio had in "seen", squashed patches 8 & 9 together as suggested by
Oswald and expanded the commit message, and added Uwe's Tested-by:
trailer to the final patch. Oswald suggested extended the use of the
enum which I think is a good idea in the long-term but I punted on
that for now because I think it would be fairly invasive and this
series has enough refactoring in it already.
base-commit: 6c3d7b73556db708feb3b16232fab1efc4353428
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-drop-notes-with-commit%2Fv3
View-Changes-At: https://github.com/phillipwood/git/compare/6c3d7b735...2ef36b9ee
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-drop-notes-with-commit/v3
Phillip Wood (9):
t3400: restore coverage for note copying with apply backend
sequencer: be more careful with external merge
sequencer: never reschedule on failed commit
sequencer: remove unnecessary "or" in pick_one_commit()
sequencer: simplify handling of fixup with conflicts
sequencer: remove unnecessary condition in pick_one_commit()
sequencer: simplify pick_one_commit()
sequencer: use an enum to represent result of picking a commit
sequencer: do not record dropped commits as rewritten
sequencer.c | 124 +++++++++++++++++++++++++---------
t/t3400-rebase.sh | 16 ++++-
t/t3404-rebase-interactive.sh | 11 +++
t/t5407-post-rewrite-hook.sh | 23 +++++++
4 files changed, 140 insertions(+), 34 deletions(-)
Range-diff against v2:
1: 65af2ac07a2 ! 1: c4705066ee0 t3400: restore coverage for note copying with apply backend
@@ Metadata
## Commit message ##
t3400: restore coverage for note copying with apply backend
- Now that the merge backend is the default we have lost coverage for
+ Now that the merge backend is the default, we have lost coverage for
"git rebase --apply" copying notes. Fix this by replacing "-m" with
"--apply" as the previous test which uses the default backend now
checks the merge backend.
2: 02670f57e7d < -: ----------- sequencer: move definition of is_final_fixup()
3: 3d79362332c ! 2: 947bb77e44f sequencer: be more careful with external merge
@@ Commit message
If an external merge strategy cannot merge (for example because it
would overwrite an untracked file) it exits with a non-zero exit
- code other than 1. This should be treated differently to a merge
- with conflicts which is signalled by an exit code of 1 because as
- the merge failed we need to reschedule the last pick. The caller
+ code other than 1. This should be treated differently from a merge
+ with conflicts, which is signaled by an exit code of 1, because, as
+ the merge failed, we need to reschedule the last pick. The caller
expects us to return -1 in this case. Also reschedule without trying
to merge if the commit message cannot be written as that prevents us
from successfully picking the commit.
4: fc89e77c6e8 = 3: bff5f319e91 sequencer: never reschedule on failed commit
5: 26eef6c0958 = 4: e785433ad3d sequencer: remove unnecessary "or" in pick_one_commit()
6: 26dc48951ce ! 5: 134d8f7e935 sequencer: simplify handing of fixup with conflicts
@@ Metadata
Author: Phillip Wood <phillip.wood@dunelm.org.uk>
## Commit message ##
- sequencer: simplify handing of fixup with conflicts
+ sequencer: simplify handling of fixup with conflicts
Commit e032abd5a0 (rebase: fix rewritten list for failed pick,
- 2023-09-06) introduced an early return when res == -1, so if we enter
- this conditional block then res is positive. After the last couple
- of commits the only possible positive value is 1 so we can simplify
- the code by removing the conditional call to intend_to_amend() and
- call it error_with_patch() instead.
+ 2023-09-06) introduced an early return when res == -1, so if
+ we enter this conditional block then res is positive. After the
+ last couple of commits the only possible positive value is 1. That
+ means we can simplify the code by removing the conditional call to
+ intend_to_amend() and have error_failed_squash() request that it is
+ called in error_with_patch() instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
7: 71ed717d322 = 6: e3091dee633 sequencer: remove unnecessary condition in pick_one_commit()
8: e8b7fa4c59e ! 7: 7c1642b0a49 sequencer: simplify pick_one_commit()
@@ Metadata
## Commit message ##
sequencer: simplify pick_one_commit()
- Unless we're rebasing all we do in pick_one_commit() is call
+ Unless we're rebasing, all we do in pick_one_commit() is call
do_pick_commit() and return its result. Simplify the code by returning
- early if we're not rebasing so that we don't have to continually call
+ early if we're not rebasing so that we don't have to repeatedly call
is_rebase_i() in the rest of the function. Note that there are a couple
of conditions that do not call is_rebase_i() but they check for either
an "edit" or a "fixup" command, both of which imply we're rebasing.
The only block that does not return early is the one guarded by
"!res". Move the return into that block to make it clear that after
- recording the commit as rewritten all we do is return from the function.
+ recording the commit as rewritten, all we do is return from the
+ function.
As the conditional blocks are all mutually exclusive (either the
conditions are mutually exclusive, or an earlier conditional block
9: 4fb641afb3c ! 8: 0a146d57266 sequencer: use an enum to represent result of picking a commit
@@ Metadata
## Commit message ##
sequencer: use an enum to represent result of picking a commit
- Rather than using an integer where -1 is an error, 0 is success and
- 1 means there were conflicts use an enum. This is clearer and lets
+ Rather than using an integer where -1 is an error, 0 is success and 1
+ indicates there were conflicts, use an enum. This is clearer and lets
us add a separate return value for commits that are dropped because
they become empty in the next commit.
10: c89234dd949 ! 9: 2ef36b9ee5a sequencer: do not record dropped commits as rewritten
@@ Commit message
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
- post-rewrite hooks this means we end up copying the notes from the
+ post-rewrite hooks, it means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
- While we do not want to record the dropped commit is rewritten, if
+ While we do not want to record the dropped commit as rewritten, if
it is the final commit in a chain of fixups then we need to flush
the list of rewritten commits. The behavior of an "edit" command
where the commit is dropped is changed so that "rebase --continue"
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply
* [PATCH v3 1/9] t3400: restore coverage for note copying with apply backend
From: Phillip Wood @ 2026-07-15 15:21 UTC (permalink / raw)
To: git
Cc: Uwe Kleine-König, Junio C Hamano, Oswald Buddenhagen,
Farid Zakaria, Andrei Rybak, Phillip Wood
In-Reply-To: <cover.1784128921.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Now that the merge backend is the default, we have lost coverage for
"git rebase --apply" copying notes. Fix this by replacing "-m" with
"--apply" as the previous test which uses the default backend now
checks the merge backend.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3400-rebase.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c0c00fbb7b1..f0e7fcf649a 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -270,9 +270,9 @@ test_expect_success 'rebase can copy notes' '
test "a note" = "$(git notes show HEAD)"
'
-test_expect_success 'rebase -m can copy notes' '
+test_expect_success 'rebase --apply can copy notes' '
git reset --hard n3 &&
- git rebase -m --onto n1 n2 &&
+ git rebase --apply --onto n1 n2 &&
test "a note" = "$(git notes show HEAD)"
'
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* Re: [PATCH] diff: ignore unmerged paths outside prefix with --relative --cached
From: Junio C Hamano @ 2026-07-15 15:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20260715060523.GA517940@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> A diff using --relative ignores entries outside the current directory.
> This results in a segfault when we try to process an unmerged entry
> that's outside of our prefix, since we end up with a NULL diff_filepair
> and use it without checking that it's valid.
> ...
> +cc Junio, as you may have some wisdom on that further exploration.
Will take a look at the history myself, but I would probably not
have much wisdom on a change from 2011. I often do not even
remember what I ate for breakfast yesterday ;-).
> diff-lib.c | 2 +-
> t/t4045-diff-relative.sh | 9 +++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index ae91027a02..a23119b852 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -467,7 +467,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
> if (cached && idx && ce_stage(idx)) {
> struct diff_filepair *pair;
> pair = diff_unmerge(&revs->diffopt, idx->name);
> - if (tree)
> + if (pair && tree)
> fill_filespec(pair->one, &tree->oid, 1,
> tree->ce_mode);
> return;
> diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
> index 2c8493fe66..167be0bdcc 100755
> --- a/t/t4045-diff-relative.sh
> +++ b/t/t4045-diff-relative.sh
> @@ -245,4 +245,13 @@ test_expect_failure 'diff --relative with change in subdir' '
> test_cmp expected out
> '
>
> +test_expect_success 'diff --relative --cached with change in subdir' '
> + git switch br3 &&
> + test_when_finished "git merge --abort" &&
> + test_must_fail git merge sub1 &&
> + echo file0 >expected &&
> + git -C subdir diff --relative --name-only --cached >out &&
> + test_cmp expected out
> +'
> +
> test_done
^ permalink raw reply
* [PATCH v9 5/5] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v9.git.git.1784128573.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message, or
the replacement body from an amend! commit targeting it. When
--reedit-message is given it only reopened that selected message, so the
messages of the other commits in the range were lost.
Gather the message of every commit in the range and build the same editor
template that "git rebase -i --autosquash" shows for a squash, reusing
add_squash_combination_header(), add_squash_message_header() and
squash_subject_comment_len(). Feed the range through
todo_list_rearrange_squash() so that each fixup!, squash! or amend! is
grouped under the commit it targets rather than shown in commit order,
exactly as autosquash would arrange them.
Only the message text differs, the changes are always folded in. A fixup!
message is commented out in full under a "will be skipped" header, a
squash! keeps its body with only the marker subject commented, and an
amend! replaces its target's message unless a squash! already folded into
that target, in which case it behaves like a squash!.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 20 +++-
builtin/history.c | 104 +++++++++++++++++
t/t3455-history-squash.sh | 201 +++++++++++++++++++++++++++++++++
3 files changed, 320 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2c0d861303..dc5580531f 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -118,11 +118,12 @@ already on `topic`. Rev-list options may also be given, but any that would
change how the range is walked are overridden with a warning.
+
The oldest commit's message is preserved by default, except that an `amend!`
-commit targeting it replaces its message. Specify `--reedit-message` to edit
-the resulting message. A merge commit inside the range is folded like any
-other, but the range must have a single base, so a range that reaches more
-than one entry point (for example a side branch that forked before the range
-and was later merged into it) is rejected.
+commit targeting it replaces its message. With `--reedit-message`, an editor
+opens pre-filled with the messages of all the folded commits so you can
+combine them. A merge commit inside the range is folded like any other, but
+the range must have a single base, so a range that reaches more than one entry
+point (for example a side branch that forked before the range and was later
+merged into it) is rejected.
+
A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
targets is also in the range, so the fold does not silently absorb a
@@ -130,6 +131,15 @@ marker meant for a commit outside it. The body after an `amend!` subject
replaces the oldest commit's message when the marker targets that commit. As
an exception, a range made up entirely of markers for one target is combined
into a single commit, keeping the last `amend!` message if there is one.
+The changes from every commit in the range are always folded in. Only the
+message text differs.
+With `--reedit-message` the template mirrors `git rebase -i --autosquash`:
+each `fixup!`, `squash!`, or `amend!` is grouped under the commit it
+targets rather than shown in commit order. A `fixup!` message is dropped
+(commented out in full), a `squash!` keeps its body with only the marker
+subject commented, and an `amend!` replaces its target's message, unless
+a `squash!` folded into that target first, in which case it keeps its
+body like a `squash!`.
+
A branch or tag that points at a commit inside the range would be left
dangling once those commits are folded away, so with the default
diff --git a/builtin/history.c b/builtin/history.c
index edf98a21d3..b1f84e8297 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1223,6 +1223,102 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
return 0;
}
+static bool amend_replaces_target(struct todo_list *todo, int target)
+{
+ int i;
+
+ for (i = target + 1; i < todo->nr &&
+ todo->items[i].command != TODO_PICK; i++) {
+ if (todo->items[i].command == TODO_SQUASH)
+ return false;
+ if (todo->items[i].flags & TODO_REPLACE_FIXUP_MSG)
+ return true;
+ }
+ return false;
+}
+
+static int build_squash_message(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct strbuf *out)
+{
+ struct rev_info revs;
+ struct commit *commit;
+ struct strvec args = STRVEC_INIT;
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ int i, nr_commits, ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs)))
+ strbuf_addf(&todo.buf, "pick %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ if (todo_list_parse_insn_buffer(repo, &opts, todo.buf.buf, &todo) < 0 ||
+ todo_list_rearrange_squash(&todo) < 0) {
+ ret = error(_("could not prepare the squash message"));
+ goto out;
+ }
+
+ nr_commits = todo.nr;
+ for (i = 0; i < nr_commits; i++) {
+ struct todo_item *item = &todo.items[i];
+ const char *message, *body;
+ size_t commented_len;
+ bool skip, squashing;
+
+ squashing = item->command == TODO_SQUASH ||
+ (item->flags & TODO_REPLACE_FIXUP_MSG);
+ if (item->command == TODO_PICK)
+ skip = amend_replaces_target(&todo, i);
+ else
+ skip = !squashing;
+
+ message = repo_logmsg_reencode(repo, item->commit, NULL, NULL);
+ find_commit_subject(message, &body);
+
+ if (skip)
+ commented_len = strlen(body);
+ else if (squashing)
+ commented_len = squash_subject_comment_len(body, 1);
+ else
+ commented_len = 0;
+
+ if (!i)
+ add_squash_combination_header(out, nr_commits);
+ strbuf_addch(out, '\n');
+ add_squash_message_header(out, i + 1, skip);
+ strbuf_addstr(out, "\n\n");
+ strbuf_add_commented_lines(out, body, commented_len, comment_line_str);
+ strbuf_addstr(out, body + commented_len);
+ strbuf_complete_line(out);
+
+ repo_unuse_commit_buffer(repo, item->commit, message);
+ }
+
+ ret = 0;
+
+out:
+ todo_list_release(&todo);
+ replay_opts_release(&opts);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1306,6 +1402,14 @@ static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ strbuf_reset(&message);
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ message_template = message.buf;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index d6199b9644..f2835f5379 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -267,6 +267,207 @@ test_expect_success 'preserves authorship of the oldest commit' '
test_cmp expect actual
'
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
+ stage_file b &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit --no-tag re-two file c &&
+ test_commit re-three file d &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # This is the 1st commit message:
+
+ re-one subject
+
+ re-one body line
+
+ # This is the commit message #2:
+
+ re-two
+
+ # This is the commit message #3:
+
+ re-three
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_subjects -1 <<-\EOF
+ combined
+ EOF
+'
+
+test_expect_success '--reedit-message handles fixup!, squash! and amend! like rebase' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "fixup! mark-base\n\nfixup body\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+ stage_file e &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ mark-base
+
+ # The commit message #2 will be skipped:
+
+ # fixup! mark-base
+ #
+ # fixup body
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # This is the commit message #4:
+
+ # amend! mark-base
+
+ amended message
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ mark-base
+
+ squash remark
+
+ amended message
+
+ EOF
+'
+
+test_expect_success '--reedit-message groups fixups under their targets' '
+ git reset --hard start &&
+ test_commit --no-tag alpha file a1 &&
+ test_commit --no-tag beta file b1 &&
+ stage_file a2 &&
+ commit_with_message "fixup! alpha\n" &&
+ stage_file b2 &&
+ commit_with_message "fixup! beta\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ alpha
+
+ # The commit message #2 will be skipped:
+
+ # fixup! alpha
+
+ # This is the commit message #3:
+
+ beta
+
+ # The commit message #4 will be skipped:
+
+ # fixup! beta
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited
+'
+
+test_expect_success '--reedit-message lets amend! replace its target message' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
+ stage_file c &&
+ commit_with_message "amend! mark-base\n\namended message\n" &&
+ stage_file d &&
+ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ cat >expect <<-EOF &&
+ # This is a combination of 3 commits.
+ # The 1st commit message will be skipped:
+
+ # mark-base
+
+ # This is the commit message #2:
+
+ # amend! mark-base
+
+ amended message
+
+ # This is the commit message #3:
+
+ # squash! mark-base
+
+ squash remark
+
+ # Please enter the commit message for the squash changes. Lines starting
+ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
+ # Changes to be committed:
+ # modified: file
+ #
+ EOF
+ test_cmp expect edited &&
+ check_log_messages -1 <<-\EOF
+ amended message
+
+ squash remark
+
+ EOF
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ write_script editor <<-\EOF &&
+ >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ test_must_fail git history squash --reedit-message start.. &&
+
+ test_cmp_rev "$head_before" HEAD
+'
+
test_expect_success '--update-refs=head only moves HEAD' '
git reset --hard three &&
git branch -f other HEAD &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 4/5] sequencer: share the squash message marker helpers and flags
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v9.git.git.1784128573.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When "git rebase -i" squashes commits it builds an editor template with a
"This is a combination of N commits." banner, a "This is the 1st/Nth
commit message:" header above each kept message (or a "will be skipped"
header for a dropped one), and a commented-out subject for any fixup!,
squash! or amend! commit. The banner, the headers and the
subject-commenting all live in static helpers in sequencer.c wired to the
rebase state, so no other command can present a squash the same way.
Pull the three pieces out into add_squash_combination_header(),
add_squash_message_header() (which takes a flag for the "will be skipped"
variant) and squash_subject_comment_len(), and use them from
update_squash_messages() and append_squash_message(). Also move the
todo_item_flags enum to the header, so a caller reading the output of
todo_list_rearrange_squash() can tell an amend! (TODO_REPLACE_FIXUP_MSG)
from a plain fixup!. A later change reuses all of this to give "git
history squash --reedit-message" the same template.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
sequencer.c | 70 +++++++++++++++++++++++++++++------------------------
sequencer.h | 30 +++++++++++++++++++++++
2 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0fe8fed6c3..2387afd9b5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1880,18 +1880,38 @@ static int is_pick_or_similar(enum todo_command command)
}
}
-enum todo_item_flags {
- TODO_EDIT_MERGE_MSG = (1 << 0),
- TODO_REPLACE_FIXUP_MSG = (1 << 1),
- TODO_EDIT_FIXUP_MSG = (1 << 2),
-};
-
static const char first_commit_msg_str[] = N_("This is the 1st commit message:");
static const char nth_commit_msg_fmt[] = N_("This is the commit message #%d:");
static const char skip_first_commit_msg_str[] = N_("The 1st commit message will be skipped:");
static const char skip_nth_commit_msg_fmt[] = N_("The commit message #%d will be skipped:");
static const char combined_commit_msg_fmt[] = N_("This is a combination of %d commits.");
+void add_squash_combination_header(struct strbuf *buf, int n)
+{
+ strbuf_addf(buf, "%s ", comment_line_str);
+ strbuf_addf(buf, _(combined_commit_msg_fmt), n);
+}
+
+void add_squash_message_header(struct strbuf *buf, int n, int skip)
+{
+ strbuf_addf(buf, "%s ", comment_line_str);
+ if (n == 1)
+ strbuf_addstr(buf, skip ? _(skip_first_commit_msg_str) :
+ _(first_commit_msg_str));
+ else
+ strbuf_addf(buf, skip ? _(skip_nth_commit_msg_fmt) :
+ _(nth_commit_msg_fmt), n);
+}
+
+size_t squash_subject_comment_len(const char *body, int squashing)
+{
+ if (starts_with(body, "amend!") ||
+ (squashing && (starts_with(body, "squash!") ||
+ starts_with(body, "fixup!"))))
+ return commit_subject_length(body);
+ return 0;
+}
+
static int is_fixup_flag(enum todo_command command, unsigned flag)
{
return command == TODO_FIXUP && ((flag & TODO_REPLACE_FIXUP_MSG) ||
@@ -2005,20 +2025,13 @@ static int append_squash_message(struct strbuf *buf, const char *body,
{
struct replay_ctx *ctx = opts->ctx;
const char *fixup_msg;
- size_t commented_len = 0, fixup_off;
- /*
- * amend is non-interactive and not normally used with fixup!
- * or squash! commits, so only comment out those subjects when
- * squashing commit messages.
- */
- if (starts_with(body, "amend!") ||
- ((command == TODO_SQUASH || seen_squash(ctx)) &&
- (starts_with(body, "squash!") || starts_with(body, "fixup!"))))
- commented_len = commit_subject_length(body);
+ size_t commented_len, fixup_off;
+
+ commented_len = squash_subject_comment_len(body,
+ command == TODO_SQUASH || seen_squash(ctx));
- strbuf_addf(buf, "\n%s ", comment_line_str);
- strbuf_addf(buf, _(nth_commit_msg_fmt),
- ++ctx->current_fixup_count + 1);
+ strbuf_addch(buf, '\n');
+ add_squash_message_header(buf, ++ctx->current_fixup_count + 1, 0);
strbuf_addstr(buf, "\n\n");
strbuf_add_commented_lines(buf, body, commented_len, comment_line_str);
/* buf->buf may be reallocated so store an offset into the buffer */
@@ -2083,9 +2096,8 @@ static int update_squash_messages(struct repository *r,
eol = !starts_with(buf.buf, comment_line_str) ?
buf.buf : strchrnul(buf.buf, '\n');
- strbuf_addf(&header, "%s ", comment_line_str);
- strbuf_addf(&header, _(combined_commit_msg_fmt),
- ctx->current_fixup_count + 2);
+ add_squash_combination_header(&header,
+ ctx->current_fixup_count + 2);
strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len);
strbuf_release(&header);
if (is_fixup_flag(command, flag) && !seen_squash(ctx))
@@ -2109,12 +2121,9 @@ static int update_squash_messages(struct repository *r,
repo_unuse_commit_buffer(r, head_commit, head_message);
return error(_("cannot write '%s'"), rebase_path_fixup_msg());
}
- strbuf_addf(&buf, "%s ", comment_line_str);
- strbuf_addf(&buf, _(combined_commit_msg_fmt), 2);
- strbuf_addf(&buf, "\n%s ", comment_line_str);
- strbuf_addstr(&buf, is_fixup_flag(command, flag) ?
- _(skip_first_commit_msg_str) :
- _(first_commit_msg_str));
+ add_squash_combination_header(&buf, 2);
+ strbuf_addch(&buf, '\n');
+ add_squash_message_header(&buf, 1, is_fixup_flag(command, flag));
strbuf_addstr(&buf, "\n\n");
if (is_fixup_flag(command, flag))
strbuf_add_commented_lines(&buf, body, strlen(body),
@@ -2133,9 +2142,8 @@ static int update_squash_messages(struct repository *r,
if (command == TODO_SQUASH || is_fixup_flag(command, flag)) {
res = append_squash_message(&buf, body, command, opts, flag);
} else if (command == TODO_FIXUP) {
- strbuf_addf(&buf, "\n%s ", comment_line_str);
- strbuf_addf(&buf, _(skip_nth_commit_msg_fmt),
- ++ctx->current_fixup_count + 1);
+ strbuf_addch(&buf, '\n');
+ add_squash_message_header(&buf, ++ctx->current_fixup_count + 1, 1);
strbuf_addstr(&buf, "\n\n");
strbuf_add_commented_lines(&buf, body, strlen(body),
comment_line_str);
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..b01f897020 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -119,6 +119,13 @@ enum todo_command {
TODO_COMMENT
};
+/* Bits for the "flags" member of struct todo_item */
+enum todo_item_flags {
+ TODO_EDIT_MERGE_MSG = (1 << 0),
+ TODO_REPLACE_FIXUP_MSG = (1 << 1),
+ TODO_EDIT_FIXUP_MSG = (1 << 2),
+};
+
struct todo_item {
enum todo_command command;
struct commit *commit;
@@ -208,6 +215,29 @@ int todo_list_rearrange_squash(struct todo_list *todo_list);
*/
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
+/*
+ * Append the "This is a combination of N commits." banner that "git rebase
+ * -i" writes at the top of a squashed commit's message, commented out with
+ * the comment character.
+ */
+void add_squash_combination_header(struct strbuf *buf, int n);
+
+/*
+ * Append the header (1-based N) that "git rebase -i" writes above each message
+ * when squashing, commented out with the comment character. With SKIP it reads
+ * "The ... commit message will be skipped" for a message that is dropped (a
+ * fixup), otherwise "This is the ... commit message".
+ */
+void add_squash_message_header(struct strbuf *buf, int n, int skip);
+
+/*
+ * Return the length of the leading subject of BODY when it should be commented
+ * out in a squash message, or 0 otherwise. An "amend!" subject always
+ * qualifies; "squash!" and "fixup!" subjects only when SQUASHING, since a
+ * plain fixup chain keeps them.
+ */
+size_t squash_subject_comment_len(const char *body, int squashing);
+
void append_conflicts_hint(struct index_state *istate,
struct strbuf *msgbuf, enum commit_msg_cleanup_mode cleanup_mode);
enum commit_msg_cleanup_mode get_cleanup_mode(const char *cleanup_arg,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 3/5] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v9.git.git.1784128573.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Folding a series of commits into one required either an interactive
rebase where each commit after the first was hand-edited to "fixup", or
a "git reset --soft" to the merge base followed by "git commit --amend".
Add "git history squash <revision-range>" to do this directly. It folds
every commit in the range into the oldest one, keeping that commit's
authorship and taking the tree of the newest commit, then replays the
commits above the range on top. The squashed message comes from the
oldest commit by default, or from the body of the last amend! commit
targeting it. An editor opens with the selected message when
--reedit-message is given. A fixup!, squash! or amend! commit is refused
unless the commit it targets is also in the range, so the fold does not
silently absorb a marker meant for a commit outside it. The check runs
the range through todo_list_rearrange_squash(), which leaves such a
marker as a plain pick. Markers whose target is in the range fold in as
usual. As an exception, a range made up entirely of markers for one
target is combined anyway, taking its message from the last amend! if
there is one, so a batch of fixups for the same commit can be collapsed.
The range is read like the arguments to "git rev-list", so several
revisions such as "HEAD~3..HEAD ^topic" may be given, and rev-list
options are accepted too. As "git replay" does, the walk options the fold
relies on are forced after setup_revisions() and a warning is printed if
an option changed them, so the first commit returned is the range's
oldest and its parent is the base regardless of what the user passed
(including after a "--"). A merge inside the range is folded when its
other parent is reachable from the base, otherwise the range has more
than one base and is rejected. By default the command also refuses when a
ref points at a commit that the fold would discard. Use --update-refs=head
to rewrite only the current branch instead.
Inspired-by: Sergey Chernov <serega.morph@gmail.com>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/advice.adoc | 4 +
Documentation/git-history.adoc | 47 ++-
advice.c | 1 +
advice.h | 1 +
builtin/history.c | 372 ++++++++++++++++++++
t/meson.build | 1 +
t/t3455-history-squash.sh | 565 +++++++++++++++++++++++++++++++
7 files changed, 988 insertions(+), 3 deletions(-)
create mode 100755 t/t3455-history-squash.sh
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 81f80a9274..e2a3487778 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -59,6 +59,10 @@ all advice messages.
forceDeleteBranch::
Shown when the user tries to delete a not fully merged
branch without the force option set.
+ historyUpdateRefs::
+ Shown when `git history squash` refuses because a ref points
+ into the range being folded, to tell the user about
+ `--update-refs=head`.
ignoredHook::
Shown when a hook is ignored because the hook is not
set as executable.
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..2c0d861303 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
+git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>
DESCRIPTION
-----------
@@ -42,8 +43,11 @@ at once.
LIMITATIONS
-----------
-This command does not (yet) work with histories that contain merges. You
-should use linkgit:git-rebase[1] with the `--rebase-merges` flag instead.
+This command does not (yet) replay merge commits onto the rewritten
+history: if a commit that would be replayed is a merge, the operation is
+rejected, and you should use linkgit:git-rebase[1] with the
+`--rebase-merges` flag instead. The `squash` subcommand can still fold a
+merge that lies inside the range, as long as the range has a single base.
Furthermore, the command does not support operations that can result in merge
conflicts. This limitation is by design as history rewrites are not intended to
@@ -97,6 +101,42 @@ linkgit:gitglossary[7].
It is invalid to select either all or no hunks, as that would lead to
one of the commits becoming empty.
+`squash <revision-range>`::
+ Fold all commits in _<revision-range>_ into the oldest commit of that
+ range. The resulting commit keeps the oldest commit's authorship and
+ takes the tree of the range's newest commit, so the whole range
+ collapses into a single commit. Commits above the range are replayed
+ on top of the result.
++
+The range is given in the usual `<base>..<tip>` form, where _<base>_ is
+the commit just below the oldest commit to squash. For example, `git
+history squash HEAD~3..HEAD` folds the three most recent commits into
+one, and `git history squash HEAD~5..HEAD~2` squashes an interior range
+while leaving the two newest commits in place. Several revisions may be
+given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
+already on `topic`. Rev-list options may also be given, but any that would
+change how the range is walked are overridden with a warning.
++
+The oldest commit's message is preserved by default, except that an `amend!`
+commit targeting it replaces its message. Specify `--reedit-message` to edit
+the resulting message. A merge commit inside the range is folded like any
+other, but the range must have a single base, so a range that reaches more
+than one entry point (for example a side branch that forked before the range
+and was later merged into it) is rejected.
++
+A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
+targets is also in the range, so the fold does not silently absorb a
+marker meant for a commit outside it. The body after an `amend!` subject
+replaces the oldest commit's message when the marker targets that commit. As
+an exception, a range made up entirely of markers for one target is combined
+into a single commit, keeping the last `amend!` message if there is one.
++
+A branch or tag that points at a commit inside the range would be left
+dangling once those commits are folded away, so with the default
+`--update-refs=branches` the command refuses. Rerun with
+`--update-refs=head` to rewrite only the current branch and leave such
+refs pointing at the old commits.
+
OPTIONS
-------
@@ -107,7 +147,8 @@ OPTIONS
ref updates is generally safe.
`--reedit-message`::
- Open an editor to modify the target commit's message.
+ Open an editor to modify the rewritten commit's message. For `squash`
+ the editor is pre-filled with the messages of all the folded commits.
`--empty=(drop|keep|abort)`::
Control what happens when a commit becomes empty as a result of the
diff --git a/advice.c b/advice.c
index 63bf8b0c5f..401d047391 100644
--- a/advice.c
+++ b/advice.c
@@ -58,6 +58,7 @@ static struct {
[ADVICE_FETCH_SHOW_FORCED_UPDATES] = { "fetchShowForcedUpdates" },
[ADVICE_FORCE_DELETE_BRANCH] = { "forceDeleteBranch" },
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated" },
+ [ADVICE_HISTORY_UPDATE_REFS] = { "historyUpdateRefs" },
[ADVICE_IGNORED_HOOK] = { "ignoredHook" },
[ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity" },
[ADVICE_MERGE_CONFLICT] = { "mergeConflict" },
diff --git a/advice.h b/advice.h
index 66f6cd6a77..3f0b4f0485 100644
--- a/advice.h
+++ b/advice.h
@@ -25,6 +25,7 @@ enum advice_type {
ADVICE_FETCH_SHOW_FORCED_UPDATES,
ADVICE_FORCE_DELETE_BRANCH,
ADVICE_GRAFT_FILE_DEPRECATED,
+ ADVICE_HISTORY_UPDATE_REFS,
ADVICE_IGNORED_HOOK,
ADVICE_IMPLICIT_IDENTITY,
ADVICE_MERGE_CONFLICT,
diff --git a/builtin/history.c b/builtin/history.c
index cbba25096f..edf98a21d3 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
+#include "advice.h"
#include "cache-tree.h"
#include "commit.h"
#include "commit-reach.h"
@@ -30,6 +31,8 @@
N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
#define GIT_HISTORY_SPLIT_USAGE \
N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
+#define GIT_HISTORY_SQUASH_USAGE \
+ N_("git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>")
static void change_data_free(void *util, const char *str UNUSED)
{
@@ -973,6 +976,373 @@ out:
return ret;
}
+/*
+ * Resolve a "<base>..<tip>" revision range into the base commit just outside
+ * the range (which becomes the parent of the squashed commit), the oldest
+ * commit contained in the range (whose message the squash reuses), and the
+ * range tip (whose tree becomes the result). A merge inside the range is fine,
+ * but the range must have a single base and must not reach a root commit.
+ */
+static int resolve_squash_range(struct repository *repo,
+ const char **argv,
+ struct commit **base_out,
+ struct commit **oldest_out,
+ struct commit **tip_out,
+ struct oidset *interior_out)
+{
+ struct rev_info revs;
+ struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
+ struct commit_list *boundaries = NULL, *b;
+ struct strvec args = STRVEC_INIT;
+ size_t i;
+ int ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ revs.reverse = 1;
+ revs.topo_order = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.simplify_history = 0;
+ revs.boundary = 1;
+
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--ancestry-path");
+ strvec_pushv(&args, argv);
+ setup_revisions_from_strvec(&args, &revs, NULL);
+ if (args.nr != 1) {
+ ret = error(_("unrecognized argument: %s"), args.v[1]);
+ goto out;
+ }
+
+ if (revs.reverse != 1 || revs.topo_order != 1 ||
+ revs.sort_order != REV_SORT_IN_GRAPH_ORDER ||
+ revs.simplify_history != 0) {
+ warning(_("ignoring rev-list options that would change how the "
+ "range is walked"));
+ revs.reverse = 1;
+ revs.topo_order = 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.simplify_history = 0;
+ }
+
+ /*
+ * A squash needs a base to reparent onto, so the range has to exclude
+ * something, as in "<base>..<tip>". A revision range with no such
+ * bottom commit cannot be squashed.
+ */
+ for (i = 0; i < revs.cmdline.nr; i++)
+ if (revs.cmdline.rev[i].flags & UNINTERESTING)
+ break;
+ if (i == revs.cmdline.nr) {
+ ret = error(_("not a '<base>..<tip>' revision range"));
+ goto out;
+ }
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ /*
+ * Set boundary commits aside for the base check below, and put every
+ * in-range commit but the tip into the interior set. A ref pointing
+ * at an interior commit would dangle once the range is folded away.
+ */
+ while ((commit = get_revision(&revs))) {
+ if (commit->object.flags & BOUNDARY) {
+ commit_list_insert(commit, &boundaries);
+ continue;
+ }
+ if (!oldest)
+ oldest = commit;
+ if (tip)
+ oidset_insert(interior_out, &tip->object.oid);
+ tip = commit;
+ }
+
+ if (!oldest) {
+ ret = error(_("the revision range is empty"));
+ goto out;
+ } else if (oldest == tip) {
+ ret = error(_("the revision range holds a single commit; "
+ "nothing to squash"));
+ goto out;
+ } else if (!oldest->parents) {
+ BUG("an in-range commit must have a parent");
+ }
+ base = oldest->parents->item;
+
+ /*
+ * A boundary other than the base is an in-range commit reaching a
+ * commit outside the range, so the range has more than one base.
+ */
+ for (b = boundaries; b; b = b->next) {
+ if (b->item != base) {
+ ret = error(_("the revision range has more than one base; "
+ "cannot squash"));
+ goto out;
+ }
+ }
+
+ *base_out = base;
+ *oldest_out = oldest;
+ *tip_out = tip;
+ ret = 0;
+
+out:
+ commit_list_free(boundaries);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
+static const char *autosquash_target(const char *subject)
+{
+ const char *rest;
+
+ while (skip_prefix(subject, "fixup! ", &rest) ||
+ skip_prefix(subject, "squash! ", &rest) ||
+ skip_prefix(subject, "amend! ", &rest))
+ subject = rest;
+ return subject;
+}
+
+static int reject_dangling_fixups(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct commit *oldest,
+ struct commit **msg_source,
+ struct commit **amend_source)
+{
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
+ struct rev_info revs;
+ struct commit *commit, *last_amend = NULL;
+ struct strvec args = STRVEC_INIT;
+ char *dangling_subject = NULL, *dangling_target = NULL;
+ bool mixed_target = false, all_fixups_one_target;
+ bool past_oldest_group = false;
+ int i, ret, nr_dangling = 0;
+
+ *msg_source = oldest;
+ *amend_source = NULL;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+ while ((commit = get_revision(&revs)))
+ strbuf_addf(&todo.buf, "pick %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ if (todo_list_parse_insn_buffer(repo, &opts, todo.buf.buf, &todo) < 0 ||
+ todo_list_rearrange_squash(&todo) < 0) {
+ ret = error(_("could not check the range for fixups"));
+ goto out;
+ }
+
+ for (i = 0; i < todo.nr; i++) {
+ const char *message, *subject_start, *target;
+ char *subject;
+ size_t sublen;
+
+ message = repo_logmsg_reencode(repo, todo.items[i].commit,
+ NULL, NULL);
+ sublen = find_commit_subject(message, &subject_start);
+
+ if (todo.items[i].command != TODO_PICK) {
+ if (!past_oldest_group &&
+ starts_with(subject_start, "amend! "))
+ *amend_source = todo.items[i].commit;
+ repo_unuse_commit_buffer(repo, todo.items[i].commit, message);
+ continue;
+ }
+ if (i)
+ past_oldest_group = true;
+
+ subject = xmemdupz(subject_start, sublen);
+ target = autosquash_target(subject);
+ if (target != subject) {
+ nr_dangling++;
+ if (!dangling_target) {
+ dangling_target = xstrdup(target);
+ dangling_subject = xstrdup(subject);
+ } else if (strcmp(dangling_target, target)) {
+ mixed_target = true;
+ }
+ if (starts_with(subject, "amend! "))
+ last_amend = todo.items[i].commit;
+ }
+ free(subject);
+ repo_unuse_commit_buffer(repo, todo.items[i].commit, message);
+ }
+
+ all_fixups_one_target = nr_dangling == todo.nr && !mixed_target;
+ if (nr_dangling && !all_fixups_one_target) {
+ ret = error(_("cannot squash '%s': its target is not in the "
+ "range"), dangling_subject);
+ } else {
+ if (last_amend)
+ *msg_source = last_amend;
+ ret = 0;
+ }
+
+out:
+ free(dangling_subject);
+ free(dangling_target);
+ todo_list_release(&todo);
+ replay_opts_release(&opts);
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
+struct interior_ref_cb {
+ const struct oidset *interior;
+ const char *name;
+};
+
+static int find_interior_ref(const struct reference *ref, void *cb_data)
+{
+ struct interior_ref_cb *data = cb_data;
+
+ if (oidset_contains(data->interior, ref->oid)) {
+ data->name = xstrdup(ref->name);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int cmd_history_squash(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_SQUASH_USAGE,
+ NULL,
+ };
+ enum ref_action action = REF_ACTION_DEFAULT;
+ enum commit_tree_flags flags = 0;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+ N_("control which refs should be updated"),
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
+ OPT_BIT(0, "reedit-message", &flags,
+ N_("open an editor to modify the commit message"),
+ COMMIT_TREE_EDIT_MESSAGE),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
+ struct oidset interior = OIDSET_INIT;
+ struct commit *base, *oldest, *tip, *rewritten, *msg_source,
+ *amend_source;
+ const struct object_id *base_tree_oid, *tip_tree_oid;
+ const char *message_template = NULL;
+ struct commit_list *parents = NULL;
+ struct rev_info revs = { 0 };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage,
+ PARSE_OPT_KEEP_UNKNOWN_OPT);
+ if (!argc) {
+ ret = error(_("command expects a revision range"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ ret = resolve_squash_range(repo, argv, &base, &oldest, &tip,
+ &interior);
+ if (ret < 0)
+ goto out;
+
+ ret = reject_dangling_fixups(repo, base, tip, oldest, &msg_source,
+ &amend_source);
+ if (ret < 0)
+ goto out;
+ if (amend_source) {
+ const char *amend_message, *body;
+
+ amend_message = repo_logmsg_reencode(repo, amend_source,
+ NULL, NULL);
+ find_commit_subject(amend_message, &body);
+ body = skip_blank_lines(body + commit_subject_length(body));
+ strbuf_addstr(&message, body);
+ message_template = message.buf;
+ repo_unuse_commit_buffer(repo, amend_source, amend_message);
+ }
+
+ if (action == REF_ACTION_BRANCHES) {
+ struct interior_ref_cb cb = { .interior = &interior };
+
+ refs_for_each_ref(get_main_ref_store(repo),
+ find_interior_ref, &cb);
+ if (cb.name) {
+ ret = error(_("'%s' points into the squashed range"),
+ cb.name);
+ advise_if_enabled(ADVICE_HISTORY_UPDATE_REFS,
+ _("Use --update-refs=head to rewrite only "
+ "the current branch and leave such refs "
+ "untouched."));
+ free((char *)cb.name);
+ goto out;
+ }
+ }
+
+ ret = setup_revwalk(repo, action, tip, &revs);
+ if (ret < 0)
+ goto out;
+
+ base_tree_oid = &repo_get_commit_tree(repo, base)->object.oid;
+ tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
+ commit_list_append(base, &parents);
+
+ ret = commit_tree_ext(repo, "squash", msg_source, message_template,
+ parents,
+ base_tree_oid, tip_tree_oid, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing squashed commit"));
+ goto out;
+ }
+
+ strbuf_addf(&reflog_msg, "squash: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, tip, rewritten,
+ reflog_msg.buf, dry_run,
+ REPLAY_EMPTY_COMMIT_ABORT);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ strbuf_release(&reflog_msg);
+ strbuf_release(&message);
+ oidset_clear(&interior);
+ commit_list_free(parents);
+ release_revisions(&revs);
+ return ret;
+}
+
int cmd_history(int argc,
const char **argv,
const char *prefix,
@@ -982,6 +1352,7 @@ int cmd_history(int argc,
GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
+ GIT_HISTORY_SQUASH_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -989,6 +1360,7 @@ int cmd_history(int argc,
OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
+ OPT_SUBCOMMAND("squash", &fn, cmd_history_squash),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 7c3c070426..459e251623 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -400,6 +400,7 @@ integration_tests = [
't3451-history-reword.sh',
't3452-history-split.sh',
't3453-history-fixup.sh',
+ 't3455-history-squash.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
new file mode 100755
index 0000000000..d6199b9644
--- /dev/null
+++ b/t/t3455-history-squash.sh
@@ -0,0 +1,565 @@
+#!/bin/sh
+
+test_description='tests for git-history squash subcommand'
+
+. ./test-lib.sh
+
+stage_file () {
+ printf "%s\n" "$1" >file &&
+ git add file
+}
+
+commit_with_message () {
+ printf "%b" "$1" >msg &&
+ git commit --allow-empty -qF msg
+}
+
+check_commit_count () {
+ git rev-list --count "$1" >actual &&
+ echo "$2" >expect &&
+ test_cmp expect actual
+}
+
+check_log_subjects () {
+ git log --format="%s" "$1" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+check_log_messages () {
+ git log --format="%B" "$1" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success 'setup linear history touching two files' '
+ test_commit base file a &&
+ git tag start &&
+ test_commit --no-tag one other x &&
+ test_commit --no-tag two file c &&
+ test_commit three file d
+'
+
+test_expect_success 'errors on missing range argument' '
+ test_must_fail git history squash 2>err &&
+ test_grep "expects a revision range" err
+'
+
+test_expect_success 'errors on an empty range' '
+ test_must_fail git history squash HEAD..HEAD 2>err &&
+ test_grep "the revision range is empty" err
+'
+
+test_expect_success 'errors on a single revision that is not a range' '
+ test_must_fail git history squash HEAD 2>err &&
+ test_grep "not a .*range" err &&
+ test_must_fail git history squash HEAD~1 2>err &&
+ test_grep "not a .*range" err
+'
+
+test_expect_success 'errors on a range holding a single commit' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash "HEAD^!" 2>err &&
+ test_grep "single commit; nothing to squash" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'accepts multiple revision arguments with an exclusion' '
+ git reset --hard three &&
+ git branch -f keep HEAD~2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..HEAD ^keep &&
+
+ check_log_subjects start..HEAD <<-\EOF &&
+ two
+ one
+ EOF
+ test_cmp_rev keep HEAD~1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+
+ git branch -D keep
+'
+
+test_expect_success 'squashes a branch the current branch is not on' '
+ git reset --hard three &&
+ main=$(git symbolic-ref --short HEAD) &&
+ head_before=$(git rev-parse HEAD) &&
+ git checkout -b off-history start &&
+ test_commit --no-tag off-one off a &&
+ test_commit --no-tag off-two off b &&
+ git checkout "$main" &&
+
+ git history squash start..off-history &&
+
+ check_commit_count start..off-history 1 &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D off-history
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash --dry-run start.. >out &&
+ predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git history squash start.. &&
+
+ test "$predicted" = "$(git rev-parse HEAD)" &&
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ check_log_subjects -1 <<-\EOF &&
+ one
+ EOF
+ git reflog >reflog &&
+ test_grep "squash: updating" reflog
+'
+
+test_expect_success 'sanitizes rev-list walk options, before and after --' '
+ git reset --hard three &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash --date-order start.. 2>err &&
+ test_grep "ignoring rev-list options" err &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+
+ git reset --hard three &&
+ git history squash -- --reverse start.. 2>err &&
+ test_grep "ignoring rev-list options" err &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes an interior range and replays descendants verbatim' '
+ git reset --hard three &&
+ final_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start..@~1 &&
+
+ check_log_subjects start..HEAD <<-\EOF &&
+ three
+ one
+ EOF
+
+ test_cmp_rev start HEAD~2 &&
+ test "$final_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes when the base is the root commit' '
+ git reset --hard three &&
+ root=$(git rev-list --max-parents=0 HEAD) &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash "$root.." &&
+
+ check_commit_count "$root..HEAD" 1 &&
+ test_cmp_rev "$root" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+
+test_expect_success 'folds fixups whose target is in the range' '
+ git reset --hard start &&
+ test_commit --no-tag target file b &&
+ git commit --allow-empty -m "fixup! target" &&
+ git commit --allow-empty -m "fixup! target" &&
+ test_commit --no-tag later file c &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF
+ target
+ EOF
+'
+
+test_expect_success 'refuses a below-range fixup! after an in-range commit' '
+ git reset --hard start &&
+ test_commit --no-tag inside file b &&
+ test_commit --no-tag "fixup! outside" file c &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "target is not in the range" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'combines a run of fixups for one commit below the range' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! base" &&
+ stage_file c && git commit -m "fixup! base" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF
+ fixup! base
+ EOF
+'
+
+test_expect_success 'combining below-range fixups keeps the last amend! message' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! base" &&
+ stage_file c &&
+ commit_with_message "amend! base\n\namended body\n" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_messages -1 <<-\EOF
+ amend! base
+
+ amended body
+
+ EOF
+'
+
+test_expect_success 'refuses fixups for two different commits below the range' '
+ git reset --hard start &&
+ stage_file b && git commit -m "fixup! aaa" &&
+ stage_file c && git commit -m "fixup! bbb" &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "target is not in the range" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'the last amend! for the oldest commit replaces its message' '
+ git reset --hard start &&
+ test_commit --no-tag marker-oldest file b &&
+ git commit --allow-empty -m "squash! marker-oldest" &&
+ commit_with_message "amend! marker-oldest\n\nearlier message\n" &&
+ commit_with_message \
+ "amend! marker-oldest\n\namended subject\n\namended body\n" &&
+ test_commit --no-tag marker-later file c &&
+ commit_with_message "amend! marker-later\n\nwrong message\n" &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_messages -1 <<-\EOF
+ amended subject
+
+ amended body
+
+ EOF
+'
+
+test_expect_success 'preserves authorship of the oldest commit' '
+ git reset --hard start &&
+ GIT_AUTHOR_NAME=Squasher GIT_AUTHOR_EMAIL=squash@example.com \
+ test_commit --no-tag oldest file b &&
+ test_commit newest file c &&
+
+ git history squash start.. &&
+
+ git log -1 --format="%an <%ae>" >actual &&
+ echo "Squasher <squash@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--update-refs=head only moves HEAD' '
+ git reset --hard three &&
+ git branch -f other HEAD &&
+ other_before=$(git rev-parse other) &&
+
+ git history squash --update-refs=head start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$other_before" other
+'
+
+test_expect_success 'refuses to fold a range a ref points into' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "error: .* points into the squashed range" err &&
+ test_grep "hint: .*--update-refs=head" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D mid
+'
+
+test_expect_success 'advice.historyUpdateRefs silences the hint' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git -c advice.historyUpdateRefs=false \
+ history squash start.. 2>err &&
+ test_grep "points into the squashed range" err &&
+ test_grep ! "hint:" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D mid
+'
+
+test_expect_success '--update-refs=head folds past a ref pointing into the range' '
+ git reset --hard three &&
+ git branch -f mid HEAD~1 &&
+ mid_before=$(git rev-parse mid) &&
+
+ git history squash --update-refs=head start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$mid_before" mid &&
+
+ git branch -D mid
+'
+
+test_expect_success 'refuses to fold a range a tag points into' '
+ git reset --hard three &&
+ git tag -f mark HEAD~1 &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "refs/tags/mark" err &&
+ test_grep "points into the squashed range" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git tag -d mark
+'
+
+test_expect_success 'squashes a range whose internal merge has a single base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag before-side file b &&
+ git checkout -b inner-side &&
+ test_commit --no-tag on-inner-side inner x &&
+ git checkout "$main" &&
+ test_commit --no-tag after-side file c &&
+ git merge --no-ff -m merge inner-side &&
+ git branch -D inner-side &&
+ test_commit --no-tag after-merge file d &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ check_log_subjects -1 <<-\EOF &&
+ before-side
+ EOF
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file inner
+'
+
+test_expect_success 'folds a merge of a branch that forked at the base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b base-fork-side &&
+ test_commit --no-tag base-fork-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag base-fork-main file b &&
+ git merge --no-ff -m "merge base-fork-side" base-fork-side &&
+ git branch -D base-fork-side &&
+ test_commit --no-tag base-fork-tail file c &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
+
+test_expect_success 'refuses a merge whose other parent is outside the range' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b outside-parent &&
+ test_commit --no-tag outside-parent outside x &&
+ git checkout "$main" &&
+ test_commit --no-tag outside-main file b &&
+ base=$(git rev-parse HEAD) &&
+ test_commit --no-tag outside-mid file c &&
+ git merge --no-ff -m "merge outside-parent" outside-parent &&
+ git branch -D outside-parent &&
+ merged=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash "$base.." 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_expect_success 'folds a range whose tip is a merge commit' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag tipmerge-base file b &&
+ git checkout -b tipmerge-side &&
+ test_commit --no-tag tipmerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag tipmerge-main file c &&
+ git merge --no-ff -m "merge tipmerge-side" tipmerge-side &&
+ git branch -D tipmerge-side &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
+
+test_expect_success 'folds a range whose base is a merge commit' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b basemerge-side &&
+ test_commit --no-tag basemerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag basemerge-main file b &&
+ git merge --no-ff -m "merge basemerge-side" basemerge-side &&
+ git branch -D basemerge-side &&
+ base=$(git rev-parse HEAD) &&
+ test_commit --no-tag basemerge-one file c &&
+ test_commit --no-tag basemerge-two file d &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash "$base.." &&
+
+ check_commit_count "$base..HEAD" 1 &&
+ test_cmp_rev "$base" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'folds a range with two interior merges' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag two-merge-a file a1 &&
+ git checkout -b two-merge-s1 &&
+ test_commit --no-tag two-merge-s1 s1 x &&
+ git checkout "$main" &&
+ git merge --no-ff -m "merge s1" two-merge-s1 &&
+ test_commit --no-tag two-merge-b file b1 &&
+ git checkout -b two-merge-s2 &&
+ test_commit --no-tag two-merge-s2 s2 y &&
+ git checkout "$main" &&
+ git merge --no-ff -m "merge s2" two-merge-s2 &&
+ git branch -D two-merge-s1 two-merge-s2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file s1 &&
+ test_path_is_file s2
+'
+
+test_expect_success 'folds a range with a nested merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b nested-outer &&
+ test_commit --no-tag nested-outer outer x &&
+ git checkout -b nested-inner &&
+ test_commit --no-tag nested-inner inner y &&
+ git checkout nested-outer &&
+ git merge --no-ff -m "merge inner" nested-inner &&
+ git checkout "$main" &&
+ test_commit --no-tag nested-main file b1 &&
+ git merge --no-ff -m "merge outer" nested-outer &&
+ git branch -D nested-outer nested-inner &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file outer &&
+ test_path_is_file inner
+'
+
+test_expect_success 'folds a range with an octopus merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag octo-base file a1 &&
+ git checkout -b octo-1 &&
+ test_commit --no-tag octo-1 o1 x &&
+ git checkout "$main" &&
+ git checkout -b octo-2 &&
+ test_commit --no-tag octo-2 o2 y &&
+ git checkout "$main" &&
+ git merge --no-ff -m octopus octo-1 octo-2 &&
+ git branch -D octo-1 octo-2 &&
+ tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+ git history squash start.. &&
+
+ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file o1 &&
+ test_path_is_file o2
+'
+
+test_expect_success 'refuses an octopus merge with an arm forked before the base' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ git checkout -b octo-pre &&
+ test_commit octo-pre-side pside x &&
+ git checkout "$main" &&
+ test_commit octo-pre-main file b1 &&
+ octo_base=$(git rev-parse HEAD) &&
+ git checkout -b octo-within &&
+ test_commit --no-tag octo-within wside y &&
+ git checkout "$main" &&
+ git merge --no-ff -m octopus octo-pre octo-within &&
+ merged=$(git rev-parse HEAD) &&
+ git branch -D octo-pre octo-within &&
+
+ test_must_fail git history squash "$octo_base.." 2>err &&
+ test_grep "more than one base" err &&
+ test_cmp_rev "$merged" HEAD
+'
+
+test_expect_success 'refuses when a descendant above the range is a merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag desc-one file b &&
+ test_commit --no-tag desc-two file c &&
+ git tag desc-tip &&
+ git checkout -b desc-above &&
+ test_commit --no-tag desc-above above x &&
+ git checkout "$main" &&
+ test_commit --no-tag desc-main file d &&
+ git merge --no-ff -m "merge desc-above" desc-above &&
+ git branch -D desc-above &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start..desc-tip 2>err &&
+ test_grep "merge commits is not supported" err &&
+ test_cmp_rev "$head_before" HEAD
+'
+
+test_expect_success 'refuses to fold a range a ref points into at a merge' '
+ git reset --hard start &&
+ main=$(git symbolic-ref --short HEAD) &&
+ test_commit --no-tag refmerge-base file b &&
+ git checkout -b refmerge-side &&
+ test_commit --no-tag refmerge-side side x &&
+ git checkout "$main" &&
+ test_commit --no-tag refmerge-main file c &&
+ git merge --no-ff -m "interior merge" refmerge-side &&
+ git branch -D refmerge-side &&
+ git branch at-merge HEAD &&
+ test_commit --no-tag refmerge-tail file d &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
+ test_grep "at-merge" err &&
+ test_grep "points into the squashed range" err &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D at-merge
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 2/5] history: give commit_tree_ext a message template
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v9.git.git.1784128573.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
commit_tree_ext() reuses the message of the commit it is handed. A
caller that folds several commits together wants to seed the message
from more than that single commit, so add an optional message_template
parameter. When NULL, the behavior is unchanged.
Pass NULL from the existing fixup and split callers.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 9f516687fe..cbba25096f 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -101,6 +101,7 @@ enum commit_tree_flags {
static int commit_tree_ext(struct repository *repo,
const char *action,
struct commit *commit_with_message,
+ const char *message_template,
const struct commit_list *parents,
const struct object_id *old_tree,
const struct object_id *new_tree,
@@ -130,13 +131,16 @@ static int commit_tree_ext(struct repository *repo,
original_author = xmemdupz(ptr, len);
find_commit_subject(original_message, &original_body);
+ if (!message_template)
+ message_template = original_body;
+
if (flags & COMMIT_TREE_EDIT_MESSAGE) {
ret = fill_commit_message(repo, old_tree, new_tree,
- original_body, action, &commit_message);
+ message_template, action, &commit_message);
if (ret < 0)
goto out;
} else {
- strbuf_addstr(&commit_message, original_body);
+ strbuf_addstr(&commit_message, message_template);
}
original_extra_headers = read_commit_extra_headers(commit_with_message,
@@ -189,7 +193,7 @@ static int commit_tree_with_edited_message(struct repository *repo,
if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
return -1;
- return commit_tree_ext(repo, action, original, original->parents,
+ return commit_tree_ext(repo, action, original, NULL, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
}
@@ -644,7 +648,7 @@ static int cmd_history_fixup(int argc,
goto out;
if (!skip_commit) {
- ret = commit_tree_ext(repo, "fixup", original, original->parents,
+ ret = commit_tree_ext(repo, "fixup", original, NULL, original->parents,
&original_tree->object.oid, &merge_result.tree->object.oid,
&rewritten, flags);
if (ret < 0) {
@@ -855,7 +859,7 @@ static int split_commit(struct repository *repo,
* The first commit is constructed from the split-out tree. The base
* that shall be diffed against is the parent of the original commit.
*/
- ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, original->parents, &parent_tree_oid,
&split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing first commit"));
@@ -872,7 +876,7 @@ static int split_commit(struct repository *repo,
old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid;
new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+ ret = commit_tree_ext(repo, "split-out", original, NULL, parents, old_tree_oid,
new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing second commit"));
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 1/5] history: extract helper for a commit's parent tree
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v9.git.git.1784128573.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Three places resolve the tree of a commit's first parent, falling back
to the empty tree for a root commit, each repeating the same parse and
oidcpy dance. Extract a first_parent_tree_oid() helper and route the
existing callers through it.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/history.c | 58 +++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 32 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index fd83de8265..9f516687fe 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -157,6 +157,25 @@ out:
return ret;
}
+static int first_parent_tree_oid(struct repository *repo,
+ struct commit *commit,
+ struct object_id *out)
+{
+ struct commit *parent = commit->parents ? commit->parents->item : NULL;
+
+ if (!parent) {
+ oidcpy(out, repo->hash_algo->empty_tree);
+ return 0;
+ }
+
+ if (repo_parse_commit(repo, parent))
+ return error(_("unable to parse parent commit %s"),
+ oid_to_hex(&parent->object.oid));
+
+ oidcpy(out, &repo_get_commit_tree(repo, parent)->object.oid);
+ return 0;
+}
+
static int commit_tree_with_edited_message(struct repository *repo,
const char *action,
struct commit *original,
@@ -164,21 +183,11 @@ static int commit_tree_with_edited_message(struct repository *repo,
{
struct object_id parent_tree_oid;
const struct object_id *tree_oid;
- struct commit *parent;
tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
- parent = original->parents ? original->parents->item : NULL;
- if (parent) {
- if (repo_parse_commit(repo, parent)) {
- return error(_("unable to parse parent commit %s"),
- oid_to_hex(&parent->object.oid));
- }
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return commit_tree_ext(repo, action, original, original->parents,
&parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
@@ -444,18 +453,10 @@ static int commit_became_empty(struct repository *repo,
struct commit *original,
struct tree *result)
{
- struct commit *parent = original->parents ? original->parents->item : NULL;
struct object_id parent_tree_oid;
- if (parent) {
- if (repo_parse_commit(repo, parent))
- return error(_("unable to parse parent of %s"),
- oid_to_hex(&original->object.oid));
-
- parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
- }
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+ return -1;
return oideq(&result->object.oid, &parent_tree_oid);
}
@@ -799,16 +800,9 @@ static int split_commit(struct repository *repo,
struct tree *split_tree;
int ret;
- if (original->parents) {
- if (repo_parse_commit(repo, original->parents->item)) {
- ret = error(_("unable to parse parent commit %s"),
- oid_to_hex(&original->parents->item->object.oid));
- goto out;
- }
-
- parent_tree_oid = *get_commit_tree_oid(original->parents->item);
- } else {
- oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
+ if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0) {
+ ret = -1;
+ goto out;
}
original_commit_tree_oid = get_commit_tree_oid(original);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 0/5] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-07-15 15:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, D. Ben Knoble, Patrick Steinhardt, Matt Hunter,
Harald Nordgren
In-Reply-To: <pull.2337.v8.git.git.1783674396.gitgitgadget@gmail.com>
Adds git history squash <revision-range> to fold a range of commits.
Changes in v9:
* Use the last amend! targeting the oldest folded commit as the default
squashed message. Ignore amend! markers targeting later commits while
selecting that replacement message.
* Improve tests.
Changes in v8:
* --reedit-message now builds the same editor template as git rebase -i
--autosquash: fixup!, squash! and amend! commits are grouped under the
commit they target instead of shown in commit order, and an amend!
replaces its target's message.
* A fixup!, squash! or amend! is refused only when its target is outside
the range, so several fixups for an in-range commit fold together. A
range that is entirely markers for one below-range target is combined
into a single commit, keeping the last amend! message.
* Merges inside the range are folded when the range has a single base, with
no dedicated opt-in flag, --ancestry-path ensures only commits descended
from the base are folded, and a range reaching more than one base is
rejected.
* Rev-list options are accepted and sanitized the way git replay does,
forcing the walk order back with a warning, which also fixes git history
squash -- --reverse slipping past the previous option check.
* Kept this as an explicit squash subcommand rather than making
--reedit-message the default or renaming the command.
Changes in v7:
* --reedit-message now builds the same editor template git rebase -i shows
for a squash (a combination of N commits banner with each folded message
under its own header) and follows autosquash for markers: a fixup!
message falls out (commented under a will be skipped header), while a
squash! or amend! keeps its body with only the marker subject commented
so its remark can be reworded in. Only the message text is affected,
every commit's changes are always folded in.
* Reuse git rebase -i's squash-message code: a preparatory sequencer:
commit extracts the banner, header and marker-comment helpers so both
rebase and git history squash build the identical template from one
source.
* Refuse a range whose oldest commit is a fixup!, squash! or amend!, since
the marker's target cannot be inside the range.
* Reorder the squash usage so dashed options come before <revision-range>,
and spell out HEAD instead of @ in the documentation and examples.
* Expand the squash commit message and documentation with this overview,
and scope the merge limitation so it no longer contradicts squash folding
a single-base interior merge.
Changes in v6:
* git history squash now accepts multiple revision arguments, read like the
arguments to git-rev-list, so a compound range such as @~3.. ^topic
works.
* The base to reparent onto is now the oldest in-range commit's parent; a
boundary other than that base means the range has more than one base and
is rejected. This also fixes the earlier overly-restrictive handling of
merges and side branches.
* A single-commit range (e.g. @^!) is rejected with "nothing to squash"
(this also covers the @^!-style example that previously succeeded
silently).
* Commit messages reworded: the squash commit now gives an overview of
fixup!/squash!/amend! handling, rewording, merge-parent and ref behavior.
Changes in v5:
* The range walk now uses --ancestry-path, so only commits descended from
the base are folded; a single revision such as HEAD or HEAD~1 is now
rejected as "not a <base>..<tip> range" rather than treated as a squash
down to the root.
* This adopts the --ancestry-path suggestion; the multi-base rejection is
unchanged, so a side branch that forked before the base and merged in is
still refused.
* Added tests covering more merge topologies: two interior merges, a nested
merge, an octopus merge, an octopus arm forked before the base, a merge
among the descendants replayed above the range, and a ref pointing at an
interior merge commit.
Changes in v4:
* git history squash now detects when another ref points at a commit inside
the range being folded and refuses, with an advice.historyUpdateRefs hint
to use --update-refs=head.
* A merge inside the range is folded fine as long as the range has a single
base; a range with merge commit at the tip or base also folds correctly.
Only a range with more than one base is rejected.
Changes in v3:
* Moved the feature out of git rebase and into a new git history squash
<revision-range> subcommand, per the list discussion. git rebase --squash
is dropped.
* Takes an arbitrary range (git history squash @~3.., git history squash
@~5..@~2), folding it into the oldest commit and replaying any
descendants on top.
* Implemented as a single tree operation rather than picking each commit,
so there are no repeated conflict stops (addresses Phillip's efficiency
point).
* A merge inside the range is folded fine, only a range with more than one
base is rejected.
* --reedit-message seeds the editor with every folded-in message, not just
the oldest.
Harald Nordgren (5):
history: extract helper for a commit's parent tree
history: give commit_tree_ext a message template
history: add squash subcommand to fold a range
sequencer: share the squash message marker helpers and flags
history: re-edit a squash with every message
Documentation/config/advice.adoc | 4 +
Documentation/git-history.adoc | 57 ++-
advice.c | 1 +
advice.h | 1 +
builtin/history.c | 550 ++++++++++++++++++++--
sequencer.c | 70 +--
sequencer.h | 30 ++
t/meson.build | 1 +
t/t3455-history-squash.sh | 766 +++++++++++++++++++++++++++++++
9 files changed, 1408 insertions(+), 72 deletions(-)
create mode 100755 t/t3455-history-squash.sh
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v9
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v9
Pull-Request: https://github.com/git/git/pull/2337
Range-diff vs v8:
1: ba77752282 = 1: 352c818c29 history: extract helper for a commit's parent tree
2: 50f3572887 = 2: e06e49095b history: give commit_tree_ext a message template
3: 2d81a40a05 ! 3: ead974c317 history: add squash subcommand to fold a range
@@ Commit message
Add "git history squash <revision-range>" to do this directly. It folds
every commit in the range into the oldest one, keeping that commit's
- message and authorship and taking the tree of the newest commit, then
- replays the commits above the range on top. The squashed message comes
- from the oldest commit, or from an editor with --reedit-message. A
- fixup!, squash! or amend! commit is refused unless the commit it targets
- is also in the range, so the fold does not silently absorb a marker meant
- for a commit outside it. The check runs the range through
- todo_list_rearrange_squash(), which leaves such a marker as a plain pick.
- Markers whose target is in the range fold in as usual. As an exception, a
- range made up entirely of markers for one target is combined anyway,
- taking its message from the last amend! if there is one, so a batch of
- fixups for the same commit can be collapsed.
+ authorship and taking the tree of the newest commit, then replays the
+ commits above the range on top. The squashed message comes from the
+ oldest commit by default, or from the body of the last amend! commit
+ targeting it. An editor opens with the selected message when
+ --reedit-message is given. A fixup!, squash! or amend! commit is refused
+ unless the commit it targets is also in the range, so the fold does not
+ silently absorb a marker meant for a commit outside it. The check runs
+ the range through todo_list_rearrange_squash(), which leaves such a
+ marker as a plain pick. Markers whose target is in the range fold in as
+ usual. As an exception, a range made up entirely of markers for one
+ target is combined anyway, taking its message from the last amend! if
+ there is one, so a batch of fixups for the same commit can be collapsed.
The range is read like the arguments to "git rev-list", so several
revisions such as "HEAD~3..HEAD ^topic" may be given, and rev-list
@@ Documentation/git-history.adoc: linkgit:gitglossary[7].
+`squash <revision-range>`::
+ Fold all commits in _<revision-range>_ into the oldest commit of that
-+ range. The resulting commit keeps the oldest commit's message and
-+ authorship and takes the tree of the range's newest commit, so the
-+ whole range collapses into a single commit. Commits above the range
-+ are replayed on top of the result.
++ range. The resulting commit keeps the oldest commit's authorship and
++ takes the tree of the range's newest commit, so the whole range
++ collapses into a single commit. Commits above the range are replayed
++ on top of the result.
++
+The range is given in the usual `<base>..<tip>` form, where _<base>_ is
+the commit just below the oldest commit to squash. For example, `git
@@ Documentation/git-history.adoc: linkgit:gitglossary[7].
+already on `topic`. Rev-list options may also be given, but any that would
+change how the range is walked are overridden with a warning.
++
-+The oldest commit's message and authorship are preserved by default,
-+unless you specify `--reedit-message`. A merge commit inside the range is
-+folded like any other, but the range must have a single base, so a range
-+that reaches more than one entry point (for example a side branch that
-+forked before the range and was later merged into it) is rejected.
++The oldest commit's message is preserved by default, except that an `amend!`
++commit targeting it replaces its message. Specify `--reedit-message` to edit
++the resulting message. A merge commit inside the range is folded like any
++other, but the range must have a single base, so a range that reaches more
++than one entry point (for example a side branch that forked before the range
++and was later merged into it) is rejected.
++
+A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
+targets is also in the range, so the fold does not silently absorb a
-+marker meant for a commit outside it. As an exception, a range made up
-+entirely of markers for one target is combined into a single commit,
-+keeping the last `amend!` message if there is one.
++marker meant for a commit outside it. The body after an `amend!` subject
++replaces the oldest commit's message when the marker targets that commit. As
++an exception, a range made up entirely of markers for one target is combined
++into a single commit, keeping the last `amend!` message if there is one.
++
+A branch or tag that points at a commit inside the range would be left
+dangling once those commits are folded away, so with the default
@@ builtin/history.c: out:
+ struct commit *base,
+ struct commit *tip,
+ struct commit *oldest,
-+ struct commit **msg_source)
++ struct commit **msg_source,
++ struct commit **amend_source)
+{
+ struct todo_list todo = TODO_LIST_INIT;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
@@ builtin/history.c: out:
+ struct strvec args = STRVEC_INIT;
+ char *dangling_subject = NULL, *dangling_target = NULL;
+ bool mixed_target = false, all_fixups_one_target;
++ bool past_oldest_group = false;
+ int i, ret, nr_dangling = 0;
+
+ *msg_source = oldest;
++ *amend_source = NULL;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
@@ builtin/history.c: out:
+ char *subject;
+ size_t sublen;
+
-+ if (todo.items[i].command != TODO_PICK)
-+ continue;
+ message = repo_logmsg_reencode(repo, todo.items[i].commit,
+ NULL, NULL);
+ sublen = find_commit_subject(message, &subject_start);
++
++ if (todo.items[i].command != TODO_PICK) {
++ if (!past_oldest_group &&
++ starts_with(subject_start, "amend! "))
++ *amend_source = todo.items[i].commit;
++ repo_unuse_commit_buffer(repo, todo.items[i].commit, message);
++ continue;
++ }
++ if (i)
++ past_oldest_group = true;
++
+ subject = xmemdupz(subject_start, sublen);
+ target = autosquash_target(subject);
+ if (target != subject) {
@@ builtin/history.c: out:
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
++ struct strbuf message = STRBUF_INIT;
+ struct oidset interior = OIDSET_INIT;
-+ struct commit *base, *oldest, *tip, *rewritten, *msg_source;
++ struct commit *base, *oldest, *tip, *rewritten, *msg_source,
++ *amend_source;
+ const struct object_id *base_tree_oid, *tip_tree_oid;
++ const char *message_template = NULL;
+ struct commit_list *parents = NULL;
+ struct rev_info revs = { 0 };
+ int ret;
@@ builtin/history.c: out:
+ if (ret < 0)
+ goto out;
+
-+ ret = reject_dangling_fixups(repo, base, tip, oldest, &msg_source);
++ ret = reject_dangling_fixups(repo, base, tip, oldest, &msg_source,
++ &amend_source);
+ if (ret < 0)
+ goto out;
++ if (amend_source) {
++ const char *amend_message, *body;
++
++ amend_message = repo_logmsg_reencode(repo, amend_source,
++ NULL, NULL);
++ find_commit_subject(amend_message, &body);
++ body = skip_blank_lines(body + commit_subject_length(body));
++ strbuf_addstr(&message, body);
++ message_template = message.buf;
++ repo_unuse_commit_buffer(repo, amend_source, amend_message);
++ }
+
+ if (action == REF_ACTION_BRANCHES) {
+ struct interior_ref_cb cb = { .interior = &interior };
@@ builtin/history.c: out:
+ tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
+ commit_list_append(base, &parents);
+
-+ ret = commit_tree_ext(repo, "squash", msg_source, NULL, parents,
++ ret = commit_tree_ext(repo, "squash", msg_source, message_template,
++ parents,
+ base_tree_oid, tip_tree_oid, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing squashed commit"));
@@ builtin/history.c: out:
+
+out:
+ strbuf_release(&reflog_msg);
++ strbuf_release(&message);
+ oidset_clear(&interior);
+ commit_list_free(parents);
+ release_revisions(&revs);
@@ t/t3455-history-squash.sh (new)
+
+. ./test-lib.sh
+
++stage_file () {
++ printf "%s\n" "$1" >file &&
++ git add file
++}
++
++commit_with_message () {
++ printf "%b" "$1" >msg &&
++ git commit --allow-empty -qF msg
++}
++
++check_commit_count () {
++ git rev-list --count "$1" >actual &&
++ echo "$2" >expect &&
++ test_cmp expect actual
++}
++
++check_log_subjects () {
++ git log --format="%s" "$1" >actual &&
++ cat >expect &&
++ test_cmp expect actual
++}
++
++check_log_messages () {
++ git log --format="%B" "$1" >actual &&
++ cat >expect &&
++ test_cmp expect actual
++}
++
+test_expect_success 'setup linear history touching two files' '
+ test_commit base file a &&
+ git tag start &&
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start..HEAD ^keep &&
+
-+ git log --format="%s" start..HEAD >actual &&
-+ cat >expect <<-\EOF &&
++ check_log_subjects start..HEAD <<-\EOF &&
+ two
+ one
+ EOF
-+ test_cmp expect actual &&
+ test_cmp_rev keep HEAD~1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start..off-history &&
+
-+ git rev-list --count start..off-history >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..off-history 1 &&
+ test_cmp_rev "$head_before" HEAD &&
+
+ git branch -D off-history
@@ t/t3455-history-squash.sh (new)
+ git history squash start.. &&
+
+ test "$predicted" = "$(git rev-parse HEAD)" &&
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
-+ git log --format="%s" -1 >subject &&
-+ echo one >expect &&
-+ test_cmp expect subject &&
++ check_log_subjects -1 <<-\EOF &&
++ one
++ EOF
+ git reflog >reflog &&
+ test_grep "squash: updating" reflog
+'
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start..@~1 &&
+
-+ git log --format="%s" start..HEAD >actual &&
-+ cat >expect <<-\EOF &&
++ check_log_subjects start..HEAD <<-\EOF &&
+ three
+ one
+ EOF
-+ test_cmp expect actual &&
+
+ test_cmp_rev start HEAD~2 &&
+ test "$final_tree" = "$(git rev-parse HEAD^{tree})"
@@ t/t3455-history-squash.sh (new)
+
+ git history squash "$root.." &&
+
-+ git rev-list --count "$root..HEAD" >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count "$root..HEAD" 1 &&
+ test_cmp_rev "$root" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
-+ git log --format="%s" -1 >actual &&
-+ echo target >expect &&
-+ test_cmp expect actual
++ check_commit_count start..HEAD 1 &&
++ check_log_subjects -1 <<-\EOF
++ target
++ EOF
+'
+
+test_expect_success 'refuses a below-range fixup! after an in-range commit' '
@@ t/t3455-history-squash.sh (new)
+
+test_expect_success 'combines a run of fixups for one commit below the range' '
+ git reset --hard start &&
-+ echo b >file && git add file && git commit -m "fixup! base" &&
-+ echo c >file && git add file && git commit -m "fixup! base" &&
++ stage_file b && git commit -m "fixup! base" &&
++ stage_file c && git commit -m "fixup! base" &&
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
-+ git log --format="%s" -1 >actual &&
-+ echo "fixup! base" >expect &&
-+ test_cmp expect actual
++ check_commit_count start..HEAD 1 &&
++ check_log_subjects -1 <<-\EOF
++ fixup! base
++ EOF
+'
+
+test_expect_success 'combining below-range fixups keeps the last amend! message' '
+ git reset --hard start &&
-+ echo b >file && git add file && git commit -m "fixup! base" &&
-+ printf "amend! base\n\namended body\n" >msg &&
-+ echo c >file && git add file && git commit -qF msg &&
++ stage_file b && git commit -m "fixup! base" &&
++ stage_file c &&
++ commit_with_message "amend! base\n\namended body\n" &&
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
-+ git log --format="%s" -1 >actual &&
-+ echo "amend! base" >expect &&
-+ test_cmp expect actual &&
-+ git log --format="%b" -1 >body &&
-+ test_grep "amended body" body
++ check_commit_count start..HEAD 1 &&
++ check_log_messages -1 <<-\EOF
++ amend! base
++
++ amended body
++
++ EOF
+'
+
+test_expect_success 'refuses fixups for two different commits below the range' '
+ git reset --hard start &&
-+ echo b >file && git add file && git commit -m "fixup! aaa" &&
-+ echo c >file && git add file && git commit -m "fixup! bbb" &&
++ stage_file b && git commit -m "fixup! aaa" &&
++ stage_file c && git commit -m "fixup! bbb" &&
+ head_before=$(git rev-parse HEAD) &&
+
+ test_must_fail git history squash start.. 2>err &&
@@ t/t3455-history-squash.sh (new)
+ test_cmp_rev "$head_before" HEAD
+'
+
-+test_expect_success 'keeps the oldest message for in-range squash! and amend!' '
++test_expect_success 'the last amend! for the oldest commit replaces its message' '
+ git reset --hard start &&
+ test_commit --no-tag marker-oldest file b &&
+ git commit --allow-empty -m "squash! marker-oldest" &&
-+ git commit --allow-empty -m "amend! marker-oldest" &&
-+ test_commit --no-tag marker-newest file c &&
++ commit_with_message "amend! marker-oldest\n\nearlier message\n" &&
++ commit_with_message \
++ "amend! marker-oldest\n\namended subject\n\namended body\n" &&
++ test_commit --no-tag marker-later file c &&
++ commit_with_message "amend! marker-later\n\nwrong message\n" &&
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
-+ git log --format="%s" -1 >actual &&
-+ echo marker-oldest >expect &&
-+ test_cmp expect actual
++ check_commit_count start..HEAD 1 &&
++ check_log_messages -1 <<-\EOF
++ amended subject
++
++ amended body
++
++ EOF
+'
+
+test_expect_success 'preserves authorship of the oldest commit' '
@@ t/t3455-history-squash.sh (new)
+
+ git history squash --update-refs=head start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$other_before" other
+'
+
@@ t/t3455-history-squash.sh (new)
+
+ git history squash --update-refs=head start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test_cmp_rev "$mid_before" mid &&
+
+ git branch -D mid
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
-+ git log --format="%s" -1 >subject &&
-+ echo before-side >expect &&
-+ test_cmp expect subject &&
++ check_commit_count start..HEAD 1 &&
++ check_log_subjects -1 <<-\EOF &&
++ before-side
++ EOF
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file inner
+'
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test_cmp_rev start HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file side
+'
@@ t/t3455-history-squash.sh (new)
+
+ git history squash "$base.." &&
+
-+ git rev-list --count "$base..HEAD" >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count "$base..HEAD" 1 &&
+ test_cmp_rev "$base" HEAD^ &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file s1 &&
+ test_path_is_file s2
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file outer &&
+ test_path_is_file inner
@@ t/t3455-history-squash.sh (new)
+
+ git history squash start.. &&
+
-+ git rev-list --count start..HEAD >count &&
-+ echo 1 >expect &&
-+ test_cmp expect count &&
++ check_commit_count start..HEAD 1 &&
+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+ test_path_is_file o1 &&
+ test_path_is_file o2
4: 0a735117ad = 4: 08915cee51 sequencer: share the squash message marker helpers and flags
5: baf7e6f0a6 ! 5: fb76afe31c history: re-edit a squash with every message
@@ Metadata
## Commit message ##
history: re-edit a squash with every message
- By default "git history squash" reuses the oldest commit's message.
- When --reedit-message is given it only reopened that one message, so the
+ By default "git history squash" reuses the oldest commit's message, or
+ the replacement body from an amend! commit targeting it. When
+ --reedit-message is given it only reopened that selected message, so the
messages of the other commits in the range were lost.
Gather the message of every commit in the range and build the same editor
@@ Commit message
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## Documentation/git-history.adoc ##
-@@ Documentation/git-history.adoc: given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
- already on `topic`. Rev-list options may also be given, but any that would
+@@ Documentation/git-history.adoc: already on `topic`. Rev-list options may also be given, but any that would
change how the range is walked are overridden with a warning.
+
--The oldest commit's message and authorship are preserved by default,
--unless you specify `--reedit-message`. A merge commit inside the range is
-+The oldest commit's message and authorship are preserved by default. With
-+`--reedit-message`, an editor opens pre-filled with the messages of all the
-+folded commits so you can combine them. A merge commit inside the range is
- folded like any other, but the range must have a single base, so a range
- that reaches more than one entry point (for example a side branch that
- forked before the range and was later merged into it) is rejected.
-@@ Documentation/git-history.adoc: A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
+ The oldest commit's message is preserved by default, except that an `amend!`
+-commit targeting it replaces its message. Specify `--reedit-message` to edit
+-the resulting message. A merge commit inside the range is folded like any
+-other, but the range must have a single base, so a range that reaches more
+-than one entry point (for example a side branch that forked before the range
+-and was later merged into it) is rejected.
++commit targeting it replaces its message. With `--reedit-message`, an editor
++opens pre-filled with the messages of all the folded commits so you can
++combine them. A merge commit inside the range is folded like any other, but
++the range must have a single base, so a range that reaches more than one entry
++point (for example a side branch that forked before the range and was later
++merged into it) is rejected.
+ +
+ A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it
targets is also in the range, so the fold does not silently absorb a
- marker meant for a commit outside it. As an exception, a range made up
- entirely of markers for one target is combined into a single commit,
--keeping the last `amend!` message if there is one.
-+keeping the last `amend!` message if there is one. The changes from every
-+commit in the range are always folded in. Only the message text differs.
+@@ Documentation/git-history.adoc: marker meant for a commit outside it. The body after an `amend!` subject
+ replaces the oldest commit's message when the marker targets that commit. As
+ an exception, a range made up entirely of markers for one target is combined
+ into a single commit, keeping the last `amend!` message if there is one.
++The changes from every commit in the range are always folded in. Only the
++message text differs.
+With `--reedit-message` the template mirrors `git rebase -i --autosquash`:
+each `fixup!`, `squash!`, or `amend!` is grouped under the commit it
+targets rather than shown in commit order. A `fixup!` message is dropped
@@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
-@@ builtin/history.c: static int cmd_history_squash(int argc,
- OPT_END(),
- };
- struct strbuf reflog_msg = STRBUF_INIT;
-+ struct strbuf message = STRBUF_INIT;
- struct oidset interior = OIDSET_INIT;
- struct commit *base, *oldest, *tip, *rewritten, *msg_source;
- const struct object_id *base_tree_oid, *tip_tree_oid;
@@ builtin/history.c: static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
++ strbuf_reset(&message);
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
++ message_template = message.buf;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
-@@ builtin/history.c: static int cmd_history_squash(int argc,
- tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
- commit_list_append(base, &parents);
-
-- ret = commit_tree_ext(repo, "squash", msg_source, NULL, parents,
-+ ret = commit_tree_ext(repo, "squash", msg_source,
-+ message.len ? message.buf : NULL, parents,
- base_tree_oid, tip_tree_oid, &rewritten, flags);
- if (ret < 0) {
- ret = error(_("failed writing squashed commit"));
-@@ builtin/history.c: static int cmd_history_squash(int argc,
-
- out:
- strbuf_release(&reflog_msg);
-+ strbuf_release(&message);
- oidset_clear(&interior);
- commit_list_free(parents);
- release_revisions(&revs);
## t/t3455-history-squash.sh ##
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the oldest commit' '
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
-+ echo b >file &&
-+ git add file &&
++ stage_file b &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit --no-tag re-two file c &&
+ test_commit re-three file d &&
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
+ #
+ EOF
+ test_cmp expect edited &&
-+ echo combined >expect &&
-+ git log --format="%s" -1 >actual &&
-+ test_cmp expect actual
++ check_log_subjects -1 <<-\EOF
++ combined
++ EOF
+'
+
+test_expect_success '--reedit-message handles fixup!, squash! and amend! like rebase' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
-+ printf "fixup! mark-base\n\nfixup body\n" >msg &&
-+ echo c >file &&
-+ git add file &&
-+ git commit -qF msg &&
-+ printf "squash! mark-base\n\nsquash remark\n" >msg &&
-+ echo d >file &&
-+ git add file &&
-+ git commit -qF msg &&
-+ printf "amend! mark-base\n\namended message\n" >msg &&
-+ echo e >file &&
-+ git add file &&
-+ git commit -qF msg &&
++ stage_file c &&
++ commit_with_message "fixup! mark-base\n\nfixup body\n" &&
++ stage_file d &&
++ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
++ stage_file e &&
++ commit_with_message "amend! mark-base\n\namended message\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
+ #
+ EOF
+ test_cmp expect edited &&
-+ git log -1 --format="%B" >final &&
-+ test_grep ! "fixup body" final &&
-+ test_grep "squash remark" final &&
-+ test_grep "amended message" final
++ check_log_messages -1 <<-\EOF
++ mark-base
++
++ squash remark
++
++ amended message
++
++ EOF
+'
+
+test_expect_success '--reedit-message groups fixups under their targets' '
+ git reset --hard start &&
+ test_commit --no-tag alpha file a1 &&
+ test_commit --no-tag beta file b1 &&
-+ printf "fixup! alpha\n" >msg &&
-+ echo a2 >file &&
-+ git add file &&
-+ git commit -qF msg &&
-+ printf "fixup! beta\n" >msg &&
-+ echo b2 >file &&
-+ git add file &&
-+ git commit -qF msg &&
++ stage_file a2 &&
++ commit_with_message "fixup! alpha\n" &&
++ stage_file b2 &&
++ commit_with_message "fixup! beta\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
+test_expect_success '--reedit-message lets amend! replace its target message' '
+ git reset --hard start &&
+ test_commit --no-tag mark-base file b &&
-+ printf "amend! mark-base\n\namended message\n" >msg &&
-+ echo c >file &&
-+ git add file &&
-+ git commit -qF msg &&
-+ printf "squash! mark-base\n\nsquash remark\n" >msg &&
-+ echo d >file &&
-+ git add file &&
-+ git commit -qF msg &&
++ stage_file c &&
++ commit_with_message "amend! mark-base\n\namended message\n" &&
++ stage_file d &&
++ commit_with_message "squash! mark-base\n\nsquash remark\n" &&
+
+ write_script editor <<-\EOF &&
+ cat "$1" >edited
@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
+ #
+ EOF
+ test_cmp expect edited &&
-+ git log -1 --format="%B" >final &&
-+ test_grep ! "mark-base" final &&
-+ test_grep "amended message" final &&
-+ test_grep "squash remark" final
++ check_log_messages -1 <<-\EOF
++ amended message
++
++ squash remark
++
++ EOF
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`
From: Junio C Hamano @ 2026-07-15 15:15 UTC (permalink / raw)
To: Toon Claes; +Cc: Patrick Steinhardt, git, Justin Tobler, Jeff King, Taylor Blau
In-Reply-To: <874ii0h2uf.fsf@emacs.iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> this patch series introduces object filters to `odb_for_each_object()`.
>> The intent of this is to make `git cat-file --batch-all-objects` work
>> with pluggable object databases. Right now it doesn't because it reaches
>> into internals of the "packed" backend to efficiently handle bitmapped
>> objects.
>>
>> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
>> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
>> info fields, 2026-07-02) merged into it.
>>
>> Changes in v4:
>> - Fix references to an old function name in commit messages.
>
> Thanks for fixing that! This version looks fine by me.
Great. Thanks all for working well together.
^ permalink raw reply
* Re: [PATCH v2 0/7] refs: remove use of `the_repository`
From: Junio C Hamano @ 2026-07-15 14:38 UTC (permalink / raw)
To: Toon Claes; +Cc: Patrick Steinhardt, git
In-Reply-To: <87y0fcfn7v.fsf@emacs.iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Hi,
>>
>> this patch series refactors the ref subsystem to drop uses of
>> `the_repository`. These patches were part of a discarded attempt to
>> make the initialization of the refdb eager. I guess they make sense by
>> themselves though, so here we go.
>>
>> Note that these patches contain a slight tangent to also adapt
>> "worktree.c". This is one of the subsystems that caused problems with
>> eager refdb initialization because of `has_worktrees()`, so I refactored
>> this subsystem while at it.
>
> Changes are all very straightforward and, except from the small comment
> on the commit message of [PATCH 2/7], I approve this series.
Sounds very good. Thanks.
^ permalink raw reply
* [PATCH] mv: report missing destination leading directory
From: Lucas Zamboni Orioli via GitGitGadget @ 2026-07-15 14:32 UTC (permalink / raw)
To: git; +Cc: Lucas Zamboni Orioli, Lucas Zamboni Orioli
From: Lucas Zamboni Orioli <lucaszam0@gmail.com>
When moving a file to a destination whose leading directory does not
exist, "git mv" fails at the rename(2) syscall with ENOENT. Because
the error is reported via die_errno() using only the source path:
fatal: renaming 'src' failed: No such file or directory
the message misleadingly blames the source, even though it is the
destination's parent directory that is missing. A user who runs
git mv a/file b/does-not-exist/file
is told the problem is with 'a/file', which exists, giving no hint
that 'b/does-not-exist/' needs to be created first.
The checking phase already rejects a missing destination directory
when the destination ends in a slash, but a destination that names a
file inside a non-existent directory is not caught and only fails
later at rename(2). As a result "git mv -n" also fails to detect the
problem, since the dry run never reaches the syscall and reports a
move that would not actually succeed.
Detect this during the checking phase instead: for entries that will
be renamed on disk, stat the destination's leading directory and, if
it is missing, fail with the existing "destination directory does not
exist" message. Guard the check with the same condition under which
rename(2) is invoked so that directory moves, whose child entries are
expanded to paths under a not-yet-created directory, and sparse or
out-of-cone destinations, which are not written to the worktree, are
not flagged incorrectly.
This gives a clear message and lets "git mv -n" report the failure.
Signed-off-by: Lucas Zamboni Orioli <lucaszam0@gmail.com>
---
mv: report missing destination leading directory
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2356%2FZamboniL%2Fmv-detect-non-existing-target-folder-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2356/ZamboniL/mv-detect-non-existing-target-folder-v1
Pull-Request: https://github.com/git/git/pull/2356
builtin/mv.c | 21 +++++++++++++++++++++
t/t7001-mv.sh | 14 ++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/builtin/mv.c b/builtin/mv.c
index e03823370c..a95531f0b2 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -444,6 +444,27 @@ dir_check:
goto act_on_entry;
}
+ /*
+ * If we are going to move SRC to DST on disk, DST's leading
+ * directories must already exist.
+ */
+ if (!(modes[i] & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) &&
+ !(dst_mode & (SKIP_WORKTREE_DIR | SPARSE))) {
+ char *dst_dir = xstrdup(dst);
+ char *slash = strrchr(dst_dir, '/');
+
+ if (slash) {
+ struct stat dir_st;
+ *slash = '\0';
+ if (lstat(dst_dir, &dir_st) < 0 && errno == ENOENT) {
+ free(dst_dir);
+ bad = _("destination directory does not exist");
+ goto act_on_entry;
+ }
+ }
+ free(dst_dir);
+ }
+
if (ignore_sparse &&
(dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) &&
index_entry_exists(the_repository->index, dst, strlen(dst))) {
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 920479e925..8a45997b33 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -114,6 +114,20 @@ test_expect_success 'clean up' '
git reset --hard
'
+test_expect_success 'moving to non-existent destination parent directory' '
+ git reset --hard &&
+ mkdir -p from &&
+ echo content >from/file &&
+ git add from/file &&
+ test_must_fail git mv from/file no-such-dir/file 2>actual &&
+ test_grep "destination directory does not exist" actual
+'
+
+test_expect_success 'mv --dry-run detects non-existent destination parent directory' '
+ test_must_fail git mv -n from/file no-such-dir/file 2>actual &&
+ test_grep "destination directory does not exist" actual
+'
+
test_expect_success 'moving to existing untracked target with trailing slash' '
mkdir path1 &&
git mv path0/ path1/ &&
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 0/7] refs: remove use of `the_repository`
From: Toon Claes @ 2026-07-15 12:31 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
In-Reply-To: <20260715-pks-refs-wo-the-repository-v2-0-d00d364f5a3e@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this patch series refactors the ref subsystem to drop uses of
> `the_repository`. These patches were part of a discarded attempt to
> make the initialization of the refdb eager. I guess they make sense by
> themselves though, so here we go.
>
> Note that these patches contain a slight tangent to also adapt
> "worktree.c". This is one of the subsystems that caused problems with
> eager refdb initialization because of `has_worktrees()`, so I refactored
> this subsystem while at it.
Changes are all very straightforward and, except from the small comment
on the commit message of [PATCH 2/7], I approve this series.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2 2/7] refs/packed: drop `USE_THE_REPOSITORY_VARIABLE`
From: Toon Claes @ 2026-07-15 12:28 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano
In-Reply-To: <20260715-pks-refs-wo-the-repository-v2-2-d00d364f5a3e@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> There's a single user of `the_repository` in the "packed" reference
> backend. Convert it to instead use the backend's repository and drop
> `USE_THE_REPOSITORY_VARIABLE`.
Well, this was removed in the previous patch. I'm fine keeping this as a
separate commmit, but the messaging is a bit confusing.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> refs/packed-backend.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index 14b27d24ec..c5d96793fa 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> #define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "../git-compat-util.h"
>
> --
> 2.55.0.313.g8d093f411d.dirty
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`
From: Toon Claes @ 2026-07-15 12:08 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this patch series introduces object filters to `odb_for_each_object()`.
> The intent of this is to make `git cat-file --batch-all-objects` work
> with pluggable object databases. Right now it doesn't because it reaches
> into internals of the "packed" backend to efficiently handle bitmapped
> objects.
>
> The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
> 2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
> info fields, 2026-07-02) merged into it.
>
> Changes in v4:
> - Fix references to an old function name in commit messages.
Thanks for fixing that! This version looks fine by me.
--
Cheers,
Toon
^ permalink raw reply
* [PATCH v5] show-branch: convert per-branch flags to commit-slab
From: Gatla Vishweshwar Reddy @ 2026-07-15 12:01 UTC (permalink / raw)
To: gitster; +Cc: git, Gatla Vishweshwar Reddy
In-Reply-To: <xmqqwluwpvme.fsf@gitster.g>
show-branch uses commit->object.flags to store per-branch
reachability bits, one bit per branch starting at REV_SHIFT.
The flags word has only a fixed number of available bits, limiting
the number of branches that can be shown simultaneously to MAX_REVS.
Convert the per-branch bits to a dedicated commit-slab using uint64_t
as the element type, initialized with a stride via
init_commit_rev_flags_with_stride(). Keep the UNINTERESTING bit in
object.flags where it belongs, as it is used for revision walking and
does not need to be in the per-branch slab. With UNINTERESTING removed
from the slab, REV_SHIFT becomes 0 and all 64 bits of uint64_t are
available for branch tracking, lifting MAX_REVS from 27 to 64 branches.
Add helper functions get_rev_flags_ptr(), peek_rev_flags_ptr(),
has_any_rev_flags(), or_rev_flag_bit(), test_rev_flag_bit(),
has_all_rev_flags(), and has_only_rev_flag_bit() to encapsulate
per-bit slab access cleanly. Use has_only_rev_flag_bit() in
show_independent() to preserve the original semantics: a commit is
independent only if reachable from exactly one tip, not merely if
the i-th bit happens to be set. Update all bit operations to use
UINT64_C(1) for correct 64-bit shifts. Initialize and clear the slab
in cmd_show_branch().
Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
Apologies to Patrick for not replying inline to the review before
sending v4. For previous review threads I did reply inline; for that
round I mistakenly folded the response into the annotation only.
Response to Junio's review of v4:
- Restored the TODO comment above REV_SHIFT since the UNINTERESTING
migration to the slab is not yet complete.
- Removed the local "#define UNINTERESTING 01" and added
#include "revision.h" to use the shared definition instead.
Changes in v5:
- Restore TODO comment above REV_SHIFT
- Remove local UNINTERESTING define, include revision.h instead
builtin/show-branch.c | 162 +++++++++++++++++++++++++++---------------
1 file changed, 103 insertions(+), 59 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..cf037c5814 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -9,6 +9,7 @@
#include "hex.h"
#include "pretty.h"
#include "refs.h"
+#include "revision.h"
#include "color.h"
#include "strvec.h"
#include "object-name.h"
@@ -35,15 +36,12 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
+ * TODO: store a pointer to ref name directly in the commit-slab
+ * instead, and use the UNINTERESTING definition from revision.h
+ * here once that is done.
*/
-#define UNINTERESTING 01
-
-#define REV_SHIFT 2
-#define MAX_REVS (FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
-
+#define REV_SHIFT 0
+#define MAX_REVS (sizeof(uint64_t) * 8)
#define DEFAULT_REFLOG 4
static const char *get_color_code(int idx)
@@ -79,11 +77,72 @@ struct commit_name {
define_commit_slab(commit_name_slab, struct commit_name *);
static struct commit_name_slab name_slab;
+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+static int flags_stride; /* number of uint64_t words per commit */
+
static struct commit_name *commit_to_name(struct commit *commit)
{
return *commit_name_slab_at(&name_slab, commit);
}
+static uint64_t *get_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_at(&rev_flags_slab, commit);
+}
+
+static uint64_t *peek_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_peek(&rev_flags_slab, commit);
+}
+
+static int has_any_rev_flags(struct commit *commit)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++)
+ if (f[i])
+ return 1;
+ return 0;
+}
+
+static void or_rev_flag_bit(struct commit *commit, int branch)
+{
+ get_rev_flags_ptr(commit)[branch / 64] |= UINT64_C(1) << (branch % 64);
+}
+
+static int test_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ return f && !!(f[branch / 64] & (UINT64_C(1) << (branch % 64)));
+}
+
+static int has_all_rev_flags(struct commit *commit, int num_rev)
+{
+ int i;
+ for (i = 0; i < num_rev; i++)
+ if (!test_rev_flag_bit(commit, i))
+ return 0;
+ return 1;
+}
+
+static int has_only_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++) {
+ uint64_t expected = (i == branch / 64)
+ ? (UINT64_C(1) << (branch % 64))
+ : 0;
+ if (f[i] != expected)
+ return 0;
+ }
+ return 1;
+}
/* Name the commit as nth generation ancestor of head_name;
* we count only the first-parent relationship for naming purposes.
@@ -215,7 +274,7 @@ static void name_commits(struct commit_list *list,
static int mark_seen(struct commit *commit, struct commit_list **seen_p)
{
- if (!commit->object.flags) {
+ if (!has_any_rev_flags(commit)) {
commit_list_insert(commit, seen_p);
return 1;
}
@@ -226,34 +285,34 @@ static void join_revs(struct prio_queue *queue,
struct commit_list **seen_p,
int num_rev, int extra)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
while (queue->nr) {
struct commit_list *parents;
int still_interesting = !!interesting(queue);
struct commit *commit = prio_queue_peek(queue);
bool get_pending = true;
- int flags = commit->object.flags & all_mask;
if (!still_interesting && extra <= 0)
break;
mark_seen(commit, seen_p);
- if ((flags & all_revs) == all_revs)
- flags |= UNINTERESTING;
+ if (has_all_rev_flags(commit, num_rev))
+ commit->object.flags |= UNINTERESTING;
parents = commit->parents;
while (parents) {
struct commit *p = parents->item;
- int this_flag = p->object.flags;
parents = parents->next;
- if ((this_flag & flags) == flags)
+ if (has_all_rev_flags(p, num_rev))
continue;
repo_parse_commit(the_repository, p);
if (mark_seen(p, seen_p) && !still_interesting)
extra--;
- p->object.flags |= flags;
+ {
+ int _b;
+ for (_b = 0; _b < num_rev; _b++)
+ if (test_rev_flag_bit(commit, _b))
+ or_rev_flag_bit(p, _b);
+ }
if (get_pending)
prio_queue_replace(queue, p);
else
@@ -263,7 +322,6 @@ static void join_revs(struct prio_queue *queue,
if (get_pending)
prio_queue_get(queue);
}
-
/*
* Postprocess to complete well-poisoning.
*
@@ -278,7 +336,7 @@ static void join_revs(struct prio_queue *queue,
struct commit *c = s->item;
struct commit_list *parents;
- if (((c->object.flags & all_revs) != all_revs) &&
+ if (!has_all_rev_flags(c, num_rev) &&
!(c->object.flags & UNINTERESTING))
continue;
@@ -410,8 +468,8 @@ static int append_ref(const char *refname, const struct object_id *oid,
return 0;
}
if (MAX_REVS <= ref_name_cnt) {
- warning(Q_("ignoring %s; cannot handle more than %d ref",
- "ignoring %s; cannot handle more than %d refs",
+ warning(Q_("ignoring %s; cannot handle more than %zu ref",
+ "ignoring %s; cannot handle more than %zu refs",
MAX_REVS), refname, MAX_REVS);
return 0;
}
@@ -511,15 +569,12 @@ static int rev_is_head(const char *head, const char *name)
static int show_merge_base(const struct commit_list *seen, int num_rev)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
int exit_status = 1;
for (const struct commit_list *s = seen; s; s = s->next) {
struct commit *commit = s->item;
- int flags = commit->object.flags & all_mask;
- if (!(flags & UNINTERESTING) &&
- ((flags & all_revs) == all_revs)) {
+ if (!(commit->object.flags & UNINTERESTING) &&
+ has_all_rev_flags(commit, num_rev)) {
puts(oid_to_hex(&commit->object.oid));
exit_status = 0;
commit->object.flags |= UNINTERESTING;
@@ -528,17 +583,13 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
return exit_status;
}
-static int show_independent(struct commit **rev,
- int num_rev,
- unsigned int *rev_mask)
+static int show_independent(struct commit **rev, int num_rev)
{
int i;
for (i = 0; i < num_rev; i++) {
struct commit *commit = rev[i];
- unsigned int flag = rev_mask[i];
-
- if (commit->object.flags == flag)
+ if (has_only_rev_flag_bit(commit, i))
puts(oid_to_hex(&commit->object.oid));
commit->object.flags |= UNINTERESTING;
}
@@ -603,13 +654,12 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
* Otherwise, if it is a merge that is reachable from only one
* tip, it is not that interesting.
*/
- int i, flag, count;
+ int i, count;
for (i = 0; i < n; i++)
if (rev[i] == commit)
return 0;
- flag = commit->object.flags;
for (i = count = 0; i < n; i++) {
- if (flag & (1u << (i + REV_SHIFT)))
+ if (test_rev_flag_bit(commit, i))
count++;
}
if (count == 1)
@@ -648,10 +698,8 @@ int cmd_show_branch(int ac,
char *reflog_msg[MAX_REVS] = {0};
struct commit_list *seen = NULL;
struct prio_queue queue = { compare_commits_by_commit_date };
- unsigned int rev_mask[MAX_REVS];
int num_rev, i, extra = 0;
int all_heads = 0, all_remotes = 0;
- int all_mask, all_revs;
enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
char *head;
struct object_id head_oid;
@@ -713,7 +761,8 @@ int cmd_show_branch(int ac,
const char **args_copy = NULL;
int ret;
- init_commit_name_slab(&name_slab);
+ flags_stride = (MAX_REVS + 63) / 64;
+ init_commit_rev_flags_with_stride(&rev_flags_slab, flags_stride);
repo_config(the_repository, git_show_branch_config, NULL);
@@ -779,8 +828,8 @@ int cmd_show_branch(int ac,
die(_("--reflog option needs one branch name"));
if (MAX_REVS < reflog)
- die(Q_("only %d entry can be shown at one time.",
- "only %d entries can be shown at one time.",
+ die(Q_("only %zu entry can be shown at one time.",
+ "only %zu entries can be shown at one time.",
MAX_REVS), MAX_REVS);
if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
&ref, 0))
@@ -870,11 +919,11 @@ int cmd_show_branch(int ac,
for (num_rev = 0; ref_name[num_rev]; num_rev++) {
struct object_id revkey;
- unsigned int flag = 1u << (num_rev + REV_SHIFT);
+ int first_seen;
if (MAX_REVS <= num_rev)
- die(Q_("cannot handle more than %d rev.",
- "cannot handle more than %d revs.",
+ die(Q_("cannot handle more than %zu rev.",
+ "cannot handle more than %zu revs.",
MAX_REVS), MAX_REVS);
if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
die(_("'%s' is not a valid ref."), ref_name[num_rev]);
@@ -885,17 +934,15 @@ int cmd_show_branch(int ac,
repo_parse_commit(the_repository, commit);
mark_seen(commit, &seen);
- /* rev#0 uses bit REV_SHIFT, rev#1 uses bit REV_SHIFT+1,
- * and so on. REV_SHIFT bits from bit 0 are used for
- * internal bookkeeping.
+ /* rev#0 uses bit 0, rev#1 uses bit 1,
+ * and so on. All bits are available for branch tracking.
*/
- commit->object.flags |= flag;
- if (commit->object.flags == flag)
+ first_seen = !has_any_rev_flags(commit);
+ or_rev_flag_bit(commit, num_rev);
+ if (first_seen)
prio_queue_put(&queue, commit);
rev[num_rev] = commit;
}
- for (i = 0; i < num_rev; i++)
- rev_mask[i] = rev[i]->object.flags;
if (0 <= extra)
join_revs(&queue, &seen, num_rev, extra);
@@ -908,7 +955,7 @@ int cmd_show_branch(int ac,
}
if (independent) {
- ret = show_independent(rev, num_rev, rev_mask);
+ ret = show_independent(rev, num_rev);
goto out;
}
@@ -958,13 +1005,9 @@ int cmd_show_branch(int ac,
if (!sha1_name && !no_name)
name_commits(seen, rev, ref_name, num_rev);
- all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
for (struct commit_list *l = seen; l; l = l->next) {
struct commit *commit = l->item;
- int this_flag = commit->object.flags;
- int is_merge_point = ((this_flag & all_revs) == all_revs);
+ int is_merge_point = has_all_rev_flags(commit, num_rev);
shown_merge_point |= is_merge_point;
@@ -973,14 +1016,14 @@ int cmd_show_branch(int ac,
commit->parents->next);
if (topics &&
!is_merge_point &&
- (this_flag & (1u << REV_SHIFT)))
+ test_rev_flag_bit(commit, 0))
continue;
if (!sparse && is_merge &&
omit_in_dense(commit, rev, num_rev))
continue;
for (i = 0; i < num_rev; i++) {
int mark;
- if (!(this_flag & (1u << (i + REV_SHIFT))))
+ if (!test_rev_flag_bit(commit, i))
mark = ' ';
else if (is_merge)
mark = '-';
@@ -1010,6 +1053,7 @@ int cmd_show_branch(int ac,
free(reflog_msg[i]);
commit_list_free(seen);
clear_prio_queue(&queue);
+ clear_commit_rev_flags(&rev_flags_slab);
free(args_copy);
free(head);
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH] t7614: avoid hiding git's exit code in a pipe
From: Shlok Kulshreshtha @ 2026-07-15 11:33 UTC (permalink / raw)
To: git; +Cc: Shlok Kulshreshtha, Junio C Hamano
The exit code of the upstream command in a pipe is ignored, so in
git cat-file commit HEAD | sed -e "1,/^\$/d" >actual
a crash of "git cat-file" would go unnoticed: the exit code of the
pipeline is that of "sed", which happily succeeds on empty input. The
test would thus pass even though "git cat-file" failed.
Write the output of "git cat-file" to a file first and run "sed" on
that file, so that the exit codes of both commands are checked by the
&&-chain.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com>
---
This is a microproject ("Avoid suppressing git's exit code in test
scripts"), applying the same fix as c6f44e1da5 (t9813: avoid using
pipes) to another script. A search of the list did not turn up anyone
working on t7614; please let me know if it is already taken.
t/t7614-merge-signoff.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
index fee258d4f0..e58bf07b7a 100755
--- a/t/t7614-merge-signoff.sh
+++ b/t/t7614-merge-signoff.sh
@@ -45,7 +45,8 @@ test_expect_success 'git merge --signoff adds a sign-off line' '
test_commit main-branch-2 file2 2 &&
git checkout other-branch &&
git merge main --signoff --no-edit &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-signed actual
'
@@ -55,7 +56,8 @@ test_expect_success 'git merge does not add a sign-off line' '
test_commit main-branch-3 file3 3 &&
git checkout other-branch &&
git merge main --no-edit &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-unsigned actual
'
@@ -65,7 +67,8 @@ test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
test_commit main-branch-4 file4 4 &&
git checkout other-branch &&
git merge main --no-edit --signoff --no-signoff &&
- git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+ git cat-file commit HEAD >commit &&
+ sed -e "1,/^\$/d" commit >actual &&
test_cmp expected-unsigned actual
'
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 1/5] tempfile: add repo_create_tempfile{,_mode}()
From: René Scharfe @ 2026-07-15 11:25 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <aldYTuMvN-8EMvYK@pks.im>
On 7/15/26 11:52 AM, Patrick Steinhardt wrote:
> On Tue, Jul 14, 2026 at 07:59:52PM +0200, René Scharfe wrote:
>> Add variants of create_tempfile_mode() that handle arbitrary
>> repositories.
>
> One thing I was wondering is whether it really makes sense to pass in a
> full repository. All we require it for is `adjust_shared_perm()`, and it
> feels quite extreme to require a full-blown repository.
>
> An alternative would be to let callers pass in the setting by
> themselves, but that would likely lead to lots of duplicated code. So
> maybe this is a good first step, and we could eventually create another
> API where users can pass in the configuration instead of a repository if
> we ever gain callers that don't have a repository available.
Had the same thought. I think it's because create_tempfile() sounds
quite generic, but is actually for creating temporary files within a
repository, not just anywhere or just for the duration of the creating
process, so shared access matters (if enabled).
I didn't find a case where a caller would not have at least
the_repository to pass in, so while a repo-less adjust_shared_perm()
or create_tempfile() might seem cleaner, we probably won't need it in
practice. We'll find out..
René
^ permalink raw reply
* [PATCH v5 2/2] fetch: add fetch.submoduleErrors to make submodule fetch errors non-fatal
From: Paulius Zaleckas @ 2026-07-15 10:35 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Ramsay Jones, Paulius Zaleckas,
Jean-Noël Avila, Glen Choo, Patrick Steinhardt,
Ævar Arnfjörð Bjarmason
In-Reply-To: <20260715103518.526326-1-paulius.zaleckas@gmail.com>
When fetching with --recurse-submodules, a submodule commit that is not
yet reachable from any of the submodule's remote refs causes the entire
fetch to fail. This is overly strict when the missing commit belongs to
an upstream branch that is still being prepared (e.g. an in-progress
merge topic): the local branch does not need that commit, so there is no
reason to treat its absence as fatal.
Add a new config key fetch.submoduleErrors (values: fail/warn) and a
corresponding --submodule-errors=(fail|warn) command-line option that
control this behaviour. The default remains fail (existing behaviour);
setting the value to warn causes submodule fetch failures to be reported
on stderr without affecting the overall exit status of git fetch / git
pull.
Forward the option to child fetches in add_options_to_argv() so that it
also takes effect for `git fetch --all` / `--multiple` (where per-remote
child processes handle the submodule recursion themselves) and for
nested submodule recursion. The resolved value is forwarded whenever it
was set explicitly, in either direction: the per-remote children re-read
the repository configuration, so a command-line --submodule-errors=fail
must be passed down to them to override fetch.submoduleErrors=warn from
the configuration. When neither the configuration nor the command line
sets a value, nothing is forwarded and the child processes fall back to
their own configuration.
Helped-by: Jean-Noël Avila <avila.jn@gmail.com>
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paulius Zaleckas <paulius.zaleckas@gmail.com>
---
Documentation/config/fetch.adoc | 14 +++++
Documentation/fetch-options.adoc | 8 +++
builtin/fetch.c | 72 +++++++++++++++++++++++++-
submodule.c | 8 ++-
submodule.h | 7 ++-
t/t5526-fetch-submodules.sh | 89 ++++++++++++++++++++++++++++++++
6 files changed, 194 insertions(+), 4 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index 04ac90912d..5c9c942a70 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -10,6 +10,20 @@
reference.
Defaults to `on-demand`, or to the value of `submodule.recurse` if set.
+`fetch.submoduleErrors`::
+ Controls how errors from submodule fetches are handled when
+ `--recurse-submodules` is in effect. When set to `fail` (the default),
+ any submodule fetch error causes the overall `git fetch` or `git pull`
+ to exit with a non-zero status. When set to `warn`, submodule fetch
+ errors are reported to standard error but do not affect the exit
+ status of the command. This is useful when working in repositories
+ where some branches reference submodule commits that are not yet
+ available on the submodule remote, but those commits are not needed
+ for the currently checked-out branch.
++
+The value of this option can be overridden by the `--submodule-errors`
+option of linkgit:git-fetch[1].
+
`fetch.fsckObjects`::
If it is set to true, git-fetch-pack will check all fetched
objects. See `transfer.fsckObjects` for what's
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 035f780e58..78525f6848 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -294,6 +294,14 @@ ifndef::git-pull[]
`--no-recurse-submodules`::
Disable recursive fetching of submodules (this has the same effect as
using the `--recurse-submodules=no` option).
+
+`--submodule-errors=(fail|warn)`::
+ Control how errors from submodule fetches are handled when
+ `--recurse-submodules` is in effect. When set to `fail` (the default),
+ any submodule fetch error causes the overall `git fetch` to exit with a
+ non-zero status. When set to `warn`, submodule fetch errors are reported
+ to standard error but do not affect the exit status of the command. Can
+ also be configured via `fetch.submoduleErrors`. See linkgit:git-config[1].
endif::git-pull[]
`--set-upstream`::
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c1d7c672f4..b0eb1eb301 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -110,8 +110,32 @@ struct fetch_config {
int recurse_submodules;
int parallel;
int submodule_fetch_jobs;
+ int submodule_errors;
};
+/* really private - use accessors below to parse and format */
+static const char *submodule_errors_names[] = {
+ [SUBMODULE_ERRORS_FAIL] = "fail",
+ [SUBMODULE_ERRORS_WARN] = "warn",
+};
+
+static const char *submodule_errors_to_string(int mode)
+{
+ if (mode < 0 || (size_t)mode >= ARRAY_SIZE(submodule_errors_names))
+ BUG("invalid submodule errors mode %d", mode);
+ return submodule_errors_names[mode];
+}
+
+static int parse_submodule_errors(const char *name)
+{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(submodule_errors_names); i++)
+ if (!strcmp(submodule_errors_names[i], name))
+ return i;
+ return -1;
+}
+
static int git_fetch_config(const char *k, const char *v,
const struct config_context *ctx, void *cb)
{
@@ -152,6 +176,19 @@ static int git_fetch_config(const char *k, const char *v,
return 0;
}
+ if (!strcmp(k, "fetch.submoduleerrors")) {
+ int mode;
+
+ if (!v)
+ return config_error_nonbool(k);
+ mode = parse_submodule_errors(v);
+ if (mode < 0)
+ die(_("invalid value for '%s': '%s'"),
+ "fetch.submoduleErrors", v);
+ fetch_config->submodule_errors = mode;
+ return 0;
+ }
+
if (!strcmp(k, "fetch.parallel")) {
fetch_config->parallel = git_config_int(k, v, ctx->kvi);
if (fetch_config->parallel < 0)
@@ -2205,6 +2242,9 @@ static void add_options_to_argv(struct strvec *argv,
strvec_push(argv, "--no-recurse-submodules");
else if (config->recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
strvec_push(argv, "--recurse-submodules=on-demand");
+ if (config->submodule_errors != -1)
+ strvec_pushf(argv, "--submodule-errors=%s",
+ submodule_errors_to_string(config->submodule_errors));
if (tags == TAGS_SET)
strvec_push(argv, "--tags");
else if (tags == TAGS_UNSET)
@@ -2464,6 +2504,23 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
return exit_code;
}
+static int option_parse_submodule_errors(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *v = opt->value;
+ int mode;
+
+ if (unset) {
+ *v = SUBMODULE_ERRORS_FAIL;
+ return 0;
+ }
+ mode = parse_submodule_errors(arg);
+ if (mode < 0)
+ die(_("invalid value for '%s': '%s'"), "--submodule-errors", arg);
+ *v = mode;
+ return 0;
+}
+
int cmd_fetch(int argc,
const char **argv,
const char *prefix,
@@ -2477,6 +2534,7 @@ int cmd_fetch(int argc,
.recurse_submodules = RECURSE_SUBMODULES_DEFAULT,
.parallel = 1,
.submodule_fetch_jobs = -1,
+ .submodule_errors = -1, /* unset */
};
const char *submodule_prefix = "";
const char *bundle_uri;
@@ -2491,6 +2549,7 @@ int cmd_fetch(int argc,
int max_jobs = -1;
int recurse_submodules_cli = RECURSE_SUBMODULES_DEFAULT;
int recurse_submodules_default = RECURSE_SUBMODULES_ON_DEMAND;
+ int submodule_errors_cli = -1; /* -1: not set on command line */
int fetch_write_commit_graph = -1;
int stdin_refspecs = 0;
int negotiate_only = 0;
@@ -2527,6 +2586,10 @@ int cmd_fetch(int argc,
OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules_cli, N_("on-demand"),
N_("control recursive fetching of submodules"),
PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
+ OPT_CALLBACK_F(0, "submodule-errors", &submodule_errors_cli,
+ N_("(fail|warn)"),
+ N_("control how submodule fetch errors are handled"),
+ 0, option_parse_submodule_errors),
OPT_BOOL(0, "dry-run", &dry_run,
N_("dry run")),
OPT_BOOL(0, "porcelain", &porcelain, N_("machine-readable output")),
@@ -2616,6 +2679,9 @@ int cmd_fetch(int argc,
if (recurse_submodules_cli != RECURSE_SUBMODULES_DEFAULT)
config.recurse_submodules = recurse_submodules_cli;
+ if (submodule_errors_cli != -1)
+ config.submodule_errors = submodule_errors_cli;
+
if (negotiate_only) {
switch (recurse_submodules_cli) {
case RECURSE_SUBMODULES_OFF:
@@ -2819,11 +2885,14 @@ int cmd_fetch(int argc,
if (!result && remote && (config.recurse_submodules != RECURSE_SUBMODULES_OFF)) {
struct strvec options = STRVEC_INIT;
int max_children = max_jobs;
+ int submodule_errors = config.submodule_errors;
if (max_children < 0)
max_children = config.submodule_fetch_jobs;
if (max_children < 0)
max_children = config.parallel;
+ if (submodule_errors < 0)
+ submodule_errors = SUBMODULE_ERRORS_FAIL;
add_options_to_argv(&options, &config);
trace2_region_enter_printf("fetch", "recurse-submodule", the_repository, "%s", submodule_prefix);
@@ -2833,7 +2902,8 @@ int cmd_fetch(int argc,
config.recurse_submodules,
recurse_submodules_default,
verbosity < 0,
- max_children);
+ max_children,
+ submodule_errors);
trace2_region_leave_printf("fetch", "recurse-submodule", the_repository, "%s", submodule_prefix);
strvec_clear(&options);
}
diff --git a/submodule.c b/submodule.c
index 8bcef68a42..da4ace751f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1409,6 +1409,7 @@ struct submodule_parallel_fetch {
int oid_fetch_tasks_nr, oid_fetch_tasks_alloc;
struct strbuf submodules_with_errors;
+ int submodule_errors;
};
#define SPF_INIT { \
.args = STRVEC_INIT, \
@@ -1565,7 +1566,8 @@ static struct fetch_task *fetch_task_create(struct submodule_parallel_fetch *spf
static void record_fetch_error(struct submodule_parallel_fetch *spf,
const char *name)
{
- spf->result = 1;
+ if (spf->submodule_errors == SUBMODULE_ERRORS_FAIL)
+ spf->result = 1;
strbuf_addf(&spf->submodules_with_errors, "\t%s\n", name);
}
@@ -1851,7 +1853,8 @@ int fetch_submodules(struct repository *r,
const struct strvec *options,
const char *prefix, int command_line_option,
int default_option,
- int quiet, int max_parallel_jobs)
+ int quiet, int max_parallel_jobs,
+ int submodule_errors)
{
struct submodule_parallel_fetch spf = SPF_INIT;
const struct run_process_parallel_opts opts = {
@@ -1871,6 +1874,7 @@ int fetch_submodules(struct repository *r,
spf.default_option = default_option;
spf.quiet = quiet;
spf.prefix = prefix;
+ spf.submodule_errors = submodule_errors;
if (!r->worktree)
goto out;
diff --git a/submodule.h b/submodule.h
index b10e16e6c0..c80b687d2a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -90,12 +90,17 @@ int should_update_submodules(void);
*/
const struct submodule *submodule_from_ce(const struct cache_entry *ce);
void check_for_new_submodule_commits(struct object_id *oid);
+/* Values for the submodule_errors parameter of fetch_submodules(). */
+#define SUBMODULE_ERRORS_FAIL 0 /* submodule fetch errors are fatal (default) */
+#define SUBMODULE_ERRORS_WARN 1 /* submodule fetch errors are non-fatal warnings */
+
int fetch_submodules(struct repository *r,
const struct strvec *options,
const char *prefix,
int command_line_option,
int default_option,
- int quiet, int max_parallel_jobs);
+ int quiet, int max_parallel_jobs,
+ int submodule_errors);
unsigned is_submodule_modified(const char *path, int ignore_untracked);
int submodule_uses_gitfile(const char *path);
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 7ad274ce04..19d17440cf 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -1307,6 +1307,57 @@ test_expect_success 'setup for submodule fetch error tests' '
git config --global protocol.file.allow always
'
+test_expect_success 'fetch --recurse-submodules fails when submodule commit is unreachable (default)' '
+ test_when_finished "rm -fr env_default" &&
+ create_err_env env_default &&
+ push_unreachable_commit env_default &&
+ test_must_fail git -C env_default/clone fetch --recurse-submodules 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success 'fetch.submoduleErrors=warn: unreachable submodule commit is non-fatal' '
+ test_when_finished "rm -fr env_warn_cfg" &&
+ create_err_env env_warn_cfg &&
+ push_unreachable_commit env_warn_cfg &&
+ git -C env_warn_cfg/clone -c fetch.submoduleErrors=warn \
+ fetch --recurse-submodules 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success '--submodule-errors=warn: unreachable submodule commit is non-fatal' '
+ test_when_finished "rm -fr env_warn_cli" &&
+ create_err_env env_warn_cli &&
+ push_unreachable_commit env_warn_cli &&
+ git -C env_warn_cli/clone fetch --recurse-submodules \
+ --submodule-errors=warn 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success '--submodule-errors=fail: unreachable submodule commit is fatal' '
+ test_when_finished "rm -fr env_fail_cli" &&
+ create_err_env env_fail_cli &&
+ push_unreachable_commit env_fail_cli &&
+ test_must_fail git -C env_fail_cli/clone fetch --recurse-submodules \
+ --submodule-errors=fail 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success 'fetch.submoduleErrors=warn does not suppress successful fetch' '
+ # A new reachable submodule commit (pushed to sub_bare) should be
+ # fetched without any error summary.
+ test_when_finished "rm -fr env_ok" &&
+ create_err_env env_ok &&
+ test_commit -C env_ok/sub_work reachable_ok &&
+ git -C env_ok/sub_work push &&
+ git -C env_ok/super_work submodule update --remote &&
+ git -C env_ok/super_work add sub &&
+ git -C env_ok/super_work commit -m "point sub to reachable commit" &&
+ git -C env_ok/super_work push &&
+ git -C env_ok/clone -c fetch.submoduleErrors=warn \
+ fetch --recurse-submodules 2>err &&
+ test_grep ! "Errors during submodule fetch" err
+'
+
test_expect_success 'failed submodule fetch is fatal even when its commits are present locally' '
# Create the same commit (unreferenced, via commit-tree with fixed
# dates) in both super_work/sub and clone/sub, point the gitlink at
@@ -1334,4 +1385,42 @@ test_expect_success 'failed submodule fetch is fatal even when its commits are p
test_grep "Errors during submodule fetch" err
'
+test_expect_success '--submodule-errors=warn is honored by fetch --all' '
+ # A second remote forces fetch_multiple(), which hands the submodule
+ # recursion off to per-remote child processes; the option must be
+ # forwarded to them.
+ test_when_finished "rm -fr env_all" &&
+ create_err_env env_all &&
+ push_unreachable_commit env_all &&
+ git -C env_all/clone remote add second "$pwd/env_all/super_bare" &&
+ git -C env_all/clone fetch --all --recurse-submodules \
+ --submodule-errors=warn 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success '--submodule-errors=fail overrides warn config for fetch --all' '
+ # The per-remote child processes re-read the repository config, so
+ # the command-line override must be forwarded to them explicitly.
+ test_when_finished "rm -fr env_override" &&
+ create_err_env env_override &&
+ push_unreachable_commit env_override &&
+ git -C env_override/clone remote add second "$pwd/env_override/super_bare" &&
+ git -C env_override/clone config fetch.submoduleErrors warn &&
+ test_must_fail git -C env_override/clone fetch --all --recurse-submodules \
+ --submodule-errors=fail 2>err &&
+ test_grep "Errors during submodule fetch" err
+'
+
+test_expect_success 'fetch.submoduleErrors=warn: inaccessible submodule is non-fatal' '
+ test_when_finished "rm -fr env_access" &&
+ create_err_env env_access &&
+ rm env_access/clone/sub/.git &&
+ rm -r env_access/clone/.git/modules/sub &&
+ git -C env_access/clone -c fetch.submoduleErrors=warn \
+ fetch --recurse-submodules 2>err &&
+ test_grep "Could not access submodule" err &&
+ test_must_fail git -C env_access/clone fetch --recurse-submodules 2>err &&
+ test_grep "Could not access submodule" err
+'
+
test_done
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox