[PATCH 0/2] rebase: add --fixup to fold a range into its oldest commit

[PATCH 0/2] rebase: add --fixup to fold a range into its oldest commit

[Harald Nordgren via GitGitGadget]

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

[PATCH 1/2] t3415: remove prepare-commit-msg hook after use

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

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.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 t/t3415-rebase-autosquash.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 5033411a43..8964d1cc88 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -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

[PATCH 2/2] rebase: add --fixup-all to fold a range

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Folding a series of commits into one required either an interactive
rebase where each commit after the first was hand-edited to "fixup", or
a "git reset --soft" to the merge base followed by "git commit --amend".

Add "git 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.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 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(-)

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index f6c22d1598..d1a3e4ef64 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -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
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fa4f5d9306..a363fbc1f2 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -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) {
diff --git a/sequencer.c b/sequencer.c
index 57855b0066..eeaf6226fe 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -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)
diff --git a/sequencer.h b/sequencer.h
index 3164bd437d..9bb6b42c94 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -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);
 
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 8964d1cc88..21d4159ebd 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -511,4 +511,109 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
 	test_commit_message HEAD -m "something"
 '
 
+test_expect_success '--fixup-all 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 &&
+	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' '
+	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 &&
+	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!' '
+	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 &&
+	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' '
+	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 rev-parse HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--fixup-all with an empty range succeeds' '
+	git reset --hard base &&
+	git rebase --autosquash --fixup-all HEAD &&
+	test_cmp_rev base HEAD
+'
+
+test_expect_success '--fixup-all 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 &&
+	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' '
+	git reset --hard base &&
+	test_commit --no-tag mid-first &&
+	git checkout -b mid-side &&
+	test_commit --no-tag mid-merged &&
+	git checkout - &&
+	git merge --no-ff -m "merge mid-side" mid-side &&
+	test_commit --no-tag mid-last &&
+	git rebase --autosquash --fixup-all 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' '
+	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 &&
+	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' '
+	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_expect_success '--fixup-all 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_grep "cannot be used together" err &&
+	test_path_is_missing .git/rebase-merge
+'
+
 test_done
-- 
gitgitgadget

Re: [PATCH 0/2] rebase: add --fixup to fold a range into its oldest commit

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

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

[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.

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

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?