Amending merge commits?

Hi folks,

I think one of my coworkers has stumbled on a git bug -- if you amend a merge commit, and then pull, your amends are lost.

Is this expected behavior?

I've reproduced the problem in a script (attached).  I ran it against a couple of versions of git (1.7.1, 1.7.9, 1.8.4, 2.0.0) and in each case it seemed to lose the amend.

- Dave

Besen, David <david.besen <at> hp.com> writes:

merge commit, and then pull, your amends

couple of versions of git (1.7.1,

Whoops, accidentally encoded the script, here it is inline:

#!/bin/bash

set -ex

if [ -z "$GIT" ]; then GIT=git; fi GIT_MERGE_AUTOEDIT=no

# Clean up from the last run rm -rf repo.git repo repo2 || :

# Set up a bare "remote" repo $GIT init --bare repo.git

# Check out the "remote" repo $GIT clone repo.git repo

# Add a commit cd repo echo "file" > file.txt $GIT add file.txt $GIT commit -m "Add file.txt" $GIT push origin master

# Make a branch $GIT checkout -b mybranch

# Add a commit on the branch echo "mybranch" >> file.txt $GIT add . $GIT commit -m "Add 'mybranch' line"

# Go back to master $GIT checkout master

# Merge in mybranch to create a merge commit $GIT merge --no-ff mybranch

# Push that back $GIT push

# Amend the merge commit echo "amended" >> file.txt $GIT add . $GIT commit -C HEAD --amend

cd ..

# Make a second checkout $GIT clone repo.git repo2 cd repo2

# Add some unrelated changes to be pulled echo "repo2" > file2.txt $GIT add . $GIT commit -m "Add file2" $GIT push

cd .. cd repo

# Pull $GIT pull --rebase

# Now, we expect the text "amended" to be in file.txt grep amended file.txt

Besen, David wrote:

This is how pull --rebase works. It turns your single-parent commits into a sequence of patches on top of upstream and completely ignores your merge commits.

There is a --rebase=preserve option that makes a halfhearted attempt to preserve your merges --- perhaps that would help? The git-rebase(1) documentation has more details.

In an ideal world, I think pull --rebase would do the following:

 1. Do the same thing it does today
 2. Behind the scenes, *also* try a 'pull --merge' but don't save
    the result.
 3. Compare the results.  If they differ, show a diff and explain
    to the user what happened.

I may be the only one that wants that, though.

Hope that helps, Jonathan

Ah thanks, I'll RTFM better in the future.

- Dave

-----Original Message-----
From: Jonathan Nieder [mailto:jrnieder@gmail.com] 
Sent: Friday, July 25, 2014 4:19 PM
To: Besen, David
Cc: git@vger.kernel.org
Subject: Re: Amending merge commits?

Besen, David wrote:

This is how pull --rebase works. It turns your single-parent commits into a sequence of patches on top of upstream and completely ignores your merge commits.

There is a --rebase=preserve option that makes a halfhearted attempt to preserve your merges --- perhaps that would help? The git-rebase(1) documentation has more details.

In an ideal world, I think pull --rebase would do the following:

 1. Do the same thing it does today
 2. Behind the scenes, *also* try a 'pull --merge' but don't save
    the result.
 3. Compare the results.  If they differ, show a diff and explain
    to the user what happened.

I may be the only one that wants that, though.

Hope that helps, Jonathan

David Besen wrote:

No, not a problem. It's very useful to see examples of where git's behavior was counterintuitive and the documentation was more obscure than it could have been.

I should also emphasize the "halfhearted" above. There's a lot of room for improvement in rebase --preserve-merges's handling of "evil" and otherwise amended merges.

Thanks again, Jonathan

Jonathan Nieder <jrnieder@gmail.com> writes:

Should documentaion warn that "git pull --rebase=true" (and pull.merge=true configuration) could be harmful, and that --rebase=preserve (and pull.merge=preserve) should better be used instead?

Is there any scenario at all where pull --rebase=true wins over preserve?

Sergei Organov wrote:

Basically always in my book. ;-)

When people turn on 'pull --rebase', they are asking for a clean, simplified history where their changes are small discrete patches in a clump on top of upstream.

When someone has made a merge by mistake (with 'git pull' before remembering to do an autosetuprebase, or with 'git merge' instead of cherry-picking some patches they needed), the current --rebase=true behavior can be a good way of cleaning up.

That said, in some specific workflows, --rebase=preserve may work better than --rebase=true. My hunch is that even those workflows are not currently handled well with --rebase=preserve, alas.

A clearer explanation of --rebase (maybe with sub-headings for each choice *true*, *false*, and *preserve*?) sounds useful. An illustration in the EXAMPLES section of git-pull(1) of the difference between these three modes and when to use them would be even more helpful.

Thanks, Jonathan

Jonathan Nieder <jrnieder@gmail.com> writes:

I think they rather ask for avoiding tons of meaningless automatic merges resulting from periodic pulling from upstream.

Those subset of the above who only do small discrete patches don't do merges to their tracking branches, except by mistake, right? If so, 'pull --rebase=preserve' makes no difference compared to --rebase=true to their normal workflow. Moreover,if someone merges something to his tracking branch by mistake, how is it different from making merge to any other branch by mistake? Just fix the mistake by resetting to previous state.

On the other hand, if someone decides to merge something else to his tracking branch by purpose, both --rebase=preserve and --rebase=false perform as expected, while --rebase=true may easily lead to huge surprise. Please refer also to this thread for one such case:

http://www.mail-archive.com/git%40vger.kernel.org/msg55605.html

Once again, in case of mistake they are free to use --rebase=true, and even then using 'git rebase' directly is probably cleaner. That said, I don't argue --rebase=true could be sometimes useful. It's just that I think --rebase=preserve is safer, so it should be a good idea to suggest to use it (in favor of --rebase=true) in general.

It does indeed, and I don't think my aforementioned workflow is too specific.

--rebase=preserve works fine for the aforementioned workflow. At least simple tests I performed so far ran fine. I'd like to learn though which nasty drawbacks --rebase=preserve has for tracking branches compared to --rebase=true, if any.

That would be an improvement anyway, indeed.