[PATCH 00/12] rebase -r: support merge strategies other than recursive

[PATCH 00/12] rebase -r: support merge strategies other than recursive

[Johannes Schindelin via GitGitGadget]

This is the most notable shortcoming that --rebase-merges has, still,
relative to --preserve-merges' capabilities: it does not support passing
custom merge strategies or custom merge strategy options.

Let's fix this.

While working on this patch series, of course I tried to copy-edit the test
cases we have, to cover --preserve-merges' support for merge strategies. Oh
my, did I regret this decision as soon as my eyes set sight on 
t3427-rebase-subtree.sh!

At first I tried my best to make heads or tails of t3427, for way too long.
In the end the only way to understand what the heck it tries to do was to
actually fix it. That's why this patch series looks as if it focuses on
t3427 rather than on adding support for custom merge strategies to the 
--rebase-merges mode.

As a consolation to myself, this work was actually worth it, surprising as
that may look. Not only is t3427 now really easy to understand, adding that
test case for --rebase-merges -Xsubtree tickled the sequencer enough to
reveal a long-standing bug: the --onto option was simply ignored when passed
together with --rebase-merges and --root. For good measure, this patch
series addresses this bug, too.

Johannes Schindelin (12):
  t3427: add a clarifying comment
  t3427: simplify the `setup` test case significantly
  t3427: move the `filter-branch` invocation into the `setup` case
  t3427: condense the unnecessarily repetitive test cases into three
  t3427: fix erroneous assumption
  t3427: accommodate for the `rebase --merge` backend having been
    replaced
  t3427: fix another incorrect assumption
  t3427: mark two test cases as requiring support for `git rebase -p`
  rebase -r: support merge strategies other than `recursive`
  t/lib-rebase: prepare for testing `git rebase --rebase-merges`
  t3418: test `rebase -r` with merge strategies
  rebase -r: do not (re-)generate root commits with `--root` *and*
    `--onto`

 Documentation/git-rebase.txt           |   2 -
 builtin/rebase.c                       |  16 +--
 sequencer.c                            |  18 ++-
 sequencer.h                            |   6 +
 t/lib-rebase.sh                        |   8 +-
 t/t3418-rebase-continue.sh             |  14 +++
 t/t3422-rebase-incompatible-options.sh |  10 --
 t/t3427-rebase-subtree.sh              | 150 ++++++++++++-------------
 t/t3430-rebase-merges.sh               |  21 ++++
 9 files changed, 134 insertions(+), 111 deletions(-)


base-commit: 082ef75b7bfc90ac236afbb857a9552a026832b8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/294
-- 
gitgitgadget

[PATCH 01/12] t3427: add a clarifying comment

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The flow of this test script is outright confusing, and to start the
endeavor to address that, let's describe what this test is all about,
and how it tries to do it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 3780877e4e..a9b93c39d0 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -11,6 +11,34 @@ commit_message() {
 	git log --pretty=format:%s -1 "$1"
 }
 
+# There are a few bugs in the rebase with regards to the subtree strategy, and
+# this test script tries to document them.  First, the following commit history
+# is generated (the onelines are shown, time flows from left to right):
+#
+# master1 - master2 - master3
+#                             \
+# README ---------------------- Add subproject master - master4 - files_subtree/master5
+#
+# Where the merge moves the files master[123].t into the subdirectory
+# files_subtree/ and master4 as well as files_subtree/master5 add files to that
+# directory directly.
+#
+# Then, in subsequent test cases, `git filter-branch` is used to distill just
+# the commits that touch files_subtree/. To give it a final pre-rebase touch,
+# an empty commit is added on top. The pre-rebase commit history looks like
+# this:
+#
+# Add subproject master - master4 - files_subtree/master5 - Empty commit
+#
+# where the root commit adds three files: master1.t, master2.t and master3.t.
+#
+# This commit history is then rebased onto `master3` with the
+# `-Xsubtree=files_subtree` option in three different ways:
+#
+# 1. using `--preserve-merges`
+# 2. using `--preserve-merges` and --keep-empty
+# 3. without specifying a rebase backend
+
 test_expect_success 'setup' '
 	test_commit README &&
 	mkdir files &&
-- 
gitgitgadget

[PATCH 02/12] t3427: simplify the `setup` test case significantly

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

It still does the very same thing as before, but expresses it in a much
more succinct (and still quite readable) manner.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index a9b93c39d0..f41a08e436 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -41,27 +41,21 @@ commit_message() {
 
 test_expect_success 'setup' '
 	test_commit README &&
-	mkdir files &&
-	(
-		cd files &&
-		git init &&
-		test_commit master1 &&
-		test_commit master2 &&
-		test_commit master3
-	) &&
-	git fetch files master &&
-	git branch files-master FETCH_HEAD &&
-	git read-tree --prefix=files_subtree files-master &&
-	git checkout -- files_subtree &&
-	tree=$(git write-tree) &&
-	head=$(git rev-parse HEAD) &&
-	rev=$(git rev-parse --verify files-master^0) &&
-	commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) &&
-	git update-ref HEAD $commit &&
-	(
-		cd files_subtree &&
-		test_commit master4
-	) &&
+
+	git init files &&
+	test_commit -C files master1 &&
+	test_commit -C files master2 &&
+	test_commit -C files master3 &&
+
+	: perform subtree merge into files_subtree/ &&
+	git fetch files refs/heads/master:refs/heads/files-master &&
+	git merge -s ours --no-commit --allow-unrelated-histories \
+		files-master &&
+	git read-tree --prefix=files_subtree -u files-master &&
+	git commit -m "Add subproject master" &&
+
+	: add two extra commits to rebase &&
+	test_commit -C files_subtree master4 &&
 	test_commit files_subtree/master5
 '
 
-- 
gitgitgadget

[PATCH 03/12] t3427: move the `filter-branch` invocation into the `setup` case

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The step to prepare a pre-rebase commit history is _identical_ in _all_
of the test cases (except of course the `setup` case). It should
therefore clearly a part of the `setup` test case instead.

As the `git filter-branch` command is quite costly on platforms where
Unix shell scripting is simply slow (meaning: on Windows), this shaves
off a noticeable part of the runtime: in this developer's setup, the
time was reduced from ~1m25s to ~1m.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index f41a08e436..c0e6a49b61 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -56,15 +56,17 @@ test_expect_success 'setup' '
 
 	: add two extra commits to rebase &&
 	test_commit -C files_subtree master4 &&
-	test_commit files_subtree/master5
+	test_commit files_subtree/master5 &&
+
+	git checkout -b to-rebase &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty
 '
 
 # FAILURE: Does not preserve master4.
 test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-preserve-merges-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
 '
@@ -72,9 +74,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
 # FAILURE: Does not preserve master5.
 test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-preserve-merges-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
@@ -82,9 +82,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
 # FAILURE: Does not preserve master4.
 test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
 '
@@ -92,9 +90,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm
 # FAILURE: Does not preserve master5.
 test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
 '
@@ -102,9 +98,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm
 # FAILURE: Does not preserve Empty.
 test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
@@ -112,9 +106,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empt
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-onto-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
 '
@@ -122,18 +114,14 @@ test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-onto-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
 '
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto empty commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
-- 
gitgitgadget

[PATCH 04/12] t3427: condense the unnecessarily repetitive test cases into three

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Previously, this test script performed essentially three rebases and
verified breakages by testing the post-rebase commits' messages.

To do so, the rebases were performed multiple times, though, once per
commit message to test. This wastes electricity (and CO2) and time.

Let's condense the test cases to the essential number: the number of
different rebases to validate.

On Windows, where the scripted nature of the `--preserve-merges` backend
hurts performance rather badly, this reduces the overall runtime in this
developer's setup from ~1m to ~28s while still performing the exact same
testing as before.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 56 ++++++++-------------------------------
 1 file changed, 11 insertions(+), 45 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index c0e6a49b61..7d3ba766de 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -64,65 +64,31 @@ test_expect_success 'setup' '
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
-'
-
-# FAILURE: Does not preserve master5.
-test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-preserve-merges-5 to-rebase &&
+	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master4" &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
-'
-
-# FAILURE: Does not preserve master5.
-test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-5 to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-
-# FAILURE: Does not preserve Empty.
-test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-empty to-rebase &&
+	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
 # FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
-	reset_rebase &&
-	git checkout -b rebase-onto-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
-'
-
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-onto-5 to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto empty commit' '
+test_expect_failure 'Rebase -Xsubtree --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-empty to-rebase &&
+	git checkout -b rebase-onto to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
+	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-- 
gitgitgadget

[PATCH 05/12] t3427: fix erroneous assumption

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Apart from the `setup` test case, `t3427-rebase-subtree.sh` is made up
exclusively of demonstrations of breakages. The tricky thing about such
demonstrations is that they are often buggy themselves.

In this instance, somewhere over the course of the six iterations
of the patch that eventually made it into Git's `master` as 5f35900849e
(contrib/subtree: Add a test for subtree rebase that loses commits,
2016-06-28), the commit message "files_subtree/master4" was changed to
just "master4", but the test cases still expected the old commit
message.

Let's fix this, at long last.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 7d3ba766de..8c4ddd3408 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -68,7 +68,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "master4" &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
 
@@ -77,7 +77,7 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm
 	reset_rebase &&
 	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
@@ -87,7 +87,7 @@ test_expect_failure 'Rebase -Xsubtree --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
-- 
gitgitgadget

[PATCH 06/12] t3427: accommodate for the `rebase --merge` backend having been replaced

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Since 68aa495b590 (rebase: implement --merge via the interactive
machinery, 2018-12-11), the job of the old `--merge` backend is now
performed by the `--interactive` backend, too.

One consequence is that empty commits are no longer rebased by default.

Meaning that the test case that calls `git rebase -Xsubtree` (which used
to be handled by the `--merge` backend) now needs to ask explicitly for
the empty commit to be rebased.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 8c4ddd3408..b490919c60 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -83,10 +83,10 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm
 '
 
 # FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit' '
+test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
+	git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
-- 
gitgitgadget

[PATCH 07/12] t3427: fix another incorrect assumption

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The test case that concerns `git rebase -Xsubtree` (with the
default rebase backend, not with `--preserve-merges`) starts out with a
pre-rebase commit history that begins with a commit that introduces
three files: master1.t, master2.t and master3.t.

This commit was generated by passing a subtree merge commit through `git
filter-branch --subdirectory-filter`, so it looks as if this commit
really introduces all those files.

The commit history onto which this commit is then rebased, however,
introduced those files in individual commits. For that reason, the
rebase will fail, it _must_ fail, because the first `pick` results in no
changes to be committed.

Let's fix the test case to expect exactly this situation.

With this change, we can mark the original bug that this test case tried
to demonstrate as fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index b490919c60..c0ff3241e4 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -82,11 +82,12 @@ test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto comm
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' '
+test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
+	: first pick results in no changes &&
+	git rebase --continue &&
 	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
-- 
gitgitgadget

[PATCH 08/12] t3427: mark two test cases as requiring support for `git rebase -p`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

To prepare for the long game, where the `--preserve-merges` backend will
be dropped eventually, we already introduced the `REBASE_P` prerequisite
to allow saving time by skipping the now-almost-pointless test cases
that verify that that backend works as expected.

Due to the nature of the tests in t3427 (`test_expect_failure` is happy
as long as the scriptlet fails, whether it is for the intended reason or
because `git-rebase--preserve-merges.sh` was deleted), these two test
cases were missed.

When running with GIT_TEST_SKIP_REBASE_P=OhYesPlease, this drops the
overall run time of t3427 on this developer's machine from ~28s to
~8.5s.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index c0ff3241e4..7a37235768 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -64,7 +64,7 @@ test_expect_success 'setup' '
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
+test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
@@ -73,7 +73,7 @@ test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
+test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-- 
gitgitgadget

[PATCH 09/12] rebase -r: support merge strategies other than `recursive`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

We already support merge strategies in the sequencer, but only for
`pick` commands.

With this commit, we now also support them in `merge` commands. The
approach is simple: if any merge strategy option is specified, or if any
merge strategy other than `recursive` is specified, we simply spawn the
`git merge` command. Otherwise, we handle the merge in-process just as
before.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-rebase.txt           |  2 --
 builtin/rebase.c                       |  9 ---------
 sequencer.c                            | 14 ++++++++++++--
 t/t3422-rebase-incompatible-options.sh | 10 ----------
 t/t3430-rebase-merges.sh               | 21 +++++++++++++++++++++
 5 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f5e6ae3907..f67f96425c 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -543,8 +543,6 @@ In addition, the following pairs of options are incompatible:
  * --preserve-merges and --interactive
  * --preserve-merges and --signoff
  * --preserve-merges and --rebase-merges
- * --rebase-merges and --strategy
- * --rebase-merges and --strategy-option
 
 BEHAVIORAL DIFFERENCES
 -----------------------
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 9c52144fc4..c1ea617125 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1815,15 +1815,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			      "'--reschedule-failed-exec'"));
 	}
 
-	if (options.rebase_merges) {
-		if (strategy_options.nr)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy-option'"));
-		if (options.strategy)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy'"));
-	}
-
 	if (!options.root) {
 		if (argc < 1) {
 			struct branch *branch;
diff --git a/sequencer.c b/sequencer.c
index 334de14542..d228448cd8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3256,6 +3256,9 @@ static int do_merge(struct repository *r,
 	struct commit *head_commit, *merge_commit, *i;
 	struct commit_list *bases, *j, *reversed = NULL;
 	struct commit_list *to_merge = NULL, **tail = &to_merge;
+	const char *strategy = !opts->xopts_nr &&
+		(!opts->strategy || !strcmp(opts->strategy, "recursive")) ?
+		NULL : opts->strategy;
 	struct merge_options o;
 	int merge_arg_len, oneline_offset, can_fast_forward, ret, k;
 	static struct lock_file lock;
@@ -3404,7 +3407,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 
-	if (to_merge->next) {
+	if (strategy || to_merge->next) {
 		/* Octopus merge */
 		struct child_process cmd = CHILD_PROCESS_INIT;
 
@@ -3418,7 +3421,14 @@ static int do_merge(struct repository *r,
 		cmd.git_cmd = 1;
 		argv_array_push(&cmd.args, "merge");
 		argv_array_push(&cmd.args, "-s");
-		argv_array_push(&cmd.args, "octopus");
+		if (!strategy)
+			argv_array_push(&cmd.args, "octopus");
+		else {
+			argv_array_push(&cmd.args, strategy);
+			for (k = 0; k < opts->xopts_nr; k++)
+				argv_array_pushf(&cmd.args,
+						 "-X%s", opts->xopts[k]);
+		}
 		argv_array_push(&cmd.args, "--no-edit");
 		argv_array_push(&cmd.args, "--no-ff");
 		argv_array_push(&cmd.args, "--no-log");
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index bb78a6ec86..596caf168a 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -75,14 +75,4 @@ test_expect_success '--preserve-merges incompatible with --rebase-merges' '
 	test_must_fail git rebase --preserve-merges --rebase-merges A
 '
 
-test_expect_success '--rebase-merges incompatible with --strategy' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -s resolve A
-'
-
-test_expect_success '--rebase-merges incompatible with --strategy-option' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -Xignore-space-change A
-'
-
 test_done
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 42ba5b9f09..8ea6ff3548 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -412,4 +412,25 @@ test_expect_success '--continue after resolving conflicts after a merge' '
 	test_path_is_missing .git/MERGE_HEAD
 '
 
+test_expect_success '--rebase-merges with strategies' '
+	git checkout -b with-a-strategy F &&
+	test_tick &&
+	git merge -m "Merge conflicting-G" conflicting-G &&
+
+	: first, test with a merge strategy option &&
+	git rebase -ir -Xtheirs G &&
+	echo conflicting-G >expect &&
+	test_cmp expect G.t &&
+
+	: now, try with a merge strategy other than recursive &&
+	git reset --hard @{1} &&
+	write_script git-merge-override <<-\EOF &&
+	echo overridden$1 >>G.t
+	git add G.t
+	EOF
+	PATH="$PWD:$PATH" git rebase -ir -s override -Xxopt G &&
+	test_write_lines G overridden--xopt >expect &&
+	test_cmp expect G.t
+'
+
 test_done
-- 
gitgitgadget

[PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The format of the todo list is quite a bit different in the
`--rebase-merges` mode; Let's prepare the fake editor to handle those
todo lists properly, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/lib-rebase.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 7ea30e5006..662a958575 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -44,10 +44,10 @@ set_fake_editor () {
 	rm -f "$1"
 	echo 'rebase -i script before editing:'
 	cat "$1".tmp
-	action=pick
+	action=\&
 	for line in $FAKE_LINES; do
 		case $line in
-		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
+		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
 			action="$line";;
 		exec_*|x_*|break|b)
 			echo "$line" | sed 's/_/ /g' >> "$1";;
@@ -61,8 +61,8 @@ set_fake_editor () {
 			echo "$action XXXXXXX False commit" >> "$1"
 			action=pick;;
 		*)
-			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
-			action=pick;;
+			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
+			action=\&;;
 		esac
 	done
 	echo 'rebase -i script after editing:'
-- 
gitgitgadget

[PATCH 12/12] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When rebasing a complete commit history onto a given commit, it is
pretty obvious that the root commits should be rebased on top of said
given commit.

To test this, let's kill two birds with one stone and add a test case to
t3427-rebase-subtree.sh that not only demonstrates that this works, but
also that `git rebase -r` works with merge strategies now.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/rebase.c          |  7 +++++--
 sequencer.c               |  4 +++-
 sequencer.h               |  6 ++++++
 t/t3427-rebase-subtree.sh | 11 +++++++++++
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index c1ea617125..6a789c4421 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -62,7 +62,7 @@ struct rebase_options {
 	const char *onto_name;
 	const char *revisions;
 	const char *switch_to;
-	int root;
+	int root, root_with_onto;
 	struct object_id *squash_onto;
 	struct commit *restrict_revision;
 	int dont_finish_rebase;
@@ -374,6 +374,7 @@ static int run_rebase_interactive(struct rebase_options *opts,
 	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
 	flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
 	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
+	flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0;
 	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
 	switch (command) {
@@ -1845,7 +1846,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			options.squash_onto = &squash_onto;
 			options.onto_name = squash_onto_name =
 				xstrdup(oid_to_hex(&squash_onto));
-		}
+		} else
+			options.root_with_onto = 1;
+
 		options.upstream_name = NULL;
 		options.upstream = NULL;
 		if (argc > 1)
diff --git a/sequencer.c b/sequencer.c
index d228448cd8..ca119c84e5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4440,6 +4440,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 {
 	int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
 	int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
+	int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
 	struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
 	struct strbuf label = STRBUF_INIT;
 	struct commit_list *commits = NULL, **tail = &commits, *iter;
@@ -4606,7 +4607,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 		if (!commit)
 			strbuf_addf(out, "%s %s\n", cmd_reset,
-				    rebase_cousins ? "onto" : "[new root]");
+				    rebase_cousins || root_with_onto ?
+				    "onto" : "[new root]");
 		else {
 			const char *to = NULL;
 
diff --git a/sequencer.h b/sequencer.h
index 0c494b83d4..d506081d3c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -142,6 +142,12 @@ int sequencer_remove_state(struct replay_opts *opts);
  */
 #define TODO_LIST_REBASE_COUSINS (1U << 4)
 #define TODO_LIST_APPEND_TODO_HELP (1U << 5)
+/*
+ * When generating a script that rebases merges with `--root` *and* with
+ * `--onto`, we do not want to re-generate the root commits.
+ */
+#define TODO_LIST_ROOT_WITH_ONTO (1U << 6)
+
 
 int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 			  const char **argv, unsigned flags);
diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 7a37235768..39e348de16 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -93,4 +93,15 @@ test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
+test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' '
+	reset_rebase &&
+	git checkout -b rebase-merges-onto to-rebase &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root &&
+	: first pick results in no changes &&
+	git rebase --continue &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
+	verbose test "$(commit_message HEAD)" = "Empty commit"
+'
+
 test_done
-- 
gitgitgadget

[PATCH 11/12] t3418: test `rebase -r` with merge strategies

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

There is a test case in this script that verifies that `git rebase
--preserve-merges` works all right with non-default merge strategies or
non-default merge strategy options.

Now that `git rebase --rebase-merges` learned about merge strategies,
let's copy-edit this test case to verify that that works as intended,
too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3418-rebase-continue.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index bdaa511bb0..fbf9addfd1 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -120,6 +120,20 @@ test_expect_success REBASE_P 'rebase passes merge strategy options correctly' '
 	git rebase --continue
 '
 
+test_expect_success 'rebase -r passes merge strategy options correctly' '
+	rm -fr .git/rebase-* &&
+	git reset --hard commit-new-file-F3-on-topic-branch &&
+	test_commit merge-theirs &&
+	git reset --hard HEAD^ &&
+	test_commit some-other-commit &&
+	test_tick &&
+	git merge --no-ff merge-theirs &&
+	FAKE_LINES="1 3 edit 4 5 7 8 9" git rebase -i -f -r -m \
+		-s recursive --strategy-option=theirs HEAD~2 &&
+	test_commit force-change-ours &&
+	git rebase --continue
+'
+
 test_expect_success '--skip after failed fixup cleans commit message' '
 	test_when_finished "test_might_fail git rebase --abort" &&
 	git checkout -b with-conflicting-fixup &&
-- 
gitgitgadget

Re: [PATCH 00/12] rebase -r: support merge strategies other than recursive

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> ... As a consolation to myself, this work was actually worth it, surprising as
> that may look. Not only is t3427 now really easy to understand, adding that
> test case for --rebase-merges -Xsubtree tickled the sequencer enough to
> reveal a long-standing bug: the --onto option was simply ignored when passed
> together with --rebase-merges and --root. For good measure, this patch
> series addresses this bug, too.

Very nice.

> base-commit: 082ef75b7bfc90ac236afbb857a9552a026832b8
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/294

Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1436 bytes --]

On 2019-07-25 at 10:11:22, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The format of the todo list is quite a bit different in the
> `--rebase-merges` mode; Let's prepare the fake editor to handle those
> todo lists properly, too.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/lib-rebase.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 7ea30e5006..662a958575 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -44,10 +44,10 @@ set_fake_editor () {
>  	rm -f "$1"
>  	echo 'rebase -i script before editing:'
>  	cat "$1".tmp
> -	action=pick
> +	action=\&

So we set action to "&" so we can use it as the result in the sed
expression below…

>  	for line in $FAKE_LINES; do
>  		case $line in
> -		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> +		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
>  			action="$line";;
>  		exec_*|x_*|break|b)
>  			echo "$line" | sed 's/_/ /g' >> "$1";;
> @@ -61,8 +61,8 @@ set_fake_editor () {
>  			echo "$action XXXXXXX False commit" >> "$1"

but then here it doesn't look like "&" is a thing we'd want to use. Is
there something I'm missing about this particular case?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 00/12] rebase -r: support merge strategies other than recursive

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On 2019-07-25 at 10:11:14, Johannes Schindelin via GitGitGadget wrote:
> This is the most notable shortcoming that --rebase-merges has, still,
> relative to --preserve-merges' capabilities: it does not support passing
> custom merge strategies or custom merge strategy options.
> 
> Let's fix this.

This looks like a great improvement. I'm glad to see --rebase-merges
gaining feature parity with --preserve-merges.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 3694 bytes --]

Hi brian,

On Fri, 26 Jul 2019, brian m. carlson wrote:

> On 2019-07-25 at 10:11:22, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The format of the todo list is quite a bit different in the
> > `--rebase-merges` mode; Let's prepare the fake editor to handle those
> > todo lists properly, too.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  t/lib-rebase.sh | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> > index 7ea30e5006..662a958575 100644
> > --- a/t/lib-rebase.sh
> > +++ b/t/lib-rebase.sh
> > @@ -44,10 +44,10 @@ set_fake_editor () {
> >  	rm -f "$1"
> >  	echo 'rebase -i script before editing:'
> >  	cat "$1".tmp
> > -	action=pick
> > +	action=\&
>
> So we set action to "&" so we can use it as the result in the sed
> expression below…
>
> >  	for line in $FAKE_LINES; do
> >  		case $line in
> > -		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> > +		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
> >  			action="$line";;
> >  		exec_*|x_*|break|b)
> >  			echo "$line" | sed 's/_/ /g' >> "$1";;
> > @@ -61,8 +61,8 @@ set_fake_editor () {
> >  			echo "$action XXXXXXX False commit" >> "$1"
>
> but then here it doesn't look like "&" is a thing we'd want to use. Is
> there something I'm missing about this particular case?

Actually, the part that uses it is not shown in the patch (one of the
many, many reasons why I try to discourage patch review and encourage
code review instead). The relevant section currently looks somewhat like
this:

-- snip --
set_fake_editor () {
	write_script fake-editor.sh <<-\EOF
	case "$1" in
	*/COMMIT_EDITMSG)
		test -z "$EXPECT_HEADER_COUNT" ||
			test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
			test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" ||
			exit
		test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
		test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
		exit
		;;
	esac
	test -z "$EXPECT_COUNT" ||
		test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
		exit
	test -z "$FAKE_LINES" && exit
	grep -v '^#' < "$1" > "$1".tmp
	rm -f "$1"
	echo 'rebase -i script before editing:'
	cat "$1".tmp
	action=pick
	for line in $FAKE_LINES; do
		case $line in
		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
			action="$line";;
		exec_*|x_*|break|b)
			echo "$line" | sed 's/_/ /g' >> "$1";;
		"#")
			echo '# comment' >> "$1";;
		">")
			echo >> "$1";;
		bad)
			action="badcmd";;
		fakesha)
			echo "$action XXXXXXX False commit" >> "$1"
			action=pick;;
		*)
			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
			action=pick;;
		esac
	done
	echo 'rebase -i script after editing:'
	cat "$1"
	EOF

	test_set_editor "$(pwd)/fake-editor.sh"
}
-- snap --

Most importantly, `action` is used here:

                        sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"

and I changed it to

			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"

In other words, rather than expecting the lines that are used by the
fake editor to start with `pick`, after this patch, the tests expect the
todo lists to start with a command consisting of lower-case ASCII
letters (which catches `pick`, of course, but also `label`, `reset` and
`merge`).

After this patch, the fake editor also does not try to replace whatever
command it finds by `pick`, but it keeps it as-is instead.

Ciao,
Dscho

Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]

On 2019-07-26 at 14:01:03, Johannes Schindelin wrote:
> Actually, the part that uses it is not shown in the patch (one of the
> many, many reasons why I try to discourage patch review and encourage
> code review instead). The relevant section currently looks somewhat like
> this:

I feel like I may have communicated poorly earlier, so let me retry
asking this in a different way.

> -- snip --
> set_fake_editor () {
> 	write_script fake-editor.sh <<-\EOF
> 	case "$1" in
> 	*/COMMIT_EDITMSG)
> 		test -z "$EXPECT_HEADER_COUNT" ||
> 			test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
> 			test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" ||
> 			exit
> 		test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> 		test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> 		exit
> 		;;
> 	esac
> 	test -z "$EXPECT_COUNT" ||
> 		test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> 		exit
> 	test -z "$FAKE_LINES" && exit
> 	grep -v '^#' < "$1" > "$1".tmp
> 	rm -f "$1"
> 	echo 'rebase -i script before editing:'
> 	cat "$1".tmp
> 	action=pick

I believe you changed this line to "action=\&".

> 	for line in $FAKE_LINES; do
> 		case $line in
> 		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> 			action="$line";;
> 		exec_*|x_*|break|b)
> 			echo "$line" | sed 's/_/ /g' >> "$1";;
> 		"#")
> 			echo '# comment' >> "$1";;
> 		">")
> 			echo >> "$1";;
> 		bad)
> 			action="badcmd";;
> 		fakesha)
> 			echo "$action XXXXXXX False commit" >> "$1"

And my question was about this line.

> 			action=pick;;
> 		*)
> 			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> 			action=pick;;
> 		esac
> 	done
> 	echo 'rebase -i script after editing:'
> 	cat "$1"
> 	EOF
> 
> 	test_set_editor "$(pwd)/fake-editor.sh"
> }
> -- snap --
> 
> Most importantly, `action` is used here:
> 
>                         sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> 
> and I changed it to
> 
> 			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
> 
> In other words, rather than expecting the lines that are used by the
> fake editor to start with `pick`, after this patch, the tests expect the
> todo lists to start with a command consisting of lower-case ASCII
> letters (which catches `pick`, of course, but also `label`, `reset` and
> `merge`).
> 
> After this patch, the fake editor also does not try to replace whatever
> command it finds by `pick`, but it keeps it as-is instead.

Right, that's how I read it, and that part I agree with. I think my
question is this: in what case do we execute the "fakesha" case? Are we
guaranteed that when we do, action isn't "&"? "&" seems fine for the
right-hand side of a sed s-statement, but not as the beginning of a
typical line in a sequencer file.

I ask because if we're testing a failure case, we want it to fail for
the right reason (e.g., the commit doesn't exist), and not because we're
producing invalid data. If the answer in this case is, "Well, we'll
always have something else before it which will set $action properly,"
then that's fine. This is test code, so it need not be bulletproof, but
I did want to ask.

If I'm still misunderstanding something, I apologize.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[Johannes Schindelin]

Hi brian,

On Fri, 26 Jul 2019, brian m. carlson wrote:

> On 2019-07-26 at 14:01:03, Johannes Schindelin wrote:
> > Actually, the part that uses it is not shown in the patch (one of the
> > many, many reasons why I try to discourage patch review and encourage
> > code review instead). The relevant section currently looks somewhat like
> > this:
>
> I feel like I may have communicated poorly earlier, so let me retry
> asking this in a different way.

Actually, your communication was just fine, the misunderstanding was
entirely on my side. My apologies.

> > -- snip --
> > set_fake_editor () {
> > 	write_script fake-editor.sh <<-\EOF
> > 	case "$1" in
> > 	*/COMMIT_EDITMSG)
> > 		test -z "$EXPECT_HEADER_COUNT" ||
> > 			test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
> > 			test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" ||
> > 			exit
> > 		test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> > 		test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> > 		exit
> > 		;;
> > 	esac
> > 	test -z "$EXPECT_COUNT" ||
> > 		test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> > 		exit
> > 	test -z "$FAKE_LINES" && exit
> > 	grep -v '^#' < "$1" > "$1".tmp
> > 	rm -f "$1"
> > 	echo 'rebase -i script before editing:'
> > 	cat "$1".tmp
> > 	action=pick
>
> I believe you changed this line to "action=\&".
>
> > 	for line in $FAKE_LINES; do
> > 		case $line in
> > 		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> > 			action="$line";;
> > 		exec_*|x_*|break|b)
> > 			echo "$line" | sed 's/_/ /g' >> "$1";;
> > 		"#")
> > 			echo '# comment' >> "$1";;
> > 		">")
> > 			echo >> "$1";;
> > 		bad)
> > 			action="badcmd";;
> > 		fakesha)
> > 			echo "$action XXXXXXX False commit" >> "$1"
>
> And my question was about this line.

Right. It would append `& XXXXXXX False commit`, which is not a valid
todo command.

So something like

-			echo "$action XXXXXXX False commit" >> "$1"
+			test \& = "$action" && c=pick || c=$action
+			echo "$c XXXXXXX False commit" >>"$1"

would be needed.

However, what makes me really worried now is that our test suite did not
have a fit about this. The CI build passes the test suite on Windows,
macOS and Linux: https://github.com/gitgitgadget/git/runs/176651523.
>
> > 			action=pick;;
> > 		*)
> > 			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> > 			action=pick;;
> > 		esac
> > 	done
> > 	echo 'rebase -i script after editing:'
> > 	cat "$1"
> > 	EOF
> >
> > 	test_set_editor "$(pwd)/fake-editor.sh"
> > }
> > -- snap --
> >
> > Most importantly, `action` is used here:
> >
> >                         sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> >
> > and I changed it to
> >
> > 			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
> >
> > In other words, rather than expecting the lines that are used by the
> > fake editor to start with `pick`, after this patch, the tests expect the
> > todo lists to start with a command consisting of lower-case ASCII
> > letters (which catches `pick`, of course, but also `label`, `reset` and
> > `merge`).
> >
> > After this patch, the fake editor also does not try to replace whatever
> > command it finds by `pick`, but it keeps it as-is instead.
>
> Right, that's how I read it, and that part I agree with. I think my
> question is this: in what case do we execute the "fakesha" case? Are we
> guaranteed that when we do, action isn't "&"? "&" seems fine for the
> right-hand side of a sed s-statement, but not as the beginning of a
> typical line in a sequencer file.

Indeed, the sequencer should throw a real tantrum about this and not
even bother to start.

But then, the same would hold true for an obviously invalid commit hash.

> I ask because if we're testing a failure case, we want it to fail for
> the right reason (e.g., the commit doesn't exist), and not because we're
> producing invalid data.

Indeed. I have even come to the conclusion that our
`test_expect_failure` command encourages exactly this type of problem:
in the beginning, those test cases might actually be correct, but over
time they are prone to fail for all the wrong reasons, because we do not
actually test for a specific failure more, we just say that we expect
this test case to fail (and that this indicates a bug).

> If the answer in this case is, "Well, we'll always have something else
> before it which will set $action properly," then that's fine. This is
> test code, so it need not be bulletproof, but I did want to ask.

I think you are perfectly sane to question this, and to expect me to
double check.

So, double check I did. Turns out there is a single user of the
`fakesha` thing, and it is hidden deep in t3404, prefixed by
`test_must_fail`:

-- snip --
test_expect_success 'static check of bad SHA-1' '
	rebase_setup_and_clean bad-sha &&
	set_fake_editor &&
	test_must_fail env FAKE_LINES="1 2 edit fakesha 3 4 5 #" \
		git rebase -i --root 2>actual &&
	test_i18ngrep "edit XXXXXXX False commit" actual &&
	test_i18ngrep "You can fix this with .git rebase --edit-todo.." actual &&
	FAKE_LINES="1 2 4 5 6" git rebase --edit-todo &&
	git rebase --continue &&
	test E = $(git cat-file commit HEAD | sed -ne \$p)
'
-- snap --

As you can see, contrary to my expectations it does verify the output.
It *also* changes the action to `edit`, which is the reason why there is
no `&` ;-)

But I think you are correct, I should make sure that the fake editor is
still correct with respect to the `pick` command.

> If I'm still misunderstanding something, I apologize.

I am really impressed and inspired by your gentle language. Thank you
for this.

Ciao,
Dscho

[PATCH v2 00/16] rebase -r: support merge strategies other than recursive

[Johannes Schindelin via GitGitGadget]

This is the most notable shortcoming that --rebase-merges has, still,
relative to --preserve-merges' capabilities: it does not support passing
custom merge strategies or custom merge strategy options.

Let's fix this.

While working on this patch series, of course I tried to copy-edit the test
cases we have, to cover --preserve-merges' support for merge strategies. Oh
my, did I regret this decision as soon as my eyes set sight on 
t3427-rebase-subtree.sh!

At first I tried my best to make heads or tails of t3427, for way too long.
In the end the only way to understand what the heck it tries to do was to
actually fix it. That's why this patch series looks as if it focuses on
t3427 rather than on adding support for custom merge strategies to the 
--rebase-merges mode.

As a consolation to myself, this work was actually worth it, surprising as
that may look. Not only is t3427 now really easy to understand, adding that
test case for --rebase-merges -Xsubtree tickled the sequencer enough to
reveal a long-standing bug: the --onto option was simply ignored when passed
together with --rebase-merges and --root. For good measure, this patch
series addresses this bug, too.

Changes since v1:

 * Rebased to the current js/rebase-cleanup: that branch itself was rebased,
   and as a consequence, one already-applied patch was dropped from this
   patch series.
 * Forward-fixed the fakesha handling, just in case that anybody wants to
   use it with a regular pick command in the future (thanks, brian).

Johannes Schindelin (16):
  Drop unused git-rebase--am.sh
  t3400: stop referring to the scripted rebase
  .gitignore: there is no longer a built-in `git-rebase--interactive`
  sequencer: the `am` and `rebase--interactive` scripts are gone
  rebase: fold git-rebase--common into the -p backend
  t3427: add a clarifying comment
  t3427: simplify the `setup` test case significantly
  t3427: move the `filter-branch` invocation into the `setup` case
  t3427: condense the unnecessarily repetitive test cases into three
  t3427: fix erroneous assumption
  t3427: accommodate for the `rebase --merge` backend having been
    replaced
  t3427: fix another incorrect assumption
  rebase -r: support merge strategies other than `recursive`
  t/lib-rebase: prepare for testing `git rebase --rebase-merges`
  t3418: test `rebase -r` with merge strategies
  rebase -r: do not (re-)generate root commits with `--root` *and*
    `--onto`

 .gitignore                             |   3 -
 Documentation/git-rebase.txt           |   2 -
 Makefile                               |   2 -
 builtin/rebase.c                       |  23 +---
 git-rebase--am.sh                      |  85 --------------
 git-rebase--common.sh                  |  69 -----------
 git-rebase--preserve-merges.sh         |  55 +++++++++
 sequencer.c                            |  20 +++-
 sequencer.h                            |   6 +
 t/lib-rebase.sh                        |   9 +-
 t/t3400-rebase.sh                      |   2 +-
 t/t3418-rebase-continue.sh             |  14 +++
 t/t3422-rebase-incompatible-options.sh |  10 --
 t/t3427-rebase-subtree.sh              | 155 +++++++++++--------------
 t/t3430-rebase-merges.sh               |  21 ++++
 15 files changed, 193 insertions(+), 283 deletions(-)
 delete mode 100644 git-rebase--am.sh
 delete mode 100644 git-rebase--common.sh


base-commit: 80dfc9242ebaba357ffedececd88641a1a752411
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-294%2Fdscho%2Frebase-r-with-strategies-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-294/dscho/rebase-r-with-strategies-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/294

Range-diff vs v1:

  -:  ---------- >  1:  e0645b3ad7 Drop unused git-rebase--am.sh
  -:  ---------- >  2:  6e8be7d873 t3400: stop referring to the scripted rebase
  -:  ---------- >  3:  9e00e66dc7 .gitignore: there is no longer a built-in `git-rebase--interactive`
  -:  ---------- >  4:  db9ec248e1 sequencer: the `am` and `rebase--interactive` scripts are gone
  -:  ---------- >  5:  38c8e3e284 rebase: fold git-rebase--common into the -p backend
  1:  05be92d921 =  6:  b3daf078e8 t3427: add a clarifying comment
  2:  df096e054d =  7:  c168c4499b t3427: simplify the `setup` test case significantly
  3:  a3944c5480 !  8:  138ff362fb t3427: move the `filter-branch` invocation into the `setup` case
     @@ -29,7 +29,8 @@
       '
       
       # FAILURE: Does not preserve master4.
     - test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-4 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -39,8 +40,8 @@
       	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
       '
      @@
     - # FAILURE: Does not preserve master5.
     - test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-5 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -50,8 +51,8 @@
       	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
       '
      @@
     - # FAILURE: Does not preserve master4.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-4 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -61,8 +62,8 @@
       	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
       '
      @@
     - # FAILURE: Does not preserve master5.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-5 master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
     @@ -72,8 +73,8 @@
       	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
       '
      @@
     - # FAILURE: Does not preserve Empty.
     - test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
     + test_expect_failure REBASE_P \
     + 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-empty master &&
      -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
  4:  9aeb57fa8f !  9:  46ef005365 t3427: condense the unnecessarily repetitive test cases into three
     @@ -25,8 +25,9 @@
       '
       
       # FAILURE: Does not preserve master4.
     --test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     -+test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
     ++test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
       	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-4 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
     @@ -34,7 +35,8 @@
      -'
      -
      -# FAILURE: Does not preserve master5.
     --test_expect_failure 'Rebase -Xsubtree --preserve-merges --onto commit 5' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
      -	reset_rebase &&
      -	git checkout -b rebase-preserve-merges-5 to-rebase &&
      +	git checkout -b rebase-preserve-merges to-rebase &&
     @@ -44,8 +46,9 @@
       '
       
       # FAILURE: Does not preserve master4.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     -+test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
     ++test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
       	reset_rebase &&
      -	git checkout -b rebase-keep-empty-4 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
     @@ -53,7 +56,8 @@
      -'
      -
      -# FAILURE: Does not preserve master5.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
      -	reset_rebase &&
      -	git checkout -b rebase-keep-empty-5 to-rebase &&
      -	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
     @@ -61,7 +65,8 @@
      -'
      -
      -# FAILURE: Does not preserve Empty.
     --test_expect_failure 'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
     +-test_expect_failure REBASE_P \
     +-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
      -	reset_rebase &&
      -	git checkout -b rebase-keep-empty-empty to-rebase &&
      +	git checkout -b rebase-keep-empty to-rebase &&
  5:  3196413c2a = 10:  8afec74cfc t3427: fix erroneous assumption
  6:  261825fe44 = 11:  d73b0a05d9 t3427: accommodate for the `rebase --merge` backend having been replaced
  7:  f7452d3740 = 12:  57c63309bf t3427: fix another incorrect assumption
  8:  7f60b8e745 <  -:  ---------- t3427: mark two test cases as requiring support for `git rebase -p`
  9:  30405df99f = 13:  75b1395dae rebase -r: support merge strategies other than `recursive`
 10:  ae9e72b73b ! 14:  8f74bfbc53 t/lib-rebase: prepare for testing `git rebase --rebase-merges`
     @@ -6,6 +6,26 @@
          `--rebase-merges` mode; Let's prepare the fake editor to handle those
          todo lists properly, too.
      
     +    The original idea was that we keep the original command unless
     +    overridden, and because the original todo lists only had `pick` lines
     +    anyway, we could be sloppy and "override" the command by the same
     +    command (i.e. use the sed replacement pattern "pick" instead of "&").
     +
     +    This actually would not have worked with `fixup` and `squash` commands,
     +    but it would appear that we never tried to use the fake editor with
     +    `--autosquash`.
     +
     +    However, in the next commit we want to use the fake editor in
     +    conjunction with `--rebase-merges`, so let's use the correct sed
     +    replacement pattern.
     +
     +    Technically, it is not necessary to take care of the `fakesha` thing
     +    (where we reuse the sed replacement pattern to craft a new todo
     +    command), at least for now, as the only user of that thing overrides the
     +    `action` anyway. Nevertheless, for completeness' sake, we do take care
     +    of it.
     +
     +    Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
     @@ -25,6 +45,10 @@
       		exec_*|x_*|break|b)
       			echo "$line" | sed 's/_/ /g' >> "$1";;
      @@
     + 		bad)
     + 			action="badcmd";;
     + 		fakesha)
     ++			test \& != "$action" || action=pick
       			echo "$action XXXXXXX False commit" >> "$1"
       			action=pick;;
       		*)
 11:  a326be77aa = 15:  0c938d02b3 t3418: test `rebase -r` with merge strategies
 12:  e6713568dc = 16:  be62d267cb rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`

-- 
gitgitgadget

[PATCH v2 01/16] Drop unused git-rebase--am.sh

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Since 21853626ea (built-in rebase: call `git am` directly, 2019-01-18),
the built-in rebase already uses the built-in `git am` directly.

Now that d03ebd411c (rebase: remove the rebase.useBuiltin setting,
2019-03-18) even removed the scripted rebase, there is no longer any
user of `git-rebase--am.sh`, so let's just remove it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 .gitignore        |  1 -
 Makefile          |  1 -
 builtin/rebase.c  |  4 ---
 git-rebase--am.sh | 85 -----------------------------------------------
 4 files changed, 91 deletions(-)
 delete mode 100644 git-rebase--am.sh

diff --git a/.gitignore b/.gitignore
index 2374f77a1a..875f3fc6e8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -122,7 +122,6 @@
 /git-range-diff
 /git-read-tree
 /git-rebase
-/git-rebase--am
 /git-rebase--common
 /git-rebase--interactive
 /git-rebase--preserve-merges
diff --git a/Makefile b/Makefile
index 8a7e235352..63e1973333 100644
--- a/Makefile
+++ b/Makefile
@@ -624,7 +624,6 @@ SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--am
 SCRIPT_LIB += git-rebase--common
 SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-sh-setup
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 21681a551b..4dd76ee576 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1153,10 +1153,6 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
 	}
 
 	switch (opts->type) {
-	case REBASE_AM:
-		backend = "git-rebase--am";
-		backend_func = "git_rebase__am";
-		break;
 	case REBASE_PRESERVE_MERGES:
 		backend = "git-rebase--preserve-merges";
 		backend_func = "git_rebase__preserve_merges";
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
deleted file mode 100644
index 6416716ee6..0000000000
--- a/git-rebase--am.sh
+++ /dev/null
@@ -1,85 +0,0 @@
-# This shell script fragment is sourced by git-rebase to implement
-# its default, fast, patch-based, non-interactive mode.
-#
-# Copyright (c) 2010 Junio C Hamano.
-#
-
-git_rebase__am () {
-
-case "$action" in
-continue)
-	git am --resolved --resolvemsg="$resolvemsg" \
-		${gpg_sign_opt:+"$gpg_sign_opt"} &&
-	move_to_original_branch
-	return
-	;;
-skip)
-	git am --skip --resolvemsg="$resolvemsg" &&
-	move_to_original_branch
-	return
-	;;
-show-current-patch)
-	exec git am --show-current-patch
-	;;
-esac
-
-if test -z "$rebase_root"
-	# this is now equivalent to ! -z "$upstream"
-then
-	revisions=$upstream...$orig_head
-else
-	revisions=$onto...$orig_head
-fi
-
-ret=0
-rm -f "$GIT_DIR/rebased-patches"
-
-git format-patch -k --stdout --full-index --cherry-pick --right-only \
-	--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
-	--pretty=mboxrd --topo-order \
-	$git_format_patch_opt \
-	"$revisions" ${restrict_revision+^$restrict_revision} \
-	>"$GIT_DIR/rebased-patches"
-ret=$?
-
-if test 0 != $ret
-then
-	rm -f "$GIT_DIR/rebased-patches"
-	case "$head_name" in
-	refs/heads/*)
-		git checkout -q "$head_name"
-		;;
-	*)
-		git checkout -q "$orig_head"
-		;;
-	esac
-
-	cat >&2 <<-EOF
-
-	git encountered an error while preparing the patches to replay
-	these revisions:
-
-	    $revisions
-
-	As a result, git cannot rebase them.
-	EOF
-	return $ret
-fi
-
-git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
-	--patch-format=mboxrd \
-	$allow_rerere_autoupdate \
-	${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
-ret=$?
-
-rm -f "$GIT_DIR/rebased-patches"
-
-if test 0 != $ret
-then
-	test -d "$state_dir" && write_basic_state
-	return $ret
-fi
-
-move_to_original_branch
-
-}
-- 
gitgitgadget

[PATCH v2 02/16] t3400: stop referring to the scripted rebase

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

One test case's title mentioned the then-current implementation detail
that the `--am` backend was implemented in `git-rebase--am.sh`.

This is no longer the case, so let's update the title to reflect the
current reality.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3400-rebase.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 42f147858d..80b23fd326 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -285,7 +285,7 @@ EOF
 	test_cmp From_.msg out
 '
 
-test_expect_success 'rebase--am.sh and --show-current-patch' '
+test_expect_success 'rebase --am and --show-current-patch' '
 	test_create_repo conflict-apply &&
 	(
 		cd conflict-apply &&
-- 
gitgitgadget

[PATCH v2 03/16] .gitignore: there is no longer a built-in `git-rebase--interactive`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This went away in 0609b741a4 (rebase -i: combine rebase--interactive.c
with rebase.c, 2019-04-17).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 .gitignore | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 875f3fc6e8..bcee4fda81 100644
--- a/.gitignore
+++ b/.gitignore
@@ -123,7 +123,6 @@
 /git-read-tree
 /git-rebase
 /git-rebase--common
-/git-rebase--interactive
 /git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
-- 
gitgitgadget

[PATCH v2 04/16] sequencer: the `am` and `rebase--interactive` scripts are gone

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Update a code comment that referred to those files as if they were still
there.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index f88a97fb10..334de14542 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -767,7 +767,7 @@ static int parse_key_value_squoted(char *buf, struct string_list *list)
  *	GIT_AUTHOR_DATE='$author_date'
  *
  * where $author_name, $author_email and $author_date are quoted. We are strict
- * with our parsing, as the file was meant to be eval'd in the old
+ * with our parsing, as the file was meant to be eval'd in the now-removed
  * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs
  * from what this function expects, it is better to bail out than to do
  * something that the user does not expect.
-- 
gitgitgadget

[PATCH v2 05/16] rebase: fold git-rebase--common into the -p backend

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The only remaining scripted part of `git rebase` is the
`--preserve-merges` backend. Meaning: there is little reason to keep the
"library of common rebase functions" as a separate file.

While moving the functions to `git-rebase--preserve-merges.sh`, we also
drop the `move_to_original_branch` function that is no longer used.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 .gitignore                     |  1 -
 Makefile                       |  1 -
 builtin/rebase.c               |  3 +-
 git-rebase--common.sh          | 69 ----------------------------------
 git-rebase--preserve-merges.sh | 55 +++++++++++++++++++++++++++
 5 files changed, 56 insertions(+), 73 deletions(-)
 delete mode 100644 git-rebase--common.sh

diff --git a/.gitignore b/.gitignore
index bcee4fda81..4470d7cfc0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -122,7 +122,6 @@
 /git-range-diff
 /git-read-tree
 /git-rebase
-/git-rebase--common
 /git-rebase--preserve-merges
 /git-receive-pack
 /git-reflog
diff --git a/Makefile b/Makefile
index 63e1973333..6c3bfb1733 100644
--- a/Makefile
+++ b/Makefile
@@ -624,7 +624,6 @@ SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_LIB += git-mergetool--lib
 SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--common
 SCRIPT_LIB += git-rebase--preserve-merges
 SCRIPT_LIB += git-sh-setup
 SCRIPT_LIB += git-sh-i18n
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4dd76ee576..74a60e8c83 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1163,8 +1163,7 @@ static int run_specific_rebase(struct rebase_options *opts, enum action action)
 	}
 
 	strbuf_addf(&script_snippet,
-		    ". git-sh-setup && . git-rebase--common &&"
-		    " . %s && %s", backend, backend_func);
+		    ". git-sh-setup && . %s && %s", backend, backend_func);
 	argv[0] = script_snippet.buf;
 
 	status = run_command_v_opt(argv, RUN_USING_SHELL);
diff --git a/git-rebase--common.sh b/git-rebase--common.sh
deleted file mode 100644
index f00e13e5d0..0000000000
--- a/git-rebase--common.sh
+++ /dev/null
@@ -1,69 +0,0 @@
-
-resolvemsg="
-$(gettext 'Resolve all conflicts manually, mark them as resolved with
-"git add/rm <conflicted_files>", then run "git rebase --continue".
-You can instead skip this commit: run "git rebase --skip".
-To abort and get back to the state before "git rebase", run "git rebase --abort".')
-"
-
-write_basic_state () {
-	echo "$head_name" > "$state_dir"/head-name &&
-	echo "$onto" > "$state_dir"/onto &&
-	echo "$orig_head" > "$state_dir"/orig-head &&
-	test t = "$GIT_QUIET" && : > "$state_dir"/quiet
-	test t = "$verbose" && : > "$state_dir"/verbose
-	test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
-	test -n "$strategy_opts" && echo "$strategy_opts" > \
-		"$state_dir"/strategy_opts
-	test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" > \
-		"$state_dir"/allow_rerere_autoupdate
-	test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt
-	test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff
-	test -n "$reschedule_failed_exec" && : > "$state_dir"/reschedule-failed-exec
-}
-
-apply_autostash () {
-	if test -f "$state_dir/autostash"
-	then
-		stash_sha1=$(cat "$state_dir/autostash")
-		if git stash apply $stash_sha1 >/dev/null 2>&1
-		then
-			echo "$(gettext 'Applied autostash.')" >&2
-		else
-			git stash store -m "autostash" -q $stash_sha1 ||
-			die "$(eval_gettext "Cannot store \$stash_sha1")"
-			gettext 'Applying autostash resulted in conflicts.
-Your changes are safe in the stash.
-You can run "git stash pop" or "git stash drop" at any time.
-' >&2
-		fi
-	fi
-}
-
-move_to_original_branch () {
-	case "$head_name" in
-	refs/*)
-		message="rebase finished: $head_name onto $onto"
-		git update-ref -m "$message" \
-			$head_name $(git rev-parse HEAD) $orig_head &&
-		git symbolic-ref \
-			-m "rebase finished: returning to $head_name" \
-			HEAD $head_name ||
-		die "$(eval_gettext "Could not move back to \$head_name")"
-		;;
-	esac
-}
-
-output () {
-	case "$verbose" in
-	'')
-		output=$("$@" 2>&1 )
-		status=$?
-		test $status != 0 && printf "%s\n" "$output"
-		return $status
-		;;
-	*)
-		"$@"
-		;;
-	esac
-}
diff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh
index afbb65765d..dec90e9af6 100644
--- a/git-rebase--preserve-merges.sh
+++ b/git-rebase--preserve-merges.sh
@@ -77,6 +77,61 @@ rewritten_pending="$state_dir"/rewritten-pending
 # and leaves CR at the end instead.
 cr=$(printf "\015")
 
+resolvemsg="
+$(gettext 'Resolve all conflicts manually, mark them as resolved with
+"git add/rm <conflicted_files>", then run "git rebase --continue".
+You can instead skip this commit: run "git rebase --skip".
+To abort and get back to the state before "git rebase", run "git rebase --abort".')
+"
+
+write_basic_state () {
+	echo "$head_name" > "$state_dir"/head-name &&
+	echo "$onto" > "$state_dir"/onto &&
+	echo "$orig_head" > "$state_dir"/orig-head &&
+	test t = "$GIT_QUIET" && : > "$state_dir"/quiet
+	test t = "$verbose" && : > "$state_dir"/verbose
+	test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
+	test -n "$strategy_opts" && echo "$strategy_opts" > \
+		"$state_dir"/strategy_opts
+	test -n "$allow_rerere_autoupdate" && echo "$allow_rerere_autoupdate" > \
+		"$state_dir"/allow_rerere_autoupdate
+	test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt
+	test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff
+	test -n "$reschedule_failed_exec" && : > "$state_dir"/reschedule-failed-exec
+}
+
+apply_autostash () {
+	if test -f "$state_dir/autostash"
+	then
+		stash_sha1=$(cat "$state_dir/autostash")
+		if git stash apply $stash_sha1 >/dev/null 2>&1
+		then
+			echo "$(gettext 'Applied autostash.')" >&2
+		else
+			git stash store -m "autostash" -q $stash_sha1 ||
+			die "$(eval_gettext "Cannot store \$stash_sha1")"
+			gettext 'Applying autostash resulted in conflicts.
+Your changes are safe in the stash.
+You can run "git stash pop" or "git stash drop" at any time.
+' >&2
+		fi
+	fi
+}
+
+output () {
+	case "$verbose" in
+	'')
+		output=$("$@" 2>&1 )
+		status=$?
+		test $status != 0 && printf "%s\n" "$output"
+		return $status
+		;;
+	*)
+		"$@"
+		;;
+	esac
+}
+
 strategy_args=${strategy:+--strategy=$strategy}
 test -n "$strategy_opts" &&
 eval '
-- 
gitgitgadget

[PATCH v2 06/16] t3427: add a clarifying comment

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The flow of this test script is outright confusing, and to start the
endeavor to address that, let's describe what this test is all about,
and how it tries to do it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index d8640522a0..3a2ae7b55d 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -11,6 +11,34 @@ commit_message() {
 	git log --pretty=format:%s -1 "$1"
 }
 
+# There are a few bugs in the rebase with regards to the subtree strategy, and
+# this test script tries to document them.  First, the following commit history
+# is generated (the onelines are shown, time flows from left to right):
+#
+# master1 - master2 - master3
+#                             \
+# README ---------------------- Add subproject master - master4 - files_subtree/master5
+#
+# Where the merge moves the files master[123].t into the subdirectory
+# files_subtree/ and master4 as well as files_subtree/master5 add files to that
+# directory directly.
+#
+# Then, in subsequent test cases, `git filter-branch` is used to distill just
+# the commits that touch files_subtree/. To give it a final pre-rebase touch,
+# an empty commit is added on top. The pre-rebase commit history looks like
+# this:
+#
+# Add subproject master - master4 - files_subtree/master5 - Empty commit
+#
+# where the root commit adds three files: master1.t, master2.t and master3.t.
+#
+# This commit history is then rebased onto `master3` with the
+# `-Xsubtree=files_subtree` option in three different ways:
+#
+# 1. using `--preserve-merges`
+# 2. using `--preserve-merges` and --keep-empty
+# 3. without specifying a rebase backend
+
 test_expect_success 'setup' '
 	test_commit README &&
 	mkdir files &&
-- 
gitgitgadget

[PATCH v2 07/16] t3427: simplify the `setup` test case significantly

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

It still does the very same thing as before, but expresses it in a much
more succinct (and still quite readable) manner.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 3a2ae7b55d..36c4f92e06 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -41,27 +41,21 @@ commit_message() {
 
 test_expect_success 'setup' '
 	test_commit README &&
-	mkdir files &&
-	(
-		cd files &&
-		git init &&
-		test_commit master1 &&
-		test_commit master2 &&
-		test_commit master3
-	) &&
-	git fetch files master &&
-	git branch files-master FETCH_HEAD &&
-	git read-tree --prefix=files_subtree files-master &&
-	git checkout -- files_subtree &&
-	tree=$(git write-tree) &&
-	head=$(git rev-parse HEAD) &&
-	rev=$(git rev-parse --verify files-master^0) &&
-	commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) &&
-	git update-ref HEAD $commit &&
-	(
-		cd files_subtree &&
-		test_commit master4
-	) &&
+
+	git init files &&
+	test_commit -C files master1 &&
+	test_commit -C files master2 &&
+	test_commit -C files master3 &&
+
+	: perform subtree merge into files_subtree/ &&
+	git fetch files refs/heads/master:refs/heads/files-master &&
+	git merge -s ours --no-commit --allow-unrelated-histories \
+		files-master &&
+	git read-tree --prefix=files_subtree -u files-master &&
+	git commit -m "Add subproject master" &&
+
+	: add two extra commits to rebase &&
+	test_commit -C files_subtree master4 &&
 	test_commit files_subtree/master5
 '
 
-- 
gitgitgadget

[PATCH v2 08/16] t3427: move the `filter-branch` invocation into the `setup` case

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The step to prepare a pre-rebase commit history is _identical_ in _all_
of the test cases (except of course the `setup` case). It should
therefore clearly a part of the `setup` test case instead.

As the `git filter-branch` command is quite costly on platforms where
Unix shell scripting is simply slow (meaning: on Windows), this shaves
off a noticeable part of the runtime: in this developer's setup, the
time was reduced from ~1m25s to ~1m.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 38 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 36c4f92e06..64ba95f3e0 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -56,16 +56,18 @@ test_expect_success 'setup' '
 
 	: add two extra commits to rebase &&
 	test_commit -C files_subtree master4 &&
-	test_commit files_subtree/master5
+	test_commit files_subtree/master5 &&
+
+	git checkout -b to-rebase &&
+	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git commit -m "Empty commit" --allow-empty
 '
 
 # FAILURE: Does not preserve master4.
 test_expect_failure REBASE_P \
 	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-preserve-merges-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
 '
@@ -74,9 +76,7 @@ test_expect_failure REBASE_P \
 test_expect_failure REBASE_P \
 	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-preserve-merges-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
@@ -85,9 +85,7 @@ test_expect_failure REBASE_P \
 test_expect_failure REBASE_P \
 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
 '
@@ -96,9 +94,7 @@ test_expect_failure REBASE_P \
 test_expect_failure REBASE_P \
 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
 '
@@ -107,9 +103,7 @@ test_expect_failure REBASE_P \
 test_expect_failure REBASE_P \
 	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-keep-empty-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
@@ -117,9 +111,7 @@ test_expect_failure REBASE_P \
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
 	reset_rebase &&
-	git checkout -b rebase-onto-4 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-4 to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
 '
@@ -127,18 +119,14 @@ test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto commit 5' '
 	reset_rebase &&
-	git checkout -b rebase-onto-5 master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-5 to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
 '
 # FAILURE: fatal: Could not parse object
 test_expect_failure 'Rebase -Xsubtree --onto empty commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-empty master &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
-	git commit -m "Empty commit" --allow-empty &&
+	git checkout -b rebase-onto-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
-- 
gitgitgadget

[PATCH v2 09/16] t3427: condense the unnecessarily repetitive test cases into three

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Previously, this test script performed essentially three rebases and
verified breakages by testing the post-rebase commits' messages.

To do so, the rebases were performed multiple times, though, once per
commit message to test. This wastes electricity (and CO2) and time.

Let's condense the test cases to the essential number: the number of
different rebases to validate.

On Windows, where the scripted nature of the `--preserve-merges` backend
hurts performance rather badly, this reduces the overall runtime in this
developer's setup from ~1m to ~28s while still performing the exact same
testing as before.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 61 +++++++--------------------------------
 1 file changed, 11 insertions(+), 50 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 64ba95f3e0..b21925d279 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -64,70 +64,31 @@ test_expect_success 'setup' '
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --preserve-merges --onto commit 4' '
+test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-preserve-merges-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master4"
-'
-
-# FAILURE: Does not preserve master5.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --preserve-merges --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-preserve-merges-5 to-rebase &&
+	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master4" &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
 
 # FAILURE: Does not preserve master4.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 4' '
+test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-keep-empty-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
-'
-
-# FAILURE: Does not preserve master5.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-5 to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-
-# FAILURE: Does not preserve Empty.
-test_expect_failure REBASE_P \
-	'Rebase -Xsubtree --keep-empty --preserve-merges --onto empty commit' '
-	reset_rebase &&
-	git checkout -b rebase-keep-empty-empty to-rebase &&
+	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
+	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
 # FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 4' '
-	reset_rebase &&
-	git checkout -b rebase-onto-4 to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4"
-'
-
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit 5' '
-	reset_rebase &&
-	git checkout -b rebase-onto-5 to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master5"
-'
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto empty commit' '
+test_expect_failure 'Rebase -Xsubtree --onto commit' '
 	reset_rebase &&
-	git checkout -b rebase-onto-empty to-rebase &&
+	git checkout -b rebase-onto to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
+	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-- 
gitgitgadget

[PATCH v2 10/16] t3427: fix erroneous assumption

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Apart from the `setup` test case, `t3427-rebase-subtree.sh` is made up
exclusively of demonstrations of breakages. The tricky thing about such
demonstrations is that they are often buggy themselves.

In this instance, somewhere over the course of the six iterations
of the patch that eventually made it into Git's `master` as 5f35900849e
(contrib/subtree: Add a test for subtree rebase that loses commits,
2016-06-28), the commit message "files_subtree/master4" was changed to
just "master4", but the test cases still expected the old commit
message.

Let's fix this, at long last.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index b21925d279..94cc532e10 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -68,7 +68,7 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --preserve-merges --onto commit'
 	reset_rebase &&
 	git checkout -b rebase-preserve-merges to-rebase &&
 	git rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~)" = "master4" &&
 	verbose test "$(commit_message HEAD)" = "files_subtree/master5"
 '
 
@@ -77,7 +77,7 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --
 	reset_rebase &&
 	git checkout -b rebase-keep-empty to-rebase &&
 	git rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
@@ -87,7 +87,7 @@ test_expect_failure 'Rebase -Xsubtree --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
 	git rebase -Xsubtree=files_subtree --onto files-master master &&
-	verbose test "$(commit_message HEAD~2)" = "files_subtree/master4" &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
-- 
gitgitgadget

[PATCH v2 11/16] t3427: accommodate for the `rebase --merge` backend having been replaced

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Since 68aa495b590 (rebase: implement --merge via the interactive
machinery, 2018-12-11), the job of the old `--merge` backend is now
performed by the `--interactive` backend, too.

One consequence is that empty commits are no longer rebased by default.

Meaning that the test case that calls `git rebase -Xsubtree` (which used
to be handled by the `--merge` backend) now needs to ask explicitly for
the empty commit to be rebased.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 94cc532e10..a734716ea3 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -83,10 +83,10 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --
 '
 
 # FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --onto commit' '
+test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
-	git rebase -Xsubtree=files_subtree --onto files-master master &&
+	git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
 	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
-- 
gitgitgadget

[PATCH v2 12/16] t3427: fix another incorrect assumption

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The test case that concerns `git rebase -Xsubtree` (with the
default rebase backend, not with `--preserve-merges`) starts out with a
pre-rebase commit history that begins with a commit that introduces
three files: master1.t, master2.t and master3.t.

This commit was generated by passing a subtree merge commit through `git
filter-branch --subdirectory-filter`, so it looks as if this commit
really introduces all those files.

The commit history onto which this commit is then rebased, however,
introduced those files in individual commits. For that reason, the
rebase will fail, it _must_ fail, because the first `pick` results in no
changes to be committed.

Let's fix the test case to expect exactly this situation.

With this change, we can mark the original bug that this test case tried
to demonstrate as fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3427-rebase-subtree.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index a734716ea3..7a37235768 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -82,11 +82,12 @@ test_expect_failure REBASE_P 'Rebase -Xsubtree --keep-empty --preserve-merges --
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
-# FAILURE: fatal: Could not parse object
-test_expect_failure 'Rebase -Xsubtree --keep-empty --onto commit' '
+test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
 	reset_rebase &&
 	git checkout -b rebase-onto to-rebase &&
-	git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --onto files-master master &&
+	: first pick results in no changes &&
+	git rebase --continue &&
 	verbose test "$(commit_message HEAD~2)" = "master4" &&
 	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
 	verbose test "$(commit_message HEAD)" = "Empty commit"
-- 
gitgitgadget

[PATCH v2 13/16] rebase -r: support merge strategies other than `recursive`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

We already support merge strategies in the sequencer, but only for
`pick` commands.

With this commit, we now also support them in `merge` commands. The
approach is simple: if any merge strategy option is specified, or if any
merge strategy other than `recursive` is specified, we simply spawn the
`git merge` command. Otherwise, we handle the merge in-process just as
before.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-rebase.txt           |  2 --
 builtin/rebase.c                       |  9 ---------
 sequencer.c                            | 14 ++++++++++++--
 t/t3422-rebase-incompatible-options.sh | 10 ----------
 t/t3430-rebase-merges.sh               | 21 +++++++++++++++++++++
 5 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 5e4e927647..bc620c44e9 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -543,8 +543,6 @@ In addition, the following pairs of options are incompatible:
  * --preserve-merges and --interactive
  * --preserve-merges and --signoff
  * --preserve-merges and --rebase-merges
- * --rebase-merges and --strategy
- * --rebase-merges and --strategy-option
 
 BEHAVIORAL DIFFERENCES
 -----------------------
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 74a60e8c83..625f50c637 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1811,15 +1811,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			      "'--reschedule-failed-exec'"));
 	}
 
-	if (options.rebase_merges) {
-		if (strategy_options.nr)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy-option'"));
-		if (options.strategy)
-			die(_("cannot combine '--rebase-merges' with "
-			      "'--strategy'"));
-	}
-
 	if (!options.root) {
 		if (argc < 1) {
 			struct branch *branch;
diff --git a/sequencer.c b/sequencer.c
index 334de14542..d228448cd8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3256,6 +3256,9 @@ static int do_merge(struct repository *r,
 	struct commit *head_commit, *merge_commit, *i;
 	struct commit_list *bases, *j, *reversed = NULL;
 	struct commit_list *to_merge = NULL, **tail = &to_merge;
+	const char *strategy = !opts->xopts_nr &&
+		(!opts->strategy || !strcmp(opts->strategy, "recursive")) ?
+		NULL : opts->strategy;
 	struct merge_options o;
 	int merge_arg_len, oneline_offset, can_fast_forward, ret, k;
 	static struct lock_file lock;
@@ -3404,7 +3407,7 @@ static int do_merge(struct repository *r,
 		goto leave_merge;
 	}
 
-	if (to_merge->next) {
+	if (strategy || to_merge->next) {
 		/* Octopus merge */
 		struct child_process cmd = CHILD_PROCESS_INIT;
 
@@ -3418,7 +3421,14 @@ static int do_merge(struct repository *r,
 		cmd.git_cmd = 1;
 		argv_array_push(&cmd.args, "merge");
 		argv_array_push(&cmd.args, "-s");
-		argv_array_push(&cmd.args, "octopus");
+		if (!strategy)
+			argv_array_push(&cmd.args, "octopus");
+		else {
+			argv_array_push(&cmd.args, strategy);
+			for (k = 0; k < opts->xopts_nr; k++)
+				argv_array_pushf(&cmd.args,
+						 "-X%s", opts->xopts[k]);
+		}
 		argv_array_push(&cmd.args, "--no-edit");
 		argv_array_push(&cmd.args, "--no-ff");
 		argv_array_push(&cmd.args, "--no-log");
diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
index a5868ea152..50e7960702 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -76,14 +76,4 @@ test_expect_success REBASE_P \
 	test_must_fail git rebase --preserve-merges --rebase-merges A
 '
 
-test_expect_success '--rebase-merges incompatible with --strategy' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -s resolve A
-'
-
-test_expect_success '--rebase-merges incompatible with --strategy-option' '
-	git checkout B^0 &&
-	test_must_fail git rebase --rebase-merges -Xignore-space-change A
-'
-
 test_done
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 42ba5b9f09..8ea6ff3548 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -412,4 +412,25 @@ test_expect_success '--continue after resolving conflicts after a merge' '
 	test_path_is_missing .git/MERGE_HEAD
 '
 
+test_expect_success '--rebase-merges with strategies' '
+	git checkout -b with-a-strategy F &&
+	test_tick &&
+	git merge -m "Merge conflicting-G" conflicting-G &&
+
+	: first, test with a merge strategy option &&
+	git rebase -ir -Xtheirs G &&
+	echo conflicting-G >expect &&
+	test_cmp expect G.t &&
+
+	: now, try with a merge strategy other than recursive &&
+	git reset --hard @{1} &&
+	write_script git-merge-override <<-\EOF &&
+	echo overridden$1 >>G.t
+	git add G.t
+	EOF
+	PATH="$PWD:$PATH" git rebase -ir -s override -Xxopt G &&
+	test_write_lines G overridden--xopt >expect &&
+	test_cmp expect G.t
+'
+
 test_done
-- 
gitgitgadget

[PATCH v2 14/16] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The format of the todo list is quite a bit different in the
`--rebase-merges` mode; Let's prepare the fake editor to handle those
todo lists properly, too.

The original idea was that we keep the original command unless
overridden, and because the original todo lists only had `pick` lines
anyway, we could be sloppy and "override" the command by the same
command (i.e. use the sed replacement pattern "pick" instead of "&").

This actually would not have worked with `fixup` and `squash` commands,
but it would appear that we never tried to use the fake editor with
`--autosquash`.

However, in the next commit we want to use the fake editor in
conjunction with `--rebase-merges`, so let's use the correct sed
replacement pattern.

Technically, it is not necessary to take care of the `fakesha` thing
(where we reuse the sed replacement pattern to craft a new todo
command), at least for now, as the only user of that thing overrides the
`action` anyway. Nevertheless, for completeness' sake, we do take care
of it.

Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/lib-rebase.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 7ea30e5006..6d87961e41 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -44,10 +44,10 @@ set_fake_editor () {
 	rm -f "$1"
 	echo 'rebase -i script before editing:'
 	cat "$1".tmp
-	action=pick
+	action=\&
 	for line in $FAKE_LINES; do
 		case $line in
-		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
+		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
 			action="$line";;
 		exec_*|x_*|break|b)
 			echo "$line" | sed 's/_/ /g' >> "$1";;
@@ -58,11 +58,12 @@ set_fake_editor () {
 		bad)
 			action="badcmd";;
 		fakesha)
+			test \& != "$action" || action=pick
 			echo "$action XXXXXXX False commit" >> "$1"
 			action=pick;;
 		*)
-			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
-			action=pick;;
+			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
+			action=\&;;
 		esac
 	done
 	echo 'rebase -i script after editing:'
-- 
gitgitgadget

[PATCH v2 16/16] rebase -r: do not (re-)generate root commits with `--root` *and* `--onto`

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When rebasing a complete commit history onto a given commit, it is
pretty obvious that the root commits should be rebased on top of said
given commit.

To test this, let's kill two birds with one stone and add a test case to
t3427-rebase-subtree.sh that not only demonstrates that this works, but
also that `git rebase -r` works with merge strategies now.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/rebase.c          |  7 +++++--
 sequencer.c               |  4 +++-
 sequencer.h               |  6 ++++++
 t/t3427-rebase-subtree.sh | 11 +++++++++++
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 625f50c637..ee2bc8b032 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -62,7 +62,7 @@ struct rebase_options {
 	const char *onto_name;
 	const char *revisions;
 	const char *switch_to;
-	int root;
+	int root, root_with_onto;
 	struct object_id *squash_onto;
 	struct commit *restrict_revision;
 	int dont_finish_rebase;
@@ -374,6 +374,7 @@ static int run_rebase_interactive(struct rebase_options *opts,
 	flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
 	flags |= opts->rebase_merges ? TODO_LIST_REBASE_MERGES : 0;
 	flags |= opts->rebase_cousins > 0 ? TODO_LIST_REBASE_COUSINS : 0;
+	flags |= opts->root_with_onto ? TODO_LIST_ROOT_WITH_ONTO : 0;
 	flags |= command == ACTION_SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
 	switch (command) {
@@ -1841,7 +1842,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 			options.squash_onto = &squash_onto;
 			options.onto_name = squash_onto_name =
 				xstrdup(oid_to_hex(&squash_onto));
-		}
+		} else
+			options.root_with_onto = 1;
+
 		options.upstream_name = NULL;
 		options.upstream = NULL;
 		if (argc > 1)
diff --git a/sequencer.c b/sequencer.c
index d228448cd8..ca119c84e5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4440,6 +4440,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 {
 	int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
 	int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
+	int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
 	struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
 	struct strbuf label = STRBUF_INIT;
 	struct commit_list *commits = NULL, **tail = &commits, *iter;
@@ -4606,7 +4607,8 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 
 		if (!commit)
 			strbuf_addf(out, "%s %s\n", cmd_reset,
-				    rebase_cousins ? "onto" : "[new root]");
+				    rebase_cousins || root_with_onto ?
+				    "onto" : "[new root]");
 		else {
 			const char *to = NULL;
 
diff --git a/sequencer.h b/sequencer.h
index 0c494b83d4..d506081d3c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -142,6 +142,12 @@ int sequencer_remove_state(struct replay_opts *opts);
  */
 #define TODO_LIST_REBASE_COUSINS (1U << 4)
 #define TODO_LIST_APPEND_TODO_HELP (1U << 5)
+/*
+ * When generating a script that rebases merges with `--root` *and* with
+ * `--onto`, we do not want to re-generate the root commits.
+ */
+#define TODO_LIST_ROOT_WITH_ONTO (1U << 6)
+
 
 int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 			  const char **argv, unsigned flags);
diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 7a37235768..39e348de16 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -93,4 +93,15 @@ test_expect_success 'Rebase -Xsubtree --keep-empty --onto commit' '
 	verbose test "$(commit_message HEAD)" = "Empty commit"
 '
 
+test_expect_success 'Rebase -Xsubtree --keep-empty --rebase-merges --onto commit' '
+	reset_rebase &&
+	git checkout -b rebase-merges-onto to-rebase &&
+	test_must_fail git rebase -Xsubtree=files_subtree --keep-empty --rebase-merges --onto files-master --root &&
+	: first pick results in no changes &&
+	git rebase --continue &&
+	verbose test "$(commit_message HEAD~2)" = "master4" &&
+	verbose test "$(commit_message HEAD~)" = "files_subtree/master5" &&
+	verbose test "$(commit_message HEAD)" = "Empty commit"
+'
+
 test_done
-- 
gitgitgadget

[PATCH v2 15/16] t3418: test `rebase -r` with merge strategies

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

There is a test case in this script that verifies that `git rebase
--preserve-merges` works all right with non-default merge strategies or
non-default merge strategy options.

Now that `git rebase --rebase-merges` learned about merge strategies,
let's copy-edit this test case to verify that that works as intended,
too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3418-rebase-continue.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index bdaa511bb0..fbf9addfd1 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -120,6 +120,20 @@ test_expect_success REBASE_P 'rebase passes merge strategy options correctly' '
 	git rebase --continue
 '
 
+test_expect_success 'rebase -r passes merge strategy options correctly' '
+	rm -fr .git/rebase-* &&
+	git reset --hard commit-new-file-F3-on-topic-branch &&
+	test_commit merge-theirs &&
+	git reset --hard HEAD^ &&
+	test_commit some-other-commit &&
+	test_tick &&
+	git merge --no-ff merge-theirs &&
+	FAKE_LINES="1 3 edit 4 5 7 8 9" git rebase -i -f -r -m \
+		-s recursive --strategy-option=theirs HEAD~2 &&
+	test_commit force-change-ours &&
+	git rebase --continue
+'
+
 test_expect_success '--skip after failed fixup cleans commit message' '
 	test_when_finished "test_might_fail git rebase --abort" &&
 	git checkout -b with-conflicting-fixup &&
-- 
gitgitgadget

[PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import

[Elijah Newren]

fast-export and fast-import can easily handle the simple rewrite that
was being done by filter-branch, and should be faster on systems with a
slow fork.  Measuring the overall time taken for all of t3427 (not just
the difference between filter-branch and fast-export/fast-import) shows
a speedup of about 5% on Linux and 11% on Mac.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
This patch is meant to be added onto the end of js/rebase-r-strategy; an
earlier version of this patch conflicted js/rebase-r-strategy so now I'm
basing on top of that series.  The speedup is also less impressive now
that there is only one filter-branch invocation being replaced instead of
a handful.  Still a nice speedup, though.

 t/t3427-rebase-subtree.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
index 39e348de16..bec48e6a1f 100755
--- a/t/t3427-rebase-subtree.sh
+++ b/t/t3427-rebase-subtree.sh
@@ -59,7 +59,10 @@ test_expect_success 'setup' '
 	test_commit files_subtree/master5 &&
 
 	git checkout -b to-rebase &&
-	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
+	git fast-export --no-data HEAD -- files_subtree/ |
+		sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" |
+		git fast-import --force --quiet &&
+	git reset --hard &&
 	git commit -m "Empty commit" --allow-empty
 '
 
-- 
2.22.0.19.ga495766805

Re: [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import

[Johannes Schindelin]

Hi Elijah,

On Wed, 4 Sep 2019, Elijah Newren wrote:

> fast-export and fast-import can easily handle the simple rewrite that
> was being done by filter-branch, and should be faster on systems with a
> slow fork.  Measuring the overall time taken for all of t3427 (not just
> the difference between filter-branch and fast-export/fast-import) shows
> a speedup of about 5% on Linux and 11% on Mac.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> This patch is meant to be added onto the end of js/rebase-r-strategy; an
> earlier version of this patch conflicted js/rebase-r-strategy so now I'm
> basing on top of that series.  The speedup is also less impressive now
> that there is only one filter-branch invocation being replaced instead of
> a handful.  Still a nice speedup, though.

ACK!

Thanks,
Dscho

>
>  t/t3427-rebase-subtree.sh | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
> index 39e348de16..bec48e6a1f 100755
> --- a/t/t3427-rebase-subtree.sh
> +++ b/t/t3427-rebase-subtree.sh
> @@ -59,7 +59,10 @@ test_expect_success 'setup' '
>  	test_commit files_subtree/master5 &&
>
>  	git checkout -b to-rebase &&
> -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
> +	git fast-export --no-data HEAD -- files_subtree/ |
> +		sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" |
> +		git fast-import --force --quiet &&
> +	git reset --hard &&
>  	git commit -m "Empty commit" --allow-empty
>  '
>
> --
> 2.22.0.19.ga495766805
>
>

Re: [PATCH v2 17/16] t3427: accelerate this test by using fast-export and fast-import

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Elijah,
>
> On Wed, 4 Sep 2019, Elijah Newren wrote:
>
>> fast-export and fast-import can easily handle the simple rewrite that
>> was being done by filter-branch, and should be faster on systems with a
>> slow fork.  Measuring the overall time taken for all of t3427 (not just
>> the difference between filter-branch and fast-export/fast-import) shows
>> a speedup of about 5% on Linux and 11% on Mac.
>>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>> ---
>> This patch is meant to be added onto the end of js/rebase-r-strategy; an
>> earlier version of this patch conflicted js/rebase-r-strategy so now I'm
>> basing on top of that series.  The speedup is also less impressive now
>> that there is only one filter-branch invocation being replaced instead of
>> a handful.  Still a nice speedup, though.
>
> ACK!
>
> Thanks,
> Dscho

Thanks, both.  This indeed is a good update.

>
>>
>>  t/t3427-rebase-subtree.sh | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
>> index 39e348de16..bec48e6a1f 100755
>> --- a/t/t3427-rebase-subtree.sh
>> +++ b/t/t3427-rebase-subtree.sh
>> @@ -59,7 +59,10 @@ test_expect_success 'setup' '
>>  	test_commit files_subtree/master5 &&
>>
>>  	git checkout -b to-rebase &&
>> -	git filter-branch --prune-empty -f --subdirectory-filter files_subtree &&
>> +	git fast-export --no-data HEAD -- files_subtree/ |
>> +		sed -e "s%\([0-9a-f]\{40\} \)files_subtree/%\1%" |
>> +		git fast-import --force --quiet &&
>> +	git reset --hard &&
>>  	git commit -m "Empty commit" --allow-empty
>>  '
>>
>> --
>> 2.22.0.19.ga495766805
>>
>>