rebase flattens history when it shouldn't?

Hello,

$ git --version git version 1.9.3

Please consider the following history:

     --C--
    /     \
   /   ----M topic,HEAD
  /   /
 A---B master

shouldn't

$ git rebase master

be a no-op here? According to my reading of the rebase manual page, it should be a no-op, as 'topic' is a descendant of the 'master'. Instead, "git rebase master" flattens the history to:

       ----C topic,HEAD
      /
 A---B master

I'd expect --force-rebase to be required for this to happen:

-f, --force-rebase
    Force the rebase even if the current branch is a descendant of the
    commit you are rebasing onto. Normally non-interactive rebase will
    exit with the message "Current branch is up to date" in such a
    situation. Incompatible with the --interactive option.

Also notice that:

$ git rebase --preserve-merges --verbose master

does perform the rebasing work, even though it does not change the history in the end.

Here is use-case where it came from and where it gave me real surprise:

I have pull.rebase=true in configuration. Being on a remote tracking branch, I've successfully pulled from the origin and had no any local changes on this branch. Then I've successfully merged another branch to the current one but didn't push the changes back upstream. A few hours later I returned to the work and issued "git pull" that instead of doing nothing (as it would be should pull.rebase be either "false" or "preserve") created a surprising mess.

Do you think it's worth fixing?

Here are reproduction commands for the example history:

git init t cd t echo A > a echo B > b git add a b git commit -m A -a git checkout -b x echo A >> a git commit -m C -a git checkout master echo B >> b git commit -m B -a git checkout -b topic git merge -m M x git branch -d x git rebase master

Hi Sergei,

Sergei Organov wrote:

[...]

[...]

Thanks for a clear report.

After a successful 'git rebase master', the current branch is always a linear string of patches on top of 'master'. The "already up to date" behavior when -f is not passed is in a certain sense an optimization --- it is about git noticing that 'git rebase' wouldn't have anything to do (except for touching timestamps) and therefore doing nothing.

So I don't think requiring -f for this case would be an improvement.

I do agree that the documentation is misleading. Any ideas for wording that could make it clearer?

Hope that helps, Jonathan

Jonathan Nieder <jrnieder@gmail.com> writes:

What actually bothers me is the unfortunate consequence that "git pull" is not always a no-op when nothing was changed at the origin since the last "git pull". THIS is really surprising and probably should better be fixed. Requiring -f is just one (obvious) way to fix this.

I can't suggest anything as I don't see why -f is there in the first place. What are use cases?

On 23.07.2014 21:33, Sergei Organov wrote:

That would invalidate the simple rule that "git pull" is equivalent to "git fetch" + "git rebase".

git rebase depends on both branches it operates on, not just one. The same goes for "git merge", I assume it is just a coincidence that git merge does have this characteristic you now expect both to have.

Holger Hellmuth <hellmuth@ira.uka.de> writes:

Sorry, I don't see how it would invalidate this. My suggestion even won't change git-pull source code at all, only git-rebase code.

git pull --reabse=false git pull --rebase=preserve

both have this property.

git pull --rebase=true

almost always has this property, unless there are local merge commits to be rebased.

So, I'd rather say it's likely behavior of "git pull --rebase=true" that is a coincidence.

Jonathan Nieder <jrnieder@gmail.com> writes:

Is this documented? Except implicitly by the: 

-p, --preserve-merges
           Instead of ignoring merges, try to recreate them.

??

Anyway, why such a requirement, and is it actually enforced by tests?

Maybe, but I'd argue it's rather sane behavior to do no rebase when new rebase point is the same as original in general. I.e., when "current branch is a descendant of the commit you are rebasing onto", as documentation says.

I still do, as it will match documentation, that in turn looks reasonable.

If the problem is in documentation, it's not only misleading, it's formally wrong.

Well, if it's indeed documentation, how about this: