rebase: add --fixup to fold a range into its oldest commit

Adds git rebase --autosquash --fixup [<upstream>] to fold a range of commits into its oldest one, reusing that commit's message.

Related idea: https://github.com/gitgitgadget/git/issues/1135

Harald Nordgren (2):
t3415: remove prepare-commit-msg hook after use
rebase: add --fixup-all to fold a range

 Documentation/git-rebase.adoc |  11 ++++
 builtin/rebase.c              |  13 ++++-
 sequencer.c                   |  24 +++++++-
 sequencer.h                   |   2 +-
 t/t3415-rebase-autosquash.sh  | 106 ++++++++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+), 4 deletions(-)

base-commit: ea97ad8d017de0c9037451a78008a0fd60abea0c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v1
Pull-Request: https://github.com/git/git/pull/2337

gitgitgadget

From: Harald Nordgren <[email protected]>

The "pick and fixup respect commit.cleanup" test left its prepare-commit-msg hook in place, leaking it into later tests. Remove it with test_when_finished.

---
 t/t3415-rebase-autosquash.sh | 1 +
 1 file changed, 1 insertion(+)

index 5033411a43..8964d1cc88 100755
@@ -490,6 +490,7 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
 	git reset --hard base &&
 	test_commit --no-tag "fixup! second commit" file1 fixup &&
 	test_commit something &&
+	test_when_finished "rm -f .git/hooks/prepare-commit-msg" &&
 	write_script .git/hooks/prepare-commit-msg <<-\EOF &&
 	printf "\n# Prepared\n" >> "$1"
 	EOF

gitgitgadget

From: Harald Nordgren <[email protected]>

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 rebase --autosquash --fixup-all [<upstream>]" to do this directly. It keeps the first commit in the range as a "pick" and turns every later commit into a "fixup", so the whole range collapses into a single commit that reuses the first commit's message. With no <upstream> argument the range is "@{upstream}..HEAD", folding all unpushed commits into one.

Fold the commits in their original order, so that any fixup!/squash! commits already present in the range are folded in as well. Allow the flag only together with --autosquash, and reject --rebase-merges since a merge commit cannot be folded into another commit.

---
 Documentation/git-rebase.adoc |  11 ++++
 builtin/rebase.c              |  13 ++++-
 sequencer.c                   |  24 +++++++-
 sequencer.h                   |   2 +-
 t/t3415-rebase-autosquash.sh  | 105 ++++++++++++++++++++++++++++++++++
 5 files changed, 151 insertions(+), 4 deletions(-)

index f6c22d1598..d1a3e4ef64 100644
@@ -602,6 +602,16 @@ option can be used to override that setting.
 +
 See also INCOMPATIBLE OPTIONS below.
 
+--fixup-all::
+	Valid only when used with `--autosquash`.  Keep the first commit in
+	the range as a `pick` and change every later commit to a `fixup`, so
+	the whole range is folded into a single commit that reuses the first
+	commit's message.  With no `<upstream>` argument this folds all commits
+	since `@{upstream}` into one.  The commits are folded in their original
+	order, so any `fixup!`/`squash!` commits already in the range are folded
+	in as well.  Cannot be combined with `--rebase-merges`, as a merge
+	commit cannot be folded into another commit.
+
 --autostash::
 --no-autostash::
 	Automatically create a temporary stash entry before the operation

@@ -652,6 +662,7 @@ are incompatible with the following options:
  * --strategy
  * --strategy-option
  * --autosquash
+ * --fixup-all
  * --rebase-merges
  * --interactive
  * --exec

index fa4f5d9306..a363fbc1f2 100644
@@ -118,6 +118,7 @@ struct rebase_options {
 	int allow_rerere_autoupdate;
 	int keep_empty;
 	int autosquash;
+	int fixup_all;
 	char *gpg_sign_opt;
 	int autostash;
 	int committer_date_is_author_date;

@@ -329,7 +330,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 	ret = complete_action(the_repository, &replay, flags,
 		shortrevisions, opts->onto_name, opts->onto,
 		&opts->orig_head->object.oid, &opts->exec,
-		opts->autosquash, opts->update_refs, &todo_list);
+		opts->autosquash, opts->fixup_all, opts->update_refs,
+		&todo_list);
 
 cleanup:
 	replay_opts_release(&replay);

@@ -1205,6 +1207,8 @@ int cmd_rebase(int argc,
 		OPT_BOOL(0, "autosquash", &options.autosquash,
 			 N_("move commits that begin with "
 			    "squash!/fixup! under -i")),
+		OPT_BOOL(0, "fixup-all", &options.fixup_all,
+			 N_("fold all commits in the range into the first one")),
 		OPT_BOOL(0, "update-refs", &options.update_refs,
 			 N_("update branches that point to commits "
 			    "that are being rebased")),

@@ -1594,6 +1598,13 @@ int cmd_rebase(int argc,
 	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
 				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
 
+	if (options.fixup_all && options.autosquash != 1)
+		die(_("--fixup-all requires --autosquash"));
+
+	if (options.fixup_all && options.rebase_merges)
+		die(_("options '%s' and '%s' cannot be used together"),
+		    "--fixup-all", "--rebase-merges");
+
 	if (options.autosquash == 1) {
 		imply_merge(&options, "--autosquash");
 	} else if (options.autosquash == -1) {

index 57855b0066..eeaf6226fe 100644
@@ -6554,11 +6554,29 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
 	return 0;
 }
 
+static void todo_list_fixup_all_but_first(struct todo_list *todo_list)
+{
+	int i, seen_first = 0;
+
+	for (i = 0; i < todo_list->nr; i++) {
+		struct todo_item *item = todo_list->items + i;
+
+		if (!item->commit || item->command == TODO_DROP)
+			continue;
+		if (!seen_first) {
+			seen_first = 1;
+			item->command = TODO_PICK;
+			continue;
+		}
+		item->command = TODO_FIXUP;
+	}
+}
+
 int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
 		    const char *shortrevisions, const char *onto_name,
 		    struct commit *onto, const struct object_id *orig_head,
 		    struct string_list *commands, unsigned autosquash,
-		    unsigned update_refs,
+		    unsigned fixup_all, unsigned update_refs,
 		    struct todo_list *todo_list)
 {
 	char shortonto[GIT_MAX_HEXSZ + 1];

@@ -6581,7 +6599,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 	if (update_refs && todo_list_add_update_ref_commands(todo_list))
 		return -1;
 
-	if (autosquash && todo_list_rearrange_squash(todo_list))
+	if (fixup_all)
+		todo_list_fixup_all_but_first(todo_list);
+	else if (autosquash && todo_list_rearrange_squash(todo_list))
 		return -1;
 
 	if (commands->nr)

index 3164bd437d..9bb6b42c94 100644
@@ -196,7 +196,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 		    const char *shortrevisions, const char *onto_name,
 		    struct commit *onto, const struct object_id *orig_head,
 		    struct string_list *commands, unsigned autosquash,
-		    unsigned update_refs,
+		    unsigned fixup_all, unsigned update_refs,
 		    struct todo_list *todo_list);
 int todo_list_rearrange_squash(struct todo_list *todo_list);
 

gitgitgadget

[2/2] seems to add "--fixup-all" but I agree with the "related idea" that naming it and modelling it after "merge --squash" would be easier to understand.

I also wonder if we can do something like this without adding any new option or command. E.g., if you have four patch series, where the initial implementation HEAD~3 is followed by "oops it was still wrong" fix-up HEAD~2, HEAD~1 and HEAD, then

    git reset --soft HEAD~3 && git commit --amend --no-edit

is what the user wants to do, no?

Sounds reasonable.

I don't think it's enough. First of all the user has to know the N for HEAD~N, and then 'git reset --soft HEAD~N && git commit --amend --no-edit' is still quite ugly.

Harald

Rename to rebase --squash.

Harald Nordgren (2):
t3415: remove prepare-commit-msg hook after use
rebase: add --squash to fold a range

 Documentation/git-rebase.adoc |  11 ++++
 builtin/rebase.c              |  16 ++++-
 sequencer.c                   |  24 ++++++-
 sequencer.h                   |   2 +-
 t/t3415-rebase-autosquash.sh  | 118 ++++++++++++++++++++++++++++++++++
 5 files changed, 166 insertions(+), 5 deletions(-)

base-commit: ea97ad8d017de0c9037451a78008a0fd60abea0c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v2
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v1:

 1:  c55b9cd6f7 = 1:  c55b9cd6f7 t3415: remove prepare-commit-msg hook after use
 2:  bd1bc62aa8 ! 2:  22d4276ff5 rebase: add --fixup-all to fold a range
     @@ Metadata
      Author: Harald Nordgren <[email protected]>
      
       ## Commit message ##
     -    rebase: add --fixup-all to fold a range
     +    rebase: add --squash to fold a range
      
          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 rebase --autosquash --fixup-all [<upstream>]" to do this
     -    directly. It keeps the first commit in the range as a "pick" and turns
     -    every later commit into a "fixup", so the whole range collapses into a
     -    single commit that reuses the first commit's message. With no <upstream>
     -    argument the range is "@{upstream}..HEAD", folding all unpushed commits
     -    into one.
     +    Add "git rebase --squash [<upstream>]" to do this directly. It keeps
     +    the first commit in the range as a "pick" and turns every later commit
     +    into a "fixup", so the whole range collapses into a single commit that
     +    reuses the first commit's message. With no <upstream> argument the range
     +    is "@{upstream}..HEAD", folding all unpushed commits into one.
      
     -    Fold the commits in their original order, so that any fixup!/squash!
     -    commits already present in the range are folded in as well. Allow the
     -    flag only together with --autosquash, and reject --rebase-merges since a
     -    merge commit cannot be folded into another commit.
     +    The option implies the merge backend, so it works on its own without
     +    --autosquash. Fold the commits in their original order, so that any
     +    fixup!/squash! commits already present in the range are folded in as
     +    well. Reject --rebase-merges since a merge commit cannot be folded into
     +    another commit.
      
     +    Inspired-by: Sergey Chernov <[email protected]>
          Signed-off-by: Harald Nordgren <[email protected]>
      
       ## Documentation/git-rebase.adoc ##
     @@ Documentation/git-rebase.adoc: option can be used to override that setting.
       +
       See also INCOMPATIBLE OPTIONS below.
       
     -+--fixup-all::
     -+	Valid only when used with `--autosquash`.  Keep the first commit in
     -+	the range as a `pick` and change every later commit to a `fixup`, so
     -+	the whole range is folded into a single commit that reuses the first
     -+	commit's message.  With no `<upstream>` argument this folds all commits
     -+	since `@{upstream}` into one.  The commits are folded in their original
     -+	order, so any `fixup!`/`squash!` commits already in the range are folded
     -+	in as well.  Cannot be combined with `--rebase-merges`, as a merge
     -+	commit cannot be folded into another commit.
     ++--squash::
     ++	Keep the first commit in the range as a `pick` and change every later
     ++	commit to a `fixup`, so the whole range is folded into a single commit
     ++	that reuses the first commit's message.  With no `<upstream>` argument
     ++	this folds all commits since `@{upstream}` into one.  The commits are
     ++	folded in their original order, so any `fixup!`/`squash!` commits
     ++	already in the range are folded in as well.  Cannot be combined with
     ++	`--rebase-merges`, as a merge commit cannot be folded into another
     ++	commit.
      +
       --autostash::
       --no-autostash::
     @@ Documentation/git-rebase.adoc: are incompatible with the following options:
        * --strategy
        * --strategy-option
        * --autosquash
     -+ * --fixup-all
     ++ * --squash
        * --rebase-merges
        * --interactive
        * --exec
     @@ builtin/rebase.c: struct rebase_options {
       	int allow_rerere_autoupdate;
       	int keep_empty;
       	int autosquash;
     -+	int fixup_all;
     ++	int squash;
       	char *gpg_sign_opt;
       	int autostash;
       	int committer_date_is_author_date;
     @@ builtin/rebase.c: static int do_interactive_rebase(struct rebase_options *opts,
       		shortrevisions, opts->onto_name, opts->onto,
       		&opts->orig_head->object.oid, &opts->exec,
      -		opts->autosquash, opts->update_refs, &todo_list);
     -+		opts->autosquash, opts->fixup_all, opts->update_refs,
     ++		opts->autosquash, opts->squash, opts->update_refs,
      +		&todo_list);
       
       cleanup:
     @@ builtin/rebase.c: int cmd_rebase(int argc,
       		OPT_BOOL(0, "autosquash", &options.autosquash,
       			 N_("move commits that begin with "
       			    "squash!/fixup! under -i")),
     -+		OPT_BOOL(0, "fixup-all", &options.fixup_all,
     ++		OPT_BOOL(0, "squash", &options.squash,
      +			 N_("fold all commits in the range into the first one")),
       		OPT_BOOL(0, "update-refs", &options.update_refs,
       			 N_("update branches that point to commits "
       			    "that are being rebased")),
     +@@ builtin/rebase.c: int cmd_rebase(int argc,
     + 	if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
     + 	    (options.action != ACTION_NONE) ||
     + 	    (options.exec.nr > 0) ||
     +-	    options.autosquash == 1) {
     ++	    options.autosquash == 1 ||
     ++	    options.squash) {
     + 		allow_preemptive_ff = 0;
     + 	}
     + 	if (options.committer_date_is_author_date || options.ignore_date)
      @@ builtin/rebase.c: int cmd_rebase(int argc,
       	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
       				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
       
     -+	if (options.fixup_all && options.autosquash != 1)
     -+		die(_("--fixup-all requires --autosquash"));
     -+
     -+	if (options.fixup_all && options.rebase_merges)
     ++	if (options.squash && options.rebase_merges)
      +		die(_("options '%s' and '%s' cannot be used together"),
     -+		    "--fixup-all", "--rebase-merges");
     ++		    "--squash", "--rebase-merges");
     ++
     ++	if (options.squash)
     ++		imply_merge(&options, "--squash");
      +
       	if (options.autosquash == 1) {
       		imply_merge(&options, "--autosquash");
     @@ sequencer.c: static int todo_list_add_update_ref_commands(struct todo_list *todo
       		    struct commit *onto, const struct object_id *orig_head,
       		    struct string_list *commands, unsigned autosquash,
      -		    unsigned update_refs,
     -+		    unsigned fixup_all, unsigned update_refs,
     ++		    unsigned squash, unsigned update_refs,
       		    struct todo_list *todo_list)
       {
       	char shortonto[GIT_MAX_HEXSZ + 1];
     @@ sequencer.c: int complete_action(struct repository *r, struct replay_opts *opts,
       		return -1;
       
      -	if (autosquash && todo_list_rearrange_squash(todo_list))
     -+	if (fixup_all)
     ++	if (squash)
      +		todo_list_fixup_all_but_first(todo_list);
      +	else if (autosquash && todo_list_rearrange_squash(todo_list))
       		return -1;
     @@ sequencer.h: int complete_action(struct repository *r, struct replay_opts *opts,
       		    struct commit *onto, const struct object_id *orig_head,
       		    struct string_list *commands, unsigned autosquash,
      -		    unsigned update_refs,
     -+		    unsigned fixup_all, unsigned update_refs,
     ++		    unsigned squash, unsigned update_refs,
       		    struct todo_list *todo_list);
       int todo_list_rearrange_squash(struct todo_list *todo_list);
       
     @@ t/t3415-rebase-autosquash.sh: test_expect_success 'pick and fixup respect commit
       	test_commit_message HEAD -m "something"
       '
       
     -+test_expect_success '--fixup-all folds the range into the first commit' '
     ++test_expect_success '--squash folds the range into the first commit' '
      +	git reset --hard base &&
      +	test_commit --no-tag fold1 file_fold a &&
      +	test_commit --no-tag fold2 file_fold b &&
      +	test_commit --no-tag fold3 file_fold c &&
     -+	git rebase --autosquash --fixup-all HEAD~3 &&
     ++	git rebase --squash HEAD~3 &&
      +	test_cmp_rev base HEAD~1 &&
      +	test_commit_message HEAD -m "fold1" &&
      +	echo c >expect &&
      +	test_cmp expect file_fold
      +'
      +
     -+test_expect_success '--fixup-all folds smoothly when a fixup! commit is in the series' '
     ++test_expect_success '--squash folds smoothly when a fixup! commit is in the series' '
      +	git reset --hard base &&
      +	test_commit --no-tag foldA file_fold a &&
      +	test_commit --no-tag foldB file_fold b &&
      +	git commit --allow-empty --fixup HEAD~1 &&
     -+	git rebase --autosquash --fixup-all HEAD~3 &&
     ++	git rebase --squash HEAD~3 &&
      +	test_cmp_rev base HEAD~1 &&
      +	test_commit_message HEAD -m "foldA" &&
      +	echo b >expect &&
      +	test_cmp expect file_fold
      +'
      +
     -+test_expect_success '--fixup-all picks the first commit even if it is a fixup!' '
     ++test_expect_success '--squash picks the first commit even if it is a fixup!' '
      +	git reset --hard base &&
      +	test_commit --no-tag fixupbase file_fix a &&
      +	git commit --allow-empty --fixup HEAD &&
      +	test_commit --no-tag fixuptail file_fix b &&
     -+	git rebase --autosquash --fixup-all HEAD~3 &&
     ++	git rebase --squash HEAD~3 &&
      +	test_cmp_rev base HEAD~1 &&
      +	echo b >expect &&
      +	test_cmp expect file_fix
      +'
      +
     -+test_expect_success '--fixup-all with a single commit in range is a no-op' '
     ++test_expect_success '--squash with a single commit in range is a no-op' '
      +	git reset --hard base &&
      +	test_commit --no-tag solo file_solo a &&
      +	git rev-parse HEAD >expect &&
     -+	git rebase --autosquash --fixup-all HEAD~1 &&
     ++	git rebase --squash HEAD~1 &&
      +	git rev-parse HEAD >actual &&
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success '--fixup-all with an empty range succeeds' '
     ++test_expect_success '--squash with an empty range succeeds' '
      +	git reset --hard base &&
     -+	git rebase --autosquash --fixup-all HEAD &&
     ++	git rebase --squash HEAD &&
      +	test_cmp_rev base HEAD
      +'
      +
     -+test_expect_success '--fixup-all skips a dropped commit in the range' '
     ++test_expect_success '--squash skips a dropped commit in the range' '
      +	git reset --hard base &&
      +	test_commit --no-tag fixdrop1 file_drop a &&
      +	git commit --allow-empty -m "empty in the middle" &&
      +	test_commit --no-tag fixdrop3 file_drop b &&
     -+	git rebase --autosquash --empty=drop --fixup-all HEAD~3 &&
     ++	git rebase --squash --empty=drop HEAD~3 &&
      +	test_cmp_rev base HEAD~1 &&
      +	test_commit_message HEAD -m "fixdrop1" &&
      +	echo b >expect &&
      +	test_cmp expect file_drop
      +'
      +
     -+test_expect_success '--fixup-all folds a merge commit in the middle of the range' '
     ++test_expect_success '--squash folds a merge commit in the middle of the range' '
      +	git reset --hard base &&
      +	test_commit --no-tag mid-first &&
      +	git checkout -b mid-side &&
     @@ t/t3415-rebase-autosquash.sh: test_expect_success 'pick and fixup respect commit
      +	git checkout - &&
      +	git merge --no-ff -m "merge mid-side" mid-side &&
      +	test_commit --no-tag mid-last &&
     -+	git rebase --autosquash --fixup-all base &&
     ++	git rebase --squash base &&
      +	test_cmp_rev base HEAD~1 &&
      +	test_commit_message HEAD -m "mid-first" &&
      +	test_path_is_file mid-merged.t
      +'
      +
     -+test_expect_success '--fixup-all keeps the first flattened commit when a merge sorts first' '
     ++test_expect_success '--squash keeps the first flattened commit when a merge sorts first' '
      +	git reset --hard base &&
      +	git checkout -b head-side &&
      +	test_commit --no-tag head-merged &&
      +	git checkout - &&
      +	git merge --no-ff -m "merge head-side" head-side &&
      +	test_commit --no-tag head-last &&
     -+	git rebase --autosquash --fixup-all base &&
     ++	git rebase --squash base &&
      +	test_cmp_rev base HEAD~1 &&
      +	test_commit_message HEAD -m "head-merged" &&
      +	test_path_is_file head-merged.t
      +'
      +
     -+test_expect_success '--fixup-all requires --autosquash' '
     ++test_expect_success '--squash takes precedence over --autosquash' '
      +	git reset --hard base &&
     -+	test_must_fail git rebase --fixup-all HEAD~1 2>err &&
     -+	test_grep "fixup-all requires --autosquash" err &&
     -+	test_must_fail git rebase --no-autosquash --fixup-all HEAD~1 2>err &&
     -+	test_grep "fixup-all requires --autosquash" err
     ++	test_commit --no-tag combo-first &&
     ++	test_commit --no-tag combo-mid &&
     ++	git commit --allow-empty --fixup HEAD~1 &&
     ++	test_commit --no-tag combo-last &&
     ++	git rebase --autosquash --squash base &&
     ++	test_cmp_rev base HEAD~1 &&
     ++	test_commit_message HEAD -m "combo-first"
     ++'
     ++
     ++test_expect_success '--squash folds the range with rebase.autosquash set' '
     ++	test_config rebase.autosquash true &&
     ++	git reset --hard base &&
     ++	test_commit --no-tag cfg-first &&
     ++	test_commit --no-tag cfg-last &&
     ++	git rebase --squash base &&
     ++	test_cmp_rev base HEAD~1 &&
     ++	test_commit_message HEAD -m "cfg-first"
      +'
      +
     -+test_expect_success '--fixup-all and --rebase-merges cannot be combined' '
     ++test_expect_success '--squash and --rebase-merges cannot be combined' '
      +	git reset --hard base &&
     -+	test_must_fail git rebase --autosquash --rebase-merges \
     -+		--fixup-all HEAD~1 2>err &&
     ++	test_must_fail git rebase --rebase-merges --squash HEAD~1 2>err &&
      +	test_grep "cannot be used together" err &&
      +	test_path_is_missing .git/rebase-merge
      +'

gitgitgadget

From: Harald Nordgren <[email protected]>

The "pick and fixup respect commit.cleanup" test left its prepare-commit-msg hook in place, leaking it into later tests. Remove it with test_when_finished.

---
 t/t3415-rebase-autosquash.sh | 1 +
 1 file changed, 1 insertion(+)

index 5033411a43..8964d1cc88 100755
@@ -490,6 +490,7 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
 	git reset --hard base &&
 	test_commit --no-tag "fixup! second commit" file1 fixup &&
 	test_commit something &&
+	test_when_finished "rm -f .git/hooks/prepare-commit-msg" &&
 	write_script .git/hooks/prepare-commit-msg <<-\EOF &&
 	printf "\n# Prepared\n" >> "$1"
 	EOF

gitgitgadget

From: Harald Nordgren <[email protected]>

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 rebase --squash [<upstream>]" to do this directly. It keeps the first commit in the range as a "pick" and turns every later commit into a "fixup", so the whole range collapses into a single commit that reuses the first commit's message. With no <upstream> argument the range is "@{upstream}..HEAD", folding all unpushed commits into one.

The option implies the merge backend, so it works on its own without --autosquash. Fold the commits in their original order, so that any fixup!/squash! commits already present in the range are folded in as well. Reject --rebase-merges since a merge commit cannot be folded into another commit.

---
 Documentation/git-rebase.adoc |  11 ++++
 builtin/rebase.c              |  16 ++++-
 sequencer.c                   |  24 ++++++-
 sequencer.h                   |   2 +-
 t/t3415-rebase-autosquash.sh  | 117 ++++++++++++++++++++++++++++++++++
 5 files changed, 165 insertions(+), 5 deletions(-)

index f6c22d1598..4244288148 100644
@@ -602,6 +602,16 @@ option can be used to override that setting.
 +
 See also INCOMPATIBLE OPTIONS below.
 
+--squash::
+	Keep the first commit in the range as a `pick` and change every later
+	commit to a `fixup`, so the whole range is folded into a single commit
+	that reuses the first commit's message.  With no `<upstream>` argument
+	this folds all commits since `@{upstream}` into one.  The commits are
+	folded in their original order, so any `fixup!`/`squash!` commits
+	already in the range are folded in as well.  Cannot be combined with
+	`--rebase-merges`, as a merge commit cannot be folded into another
+	commit.
+
 --autostash::
 --no-autostash::
 	Automatically create a temporary stash entry before the operation

@@ -652,6 +662,7 @@ are incompatible with the following options:
  * --strategy
  * --strategy-option
  * --autosquash
+ * --squash
  * --rebase-merges
  * --interactive
  * --exec

index fa4f5d9306..2df9f04728 100644
@@ -118,6 +118,7 @@ struct rebase_options {
 	int allow_rerere_autoupdate;
 	int keep_empty;
 	int autosquash;
+	int squash;
 	char *gpg_sign_opt;
 	int autostash;
 	int committer_date_is_author_date;

@@ -329,7 +330,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
 	ret = complete_action(the_repository, &replay, flags,
 		shortrevisions, opts->onto_name, opts->onto,
 		&opts->orig_head->object.oid, &opts->exec,
-		opts->autosquash, opts->update_refs, &todo_list);
+		opts->autosquash, opts->squash, opts->update_refs,
+		&todo_list);
 
 cleanup:
 	replay_opts_release(&replay);

@@ -1205,6 +1207,8 @@ int cmd_rebase(int argc,
 		OPT_BOOL(0, "autosquash", &options.autosquash,
 			 N_("move commits that begin with "
 			    "squash!/fixup! under -i")),
+		OPT_BOOL(0, "squash", &options.squash,
+			 N_("fold all commits in the range into the first one")),
 		OPT_BOOL(0, "update-refs", &options.update_refs,
 			 N_("update branches that point to commits "
 			    "that are being rebased")),

@@ -1471,7 +1475,8 @@ int cmd_rebase(int argc,
 	if ((options.flags & REBASE_INTERACTIVE_EXPLICIT) ||
 	    (options.action != ACTION_NONE) ||
 	    (options.exec.nr > 0) ||
-	    options.autosquash == 1) {
+	    options.autosquash == 1 ||
+	    options.squash) {
 		allow_preemptive_ff = 0;
 	}
 	if (options.committer_date_is_author_date || options.ignore_date)

@@ -1594,6 +1599,13 @@ int cmd_rebase(int argc,
 	options.rebase_merges = (options.rebase_merges >= 0) ? options.rebase_merges :
 				((options.config_rebase_merges >= 0) ? options.config_rebase_merges : 0);
 
+	if (options.squash && options.rebase_merges)
+		die(_("options '%s' and '%s' cannot be used together"),
+		    "--squash", "--rebase-merges");
+
+	if (options.squash)
+		imply_merge(&options, "--squash");
+
 	if (options.autosquash == 1) {
 		imply_merge(&options, "--autosquash");
 	} else if (options.autosquash == -1) {

index 57855b0066..bb42b40796 100644
@@ -6554,11 +6554,29 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
 	return 0;
 }
 
+static void todo_list_fixup_all_but_first(struct todo_list *todo_list)
+{
+	int i, seen_first = 0;
+
+	for (i = 0; i < todo_list->nr; i++) {
+		struct todo_item *item = todo_list->items + i;
+
+		if (!item->commit || item->command == TODO_DROP)
+			continue;
+		if (!seen_first) {
+			seen_first = 1;
+			item->command = TODO_PICK;
+			continue;
+		}
+		item->command = TODO_FIXUP;
+	}
+}
+
 int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
 		    const char *shortrevisions, const char *onto_name,
 		    struct commit *onto, const struct object_id *orig_head,
 		    struct string_list *commands, unsigned autosquash,
-		    unsigned update_refs,
+		    unsigned squash, unsigned update_refs,
 		    struct todo_list *todo_list)
 {
 	char shortonto[GIT_MAX_HEXSZ + 1];

@@ -6581,7 +6599,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 	if (update_refs && todo_list_add_update_ref_commands(todo_list))
 		return -1;
 
-	if (autosquash && todo_list_rearrange_squash(todo_list))
+	if (squash)
+		todo_list_fixup_all_but_first(todo_list);
+	else if (autosquash && todo_list_rearrange_squash(todo_list))
 		return -1;
 
 	if (commands->nr)

index 3164bd437d..1d5a164f02 100644
@@ -196,7 +196,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 		    const char *shortrevisions, const char *onto_name,
 		    struct commit *onto, const struct object_id *orig_head,
 		    struct string_list *commands, unsigned autosquash,
-		    unsigned update_refs,
+		    unsigned squash, unsigned update_refs,
 		    struct todo_list *todo_list);
 int todo_list_rearrange_squash(struct todo_list *todo_list);
 

gitgitgadget

[snip]

Well, there are a few ways to get this more easily than counting; for example,

- git rev-list @{u}.. | tail -n1
- the lovely ":/<pattern>" or "@^{/<pattern>}" revision notations
- etc.

---

Stepping back a moment and assuming that the important thing you want is the "squash" (and not necessarily the "rebase" moving commits onto a new base), I wonder about

git history squash <range>

which would squash all commits in the (now arbitrary!) range into the first. That makes it somewhat more versatile at selecting commits, I think, at the cost that re-basing is somewhat harder. That is, you could then do

git history squash @~3..

and things like

    git history squash @~5..@~2

As a future extension, I think we could support merge commits: merges could be replayed as a merge into the final squash instead (creating an octopus merge if there are multiple merges to replay), though I'm hand-waving what we should do for conflicts. (We _do_ know what the final tree should look like—the same as the final commit in the range—so maybe we can actually avoid all conflicts?)

Anyway, I've cc'd Patrick for his opinion about whether this fits in "git-history".

D. Ben Knoble

Yes, it does fit into git-history(1), and I do indeed already have plans to implement such a command going forward. I wouldn't mind at all though if somebody else beat me to it, I want to implement at least one more command before I get to this.

Patrick

Hi Harald

Please include the original cover letter as well so people who have not read the previous version know what the series is about.

I agree that git-history might be a better place for this but I'm not opposed to adding a --squash option to git-rebase. However I do think we need to think about the implementation - picking consecutive commits when we want to squash them together is inefficient and if we're changing the base risks the user having to stop to resolve conflicts multiple times. Regardless of whether this ends up in git-rebase or git-history I think the implementation should cherry-pick the whole commit range by doing the equivalent of

  git merge-tree --merge-base $(git merge-base HEAD @{upstream}) \
                 @{upstream} HEAD

We should also let the user edit the commit message to reflect the changes that are being squashed in. We should think about what support we want for "amend!" commits that replace the commit message when rebasing.

Thanks

Phillip

So you mean this one, should that be included in each version, and append each subsequent one:

```
Adds `git rebase --autosquash --fixup [<upstream>]` to fold a range of
commits into its oldest one, reusing that commit's message.

Related idea: https://github.com/gitgitgadget/git/issues/1135
```

Or make each message a full cover letter instead of just a diff?

Harald

What I like about 'git rebase --squash' is that when upstream is set up, it understands the commit range automatically, whereas history feels more removed from the current upstream. Maybe I'm wrong about that.

Harald

Yes, see https://lore.kernel.org/[email protected] for an example and the "Cover Letter" section of Documentation/SubmittingPatches

Thanks

Phillip

Adds git history squash <revision-range> to fold a range of commits into its oldest one, reusing that commit's message and replaying any descendants on top.

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 (4):
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
history: re-edit a squash with every message

 Documentation/git-history.adoc |  21 +++
 builtin/history.c              | 287 ++++++++++++++++++++++++++++-----
 t/meson.build                  |   1 +
 t/t3454-history-squash.sh      | 250 ++++++++++++++++++++++++++++
 4 files changed, 521 insertions(+), 38 deletions(-)
 create mode 100755 t/t3454-history-squash.sh

base-commit: 95e20213faefeb95df29277c58ac1980ab68f701
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v3
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v2:

 1:  c55b9cd6f7 < -:  ---------- t3415: remove prepare-commit-msg hook after use
 2:  22d4276ff5 < -:  ---------- rebase: add --squash to fold a range
 -:  ---------- > 1:  1e31474ef6 history: extract helper for a commit's parent tree
 -:  ---------- > 2:  498da64046 history: give commit_tree_ext a message template
 -:  ---------- > 3:  66b2f49fb4 history: add squash subcommand to fold a range
 -:  ---------- > 4:  43e4270614 history: re-edit a squash with every message

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index 091465a59e..f95f26e684 100644
@@ -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

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index f95f26e684..305bde3102 100644
@@ -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

From: Harald Nordgren <[email protected]>

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 message and authorship and taking the tree of the newest commit, so the range collapses into a single commit. Commits above the range are replayed on top of the result.

The range is given as <base>..<tip>, so "git history squash @~3.." folds the three most recent commits and "git history squash @~5..@~2" squashes an interior range. A merge inside the range is folded like any other commit, but the range must have a single base, so a range with more than one entry point is rejected.

---
 Documentation/git-history.adoc |  20 ++++
 builtin/history.c              | 154 ++++++++++++++++++++++++
 t/meson.build                  |   1 +
 t/t3454-history-squash.sh      | 213 +++++++++++++++++++++++++++++++++
 4 files changed, 388 insertions(+)
 create mode 100755 t/t3454-history-squash.sh

index 2ba8121795..d3a5ad28a3 100644
@@ -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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
 
 DESCRIPTION
 -----------

@@ -97,6 +98,25 @@ 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 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.
++
+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 @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+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.
+
 OPTIONS
 -------
 

index 3219264fe7..d7ae5a46ef 100644
@@ -399,6 +399,7 @@ integration_tests = [
   't3451-history-reword.sh',
   't3452-history-split.sh',
   't3453-history-fixup.sh',
+  't3454-history-squash.sh',
   't3500-cherry.sh',
   't3501-revert-cherry-pick.sh',
   't3502-cherry-pick-merge.sh',

gitgitgadget

From: Harald Nordgren <[email protected]>

By default "git history squash" reuses the oldest commit's message. When --reedit-message is given it only reopened that one message, so the messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and use them as the editor template when re-editing, mirroring how "git rebase -i" presents a squash. The combined message is built before the descendant walk so it is not disturbed by the flags that walk leaves on the commits.

---
 Documentation/git-history.adoc |  5 +--
 builtin/history.c              | 61 +++++++++++++++++++++++++++++++++-
 t/t3454-history-squash.sh      | 37 +++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

index d3a5ad28a3..dd3544832d 100644
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
 `git history squash @~5..@~2` squashes an interior range while leaving
 the two newest commits in place.
 +
-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.

index 6c6a75bf00..1edd148295 100755
@@ -135,6 +135,43 @@ 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 &&
+	echo b >file &&
+	git add file &&
+	git commit -m "re-one subject" -m "re-one body line" &&
+	test_commit re-two file c &&
+	test_commit re-three file d &&
+
+	write_script editor <<-\EOF &&
+	cp "$1" buffer &&
+	echo combined >"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	git history squash --reedit-message start.. &&
+
+	grep "re-one subject" buffer &&
+	grep "re-one body line" buffer &&
+	grep re-two buffer &&
+	grep re-three buffer &&
+	git log --format="%s" -1 >actual &&
+	echo combined >expect &&
+	test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
 	git reset --hard three &&
 	head_before=$(git rev-parse HEAD) &&

gitgitgadget

I think I mentioned this, too, albeit indirectly. I'm not concerned about credit, though. Just excited to have this.

Thanks!

Another tweak I had to make was to replace "grep" with "test_grep" to avoid triggering test lint added by another topic in flight.

For the one in the second hunk, it may be much better to rewrite it to process "out" directly with the awk script without preprocessing it with "grep", as awk is a programming language capable enough to recognize a line that matches a pattern and process only those matching lines by itself.

--- >8 ---

Author: Junio C Hamano <[email protected]>
Date:   Thu Jun 18 13:44:36 2026 -0700

SQUASH??? avoid test_grep lint triggering on uses of raw grep

index 1edd148295..20370c0136 100755
@@ -150,10 +150,10 @@ test_expect_success '--reedit-message offers every folded-in message' '
 	test_set_editor "$(pwd)/editor" &&
 	git history squash --reedit-message start.. &&
 
-	grep "re-one subject" buffer &&
-	grep "re-one body line" buffer &&
-	grep re-two buffer &&
-	grep re-three buffer &&
+	test_grep "re-one subject" buffer &&
+	test_grep "re-one body line" buffer &&
+	test_grep re-two buffer &&
+	test_grep re-three buffer &&
 	git log --format="%s" -1 >actual &&
 	echo combined >expect &&
 	test_cmp expect actual

@@ -177,7 +177,7 @@ test_expect_success '--dry-run predicts the rewrite without performing it' '
 	head_before=$(git rev-parse HEAD) &&
 
 	git history squash --dry-run start.. >out &&
-	grep "^update refs/heads/" out >update &&
+	test_grep "^update refs/heads/" out >update &&
 	predicted=$(awk "{print \$3}" update) &&
 	test_cmp_rev "$head_before" HEAD &&
 

[snip]

D. Ben Knoble

One thing that just occurred to me.

When you have a linear history

    o---A---B---C

you run "git history squash A..C" and come to

    o---X

where the tree of X is the same as C, with the log message of A reused for it. That is simple, clean, and easy to explain.

But what should happen to refs (i.e., branch head) that point at A or B?

I am adressing this message to Patrick as this question relates to the grand vision for the "git history" command. I think "git replay" wants to rewrite all the refs that are involved in the rewrite operation, while "git rebase" (without "--update-refs") wants to leave all others refs intact and update only the branch it was told to rewrite. Is it the same design as "rebase" and "--update-refs" controls if we update _other_ refs that happened to be in the range that are rewritten?

Now, assuming that there do exist a mode where the command can update these refs that point into the history that got rewritten, there probably are at least two possibilities.

On one hand, I think it is reasonable to _remove_ these refs that used to point at a section of history that disappeared (like the one that were pointing at A or B). Perhaps A and B were pointed at by two branches or tags that were used to mark "up to this point things are broken" and "from here on things are fixed" (i.e., imagine a manual bisection). After squashing all of the commits in this section of history, the result no longer has such transition points.

It also is plausible that users may want these refs that used to point at A or B to point at X, just like the ref that used to point at C would now point at X, even though I cannot offhand think of a good story (like "there used to be transtion points, now there isn't" I said above to explain why these refs should disappear) to support such a behaviour.

Thoughts?

It's a very good question. I had git history squash in my backlog for a while, and this very question made me defer that topic repeatedly.

Yeah.

I think just pruning references would be extremely surprising to our users.

There are two more modes:

- If a reference points at an intermediate commit then it stays there.

- We detect this case and reject the update. Optionally, we may ask the user what they intend to do with those other refs.

It really is kind of ambiguous what is supposed to happen, and I can think of different scenarios where each of the possibilities would be the best choice. So ultimately, I think the last option is the best one, as it also gives us a way to iterate.

If so, a user would already be able to achieve that other refs keep pointing at X by saying git history squash --update-refs=head. The other modes can then be added at a later point in time as the need arises.

Patrick

Yeah, sounds like we should detect and fail this case, with advice() to use --update-refs.

Adds git history squash <revision-range> to fold a range of commits.

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 (4):
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
history: re-edit a squash with every message

 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  26 +++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 328 +++++++++++++++++++++++++----
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 340 +++++++++++++++++++++++++++++++
 7 files changed, 663 insertions(+), 38 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v4
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v3:

 1:  1e31474ef6 = 1:  fc2801c0b1 history: extract helper for a commit's parent tree
 2:  498da64046 = 2:  ee591e83b4 history: give commit_tree_ext a message template
 3:  66b2f49fb4 ! 3:  80bfea642e history: add squash subcommand to fold a range
     @@ Commit message
          other commit, but the range must have a single base, so a range with
          more than one entry point is rejected.
      
     +    The folded commits leave the history, so by default the command refuses
     +    when another ref points at one of them. Use "--update-refs=head" to
     +    rewrite only the current branch and leave those refs untouched.
     +
          Inspired-by: Sergey Chernov <[email protected]>
          Signed-off-by: Harald Nordgren <[email protected]>
      
     + ## Documentation/config/advice.adoc ##
     +@@ Documentation/config/advice.adoc: 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.
     +
       ## Documentation/git-history.adoc ##
      @@ Documentation/git-history.adoc: SYNOPSIS
       git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +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 folded commits disappear from the history, so with the default
     ++`--update-refs=branches` the command refuses when another ref points at
     ++one of them. Rerun with `--update-refs=head` to rewrite only the current
     ++branch and leave those refs pointing at the old commits.
      +
       OPTIONS
       -------
       
      
     + ## advice.c ##
     +@@ advice.c: 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" },
     +
     + ## advice.h ##
     +@@ advice.h: 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,
     +
       ## builtin/history.c ##
     +@@
     + #define USE_THE_REPOSITORY_VARIABLE
     + 
     + #include "builtin.h"
     ++#include "advice.h"
     + #include "cache-tree.h"
     + #include "commit.h"
     + #include "commit-reach.h"
      @@
       	N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
       #define GIT_HISTORY_SPLIT_USAGE \
     @@ builtin/history.c: out:
      +				const char *range,
      +				struct commit **base_out,
      +				struct commit **oldest_out,
     -+				struct commit **tip_out)
     ++				struct commit **tip_out,
     ++				struct oidset *interior_out)
      +{
      +	struct rev_info revs;
      +	struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
     @@ builtin/history.c: out:
      +		}
      +		if (!oldest)
      +			oldest = commit;
     ++		if (tip)
     ++			oidset_insert(interior_out, &tip->object.oid);
      +		tip = commit;
      +	}
      +
     @@ builtin/history.c: out:
      +	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,
     @@ builtin/history.c: out:
      +		OPT_END(),
      +	};
      +	struct strbuf reflog_msg = STRBUF_INIT;
     ++	struct oidset interior = OIDSET_INIT;
      +	struct commit *base, *oldest, *tip, *rewritten;
      +	const struct object_id *base_tree_oid, *tip_tree_oid;
      +	struct commit_list *parents = NULL;
     @@ builtin/history.c: out:
      +	if (action == REF_ACTION_DEFAULT)
      +		action = REF_ACTION_BRANCHES;
      +
     -+	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip);
     ++	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip,
     ++				   &interior);
      +	if (ret < 0)
      +		goto out;
      +
     ++	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;
     @@ builtin/history.c: out:
      +
      +out:
      +	strbuf_release(&reflog_msg);
     ++	oidset_clear(&interior);
      +	commit_list_free(parents);
      +	release_revisions(&revs);
      +	return ret;
     @@ t/meson.build: integration_tests = [
         't3451-history-reword.sh',
         't3452-history-split.sh',
         't3453-history-fixup.sh',
     -+  't3454-history-squash.sh',
     ++  't3455-history-squash.sh',
         't3500-cherry.sh',
         't3501-revert-cherry-pick.sh',
         't3502-cherry-pick-merge.sh',
      
     - ## t/t3454-history-squash.sh (new) ##
     + ## t/t3455-history-squash.sh (new) ##
      @@
      +#!/bin/sh
      +
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success 'setup linear history touching two files' '
      +	test_commit base file a &&
      +	git tag start &&
     -+	test_commit one other x &&
     -+	test_commit two file c &&
     ++	test_commit --no-tag one other x &&
     ++	test_commit --no-tag two file c &&
      +	test_commit three file d
      +'
      +
     @@ t/t3454-history-squash.sh (new)
      +
      +test_expect_success 'reuses the message of a fixup! commit in the range' '
      +	git reset --hard start &&
     -+	test_commit reg1 file b &&
     ++	test_commit --no-tag reg1 file b &&
      +	git commit --allow-empty -m "fixup! reg1" &&
      +	test_commit reg2 file c &&
      +
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success 'preserves authorship of the oldest commit' '
      +	git reset --hard start &&
      +	GIT_AUTHOR_NAME=Squasher [email protected] \
     -+		test_commit oldest file b &&
     ++		test_commit --no-tag oldest file b &&
      +	test_commit newest file c &&
      +
      +	git history squash start.. &&
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success '--dry-run predicts the rewrite without performing it' '
      +	git reset --hard three &&
      +	head_before=$(git rev-parse HEAD) &&
     ++	tip_tree=$(git rev-parse HEAD^{tree}) &&
      +
      +	git history squash --dry-run start.. >out &&
     -+	grep "^update refs/heads/" out >update &&
     -+	predicted=$(awk "{print \$3}" update) &&
     ++	predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
      +	test_cmp_rev "$head_before" HEAD &&
      +
      +	git history squash start.. &&
     -+	test "$predicted" = "$(git rev-parse HEAD)"
     ++	test "$predicted" = "$(git rev-parse HEAD)" &&
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test_cmp_rev start HEAD^ &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
      +'
      +
      +test_expect_success '--update-refs=head only moves HEAD' '
     @@ t/t3454-history-squash.sh (new)
      +	test_cmp_rev "$other_before" other
      +'
      +
     -+test_expect_success '--update-refs=branches moves a branch pointing into the range' '
     ++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 &&
     ++
     ++	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 &&
     ++
     ++	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~2 &&
     ++	git branch -f mid HEAD~1 &&
      +	mid_before=$(git rev-parse mid) &&
      +
     -+	git history squash start..@~1 &&
     ++	git history squash --update-refs=head start.. &&
      +
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
      +	test_cmp_rev "$mid_before" mid &&
     -+	test_commit_message mid -m one
     ++
     ++	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 &&
     -+	test_commit before-side file b &&
     ++	test_commit --no-tag before-side file b &&
      +	git checkout -b inner-side &&
     -+	test_commit on-inner-side inner x &&
     ++	test_commit --no-tag on-inner-side inner x &&
      +	git checkout - &&
     -+	test_commit after-side file c &&
     ++	test_commit --no-tag after-side file c &&
      +	git merge --no-ff -m merge inner-side &&
     -+	test_commit after-merge file d &&
     ++	git branch -D inner-side &&
     ++	test_commit --no-tag after-merge file d &&
      +	tip_tree=$(git rev-parse HEAD^{tree}) &&
      +
      +	git history squash start.. &&
     @@ t/t3454-history-squash.sh (new)
      +	test_path_is_file inner
      +'
      +
     ++test_expect_success 'folds a range whose tip is a merge commit' '
     ++	git reset --hard start &&
     ++	test_commit --no-tag tipmerge-base file b &&
     ++	git checkout -b tipmerge-side &&
     ++	test_commit --no-tag tipmerge-side side x &&
     ++	git checkout - &&
     ++	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.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	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 &&
     ++	git checkout -b basemerge-side &&
     ++	test_commit --no-tag basemerge-side side x &&
     ++	git checkout - &&
     ++	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.." &&
     ++
     ++	git rev-list --count "$base..HEAD" >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test_cmp_rev "$base" HEAD^ &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
     ++'
     ++
      +test_expect_success 'refuses to squash a range with more than one base' '
      +	git reset --hard start &&
      +	head_before=$(git rev-parse HEAD) &&
 4:  43e4270614 ! 4:  85c7817d7e history: re-edit a squash with every message
     @@ Documentation/git-history.adoc: history squash @~3..` folds the three most recen
       forked before the range and was later merged into it) is rejected.
      
       ## builtin/history.c ##
     -@@ builtin/history.c: out:
     - 	return ret;
     +@@ builtin/history.c: static int find_interior_ref(const struct reference *ref, void *cb_data)
     + 	return 0;
       }
       
      +static int build_squash_message(struct repository *repo,
     @@ builtin/history.c: static int cmd_history_squash(int argc,
       	};
       	struct strbuf reflog_msg = STRBUF_INIT;
      +	struct strbuf message = STRBUF_INIT;
     + 	struct oidset interior = OIDSET_INIT;
       	struct commit *base, *oldest, *tip, *rewritten;
       	const struct object_id *base_tree_oid, *tip_tree_oid;
     - 	struct commit_list *parents = NULL;
      @@ builtin/history.c: static int cmd_history_squash(int argc,
     - 	if (ret < 0)
     - 		goto out;
     + 		}
     + 	}
       
      +	if (flags & COMMIT_TREE_EDIT_MESSAGE) {
      +		ret = build_squash_message(repo, base, tip, &message);
     @@ 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);
     - 	return ret;
      
     - ## t/t3454-history-squash.sh ##
     -@@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the oldest commit' '
     + ## t/t3455-history-squash.sh ##
     +@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the oldest commit' '
       	test_cmp expect actual
       '
       
     @@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	echo b >file &&
      +	git add file &&
      +	git commit -m "re-one subject" -m "re-one body line" &&
     -+	test_commit re-two file c &&
     ++	test_commit --no-tag re-two file c &&
      +	test_commit re-three file d &&
      +
      +	write_script editor <<-\EOF &&
     @@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	test_set_editor "$(pwd)/editor" &&
      +	git history squash --reedit-message start.. &&
      +
     -+	grep "re-one subject" buffer &&
     -+	grep "re-one body line" buffer &&
     -+	grep re-two buffer &&
     -+	grep re-three buffer &&
     ++	test_grep "re-one subject" buffer &&
     ++	test_grep "re-one body line" buffer &&
     ++	test_grep re-two buffer &&
     ++	test_grep re-three buffer &&
      +	git log --format="%s" -1 >actual &&
      +	echo combined >expect &&
      +	test_cmp expect actual

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index 091465a59e..f95f26e684 100644
@@ -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

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index f95f26e684..305bde3102 100644
@@ -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

From: Harald Nordgren <[email protected]>

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 message and authorship and taking the tree of the newest commit, so the range collapses into a single commit. Commits above the range are replayed on top of the result.

The range is given as <base>..<tip>, so "git history squash @~3.." folds the three most recent commits and "git history squash @~5..@~2" squashes an interior range. A merge inside the range is folded like any other commit, but the range must have a single base, so a range with more than one entry point is rejected.

The folded commits leave the history, so by default the command refuses when another ref points at one of them. Use "--update-refs=head" to rewrite only the current branch and leave those refs untouched.

---
 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  25 +++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 195 ++++++++++++++++++++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 303 +++++++++++++++++++++++++++++++
 7 files changed, 530 insertions(+)
 create mode 100755 t/t3455-history-squash.sh

index 257db58918..f4d692d136 100644
@@ -55,6 +55,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.

index 2ba8121795..6716749cde 100644
@@ -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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
 
 DESCRIPTION
 -----------

@@ -97,6 +98,30 @@ 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 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.
++
+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 @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+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 folded commits disappear from the history, so with the default
+`--update-refs=branches` the command refuses when another ref points at
+one of them. Rerun with `--update-refs=head` to rewrite only the current
+branch and leave those refs pointing at the old commits.
+
 OPTIONS
 -------
 

index 0018501b7b..5c6ff95e31 100644
@@ -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" },

index 8def280688..911b4e4643 100644
@@ -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,

index 3219264fe7..63ea26b8ed 100644
@@ -399,6 +399,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',

gitgitgadget

From: Harald Nordgren <[email protected]>

By default "git history squash" reuses the oldest commit's message. When --reedit-message is given it only reopened that one message, so the messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and use them as the editor template when re-editing, mirroring how "git rebase -i" presents a squash. The combined message is built before the descendant walk so it is not disturbed by the flags that walk leaves on the commits.

---
 Documentation/git-history.adoc |  5 +--
 builtin/history.c              | 61 +++++++++++++++++++++++++++++++++-
 t/t3455-history-squash.sh      | 37 +++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

index 6716749cde..df389015aa 100644
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
 `git history squash @~5..@~2` squashes an interior range while leaving
 the two newest commits in place.
 +
-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.

index 821c801153..1fb3b9b63e 100755
@@ -135,6 +135,43 @@ 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 &&
+	echo b >file &&
+	git add file &&
+	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 &&
+	cp "$1" buffer &&
+	echo combined >"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	git history squash --reedit-message start.. &&
+
+	test_grep "re-one subject" buffer &&
+	test_grep "re-one body line" buffer &&
+	test_grep re-two buffer &&
+	test_grep re-three buffer &&
+	git log --format="%s" -1 >actual &&
+	echo combined >expect &&
+	test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
 	git reset --hard three &&
 	head_before=$(git rev-parse HEAD) &&

gitgitgadget

Some of the conceptual questions on v3 are still open, like for example whether we should be using "--ancestry=". Please make sure that you are replying to questions like that when rerolling, especially when you don't plan to address them. Otherwise the reviewer will not know why you didn't take feedback into account. See also the thread at [1] for guidance.

I'll wait for open questions to be addressed before reviewing this (or any subsequent) version.

Thanks!

Patrick

[1]: <[email protected]>

Sorry about that.

I skipped the ancestry question because I felt the implementation would get too complex and was hoping we could do without it. Probably would have been better to be explicit about that instead of just skipping it. I don't want to balloon the code more than necessary, but I'll take a look at it now.

Harald

Adds git history squash <revision-range> to fold a range of commits.

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 (4):
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
history: re-edit a squash with every message

 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  26 ++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 341 ++++++++++++++++++---
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 497 +++++++++++++++++++++++++++++++
 7 files changed, 833 insertions(+), 38 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

base-commit: 26d8d94e94df5535eecd036f16627493506a0614
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v5
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v4:

 1:  fc2801c0b1 = 1:  0f1ae9b05a history: extract helper for a commit's parent tree
 2:  ee591e83b4 = 2:  a97ffab1e6 history: give commit_tree_ext a message template
 3:  80bfea642e ! 3:  04e18ef979 history: add squash subcommand to fold a range
     @@ builtin/history.c: out:
      +	struct rev_info revs;
      +	struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
      +	struct strvec args = STRVEC_INIT;
     ++	size_t i;
      +	int ret;
      +
      +	repo_init_revisions(repo, &revs, NULL);
     @@ builtin/history.c: out:
      +	strvec_push(&args, "--reverse");
      +	strvec_push(&args, "--topo-order");
      +	strvec_push(&args, "--boundary");
     ++	strvec_push(&args, "--ancestry-path");
      +	strvec_push(&args, range);
      +	setup_revisions_from_strvec(&args, &revs, NULL);
      +	if (args.nr != 1) {
     @@ builtin/history.c: out:
      +		goto out;
      +	}
      +
     ++	/*
     ++	 * A squash needs a base to reparent onto, so the argument has to
     ++	 * exclude something, as in "<base>..<tip>". A single revision has no
     ++	 * such bottom commit and 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(_("'%s' is not a '<base>..<tip>' range"), range);
     ++		goto out;
     ++	}
     ++
      +	if (prepare_revision_walk(&revs) < 0) {
      +		ret = error(_("error preparing revisions"));
      +		goto out;
     @@ builtin/history.c: out:
      +		goto out;
      +	}
      +
     -+	if (!base) {
     -+		ret = error(_("cannot squash the root commit"));
     -+		goto out;
     -+	}
     ++	if (!base)
     ++		BUG("a non-empty range must have a boundary commit");
      +
      +	*base_out = base;
      +	*oldest_out = oldest;
     @@ t/t3455-history-squash.sh (new)
      +	test_grep "the range .* is empty" err
      +'
      +
     -+test_expect_success 'errors when the range includes the root commit' '
     ++test_expect_success 'errors on a single revision that is not a range' '
      +	test_must_fail git history squash HEAD 2>err &&
     -+	test_grep "cannot squash the root commit" err
     ++	test_grep "is not a .*range" err &&
     ++	test_must_fail git history squash HEAD~1 2>err &&
     ++	test_grep "is not a .*range" err
      +'
      +
      +test_expect_success 'squashes a range into a single commit without changing the tree' '
     @@ t/t3455-history-squash.sh (new)
      +	test_path_is_file inner
      +'
      +
     ++test_expect_success 'folds a merge of a branch that forked at the base' '
     ++	git reset --hard start &&
     ++	git checkout -b base-fork-side &&
     ++	test_commit --no-tag base-fork-side side x &&
     ++	git checkout - &&
     ++	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.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test_cmp_rev start HEAD^ &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
     ++	test_path_is_file side
     ++'
     ++
      +test_expect_success 'folds a range whose tip is a merge commit' '
      +	git reset --hard start &&
      +	test_commit --no-tag tipmerge-base file b &&
     @@ t/t3455-history-squash.sh (new)
      +	test_cmp_rev "$merged" HEAD
      +'
      +
     ++test_expect_success 'folds a range with two interior merges' '
     ++	git reset --hard start &&
     ++	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 - &&
     ++	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 - &&
     ++	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.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	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.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	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.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	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-base file b &&
     ++	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 c &&
     ++	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

4: 85c7817d7e = 4: a758e1f084 history: re-edit a squash with every message

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index 091465a59e..f95f26e684 100644
@@ -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

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index f95f26e684..305bde3102 100644
@@ -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

From: Harald Nordgren <[email protected]>

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 message and authorship and taking the tree of the newest commit, so the range collapses into a single commit. Commits above the range are replayed on top of the result.

The range is given as <base>..<tip>, so "git history squash @~3.." folds the three most recent commits and "git history squash @~5..@~2" squashes an interior range. A merge inside the range is folded like any other commit, but the range must have a single base, so a range with more than one entry point is rejected.

The folded commits leave the history, so by default the command refuses when another ref points at one of them. Use "--update-refs=head" to rewrite only the current branch and leave those refs untouched.

---
 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  25 ++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 208 ++++++++++++++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 460 +++++++++++++++++++++++++++++++
 7 files changed, 700 insertions(+)
 create mode 100755 t/t3455-history-squash.sh

index 257db58918..f4d692d136 100644
@@ -55,6 +55,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.

index 2ba8121795..6716749cde 100644
@@ -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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
 
 DESCRIPTION
 -----------

@@ -97,6 +98,30 @@ 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 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.
++
+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 @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+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 folded commits disappear from the history, so with the default
+`--update-refs=branches` the command refuses when another ref points at
+one of them. Rerun with `--update-refs=head` to rewrite only the current
+branch and leave those refs pointing at the old commits.
+
 OPTIONS
 -------
 

index 0018501b7b..5c6ff95e31 100644
@@ -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" },

index 8def280688..911b4e4643 100644
@@ -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,

index 3219264fe7..63ea26b8ed 100644
@@ -399,6 +399,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',

gitgitgadget

From: Harald Nordgren <[email protected]>

By default "git history squash" reuses the oldest commit's message. When --reedit-message is given it only reopened that one message, so the messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and use them as the editor template when re-editing, mirroring how "git rebase -i" presents a squash. The combined message is built before the descendant walk so it is not disturbed by the flags that walk leaves on the commits.

---
 Documentation/git-history.adoc |  5 +--
 builtin/history.c              | 61 +++++++++++++++++++++++++++++++++-
 t/t3455-history-squash.sh      | 37 +++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

index 6716749cde..df389015aa 100644
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
 `git history squash @~5..@~2` squashes an interior range while leaving
 the two newest commits in place.
 +
-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.

index 7227c5c90f..af59ddf6e3 100755
@@ -137,6 +137,43 @@ 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 &&
+	echo b >file &&
+	git add file &&
+	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 &&
+	cp "$1" buffer &&
+	echo combined >"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	git history squash --reedit-message start.. &&
+
+	test_grep "re-one subject" buffer &&
+	test_grep "re-one body line" buffer &&
+	test_grep re-two buffer &&
+	test_grep re-three buffer &&
+	git log --format="%s" -1 >actual &&
+	echo combined >expect &&
+	test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
 	git reset --hard three &&
 	head_before=$(git rev-parse HEAD) &&

gitgitgadget

Hi Harald

It would be helpful to give a bit more detail here about the command so that the reader has an overview of what is actually being implemented.

- what does it do with fixup!, squash! and amend! commits? Can it use the message from amend! commits to reword the commit?
- can the user reword the commit message?
- what happens if a merge commit inside the range has a parent outside the range?
- what happens to branches that point to commits inside the range?

I had a quick play and found that it accepts ranges that containing a single commit (e.g. @^!) where there is nothing to squash. It also accepts ranges that are not ancestors of HEAD (e.g. checkout master and run "git history squash --dry-run origin/seen^2^!") without printing an error message. Only accepting a single argument is quite limiting as one cannot say

git history squash ^:/base :/tip

Thanks

Phillip

Good points, I will take a look at clarifying or giving an error (like in the case of ancestor not in history of HEAD).

Only accepting a single argument is quite limiting as one

I don't understand why this is limiting? It thought it was clear that it should be one argument REF1..REF2 ? What does '^:/base :/tip' achieve that '^:/base..:/tip' cannot?

Harald

'^/:base..:/tip' is not a range - everything after the first '/:' is treated as a regular expression to search for.

Thanks

Phillip

This particular case you can do

	HEAD^{/base}..HEAD^{/tip}

(or even go "HEAD^{/tip}~43" and fancier other forms, the point being with matching {} pair, you can do more than what the lazy short-hand form can).

But I think your point still stands, I think, as

git history squash HEAD^{/base}..:/tip ^main

may be something you would want to do to express additional constraints, like "I want this range squashed, but I should never ever touch what is already in main".

Interesting, I will take a look at handling multiple args.

Harald

Adds git history squash <revision-range> to fold a range of commits.

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 (4):
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
history: re-edit a squash with every message

 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  29 ++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 357 +++++++++++++++++---
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 550 +++++++++++++++++++++++++++++++
 7 files changed, 905 insertions(+), 38 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

base-commit: 6c3d7b73556db708feb3b16232fab1efc4353428
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v6
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v5:

 1:  0f1ae9b05a = 1:  fea6b79e60 history: extract helper for a commit's parent tree
 2:  a97ffab1e6 = 2:  e2674e0bc4 history: give commit_tree_ext a message template
 3:  04e18ef979 ! 3:  811e393ab4 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, so the
     -    range collapses into a single commit. Commits above the range are
     -    replayed on top of the result.
     +    message and authorship and taking the tree of the newest commit, then
     +    replays the commits above the range on top. fixup!, squash! and amend!
     +    commits are folded like any other and are not interpreted, so the
     +    squashed message comes from the oldest commit, or from an editor with
     +    --reedit-message.
      
     -    The range is given as <base>..<tip>, so "git history squash @~3.."
     -    folds the three most recent commits and "git history squash @~5..@~2"
     -    squashes an interior range. A merge inside the range is folded like any
     -    other commit, but the range must have a single base, so a range with
     -    more than one entry point is rejected.
     -
     -    The folded commits leave the history, so by default the command refuses
     -    when another ref points at one of them. Use "--update-refs=head" to
     -    rewrite only the current branch and leave those refs untouched.
     +    The range is read like the arguments to "git rev-list", so several
     +    arguments such as "@~3.. ^topic" are allowed. 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 <[email protected]>
          Signed-off-by: Harald Nordgren <[email protected]>
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +the commit just below the oldest commit to squash. For example, `git
      +history squash @~3..` folds the three most recent commits into one, and
      +`git history squash @~5..@~2` squashes an interior range while leaving
     -+the two newest commits in place.
     ++the two newest commits in place. _<revision-range>_ is read like the
     ++arguments to linkgit:git-rev-list[1], so several arguments may be given,
     ++for example `@~3.. ^topic` to additionally exclude what is already on
     ++`topic`.
      ++
      +The oldest commit's message and authorship are preserved by default,
      +unless you specify `--reedit-message`. A merge commit inside the range is
     @@ builtin/history.c: out:
      + * 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 *range,
     ++				const char **argv,
      +				struct commit **base_out,
      +				struct commit **oldest_out,
      +				struct commit **tip_out,
     @@ builtin/history.c: 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;
     @@ builtin/history.c: out:
      +	strvec_push(&args, "--topo-order");
      +	strvec_push(&args, "--boundary");
      +	strvec_push(&args, "--ancestry-path");
     -+	strvec_push(&args, range);
     ++	strvec_pushv(&args, argv);
      +	setup_revisions_from_strvec(&args, &revs, NULL);
      +	if (args.nr != 1) {
     -+		ret = error(_("'%s' does not name a revision range"), range);
     ++		ret = error(_("unrecognized argument: %s"), args.v[1]);
      +		goto out;
      +	}
      +
      +	/*
     -+	 * A squash needs a base to reparent onto, so the argument has to
     -+	 * exclude something, as in "<base>..<tip>". A single revision has no
     -+	 * such bottom commit and cannot be squashed.
     ++	 * 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(_("'%s' is not a '<base>..<tip>' range"), range);
     ++		ret = error(_("not a '<base>..<tip>' revision range"));
      +		goto out;
      +	}
      +
     @@ builtin/history.c: out:
      +
      +	while ((commit = get_revision(&revs))) {
      +		if (commit->object.flags & BOUNDARY) {
     -+			if (base) {
     -+				ret = error(_("range '%s' has more than one base; "
     -+					      "cannot squash"), range);
     -+				goto out;
     -+			}
     -+			base = commit;
     ++			commit_list_insert(commit, &boundaries);
      +			continue;
      +		}
      +		if (!oldest)
     @@ builtin/history.c: out:
      +	}
      +
      +	if (!oldest) {
     -+		ret = error(_("the range '%s' is empty"), range);
     ++		ret = error(_("the revision range is empty"));
     ++		goto out;
     ++	}
     ++
     ++	if (oldest == tip) {
     ++		ret = error(_("the revision range holds a single commit; "
     ++			      "nothing to squash"));
      +		goto out;
      +	}
      +
     -+	if (!base)
     -+		BUG("a non-empty range must have a boundary commit");
     ++	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;
     @@ builtin/history.c: out:
      +	ret = 0;
      +
      +out:
     ++	commit_list_free(boundaries);
      +	reset_revision_walk();
      +	release_revisions(&revs);
      +	strvec_clear(&args);
     @@ builtin/history.c: out:
      +	int ret;
      +
      +	argc = parse_options(argc, argv, prefix, options, usage, 0);
     -+	if (argc != 1) {
     -+		ret = error(_("command expects a single revision range"));
     ++	if (!argc) {
     ++		ret = error(_("command expects a revision range"));
      +		goto out;
      +	}
      +	repo_config(repo, git_default_config, NULL);
     @@ builtin/history.c: out:
      +	if (action == REF_ACTION_DEFAULT)
      +		action = REF_ACTION_BRANCHES;
      +
     -+	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip,
     ++	ret = resolve_squash_range(repo, argv, &base, &oldest, &tip,
      +				   &interior);
      +	if (ret < 0)
      +		goto out;
     @@ t/t3455-history-squash.sh (new)
      +
      +test_expect_success 'errors on missing range argument' '
      +	test_must_fail git history squash 2>err &&
     -+	test_grep "command expects a single revision range" err
     -+'
     -+
     -+test_expect_success 'errors on too many arguments' '
     -+	test_must_fail git history squash start.. HEAD 2>err &&
     -+	test_grep "command expects a single revision range" 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 range .* is empty" 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 "is not a .*range" err &&
     ++	test_grep "not a .*range" err &&
      +	test_must_fail git history squash HEAD~1 2>err &&
     -+	test_grep "is not a .*range" 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 &&
     ++
     ++	git log --format="%s" start..HEAD >actual &&
     ++	cat >expect <<-\EOF &&
     ++	two
     ++	one
     ++	EOF
     ++	test_cmp expect actual &&
     ++	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 &&
     ++
     ++	git rev-list --count start..off-history >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	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' '
     @@ t/t3455-history-squash.sh (new)
      +	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
      +'
      +
     -+test_expect_success 'squashing a single-commit range replays the rest' '
     -+	git reset --hard three &&
     -+	tip_tree=$(git rev-parse HEAD^{tree}) &&
     -+
     -+	git history squash start..@~2 &&
     -+
     -+	git log --format="%s" start..HEAD >actual &&
     -+	cat >expect <<-\EOF &&
     -+	three
     -+	two
     -+	one
     -+	EOF
     -+	test_cmp expect actual &&
     -+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
     -+'
      +
      +test_expect_success 'reuses the message of a fixup! commit in the range' '
      +	git reset --hard start &&
     @@ t/t3455-history-squash.sh (new)
      +
      +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 - &&
     ++	git checkout "$main" &&
      +	test_commit --no-tag after-side file c &&
      +	git merge --no-ff -m merge inner-side &&
      +	git branch -D inner-side &&
     @@ t/t3455-history-squash.sh (new)
      +
      +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 - &&
     ++	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 &&
     @@ t/t3455-history-squash.sh (new)
      +	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 - &&
     ++	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 &&
     @@ t/t3455-history-squash.sh (new)
      +
      +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 - &&
     ++	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 &&
     @@ t/t3455-history-squash.sh (new)
      +
      +test_expect_success 'refuses to squash a range with more than one base' '
      +	git reset --hard start &&
     -+	head_before=$(git rev-parse HEAD) &&
     ++	main=$(git symbolic-ref --short HEAD) &&
      +	git checkout -b forked-before &&
      +	test_commit forked-side fside x &&
     -+	git checkout - &&
     -+	test_commit forked-main file b &&
     ++	git checkout "$main" &&
     ++	test_commit forked-base file b &&
     ++	base=$(git rev-parse HEAD) &&
     ++	test_commit forked-main file c &&
      +	git merge --no-ff -m merge forked-before &&
      +	merged=$(git rev-parse HEAD) &&
      +
     -+	test_must_fail git history squash forked-main.. 2>err &&
     ++	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 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 - &&
     ++	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 - &&
     ++	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}) &&
     @@ t/t3455-history-squash.sh (new)
      +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-base file b &&
     ++	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 c &&
     ++	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) &&

4: a758e1f084 ! 4: 4edf012b77 history: re-edit a squash with every message
@@ Commit message

Gather the messages of every commit in the range, oldest first, and use them as the editor template when re-editing, mirroring how "git rebase
- -i" presents a squash. The combined message is built before the
- descendant walk so it is not disturbed by the flags that walk leaves on
- the commits.
+ -i" presents a squash.

## Documentation/git-history.adoc ##
-@@ Documentation/git-history.adoc: history squash @~3..` folds the three most recent commits into one, and
- git history squash @~5..@~2 squashes an interior range while leaving
- the two newest commits in place.
+@@ Documentation/git-history.adoc: arguments to linkgit:git-rev-list[1], so several arguments may be given,
+ for example @~3.. ^topic to additionally exclude what is already on
+ topic.
+
-The oldest commit's message and authorship are preserved by default,
-unless you specify --reedit-message. A merge commit inside the range is

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index 091465a59e..f95f26e684 100644
@@ -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

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index f95f26e684..305bde3102 100644
@@ -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

From: Harald Nordgren <[email protected]>

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 message and authorship and taking the tree of the newest commit, then replays the commits above the range on top. fixup!, squash! and amend! commits are folded like any other and are not interpreted, so the squashed message comes from the oldest commit, or from an editor with --reedit-message.

The range is read like the arguments to "git rev-list", so several arguments such as "@~3.. ^topic" are allowed. 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.

---
 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  28 ++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 224 ++++++++++++++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 513 +++++++++++++++++++++++++++++++
 7 files changed, 772 insertions(+)
 create mode 100755 t/t3455-history-squash.sh

index 257db58918..f4d692d136 100644
@@ -55,6 +55,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.

index 2ba8121795..123ad5d4bc 100644
@@ -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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
 
 DESCRIPTION
 -----------

@@ -97,6 +98,33 @@ 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 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.
++
+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 @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place. _<revision-range>_ is read like the
+arguments to linkgit:git-rev-list[1], so several arguments may be given,
+for example `@~3.. ^topic` to additionally exclude what is already on
+`topic`.
++
+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 folded commits disappear from the history, so with the default
+`--update-refs=branches` the command refuses when another ref points at
+one of them. Rerun with `--update-refs=head` to rewrite only the current
+branch and leave those refs pointing at the old commits.
+
 OPTIONS
 -------
 

index 0018501b7b..5c6ff95e31 100644
@@ -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" },

index 8def280688..911b4e4643 100644
@@ -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,

index 3219264fe7..63ea26b8ed 100644
@@ -399,6 +399,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',

gitgitgadget

From: Harald Nordgren <[email protected]>

By default "git history squash" reuses the oldest commit's message. When --reedit-message is given it only reopened that one message, so the messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and use them as the editor template when re-editing, mirroring how "git rebase -i" presents a squash.

---
 Documentation/git-history.adoc |  5 +--
 builtin/history.c              | 61 +++++++++++++++++++++++++++++++++-
 t/t3455-history-squash.sh      | 37 +++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

index 123ad5d4bc..8d4398ab1b 100644
@@ -114,8 +114,9 @@ arguments to linkgit:git-rev-list[1], so several arguments may be given,
 for example `@~3.. ^topic` to additionally exclude what is already on
 `topic`.
 +
-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.

index 94ee54eb24..5ef6768826 100755
@@ -164,6 +164,43 @@ 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 &&
+	echo b >file &&
+	git add file &&
+	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 &&
+	cp "$1" buffer &&
+	echo combined >"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	git history squash --reedit-message start.. &&
+
+	test_grep "re-one subject" buffer &&
+	test_grep "re-one body line" buffer &&
+	test_grep re-two buffer &&
+	test_grep re-three buffer &&
+	git log --format="%s" -1 >actual &&
+	echo combined >expect &&
+	test_cmp expect actual
+'
+
+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 '--dry-run predicts the rewrite without performing it' '
 	git reset --hard three &&
 	head_before=$(git rev-parse HEAD) &&

gitgitgadget

I doubt it would make practical difference, but one thing I notice is that unlike "git rebase -i", this one does not intersperse markers like "# This is the 1st commit message" in between the messages taken from the squashed commits, so it is not exactly "mirroring".

Good things to document/discuss.

Yeah, I agree that we should punt on merge commits for now. They are a can of worms, and I'm not sure that we should just squash them. I would at least like the user to ask a flag that tells us that it's fine squashing those.

Yeah, this should be documented indeed.

Note that it is intentional that you can rewrite branches that are not currently checked out, and the other subcommands work the same. So I'd argue this should also be the case for "squash".

Patrick

I wouldn't mind extracting that logic from 'rebase -i' to show it here. It would be nice to have.

Harald

If we can share more code (not necessarily the exact existing code---after cleaning it up if needed is perfectly fine and may even be better) across codebaes that would be excellent. Thanks.

Hi Harald and Patrick

Thanks, I was disappointed that these questions were not addressed in the cover letter for v6 which contains no more detail than this one.

We should make rewording the commit as easy as possible to encourage users to create useful commit histories. I think that means having some support for fixup! style commits. We could just do what rebase does and comment out the fixup! style commit subjects and the commit messages replaced by amend! commits but I think we have the opportunity to do something nicer (I find the commented-out messages annoying). We could have a comment saying "this is the combination of the following commits" followed by a list of the subject lines and then in the template message we'd simply omit the useless fixup! subjects when the commit body is empty and also omit the original commit messages that have been replaced by an amend! commit.

So instead of

# This is the combination of 4 commits
# This is the first commit message
Base subject

Base body

# This is the second commit message
# Another subject

# Another body

# This is the third commit message
# fixup! Base subject

# This is the fourth commit message
# amend! Another subject
A better subject

A better body

We'd have

# This is the combination of 4 commits
# 123 Base subject
# 456 Another subject
# 789 fixup! Base subject
# abc amend! Another subject

Base Subject

Base Body

Another subject

Another Body

Possibly with a comment before each message saying where it came from.

It would be good to error out if the user tries squash a fixup! style commit and range does not contain its target commit.

In the long run we should provide a way to squash an arbitrary list of commits rather than just a range.

There does seem to be some support for merges in this patch series which I think behaves pretty sensibly. If we have

C - D
/ \
- A - B - E - F - G

Then squashing A..G should be fine because the parents of F are in the range and it looks like we support that. Squashing should B..G without --ancestry-path should be safe as well because B ends up as the parent of the squashed commit but we don't have a way to disable --ancestry-path (and maybe we don't want to add one). Squashing F^@..G might be useful to fixup a merge (though perhaps amending F rather than creating G is a simpler way to fix a broken merge). Squashing E..G does not make sense because the range does not include one of the merge parents.

We should sanitize what the user passes though - we do not want to accept arbitrary rev-list options. Off the top of my head "--left-only" and "--right-only" would allow the use of "A...B" and allowing "--not" seems reasonable.

Ah, thanks for clarifying that - it does not seem to check that there is a branch to be rewritten though if I do

./git checkout origin/master
./git history squash --dry-run origin/seen^2^^..origin/seen^2

there is no error message and it exits 0. If I create a branch pointing at origin/seen^2 then it does print a sensible ref-update.

Thanks

Phillip

Do you mean not even handling simple merges that are fully within the range? I'm already handling those, so remove?

Harald

That's interesting. It at least deserves a warning, even though the choice to leave them out, with intentions to make it a separate fix on top, may be deliberate. If such a change-of-mind-in-the-middle is rare, then erroring out would be perfect. The user can first reword the !fixup to make it a separate fix and then squash the range in quesiton.

I would not recommend guessing what these rev-list "expressions" would produce and blacklist some of the operations and notations. It would be a more robust approach to let the machinery do its thing to determine the set of commits, *and* inspect the shape of the history these commits represent. Are they connected? Do they have a single "bottom" that is just outside and below the range so that we can replace it with the result of squashing everything together? Do they have a single "top" whose children can be rewritten to have the resulting single commit as one of their parents? Starting from the acceptable shape of the history we want to deal with, rather than trying to enumerate rev-list operations and notations that would prevent the resulting set of commits to fall outside the acceptable shape of the history (and I am reasonably sure anybody who attempts to do so would either end up with unusablly narrow subset of what we can reasonably handle, or miss some cases that we do not want to handle), would be a better approach.

After looking at what Phillip said in a side thread (look for "So instead of ... We'd have") [*], I retract my "I doubt it would make practical difference". Without boundary that shows where each message begins, the result is much harder to look at.

[Reference]

* https://lore.kernel.org/git/[email protected]/

I think this makes it a lot harder to read. If every commit body was always just a single paragraph it could make sense, but it's generally not. Look at the commits in this series, with no delimiter of where one commit message ends and the next one starts, it would be very confusing.

Good point, I don't see a good reason to allow this.

Thanks for a good explanation here!

Harald

You've trimmed the line where I said

So I'm not against adding a comment before each message, but I do think we should omit any messages that would be commented out completely. If you look at the rebase example above you can see there is a mass of comments between the two pieces of text that make up the new message. That makes it hard to see what is actually going to be included in the new message.

Thanks

Phillip

Sorry about that! Can you show me the example of what it would look like? It's much easier for me to reason about it then.

Harald

Hi!

I haven't experimented much with the new 'git history' commands, but this discussion caught my eye and wanted to chime in. (aka: please forgive my stupid questions)

Note: I have built and am testing with v6 of this topic.

I agree with this idea as well. And letting fixup! messages completely fall out makes more sense here than I want to say in git rebase. The commented list of commits at the top is a nice compromise for the todo list you would have seen had you run 'git rebase -i' instead, and a glance at the list would confirm that the fixup!s you thought you included _actually are_ in the squash range.

I assume the same treatment would _not_ be completely given to squash! messages, since there is at least some expectation that they carry an additional remark that the author might want to reword into the base commit?

Given the above, and how 'git rebase' usually works, I'm suprised that --reedit-message isn't the default behavior. This may be my bias towards rebase showing... Perhaps the typical use of this command is to just work fast, and expect a follow up 'git history reword' if needed - when the original just needs a little extra context?

This is probably a larger question, since (according to the man page) it affects the other 'git history' commands as well. When I run 'git history ...' and discover that I made a mistake after inspecting the results, is there a fool-proof way to undo the change and return to the previous state? My first thought was to run 'git reset --hard ...', but the default behavior of --update-refs (moving other branches) can make this more complicated.

Thanks

This is a larger question: But I would love to have a reflog that is more human-centered. When e.g. rebasing a series with N commits, it's very tricky in the reflog to find what was the state before that.

I feel like branch switching is given too much space in the reflog, since it's not a destructive action, I don't care about it.

And when handling multiple commits in on go (squashing, rebasing), I would love to see a visual hierarchy (with indentation for sub-steps) instead of treating each action as equally important when it isn't.

Harald

I share these headaches to an extent. When dealing with the first problem (seeing an atomic entry in the reflog), I usually look at the branch's own reflog instead of HEAD's

But my question is about doing a comprehensive reset from a botched operation. If any history operation updates branch refs besides the current one. I don't think there's an obvious way to see which other ones were affected, and a naive 'git reset --hard my-branch@{1}' leaves them pointed at unwanted commits. Is this right?

tangent: I'm pretty sure that git-status relies on checkout / branch switching reflog entries in order to know which tag your detached HEAD started from, eg: when it says something like

HEAD detached from v2.55.0-rc2
nothing to commit, working tree clean

Sounds compelling!

For the commits

123 Commit one
456 Commit two
789 fixup! Commit one
abc squash! Commit two
def amend! Commit one

It would look something like

# This is the combination of the following commits:
# 123 Commit one
# 789 fixup! Commit one
# def amend! Commit one
# 456 Commit two
# abc squash! Commit two

A better commit one subject

A better commit one body

# ------------------------------------

Commit two

Commit two body

# squash! Commit two

More text for commit two

I've rearranged the commits in the summary to keep fixup commits with their targets instead of listing them in the same order of "git log --reverse". As we're dropping uninteresting messages I not sure there is much point in numbering them[1] so I've just used a plain separator to show where a message ends. For squash! commits commenting out the subject shows where the original message ends and the message from the squash! commit begins. I'm on the fence as to whether it is worth having

# amend! Commit one

above the first message

Thanks

Phillip

[1] If we really want to number things we could number each commit in the summary and then use that to label it in the message template with separators like

# This is the message from commit <n>

instead of separating them with

# ----------------------------

Hi Matt

Thanks for testing it, and also for your questions - it is always helpful to get the perspective of someone using the command, especially while we're still designing it.

Yes for "squash! Some commit" I think we'd want to have

Some commit

Some commit body

# squash! Some commit

The body of the squash! commit

That's a good point - if we want to encourage good commit histories then opening the editor by default gives the user an opportunity to double check which commits are being squashed and edit the message as appropriate.

"--reedit-message" is a bit of a mouthful - maybe we should call it "--edit" (or "--no-edit" if we want to open the editor by default) like "git commit"

Yes this is a problem to which we don't have a good solution at the moment. I believe Jujitsu and git-branchless both have some kind of operations log that lets you revert a whole operation rather than just a single ref-update. We'd need some way to tie all the ref updates from a single ref transaction together either by logging the separately or adding some form of transaction id to the reflog. That would be a big change.

Thanks

Phillip

I want to avoid creating drift between this and the format of 'git rebase -i', so if we want to change this, maybe better to change both at a later point instead?

Harald

Hi Harald

The reason we're introducing the history command is to experiment with providing a better user interface for rewriting history without being bound by the limitations of "git rebase". So I think it would entirely appropriate to try a different format for the squash message here. If it turns out to be a success then we can see if we want to use it in "git rebase" as well.

Thanks

Phillip

Hi Junio

I think we still want some sanity checks similar to "git replay" though to ensure the user has not overridden "--reverse", "--topo-order", and "--boundary". It will be easier to sanity check the list of commits if we can at least rely on those options being set as we know what order to expect them in and can detect merge parents that are outside the range by looking for BOUNDARY commits.

Thanks

Phillip

Do we know concretely things that are bad in the current way "rebase -i" works, so that we can experiment deviation from? If not, the above is going backwards, I'm afraind.

Thanks

Yes - if you apply the way "rebase -i" works to multiple commits you can end up with a message template that has a screen full of commented lines between uncommitted parts of the message. See the example below from earlier in the thread. It is not so much of a problem in "rebase -i" because it only fixes up a single commit at a time so all the commented messages end up at the top of the buffer and at worst you have a few "# fixup! ..." or "# squash! ..." lines mixed in with the uncommitted text.

# This is the combination of 4 commits
# This is the first commit message
Base subject

Base body

# This is the second commit message
# Another subject

# Another body

# This is the third commit message
# fixup! Base subject

# This is the fourth commit message
# amend! Another subject
A better subject

A better body

Thanks

Phillip

In the example, the second one becomes completely empty?

Is the proposal not to show any messages that will be discarded anyway and not even show them in commented form? I think that makes sense, and leaving only commit titles for these commits that would not contribute to the text in the editor given to the user to edit would indeed be an improvement. For the same reason, as "# amend!" will replace the message wholesale, it would also be a good idea for the first commit to be hidden like all the other commits that would not contribute to the text, so an improved version of the above may be:

# This is the combination fo 4 commits
# 1. Base subject
# 2. Another subject
# 3. fixup! Base subject
# 4. amend! Another subject
A better subject

A better body.

or something? Is that the direction you want us to take?

Thanks.

[?nip]

Yeah, agreed. I think that the reflog is insufficient for a lot of Git's operations and that it is way too hard to reason about. It's both too detailed and not detailed enough at the same time:

- It provides way too much detail about individual reference updates when all the user cares about is the high-level operation on the logical level.

- It does not provide enough detail to give information about what the high-level operation even was.

I don't think that we can fix the reflog to work properly in this case. But I certainly think that we should explore whether we can eventually introduce something like an oplog, as well, so that we can easily have the equivalent of jj undo.

It's something that I'd eventually want to get to, but it'll take a while. So if anybody else beats me to it then please go ahead :)

Patrick

Yes because there is an amend! commit that replaces its message

But the amend! commit does not target the first commit - it replaces the message of the second commit so I think we should keep the first message as shown below. You can see a different example with a squash! message at [1]

       Base subject

       Base body

       # -------------------------------------

Thanks

Phillip

[1] https://lore.kernel.org/git/[email protected]

As the reflog has never been about undoing, this is understandable.

Things like @{-N} notation and "git push --force-if-includes" make good use of the "what was this ref pointing at historically?" information, so they give us a proof that it is possible to programmatically go back and find the necessary state, though.

Adds git history squash <revision-range> to fold a range of commits.

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: extract helpers for the squash message markers
history: re-edit a squash with every message

 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  49 ++-
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 390 +++++++++++++++++--
 sequencer.c                      |  64 ++--
 sequencer.h                      |  23 ++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 632 +++++++++++++++++++++++++++++++
 9 files changed, 1099 insertions(+), 66 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v7
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v6:

 1:  fea6b79e60 = 1:  56ed8fadbb history: extract helper for a commit's parent tree
 2:  e2674e0bc4 = 2:  212e9c228f history: give commit_tree_ext a message template
 3:  811e393ab4 ! 3:  cf3346a1cd 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. fixup!, squash! and amend!
     -    commits are folded like any other and are not interpreted, so the
     -    squashed message comes from the oldest commit, or from an editor with
     -    --reedit-message.
     +    replays the commits above the range on top. The squashed message comes
     +    from the oldest commit, or from an editor with --reedit-message. As that
     +    message is reused, a range whose oldest commit is a fixup!, squash! or
     +    amend! is refused, since the marker's target cannot be in the range.
      
          The range is read like the arguments to "git rev-list", so several
     -    arguments such as "@~3.. ^topic" are allowed. 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.
     +    arguments such as "HEAD~3..HEAD ^topic" are allowed. 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 <[email protected]>
          Signed-off-by: Harald Nordgren <[email protected]>
     @@ Documentation/git-history.adoc: 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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
     ++git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>
       
       DESCRIPTION
       -----------
     +@@ Documentation/git-history.adoc: 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
      @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
       It is invalid to select either all or no hunks, as that would lead to
       one of the commits becoming empty.
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      ++
      +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 @~3..` folds the three most recent commits into one, and
     -+`git history squash @~5..@~2` squashes an interior range while leaving
     -+the two newest commits in place. _<revision-range>_ is read like the
     -+arguments to linkgit:git-rev-list[1], so several arguments may be given,
     -+for example `@~3.. ^topic` to additionally exclude what is already on
     -+`topic`.
     ++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. _<revision-range>_ is read
     ++like the arguments to linkgit:git-rev-list[1], so several arguments may be
     ++given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
     ++already on `topic`.
      ++
      +The oldest commit's message and authorship are preserved by default,
      +unless you specify `--reedit-message`. A merge commit inside the range is
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +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 folded commits disappear from the history, so with the default
     -+`--update-refs=branches` the command refuses when another ref points at
     -+one of them. Rerun with `--update-refs=head` to rewrite only the current
     -+branch and leave those refs pointing at the old commits.
     ++Because the oldest commit's message is reused, the range may not begin
     ++with a `fixup!`, `squash!`, or `amend!` commit, whose target is
     ++necessarily outside the range.
     +++
     ++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
       -------
       
     +@@ Documentation/git-history.adoc: 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
      
       ## advice.c ##
      @@ advice.c: static struct {
     @@ builtin/history.c
       #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 <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
     ++	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)
       {
     @@ builtin/history.c: out:
      +	return ret;
      +}
      +
     ++static int reject_fixupish_oldest(struct repository *repo,
     ++				  struct commit *oldest)
     ++{
     ++	const char *message, *subject;
     ++	int ret = 0;
     ++
     ++	message = repo_logmsg_reencode(repo, oldest, NULL, NULL);
     ++	find_commit_subject(message, &subject);
     ++	if (starts_with(subject, "fixup! ") ||
     ++	    starts_with(subject, "squash! ") ||
     ++	    starts_with(subject, "amend! "))
     ++		ret = error(_("the range begins with a fixup!, squash! or amend! "
     ++			      "commit whose target is not in the range"));
     ++	repo_unuse_commit_buffer(repo, oldest, message);
     ++	return ret;
     ++}
     ++
      +struct interior_ref_cb {
      +	const struct oidset *interior;
      +	const char *name;
     @@ builtin/history.c: out:
      +	if (ret < 0)
      +		goto out;
      +
     ++	ret = reject_fixupish_oldest(repo, oldest);
     ++	if (ret < 0)
     ++		goto out;
     ++
      +	if (action == REF_ACTION_BRANCHES) {
      +		struct interior_ref_cb cb = { .interior = &interior };
      +
     @@ t/t3455-history-squash.sh (new)
      +
      +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)" &&
      +	git rev-list --count start..HEAD >count &&
      +	echo 1 >expect &&
      +	test_cmp expect count &&
     @@ t/t3455-history-squash.sh (new)
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success 'keeps the oldest message even if it is a fixup!' '
     ++test_expect_success 'refuses a range whose oldest commit is a fixup!' '
      +	git reset --hard start &&
      +	test_commit --no-tag "fixup! something" file b &&
     -+	test_commit tail file c &&
     ++	test_commit --no-tag tail 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 'does not interpret squash! or amend! markers' '
     ++	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 &&
      +
      +	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! something" >expect &&
     ++	echo marker-oldest >expect &&
      +	test_cmp expect actual
      +'
      +
     @@ t/t3455-history-squash.sh (new)
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success '--dry-run predicts the rewrite without performing it' '
     -+	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)" &&
     -+	git rev-list --count start..HEAD >count &&
     -+	echo 1 >expect &&
     -+	test_cmp expect count &&
     -+	test_cmp_rev start HEAD^ &&
     -+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
     -+'
     -+
      +test_expect_success '--update-refs=head only moves HEAD' '
      +	git reset --hard three &&
      +	git branch -f other HEAD &&
 -:  ---------- > 4:  001356db93 sequencer: extract helpers for the squash message markers
 4:  4edf012b77 ! 5:  615fe4dd3f history: re-edit a squash with every message
     @@ Commit message
          When --reedit-message is given it only reopened that one message, so the
          messages of the folded-in commits were lost.
      
     -    Gather the messages of every commit in the range, oldest first, and use
     -    them as the editor template when re-editing, mirroring how "git rebase
     -    -i" presents a squash.
     +    Gather the messages of every commit in the range, oldest first, and build
     +    the same editor template that "git rebase -i" shows for a squash, using
     +    add_squash_combination_header(), add_squash_message_header() and
     +    squash_subject_comment_len(). Only the message text differs, the changes
     +    are always folded in. Following autosquash, a fixup!'s message is
     +    commented out in full under a "will be skipped" header, while a squash! or
     +    amend! keeps its body with only the marker subject commented.
      
          Signed-off-by: Harald Nordgren <[email protected]>
      
       ## Documentation/git-history.adoc ##
     -@@ Documentation/git-history.adoc: arguments to linkgit:git-rev-list[1], so several arguments may be given,
     - for example `@~3.. ^topic` to additionally exclude what is already on
     - `topic`.
     +@@ Documentation/git-history.adoc: like the arguments to linkgit:git-rev-list[1], so several arguments may be
     + given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
     + already on `topic`.
       +
      -The oldest commit's message and authorship are preserved by default,
      -unless you specify `--reedit-message`. A merge commit inside the range is
     @@ Documentation/git-history.adoc: arguments to linkgit:git-rev-list[1], so several
       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.
     + +
     + Because the oldest commit's message is reused, the range may not begin
     + with a `fixup!`, `squash!`, or `amend!` commit, whose target is
     +-necessarily outside the range.
     ++necessarily outside the range. 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`: the message of a
     ++`fixup!` elsewhere in the range is commented out in full, while a
     ++`squash!` or `amend!` keeps its message body with only the marker subject
     ++commented, so you can fold the remark into the result.
     + +
     + 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 ##
      @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, void *cb_data)
     @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
      +				struct commit *tip,
      +				struct strbuf *out)
      +{
     ++	struct commit_list *commits = NULL, **tail = &commits, *c;
      +	struct rev_info revs;
      +	struct commit *commit;
      +	struct strvec args = STRVEC_INIT;
     -+	int n = 0, ret;
     ++	int n = 0, total, ret;
      +
      +	repo_init_revisions(repo, &revs, NULL);
      +	strvec_push(&args, "ignored");
     @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
      +		goto out;
      +	}
      +
     -+	while ((commit = get_revision(&revs))) {
     ++	while ((commit = get_revision(&revs)))
     ++		tail = &commit_list_insert(commit, tail)->next;
     ++	total = commit_list_count(commits);
     ++
     ++	for (c = commits; c; c = c->next) {
      +		const char *message, *body;
     -+		struct strbuf one = STRBUF_INIT;
     ++		size_t commented_len;
     ++		int skip;
      +
     -+		message = repo_logmsg_reencode(repo, commit, NULL, NULL);
     ++		message = repo_logmsg_reencode(repo, c->item, NULL, NULL);
      +		find_commit_subject(message, &body);
     -+		strbuf_addstr(&one, body);
     -+		strbuf_trim_trailing_newline(&one);
      +
     -+		if (n++)
     -+			strbuf_addch(out, '\n');
     -+		strbuf_addbuf(out, &one);
     ++		skip = starts_with(body, "fixup! ");
     ++		commented_len = skip ? strlen(body) :
     ++			squash_subject_comment_len(body, 1);
     ++
     ++		if (!n)
     ++			add_squash_combination_header(out, total);
      +		strbuf_addch(out, '\n');
     ++		add_squash_message_header(out, ++n, 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);
      +
     -+		strbuf_release(&one);
     -+		repo_unuse_commit_buffer(repo, commit, message);
     ++		repo_unuse_commit_buffer(repo, c->item, message);
      +	}
      +
      +	ret = 0;
      +
      +out:
     ++	commit_list_free(commits);
      +	reset_revision_walk();
      +	release_revisions(&revs);
      +	strvec_clear(&args);
     @@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	test_commit re-three file d &&
      +
      +	write_script editor <<-\EOF &&
     -+	cp "$1" buffer &&
     ++	cat "$1" >edited &&
      +	echo combined >"$1"
      +	EOF
      +	test_set_editor "$(pwd)/editor" &&
      +	git history squash --reedit-message start.. &&
      +
     -+	test_grep "re-one subject" buffer &&
     -+	test_grep "re-one body line" buffer &&
     -+	test_grep re-two buffer &&
     -+	test_grep re-three buffer &&
     -+	git log --format="%s" -1 >actual &&
     ++	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 &&
      +	echo combined >expect &&
     ++	git log --format="%s" -1 >actual &&
      +	test_cmp expect actual
      +'
      +
     ++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 &&
     ++
     ++	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 &&
     ++	git log -1 --format="%B" >final &&
     ++	test_grep ! "fixup body" final &&
     ++	test_grep "squash remark" final &&
     ++	test_grep "amended message" final
     ++'
     ++
      +test_expect_success '--reedit-message aborts on an empty message' '
      +	git reset --hard three &&
      +	head_before=$(git rev-parse HEAD) &&
     @@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	test_cmp_rev "$head_before" HEAD
      +'
      +
     - test_expect_success '--dry-run predicts the rewrite without performing it' '
     + test_expect_success '--update-refs=head only moves HEAD' '
       	git reset --hard three &&
     - 	head_before=$(git rev-parse HEAD) &&
     + 	git branch -f other HEAD &&

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index 091465a59e..f95f26e684 100644
@@ -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

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

index f95f26e684..305bde3102 100644
@@ -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

From: Harald Nordgren <[email protected]>

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 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. As that message is reused, a range whose oldest commit is a fixup!, squash! or amend! is refused, since the marker's target cannot be in the range.

The range is read like the arguments to "git rev-list", so several arguments such as "HEAD~3..HEAD ^topic" are allowed. 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.

---
 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  43 ++-
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 245 +++++++++++++++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 517 +++++++++++++++++++++++++++++++
 7 files changed, 809 insertions(+), 3 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

index 257db58918..f4d692d136 100644
@@ -55,6 +55,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.

index 0018501b7b..5c6ff95e31 100644
@@ -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" },

index 8def280688..911b4e4643 100644
@@ -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,

index 3219264fe7..63ea26b8ed 100644
@@ -399,6 +399,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',

gitgitgadget

From: Harald Nordgren <[email protected]>

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(). A later change reuses them to give "git history squash --reedit-message" the same template.

No change in behavior.

---
 sequencer.c | 64 ++++++++++++++++++++++++++++++++---------------------
 sequencer.h | 23 +++++++++++++++++++
 2 files changed, 62 insertions(+), 25 deletions(-)

index 57855b0066..f4893e8f40 100644
@@ -1892,6 +1892,32 @@ static const char skip_first_commit_msg_str[] = N_("The 1st commit message will
 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 +2031,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 +2102,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 +2127,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 +2148,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);

index 3164bd437d..feed0e9de3 100644
@@ -208,6 +208,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

From: Harald Nordgren <[email protected]>

By default "git history squash" reuses the oldest commit's message. When --reedit-message is given it only reopened that one message, so the messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and build the same editor template that "git rebase -i" shows for a squash, using add_squash_combination_header(), add_squash_message_header() and squash_subject_comment_len(). Only the message text differs, the changes are always folded in. Following autosquash, a fixup!'s message is commented out in full under a "will be skipped" header, while a squash! or amend! keeps its body with only the marker subject commented.

---
 Documentation/git-history.adoc |  12 +++-
 builtin/history.c              |  73 ++++++++++++++++++++-
 t/t3455-history-squash.sh      | 115 +++++++++++++++++++++++++++++++++
 3 files changed, 196 insertions(+), 4 deletions(-)

index 783ddf4a51..f51332e731 100644
@@ -117,15 +117,21 @@ like the arguments to linkgit:git-rev-list[1], so several arguments may be
 given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
 already on `topic`.
 +
-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.
 +
 Because the oldest commit's message is reused, the range may not begin
 with a `fixup!`, `squash!`, or `amend!` commit, whose target is
-necessarily outside the range.
+necessarily outside the range. 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`: the message of a
+`fixup!` elsewhere in the range is commented out in full, while a
+`squash!` or `amend!` keeps its message body with only the marker subject
+commented, so you can fold the remark into the result.
 +
 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

gitgitgadget

Hi Harald

There was some discussion [1] about making that the default and renaming it - was that overlooked? If not it would be helpful to comment on those discussions to explain why you don't think it is a good idea.

Rebase re-orders commits so that fixups immediately follow their target - do you do that here? I think that is very relevant because here we may be dealing with several different commits each being targeted by a set of fixups and presenting them mixed together will be confusing. When rebase sees an "amend!" commit it comments out the message that is being replaced - it is not clear from this description whether that happens here.

As I've said before I think we would be better off with a summary of the commits that are being squashed and a more compact template message that only contains the messages we want to keep [2]. What are the advantages of having lots of commented lines (or redundant messages if you don't comment out the original when there is an amend! commit) in the middle of the template message?

I think it should allow squashing a bunch of fixups together though. I thought there was a plan [3] to refuse to squash a fixup unless the range included its target.

The range-diff does not show any input sanitization - what happens when the user passes "--reverse" for example? As I said in [4] we should copy what "git replay" does to sanity check the rev-list options, otherwise we've got no idea whether the parent of the first commit returned by get_revision() is the commit we want to use as the parent of the squashed commit.

Thanks

Phillip

[1] https://lore.kernel.org/git/[email protected]/
[2] https://lore.kernel.org/git/[email protected]
[3] https://lore.kernel.org/git/CAHwyqnWQmObWr3N81_EU6F13iyKp3FfY8KSNFfoAjS4r_0qJrQ@mail.gmail.com/
[4] https://lore.kernel.org/git/[email protected]/

What I saw in the range-diff looked all reasonable.

Nice. I often see myself getting rescued by the corresponding sanity checks in the sequencer.

Will replace. Thanks.

Not overlooked, but I side-stepped it because the discussion died down, and yes I don't agree that it needs to be the default. I could have mentioned my thinking in the cover letter.

No, I'm not doing that now, but I can take a look at that.

I attempted this with reject_fixupish_oldest(), assuming only the first commit needs to be checked as not being a fixup/squash/amend.

But now I realize that maybe we need to check all of the commits, and also check if the target is in the range or not. It just makes the logic a lot bigger.

Yeah, good point.

Harald

Well, the code already blocks "--reverse" and other unknown options, but I can clarify that better in the commit message.

Harald

Hi Harald

Well it accepts

git history squash -- --reverse ...

because after calling parse_options() everything after the "--" is passed to setup_revisions(). There was some discussion about accepting rev-list options [2] so it would have been helpful to reference that in the cover letter. The cover letter should explain both the changes you have made and the suggestions that were discussed that have not been implemented so readers can get an overview of how this version relates to the previous discussion. Without that it is impossible to know if you disagree with a suggestion or have just forgotten it.

"git replay" supports arbitrary rev-list options by passing

PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT

to parse_options(), then passing the remaining options to setup_revisions(). After that it checks the various members of struct rev_info that it cares about are still set appropriately.

Thanks

Phillip

[1] https://lore.kernel.org/git/[email protected]

Hi Harald

That's great, it is fine to punt things like this which require quite a bit of work to implement to a later re-roll but please be clear in the cover letter so reviewers know what to expect.

Yes it is a bit more involved. If the first commit is a fixup! then we should allow the user to squash other fixups with the same target and take the message from the last "amend!" commit if we see one. If there are other commits it the range then we should refuse to squash as you do here.

If the first target is not a fixup then we should refuse fixup commits whose target we have not seen. As well as exact subject matches "git rebase" accepts prefix matches and "fixup! $objectid". I think it is fine to skip the prefix matches to start with here. The $objectid matches shouldn't be too much extra work and I think they are worth supporting because if I remember correctly git-gui creates them. Another gotcha is that fixuping up a fixup prepends a "fixup!" to the subject line so you need to be able to handle things like

fixup! fixup! the real target
fixup! amend! the real target
squash! fixup! the real target

etc. Hopefully looking at the code that handles fixups in the sequencer will help

Thanks

Phillip

Adds git history squash <revision-range> to fold a range of commits.

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   |  54 ++-
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 522 +++++++++++++++++++--
 sequencer.c                      |  70 +--
 sequencer.h                      |  30 ++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 776 +++++++++++++++++++++++++++++++
 9 files changed, 1387 insertions(+), 72 deletions(-)
 create mode 100755 t/t3455-history-squash.sh

base-commit: f85a7e662054a7b0d9070e432508831afa214b47
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v8
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v7:

 1:  56ed8fadbb = 1:  ba77752282 history: extract helper for a commit's parent tree
 2:  212e9c228f = 2:  50f3572887 history: give commit_tree_ext a message template
 3:  cf3346a1cd ! 3:  2d81a40a05 history: add squash subcommand to fold a range
     @@ Commit message
          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. As that
     -    message is reused, a range whose oldest commit is a fixup!, squash! or
     -    amend! is refused, since the marker's target cannot be in the range.
     +    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.
      
          The range is read like the arguments to "git rev-list", so several
     -    arguments such as "HEAD~3..HEAD ^topic" are allowed. 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.
     +    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 <[email protected]>
          Signed-off-by: Harald Nordgren <[email protected]>
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +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. _<revision-range>_ is read
     -+like the arguments to linkgit:git-rev-list[1], so several arguments may be
     ++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`.
     ++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
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +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.
      ++
     -+Because the oldest commit's message is reused, the range may not begin
     -+with a `fixup!`, `squash!`, or `amend!` commit, whose target is
     -+necessarily outside the range.
     ++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.
      ++
      +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:
      +	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, "--reverse");
     -+	strvec_push(&args, "--topo-order");
     -+	strvec_push(&args, "--boundary");
      +	strvec_push(&args, "--ancestry-path");
      +	strvec_pushv(&args, argv);
      +	setup_revisions_from_strvec(&args, &revs, NULL);
     @@ builtin/history.c: out:
      +		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
     @@ builtin/history.c: out:
      +		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);
     @@ builtin/history.c: out:
      +	if (!oldest) {
      +		ret = error(_("the revision range is empty"));
      +		goto out;
     -+	}
     -+
     -+	if (oldest == tip) {
     ++	} else if (oldest == tip) {
      +		ret = error(_("the revision range holds a single commit; "
      +			      "nothing to squash"));
      +		goto out;
     -+	}
     -+
     -+	if (!oldest->parents)
     ++	} else if (!oldest->parents) {
      +		BUG("an in-range commit must have a parent");
     ++	}
      +	base = oldest->parents->item;
      +
      +	/*
     @@ builtin/history.c: out:
      +	return ret;
      +}
      +
     -+static int reject_fixupish_oldest(struct repository *repo,
     -+				  struct commit *oldest)
     ++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)
      +{
     -+	const char *message, *subject;
     -+	int ret = 0;
     -+
     -+	message = repo_logmsg_reencode(repo, oldest, NULL, NULL);
     -+	find_commit_subject(message, &subject);
     -+	if (starts_with(subject, "fixup! ") ||
     -+	    starts_with(subject, "squash! ") ||
     -+	    starts_with(subject, "amend! "))
     -+		ret = error(_("the range begins with a fixup!, squash! or amend! "
     -+			      "commit whose target is not in the range"));
     -+	repo_unuse_commit_buffer(repo, oldest, message);
     ++	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;
     ++	int i, ret, nr_dangling = 0;
     ++
     ++	*msg_source = oldest;
     ++
     ++	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;
     ++
     ++		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);
     ++		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;
      +}
      +
     @@ builtin/history.c: out:
      +	};
      +	struct strbuf reflog_msg = STRBUF_INIT;
      +	struct oidset interior = OIDSET_INIT;
     -+	struct commit *base, *oldest, *tip, *rewritten;
     ++	struct commit *base, *oldest, *tip, *rewritten, *msg_source;
      +	const struct object_id *base_tree_oid, *tip_tree_oid;
      +	struct commit_list *parents = NULL;
      +	struct rev_info revs = { 0 };
      +	int ret;
      +
     -+	argc = parse_options(argc, argv, prefix, options, usage, 0);
     ++	argc = parse_options(argc, argv, prefix, options, usage,
     ++			     PARSE_OPT_KEEP_UNKNOWN_OPT);
      +	if (!argc) {
      +		ret = error(_("command expects a revision range"));
      +		goto out;
     @@ builtin/history.c: out:
      +	if (ret < 0)
      +		goto out;
      +
     -+	ret = reject_fixupish_oldest(repo, oldest);
     ++	ret = reject_dangling_fixups(repo, base, tip, oldest, &msg_source);
      +	if (ret < 0)
      +		goto out;
      +
     @@ 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", oldest, NULL, parents,
     ++	ret = commit_tree_ext(repo, "squash", msg_source, NULL, parents,
      +			      base_tree_oid, tip_tree_oid, &rewritten, flags);
      +	if (ret < 0) {
      +		ret = error(_("failed writing squashed commit"));
     @@ t/t3455-history-squash.sh (new)
      +	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}) &&
     @@ t/t3455-history-squash.sh (new)
      +'
      +
      +
     -+test_expect_success 'reuses the message of a fixup! commit in the range' '
     ++test_expect_success 'folds fixups whose target is in the range' '
      +	git reset --hard start &&
     -+	test_commit --no-tag reg1 file b &&
     -+	git commit --allow-empty -m "fixup! reg1" &&
     -+	test_commit reg2 file c &&
     ++	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.. &&
      +
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
      +	git log --format="%s" -1 >actual &&
     -+	echo reg1 >expect &&
     ++	echo target >expect &&
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success 'refuses a range whose oldest commit is a fixup!' '
     ++test_expect_success 'refuses a below-range fixup! after an in-range commit' '
      +	git reset --hard start &&
     -+	test_commit --no-tag "fixup! something" file b &&
     -+	test_commit --no-tag tail file c &&
     ++	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 &&
     @@ t/t3455-history-squash.sh (new)
      +	test_cmp_rev "$head_before" HEAD
      +'
      +
     -+test_expect_success 'does not interpret squash! or amend! markers' '
     ++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" &&
     ++
     ++	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
     ++'
     ++
     ++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 &&
     ++
     ++	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
     ++'
     ++
     ++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" &&
     ++	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 'keeps the oldest message for in-range squash! and amend!' '
      +	git reset --hard start &&
      +	test_commit --no-tag marker-oldest file b &&
      +	git commit --allow-empty -m "squash! marker-oldest" &&
     @@ t/t3455-history-squash.sh (new)
      +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
      +'
     @@ t/t3455-history-squash.sh (new)
      +	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
      +'
      +
     -+test_expect_success 'refuses to squash a range with more than one base' '
     -+	git reset --hard start &&
     -+	main=$(git symbolic-ref --short HEAD) &&
     -+	git checkout -b forked-before &&
     -+	test_commit forked-side fside x &&
     -+	git checkout "$main" &&
     -+	test_commit forked-base file b &&
     -+	base=$(git rev-parse HEAD) &&
     -+	test_commit forked-main file c &&
     -+	git merge --no-ff -m merge forked-before &&
     -+	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 with two interior merges' '
      +	git reset --hard start &&
      +	main=$(git symbolic-ref --short HEAD) &&

4: 001356db93 ! 4: 0a735117ad sequencer: extract helpers for the squash message markers
@@ Metadata
Author: Harald Nordgren <[email protected]>

## Commit message ##
- sequencer: extract helpers for the squash message markers
+ sequencer: share the squash message marker helpers and flags

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
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(). A later change
- reuses them to give "git history squash --reedit-message" the same
- template.
+ 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.

## sequencer.c ##
-@@ sequencer.c: static const char skip_first_commit_msg_str[] = N_("The 1st commit message will +@@ sequencer.c: 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.");

@@ sequencer.c: static int update_squash_messages(struct repository *r,
comment_line_str);

## sequencer.h ##
+@@ sequencer.h: 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;
@@ sequencer.h: int todo_list_rearrange_squash(struct todo_list *todo_list);
*/
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
5: 615fe4dd3f ! 5: baf7e6f0a6 history: re-edit a squash with every message

     @@ Commit 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
     -    messages of the folded-in commits were lost.
     +    messages of the other commits in the range were lost.
      
     -    Gather the messages of every commit in the range, oldest first, and build
     -    the same editor template that "git rebase -i" shows for a squash, using
     +    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(). Only the message text differs, the changes
     -    are always folded in. Following autosquash, a fixup!'s message is
     -    commented out in full under a "will be skipped" header, while a squash! or
     -    amend! keeps its body with only the marker subject commented.
     +    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 <[email protected]>
      
       ## Documentation/git-history.adoc ##
     -@@ Documentation/git-history.adoc: like the arguments to linkgit:git-rev-list[1], so several arguments may be
     - given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
     - already on `topic`.
     +@@ 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
     + 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
     @@ Documentation/git-history.adoc: like the arguments to linkgit:git-rev-list[1], s
       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.
     - +
     - Because the oldest commit's message is reused, the range may not begin
     - with a `fixup!`, `squash!`, or `amend!` commit, whose target is
     --necessarily outside the range.
     -+necessarily outside the range. 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`: the message of a
     -+`fixup!` elsewhere in the range is commented out in full, while a
     -+`squash!` or `amend!` keeps its message body with only the marker subject
     -+commented, so you can fold the remark into the result.
     +@@ Documentation/git-history.adoc: 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.
     ++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
     @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
       	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 commit_list *commits = NULL, **tail = &commits, *c;
      +	struct rev_info revs;
      +	struct commit *commit;
      +	struct strvec args = STRVEC_INIT;
     -+	int n = 0, total, ret;
     ++	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");
     @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
      +	}
      +
      +	while ((commit = get_revision(&revs)))
     -+		tail = &commit_list_insert(commit, tail)->next;
     -+	total = commit_list_count(commits);
     ++		strbuf_addf(&todo.buf, "pick %s\n",
     ++			    oid_to_hex(&commit->object.oid));
      +
     -+	for (c = commits; c; c = c->next) {
     ++	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;
     -+		int skip;
     ++		bool skip, squashing;
      +
     -+		message = repo_logmsg_reencode(repo, c->item, NULL, NULL);
     ++		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);
      +
     -+		skip = starts_with(body, "fixup! ");
     -+		commented_len = skip ? strlen(body) :
     -+			squash_subject_comment_len(body, 1);
     ++		if (skip)
     ++			commented_len = strlen(body);
     ++		else if (squashing)
     ++			commented_len = squash_subject_comment_len(body, 1);
     ++		else
     ++			commented_len = 0;
      +
     -+		if (!n)
     -+			add_squash_combination_header(out, total);
     ++		if (!i)
     ++			add_squash_combination_header(out, nr_commits);
      +		strbuf_addch(out, '\n');
     -+		add_squash_message_header(out, ++n, skip);
     ++		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, c->item, message);
     ++		repo_unuse_commit_buffer(repo, item->commit, message);
      +	}
      +
      +	ret = 0;
      +
      +out:
     -+	commit_list_free(commits);
     ++	todo_list_release(&todo);
     ++	replay_opts_release(&opts);
      +	reset_revision_walk();
      +	release_revisions(&revs);
      +	strvec_clear(&args);
     @@ builtin/history.c: static int cmd_history_squash(int argc,
       	struct strbuf reflog_msg = STRBUF_INIT;
      +	struct strbuf message = STRBUF_INIT;
       	struct oidset interior = OIDSET_INIT;
     - 	struct commit *base, *oldest, *tip, *rewritten;
     + 	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,
       		}
     @@ 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", oldest, NULL, parents,
     -+	ret = commit_tree_ext(repo, "squash", oldest,
     +-	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) {
     @@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	test_grep "amended message" final
      +'
      +
     ++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 &&
     ++
     ++	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 &&
     ++	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 &&
     ++
     ++	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 &&
     ++	git log -1 --format="%B" >final &&
     ++	test_grep ! "mark-base" final &&
     ++	test_grep "amended message" final &&
     ++	test_grep "squash remark" final
     ++'
     ++
      +test_expect_success '--reedit-message aborts on an empty message' '
      +	git reset --hard three &&
      +	head_before=$(git rev-parse HEAD) &&

gitgitgadget

From: Harald Nordgren <[email protected]>

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.

---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

index fd83de8265..9f516687fe 100644
@@ -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);
 }