[PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Andreas Ericsson]

This patch adds two tests (really three, but one of
them just handles setup) which we currently expect
to fail.

One of them tests "git rebase -p", without the -i flag,
to make sure it works without phony editors and suchlike.

The other tests "git pull --rebase --preserve-merges"
to make sure that the same functionality exists there.

The test was originally written by Stephen Habermann
<stephen@exigencecorp.com> but has been significantly
modified since its creation.

Signed-off-by: Andreas Ericsson <ae@op5.se>
---

Stephen, I had to modify the tests a bit to get them to work with
how I implemented the merge-preserving rebase, and also to remove
a lot of the cruft that was previously in there. Hope you're ok
with the attribution in the commit message.

 t/t3409-rebase-preserve-merges.sh |   68 +++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100644 t/t3409-rebase-preserve-merges.sh

diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
new file mode 100644
index 0000000..532b220
--- /dev/null
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+#
+# Copyright(C) 2008 Stephen Habermann & Andreas Ericsson
+#
+test_description='git rebase -p should preserve merges
+
+This test runs various incantations of "git rebase -p" and checks
+that merges are properly carried along
+'
+. ./test-lib.sh
+
+GIT_AUTHOR_EMAIL=bogus_email_address
+export GIT_AUTHOR_EMAIL
+
+#echo 'Setting up:
+#
+#A1--A2  <-- origin/master
+# \   \
+#  B1--M  <-- topic
+#   \
+#    B2  <-- origin/topic
+#
+#'
+
+test_expect_success 'setup for merge-preserving rebase' \
+	'echo First > A &&
+	git add A &&
+	git-commit -m "Add A1" &&
+	git checkout -b topic &&
+	echo Second > B &&
+	git add B &&
+	git-commit -m "Add B1" &&
+	git checkout -f master &&
+	echo Third >> A &&
+	git-commit -a -m "Modify A2" &&
+
+	git clone ./. clone1 &&
+	cd clone1 &&
+	git checkout -b topic origin/topic &&
+	git merge origin/master &&
+	cd ..
+
+	git clone ./. clone2
+	cd clone2 &&
+	git checkout -b topic origin/topic &&
+	git merge origin/master &&
+	cd .. &&
+
+	git checkout topic &&
+	echo Fourth >> B &&
+	git commit -a -m "Modify B2"
+'
+
+test_expect_failure 'git pull --rebase -p on moved topic' '
+	cd clone1 &&
+	git pull --rebase --preserve-merges &&
+	test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
+'
+
+test_expect_failure 'rebase -p merge on moved topic' '
+	cd ../clone2 &&
+	git fetch &&
+	git rebase -p origin/topic &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+'
+
+test_done
-- 
1.6.0.2.307.gc4275.dirty

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Stephen Haberman]

> Stephen, I had to modify the tests a bit to get them to work with how
> I implemented the merge-preserving rebase, and also to remove a lot of
> the cruft that was previously in there. Hope you're ok with the
> attribution in the commit message.

No problem, it looks great.

This is awesome. Thanks for the insanely short turnaround. The
GIT_EDITOR=: hack is neat. I did not think it would be that simple.

- Stephen

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Andreas Ericsson]

Stephen Haberman wrote:
>> Stephen, I had to modify the tests a bit to get them to work with how
>> I implemented the merge-preserving rebase, and also to remove a lot of
>> the cruft that was previously in there. Hope you're ok with the
>> attribution in the commit message.
> 
> No problem, it looks great.
> 
> This is awesome. Thanks for the insanely short turnaround.


It requires a bit of testing though. All the t/t34* tests pass with
all the patches applied, and some manual tries worked just fine too,
but if you wanna give it a twirl where you work, that'd be great.


> The
> GIT_EDITOR=: hack is neat. I did not think it would be that simple.
> 

Actually, you should be able to use vanilla "git-rebase -i -p" without
getting an editor by doing something like this:

GIT_EDITOR=: git rebase -i -p

but recommending a hack like that to work around a UI deficiency didn't
really appeal to me. If Junio doesn't like the patches though, you could
try using that.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[SZEDER Gábor]

Hi Andreas,

First of all, thanks for the work!

On Tue, Sep 23, 2008 at 11:30:09PM +0200, Andreas Ericsson wrote:
> It requires a bit of testing though. All the t/t34* tests pass with
> all the patches applied, and some manual tries worked just fine too,
> but if you wanna give it a twirl where you work, that'd be great.
Unfortunately in my example workflow[1] posted earlier today your
patch series does not work in the way I would like it to behave.

The following DAG is created by the commands below:

  -A---B      master
    \
     C---M    topic
      \ /
       D

  git init
  echo 1 >foo
  git add foo
  git commit -m 'first on master'       # A
  echo 2 >>foo
  git commit -m 'second on master' foo  # B
  git checkout -b topic HEAD^
  echo 1 >bar
  git add bar
  git commit -m 'first on topic'        # C
  git checkout -b subtopic
  echo 1 >baz
  git add baz
  git commit -m 'first on subtopic'     # D
  git checkout topic
  git merge --no-ff subtopic            # M

If I now execute 'git rebase -p master topic', I get the following:

  -A---B            master
    \   \
     \   C'---M'    topic
      \      /
       C----D

But I would rather like to have the following:

  -A---B            master
        \
         C'---M'    topic
          \  /
           D'

Would such a behaviour possible at all?


Thanks,
Gábor


[1] http://article.gmane.org/gmane.comp.version-control.git/96548

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Johannes Sixt]

SZEDER Gábor schrieb:
>   -A---B      master
>     \
>      C---M    topic
>       \ /
>        D
...
> If I now execute 'git rebase -p master topic', I get the following:
> 
>   -A---B            master
>     \   \
>      \   C'---M'    topic
>       \      /
>        C----D
> 
> But I would rather like to have the following:
> 
>   -A---B            master
>         \
>          C'---M'    topic
>           \  /
>            D'
> 
> Would such a behaviour possible at all?

I think that rebase -i -p was meant to deal only with the situation where
the merged-in branch is from outside the topic branch. What you want is
called "git-sequencer". Search the archives.

-- Hannes

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Andreas Ericsson]

SZEDER Gábor wrote:
> Hi Andreas,
> 
> First of all, thanks for the work!
> 
> On Tue, Sep 23, 2008 at 11:30:09PM +0200, Andreas Ericsson wrote:
>> It requires a bit of testing though. All the t/t34* tests pass with
>> all the patches applied, and some manual tries worked just fine too,
>> but if you wanna give it a twirl where you work, that'd be great.
> Unfortunately in my example workflow[1] posted earlier today your
> patch series does not work in the way I would like it to behave.
> 
> The following DAG is created by the commands below:
> 
>   -A---B      master
>     \
>      C---M    topic
>       \ /
>        D
> 
>   git init
>   echo 1 >foo
>   git add foo
>   git commit -m 'first on master'       # A
>   echo 2 >>foo
>   git commit -m 'second on master' foo  # B
>   git checkout -b topic HEAD^
>   echo 1 >bar
>   git add bar
>   git commit -m 'first on topic'        # C
>   git checkout -b subtopic
>   echo 1 >baz
>   git add baz
>   git commit -m 'first on subtopic'     # D
>   git checkout topic
>   git merge --no-ff subtopic            # M
> 
> If I now execute 'git rebase -p master topic', I get the following:
> 
>   -A---B            master
>     \   \
>      \   C'---M'    topic
>       \      /
>        C----D
> 
> But I would rather like to have the following:
> 
>   -A---B            master
>         \
>          C'---M'    topic
>           \  /
>            D'
> 
> Would such a behaviour possible at all?
> 

See Johannes Sixt's reply (git sequencer).
What I provided was a hack to access existing functionality in a way
that was previously not possible. While that can be neat in itself,
the patch series doesn't alter how the merge-preserving rebase works
in the slightest.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Andreas Ericsson]

Stephen Haberman wrote:
>> Stephen, I had to modify the tests a bit to get them to work with how
>> I implemented the merge-preserving rebase, and also to remove a lot of
>> the cruft that was previously in there. Hope you're ok with the
>> attribution in the commit message.
> 
> No problem, it looks great.
> 
> This is awesome. Thanks for the insanely short turnaround. The
> GIT_EDITOR=: hack is neat. I did not think it would be that simple.
> 

Stephen, are you using this in production? How's it turning out?

Shawn, I haven't seen this in any of your branches. Overlooked or
dropped? I think 1-2 are probably master material, while I'm not
so sure about 3/3. Would you prefer a re-send that turns it into
a 2-patch series, adding each test with the functionality it tests?

Let me know how you want it and I'll work something up tomorrow
morning, gmt + 1.

Thanks

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Stephen Haberman]

> Stephen, are you using this in production?

Kind of--I have not distributed a patched version of pull. But I have
written test cases on our side and manually executing `GIT_EDITOR=:
git rebase -i -p` works very well.

Past occurrences aside, no one has needed to rebase a local merge yet.

> How's it turning out?

I think it's great, but the primary problem will be getting devs to
actually remember to use it. E.g. I don't think they will type out:

    git pull --rebase --preserve-rebase

Every time they pull. And they definitely don't do our current hack:

    git fetch
    GIT_EDITOR=: git rebase -i -p

I do have a wrapper shell script for people to use, but it hasn't seen
wide adoption yet. We have a draconian hook script that tries to
detect merges that should have been rebases and reject them, but
it's disabled for tweaking right now--when it gets turned back on,
I think more people will use the script.

In the long term, having "branch.name.preservemerges" and
"branch.autosetuppreservemerges" config options to parallel the
"branch.name.rebase" option and get us back to just "git pull"
would be great.

I've been meaning to submit patches for these two config options--I
figure I can hunt down how "branch.name.rebase" works and do the
appropriate copy/paste, but I haven't dedicated any time to it yet.

Thanks,
Stephen

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Shawn O. Pearce]

Andreas Ericsson <ae@op5.se> wrote:
>
> Shawn, I haven't seen this in any of your branches. Overlooked or
> dropped? I think 1-2 are probably master material, while I'm not
> so sure about 3/3. Would you prefer a re-send that turns it into
> a 2-patch series, adding each test with the functionality it tests?

Thanks for the reminder.  It just got lost in the shuffle.  I dragged
them out of the archives and will queue into this morning's update,
so no need for a resend.

-- 
Shawn.

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Andreas Ericsson]

Shawn O. Pearce wrote:
> Andreas Ericsson <ae@op5.se> wrote:
>> Shawn, I haven't seen this in any of your branches. Overlooked or
>> dropped? I think 1-2 are probably master material, while I'm not
>> so sure about 3/3. Would you prefer a re-send that turns it into
>> a 2-patch series, adding each test with the functionality it tests?
> 
> Thanks for the reminder.  It just got lost in the shuffle.  I dragged
> them out of the archives and will queue into this morning's update,
> so no need for a resend.
> 

Hold off on that if you haven't already applied them. I just noticed
something strange in passing 15 minutes ago that I need to investigate
a bit more. I need to get home now though, so I won't have time to
test it further until later tonight.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Shawn O. Pearce]

Andreas Ericsson <ae@op5.se> wrote:
> Shawn O. Pearce wrote:
>> Andreas Ericsson <ae@op5.se> wrote:
>>> Shawn, I haven't seen this in any of your branches. Overlooked or
>>> dropped? 
>>
>> Thanks for the reminder.  It just got lost in the shuffle.
>
> Hold off on that if you haven't already applied them. I just noticed
> something strange in passing 15 minutes ago that I need to investigate
> a bit more. I need to get home now though, so I won't have time to
> test it further until later tonight.

Its only in a topic branch right now.  I'll schedule them into 'pu'
today just so they are available, but I'll be happy to replace any
(or all) of the patches when you come up with something better.

-- 
Shawn.

[PATCH] Add branch.autosetuppreservemerges and branch.<name>.preservemerges.

[Stephen Haberman]

Signed-off-by: Stephen Haberman <stephen@exigencecorp.com>
---

This builds on top of Andreas's work on `git rebase -p`. I
basically copy/pasted how autosetuprebase works so that, if
appropriately configured, `git pull` will do the "right thing"
for our environment, i.e. rebasing and preserving merges.

I'm not sure how to handle patches on patches, so apologies
if I did this wrong. Let me know if there are things I should
be doing differently.

Thanks.

 Documentation/config.txt          |   20 ++++++++
 branch.c                          |   20 ++++++++
 cache.h                           |    9 ++++
 config.c                          |   15 ++++++
 environment.c                     |    1 +
 git-pull.sh                       |   10 +++-
 t/t3200-branch.sh                 |   89 ++++++++++++++++++++++++++++--------
 t/t3409-rebase-preserve-merges.sh |   65 ++++++++++++++++----------
 8 files changed, 182 insertions(+), 47 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bea867d..0abf1b8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -438,6 +438,21 @@ branch.autosetuprebase::
 	branch to track another branch.
 	This option defaults to never.
 
+branch.autosetuppreservemerges::
+	When a new branch is created with 'git-branch' or 'git-checkout'
+	that tracks another branch, this variable tells git to set
+	up pull to rebase with preserve merges (see "branch.<name>.rebase").
+	When `never`, rebase is never automatically set to true.
+	When `local`, rebase is set to true for tracked branches of
+	other local branches.
+	When `remote`, rebase is set to true for tracked branches of
+	remote branches.
+	When `always`, rebase will be set to true for all tracking
+	branches.
+	See "branch.autosetupmerge" for details on how to set up a
+	branch to track another branch.
+	This option defaults to never.
+
 branch.<name>.remote::
 	When in branch <name>, it tells 'git-fetch' which remote to fetch.
 	If this option is not given, 'git-fetch' defaults to remote "origin".
@@ -471,6 +486,11 @@ branch.<name>.rebase::
 	it unless you understand the implications (see linkgit:git-rebase[1]
 	for details).
 
+branch.<name>.preservemerges::
+	When true, and branch.<name>.rebase is true, preserve merges when
+	rebasing the branch <name> on top of the fetched branch when
+	"git pull" is run.
+
 browser.<tool>.cmd::
 	Specify the command to invoke the specified browser. The
 	specified command is evaluated in shell with the URLs passed
diff --git a/branch.c b/branch.c
index b1e59f2..a5e62c8 100644
--- a/branch.c
+++ b/branch.c
@@ -47,6 +47,21 @@ static int should_setup_rebase(const struct tracking *tracking)
 	return 0;
 }
 
+static int should_setup_preservemerges(const struct tracking *tracking)
+{
+	switch (autopreservemerges) {
+	case AUTOPRESERVEMERGES_NEVER:
+		return 0;
+	case AUTOPRESERVEMERGES_LOCAL:
+		return tracking->remote == NULL;
+	case AUTOPRESERVEMERGES_REMOTE:
+		return tracking->remote != NULL;
+	case AUTOPRESERVEMERGES_ALWAYS:
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * This is called when new_ref is branched off of orig_ref, and tries
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
@@ -91,6 +106,11 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 		git_config_set(key, "true");
 		printf("This branch will rebase on pull.\n");
 	}
+	if (should_setup_preservemerges(&tracking)) {
+		sprintf(key, "branch.%s.preservemerges", new_ref);
+		git_config_set(key, "true");
+		printf("This branch will preserve merges on pull.\n");
+	}
 	free(tracking.src);
 
 	return 0;
diff --git a/cache.h b/cache.h
index de8c2b6..97be98c 100644
--- a/cache.h
+++ b/cache.h
@@ -467,8 +467,17 @@ enum rebase_setup_type {
 	AUTOREBASE_ALWAYS,
 };
 
+enum preservemerges_setup_type {
+	AUTOPRESERVEMERGES_NEVER = 0,
+	AUTOPRESERVEMERGES_LOCAL,
+	AUTOPRESERVEMERGES_REMOTE,
+	AUTOPRESERVEMERGES_ALWAYS,
+};
+
+
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
+extern enum preservemerges_setup_type autopreservemerges;
 
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
diff --git a/config.c b/config.c
index 53f04a0..6302f5a 100644
--- a/config.c
+++ b/config.c
@@ -536,6 +536,21 @@ static int git_default_branch_config(const char *var, const char *value)
 			return error("Malformed value for %s", var);
 		return 0;
 	}
+	if (!strcmp(var, "branch.autosetuppreservemerges")) {
+		if (!value)
+			return config_error_nonbool(var);
+		else if (!strcmp(value, "never"))
+			autopreservemerges = AUTOPRESERVEMERGES_NEVER;
+		else if (!strcmp(value, "local"))
+			autopreservemerges = AUTOPRESERVEMERGES_LOCAL;
+		else if (!strcmp(value, "remote"))
+			autopreservemerges = AUTOPRESERVEMERGES_REMOTE;
+		else if (!strcmp(value, "always"))
+			autopreservemerges = AUTOPRESERVEMERGES_ALWAYS;
+		else
+			return error("Malformed value for %s", var);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index 0c6d11f..72e735c 100644
--- a/environment.c
+++ b/environment.c
@@ -42,6 +42,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+enum preservemerges_setup_type autopreservemerges = AUTOPRESERVEMERGES_NEVER;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
diff --git a/git-pull.sh b/git-pull.sh
index 270a50d..03b7da0 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -20,6 +20,7 @@ strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
+preservemerges=$(git config --bool branch.$curr_branch_short.preservemerges)
 while :
 do
 	case "$1" in
@@ -59,7 +60,7 @@ do
 		rebase=true
 		;;
 	--preserve-merges) # no short option for this
-		preserve_merges="--preserve-merges"
+		preservemerges=true
 		rebase=true
 		;;
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
@@ -181,9 +182,14 @@ then
 	exit
 fi
 
+if test true = "$preservemerges"
+then
+	preservemerges_flag="--preserve-merges"
+fi
+
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
-	exec git-rebase $preserve_merges $strategy_args --onto $merge_head \
+	exec git-rebase $preservemerges_flag $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
 exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 2147eac..f10b8ac 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -228,103 +228,121 @@ test_expect_success 'autosetuprebase local on a tracked local branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase local &&
+	git config branch.autosetuppreservemerges local &&
 	(git show-ref -q refs/remotes/local/o || git fetch local) &&
 	git branch mybase &&
 	git branch --track myr1 mybase &&
 	test "$(git config branch.myr1.remote)" = . &&
 	test "$(git config branch.myr1.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr1.rebase)" = true
+	test "$(git config branch.myr1.rebase)" = true &&
+	test "$(git config branch.myr1.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase always on a tracked local branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase always &&
+	git config branch.autosetuppreservemerges always &&
 	(git show-ref -q refs/remotes/local/o || git fetch local) &&
 	git branch mybase2 &&
 	git branch --track myr2 mybase &&
 	test "$(git config branch.myr2.remote)" = . &&
 	test "$(git config branch.myr2.merge)" = refs/heads/mybase &&
-	test "$(git config branch.myr2.rebase)" = true
+	test "$(git config branch.myr2.rebase)" = true &&
+	test "$(git config branch.myr2.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase remote on a tracked local branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase remote &&
+	git config branch.autosetuppreservemerges remote &&
 	(git show-ref -q refs/remotes/local/o || git fetch local) &&
 	git branch mybase3 &&
 	git branch --track myr3 mybase2 &&
 	test "$(git config branch.myr3.remote)" = . &&
 	test "$(git config branch.myr3.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr3.rebase)" = true
+	! test "$(git config branch.myr3.rebase)" = true &&
+	! test "$(git config branch.myr3.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase never on a tracked local branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase never &&
+	git config branch.autosetuppreservemerges never &&
 	(git show-ref -q refs/remotes/local/o || git fetch local) &&
 	git branch mybase4 &&
 	git branch --track myr4 mybase2 &&
 	test "$(git config branch.myr4.remote)" = . &&
 	test "$(git config branch.myr4.merge)" = refs/heads/mybase2 &&
-	! test "$(git config branch.myr4.rebase)" = true
+	! test "$(git config branch.myr4.rebase)" = true &&
+	! test "$(git config branch.myr4.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase local on a tracked remote branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase local &&
+	git config branch.autosetuppreservemerges local &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --track myr5 local/master &&
 	test "$(git config branch.myr5.remote)" = local &&
 	test "$(git config branch.myr5.merge)" = refs/heads/master &&
-	! test "$(git config branch.myr5.rebase)" = true
+	! test "$(git config branch.myr5.rebase)" = true &&
+	! test "$(git config branch.myr5.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase never on a tracked remote branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase never &&
+	git config branch.autosetuppreservemerges never &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --track myr6 local/master &&
 	test "$(git config branch.myr6.remote)" = local &&
 	test "$(git config branch.myr6.merge)" = refs/heads/master &&
-	! test "$(git config branch.myr6.rebase)" = true
+	! test "$(git config branch.myr6.rebase)" = true &&
+	! test "$(git config branch.myr6.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase remote on a tracked remote branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase remote &&
+	git config branch.autosetuppreservemerges remote &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --track myr7 local/master &&
 	test "$(git config branch.myr7.remote)" = local &&
 	test "$(git config branch.myr7.merge)" = refs/heads/master &&
-	test "$(git config branch.myr7.rebase)" = true
+	test "$(git config branch.myr7.rebase)" = true &&
+	test "$(git config branch.myr7.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase always on a tracked remote branch' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	git config branch.autosetuprebase remote &&
+	git config branch.autosetuppreservemerges remote &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --track myr8 local/master &&
 	test "$(git config branch.myr8.remote)" = local &&
 	test "$(git config branch.myr8.merge)" = refs/heads/master &&
-	test "$(git config branch.myr8.rebase)" = true
+	test "$(git config branch.myr8.rebase)" = true &&
+	test "$(git config branch.myr8.preservemerges)" = true
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked remote branch' '
 	git config --unset branch.autosetuprebase &&
+	git config --unset branch.autosetuppreservemerges &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --track myr9 local/master &&
 	test "$(git config branch.myr9.remote)" = local &&
 	test "$(git config branch.myr9.merge)" = refs/heads/master &&
-	test "z$(git config branch.myr9.rebase)" = z
+	test "z$(git config branch.myr9.rebase)" = z &&
+	test "z$(git config branch.myr9.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
@@ -335,7 +353,8 @@ test_expect_success 'autosetuprebase unconfigured on a tracked local branch' '
 	git branch --track myr10 mybase2 &&
 	test "$(git config branch.myr10.remote)" = . &&
 	test "$(git config branch.myr10.merge)" = refs/heads/mybase2 &&
-	test "z$(git config branch.myr10.rebase)" = z
+	test "z$(git config branch.myr10.rebase)" = z &&
+	test "z$(git config branch.myr10.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
@@ -345,7 +364,8 @@ test_expect_success 'autosetuprebase unconfigured on untracked local branch' '
 	git branch --no-track myr11 mybase2 &&
 	test "z$(git config branch.myr11.remote)" = z &&
 	test "z$(git config branch.myr11.merge)" = z &&
-	test "z$(git config branch.myr11.rebase)" = z
+	test "z$(git config branch.myr11.rebase)" = z &&
+	test "z$(git config branch.myr11.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
@@ -355,95 +375,112 @@ test_expect_success 'autosetuprebase unconfigured on untracked remote branch' '
 	git branch --no-track myr12 local/master &&
 	test "z$(git config branch.myr12.remote)" = z &&
 	test "z$(git config branch.myr12.merge)" = z &&
-	test "z$(git config branch.myr12.rebase)" = z
+	test "z$(git config branch.myr12.rebase)" = z &&
+	test "z$(git config branch.myr12.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase never on an untracked local branch' '
 	git config branch.autosetuprebase never &&
+	git config branch.autosetuppreservemerges never &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr13 mybase2 &&
 	test "z$(git config branch.myr13.remote)" = z &&
 	test "z$(git config branch.myr13.merge)" = z &&
-	test "z$(git config branch.myr13.rebase)" = z
+	test "z$(git config branch.myr13.rebase)" = z &&
+	test "z$(git config branch.myr13.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase local on an untracked local branch' '
 	git config branch.autosetuprebase local &&
+	git config branch.autosetuppreservemerges local &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr14 mybase2 &&
 	test "z$(git config branch.myr14.remote)" = z &&
 	test "z$(git config branch.myr14.merge)" = z &&
-	test "z$(git config branch.myr14.rebase)" = z
+	test "z$(git config branch.myr14.rebase)" = z &&
+	test "z$(git config branch.myr14.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase remote on an untracked local branch' '
 	git config branch.autosetuprebase remote &&
+	git config branch.autosetuppreservemerges remote &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr15 mybase2 &&
 	test "z$(git config branch.myr15.remote)" = z &&
 	test "z$(git config branch.myr15.merge)" = z &&
-	test "z$(git config branch.myr15.rebase)" = z
+	test "z$(git config branch.myr15.rebase)" = z &&
+	test "z$(git config branch.myr15.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase always on an untracked local branch' '
 	git config branch.autosetuprebase always &&
+	git config branch.autosetuppreservemerges always &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr16 mybase2 &&
 	test "z$(git config branch.myr16.remote)" = z &&
 	test "z$(git config branch.myr16.merge)" = z &&
-	test "z$(git config branch.myr16.rebase)" = z
+	test "z$(git config branch.myr16.rebase)" = z &&
+	test "z$(git config branch.myr16.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase never on an untracked remote branch' '
 	git config branch.autosetuprebase never &&
+	git config branch.autosetuppreservemerges never &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr17 local/master &&
 	test "z$(git config branch.myr17.remote)" = z &&
 	test "z$(git config branch.myr17.merge)" = z &&
-	test "z$(git config branch.myr17.rebase)" = z
+	test "z$(git config branch.myr17.rebase)" = z &&
+	test "z$(git config branch.myr17.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase local on an untracked remote branch' '
 	git config branch.autosetuprebase local &&
+	git config branch.autosetuppreservemerges local &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr18 local/master &&
 	test "z$(git config branch.myr18.remote)" = z &&
 	test "z$(git config branch.myr18.merge)" = z &&
-	test "z$(git config branch.myr18.rebase)" = z
+	test "z$(git config branch.myr18.rebase)" = z &&
+	test "z$(git config branch.myr18.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase remote on an untracked remote branch' '
 	git config branch.autosetuprebase remote &&
+	git config branch.autosetuppreservemerges remote &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr19 local/master &&
 	test "z$(git config branch.myr19.remote)" = z &&
 	test "z$(git config branch.myr19.merge)" = z &&
-	test "z$(git config branch.myr19.rebase)" = z
+	test "z$(git config branch.myr19.rebase)" = z &&
+	test "z$(git config branch.myr19.preservemerges)" = z
 '
 
 test_expect_success 'autosetuprebase always on an untracked remote branch' '
 	git config branch.autosetuprebase always &&
+	git config branch.autosetuppreservemerges always &&
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
 	(git show-ref -q refs/remotes/local/master || git fetch local) &&
 	git branch --no-track myr20 local/master &&
 	test "z$(git config branch.myr20.remote)" = z &&
 	test "z$(git config branch.myr20.merge)" = z &&
-	test "z$(git config branch.myr20.rebase)" = z
+	test "z$(git config branch.myr20.rebase)" = z &&
+	test "z$(git config branch.myr20.preservemerges)" = z
 '
 
 test_expect_success 'detect misconfigured autosetuprebase (bad value)' '
@@ -451,6 +488,11 @@ test_expect_success 'detect misconfigured autosetuprebase (bad value)' '
 	test_must_fail git branch
 '
 
+test_expect_success 'detect misconfigured autosetuppreservemerges (bad value)' '
+	git config branch.autosetuppreservemerges garbage &&
+	test_must_fail git branch
+'
+
 test_expect_success 'detect misconfigured autosetuprebase (no value)' '
 	git config --unset branch.autosetuprebase &&
 	echo "[branch] autosetuprebase" >> .git/config &&
@@ -458,4 +500,11 @@ test_expect_success 'detect misconfigured autosetuprebase (no value)' '
 	git config --unset branch.autosetuprebase
 '
 
+test_expect_success 'detect misconfigured autosetuppreservemerges (no value)' '
+	git config --unset branch.autosetuppreservemerges &&
+	echo "[branch] autosetuppreservemerges" >> .git/config &&
+	test_must_fail git branch &&
+	git config --unset branch.autosetuppreservemerges
+'
+
 test_done
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 9a376ef..5f4ce56 100644
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -25,44 +25,59 @@ export GIT_AUTHOR_EMAIL
 test_expect_success 'setup for merge-preserving rebase' \
 	'echo First > A &&
 	git add A &&
-	git-commit -m "Add A1" &&
+	git-commit -m A1 &&
 	git checkout -b topic &&
 	echo Second > B &&
 	git add B &&
-	git-commit -m "Add B1" &&
+	git-commit -m B1 &&
+	git tag B1 &&
+	echo Third >> B &&
+	git commit -a -m B2
 	git checkout -f master &&
-	echo Third >> A &&
-	git-commit -a -m "Modify A2" &&
+	echo Fourth >> A &&
+	git-commit -a -m A2 &&
+	git clone . clone
+'
 
-	git clone ./. clone1 &&
-	cd clone1 &&
+test_expect_success 'git pull --rebase -p on moved topic' '
+	cd clone &&
 	git checkout -b topic origin/topic &&
+	git reset --hard B1 &&
 	git merge origin/master &&
+	git pull --rebase --preserve-merges &&
+	test $(git rev-list --all --pretty=oneline | grep A2 | wc -l) = 1 &&
+	git checkout origin/topic &&
+	git branch -D topic &&
 	cd ..
+'
 
-	git clone ./. clone2
-	cd clone2 &&
+test_expect_success 'rebase -p merge on moved topic' '
+	cd clone &&
 	git checkout -b topic origin/topic &&
+	git reset --hard B1 &&
 	git merge origin/master &&
-	cd .. &&
-
-	git checkout topic &&
-	echo Fourth >> B &&
-	git commit -a -m "Modify B2"
-'
-
-test_expect_success 'git pull --rebase -p on moved topic' '
-	cd clone1 &&
-	git pull --rebase --preserve-merges &&
-	test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
+	git rebase -p origin/topic &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep A2 | wc -l) &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l) &&
+	git checkout origin/topic &&
+	git branch -D topic &&
+	cd ..
 '
 
-test_expect_success 'rebase -p merge on moved topic' '
-	cd ../clone2 &&
-	git fetch &&
-	git rebase -p origin/topic &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
-	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+test_expect_success 'git pull on moved topic' '
+	cd clone &&
+	git config branch.autosetuppreservemerges always &&
+	git checkout -b topic origin/topic &&
+	test true = $(git config branch.topic.preservemerges) &&
+	git reset --hard B1 &&
+	git merge origin/master &&
+	git pull &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep A2 | wc -l) &&
+	test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l) &&
+	git checkout origin/topic &&
+	git branch -D topic &&
+	cd ..
 '
 
 test_done
+
-- 
1.6.0.2

Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase

[Stephen Haberman]

> The following DAG is created by the commands below:
> 
>   -A---B      master
>     \
>      C---M    topic
>       \ /
>        D
> 
>   git init
>   echo 1 >foo
>   git add foo
>   git commit -m 'first on master'       # A
>   echo 2 >>foo
>   git commit -m 'second on master' foo  # B
>   git checkout -b topic HEAD^
>   echo 1 >bar
>   git add bar
>   git commit -m 'first on topic'        # C
>   git checkout -b subtopic
>   echo 1 >baz
>   git add baz
>   git commit -m 'first on subtopic'     # D
>   git checkout topic
>   git merge --no-ff subtopic            # M
> 
> If I now execute 'git rebase -p master topic', I get the following:
> 
>   -A---B            master
>     \   \
>      \   C'---M'    topic
>       \      /
>        C----D

Following up on this old thread, I can't get M' to have the old parent
D. I always see D change to D' and then topic is fast fowarded to D'
instead of an M' showing up. (I've tried 1.6.0.2, my rebase-i-p changes,
and sp/maint.)

> But I would rather like to have the following:
> 
>   -A---B            master
>         \
>          C'---M'    topic
>           \  /
>            D'
> 
> Would such a behaviour possible at all?

Yes, I think it just takes the following patch:

--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -251,7 +251,7 @@ pick_one_preserving_merges () {
                                GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
                                GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
                                output git merge $STRATEGY -m "$msg" \
-                                       $new_parents
+                                       --no-ff $new_parents

Applying this to either sp/maint or my rebase-i-p changes gets your
desired output.

With the only caveat being that the subtopic branch stays pointing at
the old D--since you are rebasing topic, it does not change where
subtopic points when rewriting D -> D'.

Musing, I could see moving subtopic being possible, definitely with git
sequencer, but also with a --other-branches-follow-rewrites flag of some
sort that, after rewriting hash1->hash2, just finds any local branches
pointing at hash1 and updates their refs to be hash2. Not that I'm
really suggesting it, but I don't think it would be that hard.

Anyway, subtopic still pointing at D aside, I think your desired output
makes sense, given you've explicitly told rebase to preserve merges. If
you wanted a non-ff M in the first place, I think passing along a
--no-ff to keep M' around is reasonable. And would otherwise be harmless.

I can write a test/patch for this unless you beat me to it or other
think it is unreasonable.

- Stephen