rebasing merges

rebasing merges

[Stephen Haberman]

Hello,

I was wondering if there are any plans to enhance the rebase
functionality to match something like [1], specifically to handle
rebasing merge commits.

My understanding of the current rebase implementation is that it dumps
all the changes as a patch, moves HEAD, and applies the patch.

This works great for regular commits, but if one of the new commits was
a merge, rebase seems to "flatten" the merge. Because the patch dump
doesn't include any information about the merged commit's existing
hashes/parents, when the patch is applied, each merged commit gets
reapplied as a new commit, with a new hash, and no DAG link back to
where the original pre-rebase merge came from.

We've had this happen on topic branches when devs merged in a new
stable release that contains ~100 commits, but had their branch's
remote head move on them, so they pull --rebase as usual, but end up
with all ~100 commits from the release essentially cherry-picked into
their topic branch. So now we have the disclaimer of "usually pull
--rebase...unless you've done a merge, then do a pull --merge".

[1] seems to solve this with some fairly complex cherry-picking and
ancestry manipulation that, admittedly, I don't quite understand, but
it seems like it might be able to bring along a merge's multiple
parents information throughout the rebase and maintain the merge as a
single, non-flattened merge commit.

Thanks,
Stephen

1: http://www.selenic.com/mercurial/wiki/index.cgi/RebasePlan

Re: rebasing merges

[Stephen Haberman]

> [1] seems to solve this with some fairly complex cherry-picking and
> ancestry manipulation that, admittedly, I don't quite understand, but
> it seems like it might be able to bring along a merge's multiple
> parents information throughout the rebase and maintain the merge as a
> single, non-flattened merge commit.

Hm. Sorry for the noise about that Mercurial link--mostly talking to
myself now, but I've discovered `git rebase -i -p` does exactly what I
want (I think).

Is there a reason the "preserve merge" option in the
git-rebase--interactive isn't also in the non-interactive git-rebase
that is invoked by a git pull --rebase?

I noticed the t3400.sh test explicitly tests for the flattening
behavior, but I can't tell if that is because it's testing for
explicitly desired behavior or if the "linear-izing" is something
that is up for debate (or a command line/config option).

Would it be foolish for me to work on something like this? I can
probably hold my own at copy/pasting around in the shell scripts.

Thanks,
Stephen

Re: rebasing merges

[Johannes Sixt]

Stephen Haberman schrieb:
> I noticed the t3400.sh test explicitly tests for the flattening
> behavior, but I can't tell if that is because it's testing for
> explicitly desired behavior or if the "linear-izing" is something
> that is up for debate (or a command line/config option).

This is the expected behavior and not up for debate.

Consider this use-case, for example: You keep a private patch or two on
top of upstream, and you also regularly pull from upstream. You get this
history:

  ---o--o--o--o--o--o    <-- origin
      \     \        \
       A--B--M--------N  <-- master

A and B are the private patches. From time to time you want to update them
if they get out of date, which is indicated by merge conflicts in the
merges M and N. Then you want this result:

  ---o--o--o--o--o--o         <-- origin
                     \
                      A'--B'  <-- master

i.e. linearized history without merges.

-- Hannes

Re: rebasing merges

[Samuel Tardieu]

>>>>> "Johannes" == Johannes Sixt <j.sixt@viscovery.net> writes:

Johannes> i.e. linearized history without merges.

A few days ago, I had a question in my team quite similar to Stephen
concern. A developer had performed a merge of a complex feature and
was ready to commit it

  --o--o--o--o--X        <-- origin
     \           \
      A--B--C--D--E      <-- master

when he realized that someone just pushed another change on origin
while he was doing the complicating merge (with lots of conflicts to
resolve). The configuration was then:

  --o--o--o--o--X--Y     <-- origin
     \           \
      A--B--C--D--E      <-- master

He would have wanted to have the merge rebased on E and Y instead of E
and X before pushing, without going through all the conflicts
resolution again (he didn't have "rerere" enabled).

Is that possible with "git rebase"?

Btw, would it be a good idea to unconditionally enable "rerere"
conflict resolution *recording*, and add an option to "rebase" and
"merge" to use "rerere" even when it is not enabled in the
configuration file? I can't think of any drawback.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

Re: rebasing merges

[Stephen Haberman]

> This is the expected behavior and not up for debate.

Cool, thanks for the reply. However, I debate... :-)

>   ---o--o--o--o--o--o         <-- origin
>                      \
>                       A'--B'  <-- master

Nice. That makes sense in your scenario.

Here is mine:

 ---A--B--C--D           <-- origin/stable
     \       |
      E--F   |           <-- origin/topica
          \  |
             g--h        <-- topica

All the upper case commits have been published to origin. Other
devs, etc., know about them, their hashes are in the bug tracking
system.

I'm bringing topica up to date, but with a merge because I have
published history already on topica, so I merge stable and get a
new merge commit: g. And maybe make another change: h.

Everything's cool...now, with surprising frequency, someone beats
me to moving origin/topica:

 ---A--B--C--D           <-- origin/stable
     \       |
      E--F---|--I        <-- origin/topica
          \  |
             g--h        <-- topica

Pushing h gets rejected as a rewind. Good. I want to pull, which
we had previously always used "--rebase" for, and the desired output
of a pull --rebase, to me, would be:

 ---A--B--C--D           <-- origin/stable
     \        \
      E--F--I |          <-- origin/topica
             \|
              g'--h'     <-- topica

Instead, I get:

 ---A--B--C--D           <-- origin/stable
     \
      E--F--I            <-- origin/topica
             \
              B'-C'-D'-h'<-- topica

So, yes, linearized history with no merges. However, this leads
to quizzical looks when B'/C'/D' hit the email list, bug tracker, etc.
as new commits.

Currently I just try to pull/merge/push in quick succession, but
it's a manual collaboration hack ("okay, I'm merging now, no
committing...") that would be nice to not have to worry about.

I need to investigate the interactive rebase more, but my hesitant
assertion is that it's parent rewriting seems smart enough to handle
this. Perhaps not, and I admit our desired DAG output may not be
attainable without manual intervention.

I apologize--I should have included the example DAGs in my first
post, but since I didn't I felt the need to clarify. So, humoring
me, is the B'/C'/D' from this example really the expected behavior?

I can work on a new test in t3400/etc. if that is of interest.

Thanks for your time,
Stephen

Re: rebasing merges

[Johannes Sixt]

Samuel Tardieu schrieb:
>>>>>> "Johannes" == Johannes Sixt <j.sixt@viscovery.net> writes:
> 
> Johannes> i.e. linearized history without merges.
> 
> A few days ago, I had a question in my team quite similar to Stephen
> concern. A developer had performed a merge of a complex feature and
> was ready to commit it
> 
>   --o--o--o--o--X        <-- origin
>      \           \
>       A--B--C--D--E      <-- master
> 
> when he realized that someone just pushed another change on origin
> while he was doing the complicating merge (with lots of conflicts to
> resolve). The configuration was then:
> 
>   --o--o--o--o--X--Y     <-- origin
>      \           \
>       A--B--C--D--E      <-- master
> 
> He would have wanted to have the merge rebased on E and Y instead of E
> and X before pushing, without going through all the conflicts
> resolution again (he didn't have "rerere" enabled).

If you mean "merge", and you are willing to keep E, then you do another
merge to get this history:

  --o--o--o--o--X--Y     <-- origin
     \           \  \
      A--B--C--D--E--F   <-- master

and the previous conflicts will not show up again. But if you do not want
E to show up in the history, like this:

  --o--o--o--o--X--Y     <-- origin
     \              \
      A--B--C--D-----F   <-- master

then you will have to resolve all conflicts again. Of course, you can keep
 E in a temporary branch, and you can selectively check out files from
that tree to safe some work, but whether this is possible depends on the
changes that Y introduces.

If you mean "rebase", then you get this after the first rebase:

  --o--o--o--o--X--Y               <-- origin
                 \
                  A'--B'--C'--D'   <-- master

Then you can rebase again to get this:

  --o--o--o--o--X--Y                  <-- origin
                    \
                     A"--B"--C"--D'   <-- master

at which time you don't see the old conflicts again.

-- Hannes

Re: rebasing merges

[Johannes Sixt]

Stephen Haberman schrieb:
>> This is the expected behavior and not up for debate.
> 
> Cool, thanks for the reply. However, I debate... :-)
> 
>>   ---o--o--o--o--o--o         <-- origin
>>                      \
>>                       A'--B'  <-- master
> 
> Nice. That makes sense in your scenario.
> 
> Here is mine:

I understand your problem very well. pull --rebase does not help your case
because it was not designed for your workflow (it was designed to help the
one that I sketched).

You are probably better served using fetch + rebase -i -p.

-- Hannes

Re: rebasing merges

[Andreas Ericsson]

Samuel Tardieu wrote:
> 
> Btw, would it be a good idea to unconditionally enable "rerere"
> conflict resolution *recording*, and add an option to "rebase" and
> "merge" to use "rerere" even when it is not enabled in the
> configuration file? I can't think of any drawback.
> 

I'm all for that. I actually thought (4 months ago) that that was
how it worked.

It would be a backwards incompatible change though, as we currently
fall back to "does rr-cache exist?" to determine if rerere is enabled
in case it isn't explicitly so in the configuration. Perhaps print
a warning if !rerere.enabled but $GIT_DIR/rr-cache exists. OTOH,
that will create a lot of warnings since we'd have to create that
rr-cache directory to record the resolutions.

To maintain backwards compatibility, we could ofcourse do like this;
* move rr-cache to rere ("recorded resolutions"; bikeshed color goes here)
* if rr-cache exists, implicitly enable rerere
* if rr-cache holds resolutions when setting up rerere, move them
  to rere
OR
* look for resolutions in both rr-cache and rere indefinitely.

It's not exactly performance critical, so swapping the pattern from
O(n) (or whatever it is now) to O(2n) for replaying resolutions
probably won't make that much of a difference.

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

Re: rebasing merges

[Andreas Ericsson]

Stephen Haberman wrote:
>> This is the expected behavior and not up for debate.
> 
> Cool, thanks for the reply. However, I debate... :-)
> 
>>   ---o--o--o--o--o--o         <-- origin
>>                      \
>>                       A'--B'  <-- master
> 
> Nice. That makes sense in your scenario.
> 
> Here is mine:
> 
>  ---A--B--C--D           <-- origin/stable
>      \       |
>       E--F   |           <-- origin/topica
>           \  |
>              g--h        <-- topica
> 
> All the upper case commits have been published to origin. Other
> devs, etc., know about them, their hashes are in the bug tracking
> system.
> 
> I'm bringing topica up to date, but with a merge because I have
> published history already on topica, so I merge stable and get a
> new merge commit: g. And maybe make another change: h.
> 

Why do you merge stable at this point?

If you want the latest and greatest for testing/conflict resolution
purposes, you can simply throw away the merge later and just know
that it works.
If you need some bugfix on stable but not everything else, cherrypick
only that change. Otherwise you're applying a huge patch to fix a
small problem.

> Everything's cool...now, with surprising frequency, someone beats
> me to moving origin/topica:
> 
>  ---A--B--C--D           <-- origin/stable
>      \       |
>       E--F---|--I        <-- origin/topica
>           \  |
>              g--h        <-- topica
> 
> Pushing h gets rejected as a rewind. Good. I want to pull, which
> we had previously always used "--rebase" for, and the desired output
> of a pull --rebase, to me, would be:
> 
>  ---A--B--C--D           <-- origin/stable
>      \        \
>       E--F--I |          <-- origin/topica
>              \|
>               g'--h'     <-- topica
> 
> Instead, I get:
> 
>  ---A--B--C--D           <-- origin/stable
>      \
>       E--F--I            <-- origin/topica
>              \
>               B'-C'-D'-h'<-- topica
> 
> So, yes, linearized history with no merges. However, this leads
> to quizzical looks when B'/C'/D' hit the email list, bug tracker, etc.
> as new commits.
> 
> Currently I just try to pull/merge/push in quick succession, but
> it's a manual collaboration hack ("okay, I'm merging now, no
> committing...") that would be nice to not have to worry about.
> 

I think you just need to ask yourself *why* you're doing that
first merge of "stable" into your topic. If they aren't really
separate, using a topic-branch doesn't make so much sense. If
they *are* separate, doing the merge doesn't make much sense,
unless you're integration testing a snapshot build, but in that
case you'd want to throw away the merge once it's done and
tested.

> I need to investigate the interactive rebase more, but my hesitant
> assertion is that it's parent rewriting seems smart enough to handle
> this. Perhaps not, and I admit our desired DAG output may not be
> attainable without manual intervention.
> 
> I apologize--I should have included the example DAGs in my first
> post, but since I didn't I felt the need to clarify. So, humoring
> me, is the B'/C'/D' from this example really the expected behavior?
> 

Assuming the person who did "h" doesn't have the merge commit, then
yes.

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

Re: rebasing merges

[Stephen Haberman]

> >  ---A--B--C--D           <-- origin/stable
> >      \       |
> >       E--F   |           <-- origin/topica
> >           \  |
> >              g--h        <-- topica
> > 
> > All the upper case commits have been published to origin. Other
> > devs, etc., know about them, their hashes are in the bug tracking
> > system.
> > 
> > I'm bringing topica up to date, but with a merge because I have
> > published history already on topica, so I merge stable and get a
> > new merge commit: g. And maybe make another change: h.
> > 
> 
> Why do you merge stable at this point?

Good question--I appreciate the sanity check.

I merged stable because we had a new release of the software. E.g.
A=1.0, B=1.1, C=1.2, and finally D=1.3.

So, topica is a new feature, "Add widgets/whatever", but it's not ready
for stable (production) yet, so, yes, I think it is a topic branch.

However, D=1.4 is now out the door, I've had two commits E and F on
topica that I had already committed and pushed out for code review, our
email list, and our bug tracker, and now, post-1.4, qa wants to see
topica up and running to see if it's good enough to go into the next
release.

If our deployment guy pushes out F, qa is going to (and did) complain
that they're not seeing the latest features from 1.4 in topica.

As you said, integration testing.

Okay, so I merge g, however, I really want to push it back out so that
the deployment guy can push it to qa (he would rather not resolve my
conflicts by making his own local g). And even if I did the deployment
myself, locally against g, I would prefer to share g in case another
dev working on the same topic gets feedback about funkiness from qa and
would like to see the code as it is in qa. E.g.: g.

I can appreciate that if I was doing integration testing all by myself,
with only automated tests, I could throw g away. However, even then, I
would prefer to push g out and let our integration server run the tests
for me.

Does this sound reasonable?

Thanks,
Stephen

Re: rebasing merges

[Andreas Ericsson]

Stephen Haberman wrote:
>>>  ---A--B--C--D           <-- origin/stable
>>>      \       |
>>>       E--F   |           <-- origin/topica
>>>           \  |
>>>              g--h        <-- topica
>>>
>>> All the upper case commits have been published to origin. Other
>>> devs, etc., know about them, their hashes are in the bug tracking
>>> system.
>>>
>>> I'm bringing topica up to date, but with a merge because I have
>>> published history already on topica, so I merge stable and get a
>>> new merge commit: g. And maybe make another change: h.
>>>
>> Why do you merge stable at this point?
> 
> Good question--I appreciate the sanity check.
> 
> I merged stable because we had a new release of the software. E.g.
> A=1.0, B=1.1, C=1.2, and finally D=1.3.
> 
> So, topica is a new feature, "Add widgets/whatever", but it's not ready
> for stable (production) yet, so, yes, I think it is a topic branch.
> 
> However, D=1.4 is now out the door, I've had two commits E and F on
> topica that I had already committed and pushed out for code review, our
> email list, and our bug tracker, and now, post-1.4, qa wants to see
> topica up and running to see if it's good enough to go into the next
> release.
> 
> If our deployment guy pushes out F, qa is going to (and did) complain
> that they're not seeing the latest features from 1.4 in topica.
> 
> As you said, integration testing.
> 
> Okay, so I merge g, however, I really want to push it back out so that
> the deployment guy can push it to qa (he would rather not resolve my
> conflicts by making his own local g). And even if I did the deployment
> myself, locally against g, I would prefer to share g in case another
> dev working on the same topic gets feedback about funkiness from qa and
> would like to see the code as it is in qa. E.g.: g.
> 
> I can appreciate that if I was doing integration testing all by myself,
> with only automated tests, I could throw g away. However, even then, I
> would prefer to push g out and let our integration server run the tests
> for me.
> 
> Does this sound reasonable?
> 

It sounds very reasonable indeed, but then I don't understand why you
held off pushing the merge.

That's beside the point though, as I firmly believe git should be more
helpful in this situation. If "git rebase -i -p" doesn't help you fix
the problems, I'll see what I can do to help.

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

Re: rebasing merges

[Stephen Haberman]

> > Does this sound reasonable?
> 
> It sounds very reasonable indeed

Ah, cool, thanks for validating our approach. We have so far been
slowly figuring it out and it has been nice.

> but then I don't understand why you held off pushing the merge.

Ah, yes, that is the trick, to get it out quickly. Usually it is not a
problem, but in ~2-3 months of using git, it's happened on high-churn
branches about 4-5 times (usually it is the next release candidate
branch where people are actively making small bug fixes and one guy
gets caught with a non-trivial merge in from stable).

Not all that bad, really, other than it caused a few "wtf git"
comments. Which is unfortunate as, while I enjoy git, the team as a
whole is still learning. Seeing the DAGs, and understanding the
patch/apply approach of the non-interactive rebase, it makes sense to
me what git is doing. And so while I know to watch for it, I'm trying to
find a more bullet proof approach.

> That's beside the point though, as I firmly believe git should be more
> helpful in this situation. If "git rebase -i -p" doesn't help you fix
> the problems, I'll see what I can do to help.

That's very cool, thanks. I'll start writing a test now.

- Stephen

Re: rebasing merges

[SZEDER Gábor]

Hi,

On Tue, Sep 23, 2008 at 11:11:29AM +0200, Andreas Ericsson wrote:
> That's beside the point though, as I firmly believe git should be more
> helpful in this situation. If "git rebase -i -p" doesn't help you fix
> the problems, I'll see what I can do to help.
I will just throw in an other workflow, where keeping merges during
(non-interactive) rebase would be really helpful for me.

The DAG looks like this:

  -A--------------H    master
    \
     B--C------F--G    topic
         \    /
          D--E         subtopic

I develop a new feature in my private repository on branch 'topic'.
Every now and then there are two or more commits that somehow belong
together (e.g. a refactoring consisting of multiple commits).  I
prefer having this "belong together" information explicitly in the
repository, therefore for these commits (D and E) I create the new
branch 'subtopic' that will be merged back into 'topic' (with
--no-ff).  This way I can see in the logs or in gitk explicitly that
those commits belong together.  While working on a bigger feature
there might be multiple 'subtopic' branches that get merged back into
'topic'.

But now I can't rebase 'topic' on top of master, because rebase would
linearize the history, losing the subtopic merges.


Best,
Gábor

Re: rebasing merges

[Stephen Haberman]

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


> > That's beside the point though, as I firmly believe git should be more
> > helpful in this situation. If "git rebase -i -p" doesn't help you fix
> > the problems, I'll see what I can do to help.
> 
> That's very cool, thanks. I'll start writing a test now.

I've attached a test. You were both correct, `git rebase -i -p` keeps
the merge intact and doesn't duplicate the commits. This is awesome.

So, I'm wondering what the best way to integrate this into our workflow
is. Ideally, I think it would be great to just do:

    git pull --rebase --preserve-merges

Or even better just `git pull` with branch.x.rebase=true and
branch.x.preserve-merges=true property (throw in a
branch.autosetuppreservemerges and I'll be set, haha).

While `git fetch + git rebase -i -p` will work every time, it won't be
fun to have the editor pop open, just exit it because we're not really
doing an interactive rebase, we're just borrowing its more powerful
rebase implementation, and repeat this each time a dev wants to do a
pull. Or, tell them to do a "git pull --rebase" most of the time, until
it screws up, then back up and do the other. Both doable, but not ideal.

I understand Johannes's scenario of rebasing the entire branch, and it
looks like rebase--interactive already supports this as well--e.g. if
you pass `-p`, you get our desired behavior, and if you don't pass `-p`,
you get Johannes's desired behavior of merge squashing.

Given rebase--interactive seems to be a superset of the current rebase
implementation, could the current rebase implementation just go away and
instead become a non-interactive caller (e.g. just avoid popping up the
editor with the pick list) of rebase--interactive's more powerful
rebase logic?

(Disclaimer: I'm throwing the term "more powerful" around without really
knowing whether that is true or not--for our scenario, it seems that
way, but there could be other cases I don't know about where the current
rebase implementation is superior to rebase--interactive's.)

Thanks,
Stephen


[-- Attachment #2: t3402-rebase-merge2.sh --]
[-- Type: text/x-sh, Size: 2407 bytes --]

#!/bin/sh
#
# Copyright (c) 2005 Amos Waterland
#

test_description='git rebase should not destroy author information

This test runs git rebase and checks that the author information is not lost.
'
. ./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 'prepare repository with topic branches' \
    '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"
'

# Blatant copy/paste from t3404-rebase-interactive.sh--needs cleaned up
echo "#!$SHELL_PATH" >fake-editor.sh
cat >> fake-editor.sh <<\EOF
case "$1" in
*/COMMIT_EDITMSG)
	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"
cat "$1".tmp
action=pick
for line in $FAKE_LINES; do
	case $line in
	squash|edit)
		action="$line";;
	*)
		echo sed -n "${line}s/^pick/$action/p"
		sed -n "${line}p" < "$1".tmp
		sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
		action=pick;;
	esac
done
EOF

test_set_editor "$(pwd)/fake-editor.sh"
chmod a+x fake-editor.sh

test_expect_failure 'rebase merge on moved topic' '
     cd clone1 &&
     git pull --rebase &&
     test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
'

test_expect_success 'rebase -i -p merge on moved topic' '
     cd ../clone2 &&
     git fetch &&
     FAKE_LINES="1 2" git rebase -i -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