[RFC] allow git pull to preserve merges

[RFC] allow git pull to preserve merges

[Stephen Haberman]

Hey,

Following up on an old thread (2008):

http://git.661346.n2.nabble.com/pull-preserve-merges-td1471688.html

I'd like to finally add a config parameter/setting to allow git pull to preserve
merges when it's rebasing. This addresses a somewhat common boundary case of a
locally merged feature branch getting flattened into master, as described here:

http://notes.envato.com/developers/rebasing-merge-commits-in-git/

This current patch adds a new `pull.preserve-merges` boolean config setting, but
we could also change the existing `pull.rebase` to be tri-state (so
`pull.rebase` can be true, false, or preserve-merges), or add a more generic
`pull.rebaseoptions` that is just a string of flags to pass to rebase.

Any of these would be fine with me--what would be preferred?

This patch doesn't update the docs, but I wanted to get an initial sanity check
on the preferred config setting before doing that.

Thanks!

- Stephen


Stephen Haberman (1):
  pull: Allow pull to preserve merges when rebasing.

 git-pull.sh     | 11 +++++++++--
 t/t5520-pull.sh | 15 +++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

-- 
1.8.1.2

[PATCH] pull: Allow pull to preserve merges when rebasing.

[Stephen Haberman]

If a user is working on master, and has merged in their feature branch, but now
has to "git pull" because master moved, with pull.rebase their feature branch
will be flattened into master.

This is because "git pull" currently does not know about rebase's preserve
merges flag, which would this behavior, and instead replay on the merge commit
of the feature branch onto the new master, and not the entire feature branch
itself.

Add a -p/--preserve-merges, to pass along git rebase if --rebase is in affect.

Also add a new pull.preserve-merges config setting, to enable this behavior as
the default.

Signed-off-by: Stephen Haberman <stephen@exigencecorp.com>
---
 git-pull.sh     | 11 +++++++++--
 t/t5520-pull.sh | 15 +++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..61d1efb 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -40,7 +40,7 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules= verify_signatures=
-merge_args= edit=
+merge_args= edit= rebase_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -48,6 +48,10 @@ if test -z "$rebase"
 then
 	rebase=$(git config --bool pull.rebase)
 fi
+if [ $(git config --bool pull.preserve-merges) = "true" ] ;
+then
+	rebase_args=--preserve-merges
+fi
 dry_run=
 while :
 do
@@ -116,6 +120,9 @@ do
 	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
 		rebase=false
 		;;
+	-p|--preserve-merges)
+		rebase_args=--preserve-merges
+		;;
 	--recurse-submodules)
 		recurse_submodules=--recurse-submodules
 		;;
@@ -292,7 +299,7 @@ fi
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
-	eval="git-rebase $diffstat $strategy_args $merge_args $verbosity"
+	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
 	eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
 	;;
 *)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index ed4d9c8..2a2ee97 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -148,6 +148,21 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
 	test new = $(git show HEAD:file2)
 '
 
+test_expect_success 'preserve merges' '
+	git reset --hard before-rebase &&
+	test_config pull.rebase true &&
+	test_config pull.preserve-merges true &&
+	git checkout -b keep-merge second^ &&
+	echo new > file3 &&
+	git add file3 &&
+	git commit -m "new file3" &&
+	git checkout to-rebase &&
+	git merge keep-merge &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
+	test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
+'
+
 test_expect_success '--rebase with rebased upstream' '
 
 	git remote add -f me . &&
-- 
1.8.1.2

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Stephen Haberman]

> This is because "git pull" currently does not know about rebase's
> preserve merges flag, which would this behavior, and instead replay
> on the merge commit of the feature branch onto the new master, and
> not the entire feature branch itself.

Ack, sorry, I was doing this too late last night--should say:

This is because "git pull" currently does not know about rebase's
preserve merges flag, which would avoid this behavior by replaying the
merge commit of the feature branch onto the new master, and not
replaying each individual commit in the feature branch.

- Stephen

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Johannes Schindelin]

Hi Stephen,

On Thu, 8 Aug 2013, Stephen Haberman wrote:

> If a user is working on master, and has merged in their feature branch,
> but now has to "git pull" because master moved, with pull.rebase their
> feature branch will be flattened into master.
> 
> This is because "git pull" currently does not know about rebase's
> preserve merges flag, which would this behavior, and instead replay on
> the merge commit of the feature branch onto the new master, and not the
> entire feature branch itself.
> 
> Add a -p/--preserve-merges, to pass along git rebase if --rebase is in affect.

ACK!

> Also add a new pull.preserve-merges config setting, to enable this
> behavior as the default.

This should probably be added to config.txt and
Documentation/git-pull.txt, too, right?

FYI I started to rewrite the complete --preserve-merges support for the
interactive rebase some time ago, based on the experience of the merging
rebases:

	https://github.com/msysgit/git/commit/b733454b

(part of the rebase-i-p branch). The idea is to use the 'exec' command of
the interactive rebase to do a much better job, and to allow reordering
(and in particular fixup commits) even when trying to preserve merges.

Unfortunately, the resulting branches look slightly differently now,
breaking the (horribly complicated -- my fault!) unit tests, and due to an
utter lack of time I had to stall that project.

Feel free to play with it if you want!

Ciao,
Johannes

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Stephen Haberman]

Hi Johannes,

> This should probably be added to config.txt and
> Documentation/git-pull.txt, too, right?

Yep, I meant to note that I'd do that after getting an initial
confirmation that the pull.preserve-merges was the preferred approach.

(I was being lazy and didn't want to write up docs only to switch to
overloading pull.rebase or what not.)

But I'll go ahead and do that.

> 	https://github.com/msysgit/git/commit/b733454b

Interesting!

> Feel free to play with it if you want!

I'll poke around out of curiosity, but no promises, as, yes, this is a
tricky bit of functionality that can quickly lead to a lot of lost
sleep. :-)

- Stephen

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Philip Oakley]

From: "Stephen Haberman" <stephen@exigencecorp.com>
> Hi Johannes,
>
>> This should probably be added to config.txt and
>> Documentation/git-pull.txt, too, right?
>
> Yep, I meant to note that I'd do that after getting an initial
> confirmation that the pull.preserve-merges was the preferred approach.
>
> (I was being lazy and didn't want to write up docs only to switch to
> overloading pull.rebase or what not.)
>
> But I'll go ahead and do that.
>
>> https://github.com/msysgit/git/commit/b733454b
>
> Interesting!
>
>> Feel free to play with it if you want!
>
> I'll poke around out of curiosity, but no promises, as, yes, this is a
> tricky bit of functionality that can quickly lead to a lot of lost
> sleep. :-)
>
> - Stephen
>
>
Johannes also kindly explained his merging-rebase script to me on the 
msysgit list a few days ago 
https://groups.google.com/forum/?hl=en_US?hl%3Den#!topic/msysgit/LiPa2T_K4C4 
which shows how msysgit and git both keep parallel lines of development 
with fast forwarding and rebasing at the same time.

The technique should also help those case for keeping 
private/independent lines of development that are discussed often.

Philip

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Junio C Hamano]

Stephen Haberman <stephen@exigencecorp.com> writes:

> Hi Johannes,
>
>> This should probably be added to config.txt and
>> Documentation/git-pull.txt, too, right?
>
> Yep, I meant to note that I'd do that after getting an initial
> confirmation that the pull.preserve-merges was the preferred approach.

If you were to go that route, no dashes in the last component of
configuration variable names, please.

> (I was being lazy and didn't want to write up docs only to switch to
> overloading pull.rebase or what not.)

I think we have a recent update that allows you to say

	[pull]
        	rebase = false

to mean "I want 'git pull' to use merge".  Currently the other
choice is:

	[pull]
		rebase = true

to say "I want to run 'git pull --rebase'".  I do not think it is
unreasonable to extend it further so that

	[pull]
		rebase = preserve

is understood.

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Johannes Schindelin]

Hi,

On Thu, 8 Aug 2013, Junio C Hamano wrote:

> Stephen Haberman <stephen@exigencecorp.com> writes:
> 
> > Hi Johannes,
> >
> >> This should probably be added to config.txt and
> >> Documentation/git-pull.txt, too, right?
> >
> > Yep, I meant to note that I'd do that after getting an initial
> > confirmation that the pull.preserve-merges was the preferred approach.
> 
> If you were to go that route, no dashes in the last component of
> configuration variable names, please.
> 
> > (I was being lazy and didn't want to write up docs only to switch to
> > overloading pull.rebase or what not.)
> 
> I think we have a recent update that allows you to say
> 
> 	[pull]
>         	rebase = false
> 
> to mean "I want 'git pull' to use merge".  Currently the other
> choice is:
> 
> 	[pull]
> 		rebase = true
> 
> to say "I want to run 'git pull --rebase'".  I do not think it is
> unreasonable to extend it further so that
> 
> 	[pull]
> 		rebase = preserve
> 
> is understood.

We have a patch in Git for Windows allowing rebase = interactive which I
did not have time to send upstream.

Ciao,
Johannes

Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

[Stephen Haberman]

> We have a patch in Git for Windows allowing rebase = interactive
> which I did not have time to send upstream.

Cool, so, would rebase=preserve and rebase=interactive be completely
orthogonal?

E.g. do we have to worry about the user wanting to do both, like with
something ugly like rebase=preserve-interactive?

Assuming not, rebase=preserve sounds good to me. I have a patch
that does that about ready to submit.

- Stephen