[BUG] - git rebase -i performs rebase when it shouldn't?

Hi,

In case of this situation

       A  master
        \
         B  next
          \
           C  topic

$ git rebase --onto master topic First, rewinding head to replay your work on top of it... fatal: Not a range. Nothing to do.

Which is OK.

When I fired up interactive rebase, it printed me “noop”, so I was expecting it to do nothing and exited the vi, without deleting this "noop". But it did performed some operation and I actually have commit C lost, with topic pointing to the same commit as master.

$ git rebase -i --onto master topic Successfully rebased and updated refs/heads/topic.

I believe this is wrong.

Thanks, Eugene

On Fri, Apr 09, 2010 at 03:35:42PM -0400, Eugene Sajine wrote:

I think this doesn't do quite what you thought. It's true there is "nothing to do" as in "nothing to apply", but it _did_ in fact rewind topic back to "master".

You seem to be thinking that

  git rebase --onto master topic

means "rebase everything from master to topic onto master". It doesn't. That would be:

  git rebase master topic

or, if you are already on topic, just

  git rebase master

The "--onto" option takes an argument, which says "put the commits on top of here, even though it was not the upstream base otherwise specified". So what your command does is say "using the current branch (which is topic), take everything built on top of topic (which is nothing), and rebuild it on top of master".

So no, it's not a bug. Yes, it's a terrible interface. There is really no reason IMHO for rebase to take a "which branch to operate on" argument at all. It should just operate on HEAD, like merge does. If you want to merge on a different branch, you "git checkout" that branch first.

That would have made your error less likely, because you would have had no reason to think you needed to say "topic" at all.

-Peff

Jeff King <peff@peff.net> writes:

I guess it's my turn to say "sorry it was my fault made in pre-maintainer days around 59e6b23 ([PATCH] git-rebase-script: rebase local commits to new upstream head., 2005-06-25).

I used to religiously rebase slushy topics that are not in any stable integration branches (we used to have only 'master' and 'pu'), and

	git rebase master foo
        git rebase master bar
        git rebase master baz

was far easier to type than

	git checkout foo && git rebase master
	git checkout bar && git rebase master
	git checkout baz && git rebase master

when you have many topics. Especially because the exact keystroke sequences are:

	git rebase master foo ENTER
	^P (recall the last line) ^W (delete the last word) bar ENTER
	^P ^W baz ENTER

for the former.  Of course, I can do the same with the latter interface

	sh -c 'git checkout "$1" && git rebase master' - foo ENTER
	^P (recall the last line) ^W (delete the last word) bar ENTER
	^P ^W baz ENTER

so that is not a very good excuse, but at least now people know where it came from ;-)

On Fri, Apr 09, 2010 at 09:39:22PM -0700, Junio C Hamano wrote:

Interestingly, I faced a similar problem a few months ago. I have many half-finished topics, and I aggressively rebase them against master (mostly to see and fix conflicts early). But I wrote a script so I don't have to type each one individually. :)

It uses for-each-ref with a filter to get the list of topics, and then rebases in a loop. It just barfs when a rebase has conflicts, which is OK. You fix them, finish the rebase, and restart the script, which just skips over the noop rebases. Eventually it finishes successfully.

-Peff

Am 10.04.2010 06:39, schrieb Junio C Hamano:

I'm actually very glad that the current interface is the way it is - because it can do the rebase *without* the checkout. This way you can save a lot of recompilation due to changed timestamps if the topic is based on an old version.

Only that some (all?) variants of rebase still unnecessarily do the checkout...

Oh, and it would naturally extend to a master..topic syntax.

-- Hannes

On Sat, Apr 10, 2010 at 09:58:54PM +0200, Johannes Sixt wrote:

Good point. Originally, we did the rebase directly on the branch, though I'm not sure if we did "checkout $branch && reset $onto" or "branch -f $branch $onto && checkout $branch". These days we operate on a detached HEAD, and we seem to "checkout $onto^0", which should do the optimization you mention.

-Peff

On Sonntag, 11. April 2010, Jeff King wrote:

But before this "checkout $onto^0" happens, some (all?) variants still do "checkout topic && rev-list upstream..HEAD" instead of just "rev-list upstream..topic".

-- Hannes

On Sun, Apr 11, 2010 at 07:54:37PM +0200, Johannes Sixt wrote:

I don't think this is the case any longer for regular rebase, from my reading of the code and doing a rebase under GIT_TRACE. Grepping the history turns up 0cb0664 (rebase [--onto O] A B: omit needless checkout, 2008-03-15).

But doing an interactive rebase under GIT_TRACE, it looks like it still does the unnecessary checkout.

-Peff

On 10.04.2010 21:58 CEST, Johannes Sixt wrote:

 Is there any chance for merge to behave the same? I really like that I
 don't have to do a checkout prior rebasing.

        Thanks,

On Montag, 12. April 2010, Michal Vitecek wrote:

If you mean "prior to merging", i.e., you want a "git merge --into master", you can do it this way:

# we are on branch 'topic' here git checkout HEAD^0 # detach git merge master git reset --soft master # keep index(!!!) and worktree git checkout master # re-attach git merge topic

The point is that the second merge will succeed even with a dirty index because for each file the result of the merge is identical to the index!

If you have conflicts during the merge, resolve them in the first merge and commit to prime the rerere database; the second merge will fail for each conflicting file; just 'git checkout HEAD -- the/conflicting/file' and retry. rerere will resolve the conflicts for you.

You can of course get away with only one merge, but then you will have to do extra work to switch the parents around and to fix the commit message.

-- Hannes

On Sat, Apr 10, 2010 at 12:26 AM, Jeff King <peff@peff.net> wrote:

Actually no, i was not thinking about what you think i was;). What i was trying to understand with this command (git rebase --onto master topic) is the behavior of the system when the topic branch is indirect descendant of the master and the direct parent of topic (next) is omitted, skipped.

In this situatiion

git rebase master topic

does not work (see thread "git rebase command and docs questions") p1.

So, once again i think that the interface in this case worked properly:

as topic is not direct descendant, master option value goes to --onto and there is no range defined properly. Therefore the command worked ok, when it refused to do anything.

Now the problem i have is that:

git rebase -i --onto master topic

actually worked and did something, what i would not expect it to do.

So, the problem is: non-interactive rebase DOES NOT execute the command, interactive DOES execute.

The bug is in the fact that rebase works differently in interactive and non-interactive variants.

Thanks, Eugene

On Sat, Apr 10, 2010 at 06:10:53PM -0400, Eugene Sajine wrote:

But in "git rebase --onto master topic", the relationship between master and topic is irrelevant. It is the same as:

  git rebase --onto master topic HEAD

which will consider the range between topic and HEAD as the set of commits to rebase.

Did you want to do:

  git rebase --onto master next topic

? That would take the commits between next and topic (i.e., just "topic" in your example), and rebuild them on top of master.

That's not the result I get. The non-interactive rebase _does_ do the same thing. Try this:

  mkdir repo
  cd repo
  git init

  echo content >>file && git add file && git commit -m one
  git checkout -b next
  echo content >>file && git add file && git commit -m two
  git checkout -b topic
  echo content >>file && git add file && git commit -m three

  git rebase --onto master topic

You will see that "topic" has been reset back to commit one, the same as master.

If that was not happening before, it was likely because you were not actually on the "topic" branch before. So who knows what the implicit "HEAD" argument referred to.

I don't think it does, as shown by my example above. If you still think so, please create a short test case that demonstrates the difference.

-Peff

I got your point about the HEAD which is by default get's added to the end of the command, so it becomes

git rebase --onto master topic HEAD

I will think about it it more.

I'm pretty sure that i was surprised by the fact that i've got different behavior in interactive and non-interactive variants, but i will recheck.

I forgot to mention it was in windows version of git. I will try to provide the printout of my actions tomorrow when i will get to this machine.

Thanks a lot, Eugene

On Sun, Apr 11, 2010 at 6:22 AM, Jeff King <peff@peff.net> wrote:

On Sun, Apr 11, 2010 at 10:06 AM, Eugene Sajine <euguess@gmail.com> wrote:

Here is the printout of my actions with some comments:

esajine@ESAJINEWWW /c/git_repos $ mkdir test2; cd test2; git init; echo "initial" > 1.txt; git add .; git commit -m "initial commit"; git checkout -b next; echo "commit from next" > 2.txt; git add .; git commit -m "commit from next"; git checkout -b topic; echo "commit from topic" > 3.txt; git add .; git commit -m "commit from topic" Initialized empty Git repository in c:/git_repos/test2/.git/ warning: LF will be replaced by CRLF in 1.txt [master (root-commit) 89c5353] initial commit warning: LF will be replaced by CRLF in 1.txt 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 1.txt Switched to a new branch 'next' warning: LF will be replaced by CRLF in 1.txt warning: LF will be replaced by CRLF in 2.txt [next cec1eeb] commit from next warning: LF will be replaced by CRLF in 2.txt 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 2.txt Switched to a new branch 'topic' warning: LF will be replaced by CRLF in 1.txt warning: LF will be replaced by CRLF in 2.txt warning: LF will be replaced by CRLF in 3.txt [topic 9975772] commit from topic warning: LF will be replaced by CRLF in 3.txt 1 files changed, 1 insertions(+), 0 deletions(-) create mode 100644 3.txt

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ pwd /c/git_repos/test2

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ git status # On branch topic nothing to commit (working directory clean)

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git log
commit 9975772c641c438a0a77932c18c159e9551706ce
Author: Eugene Sajine <esajine@foo.com>
Date:   Fri Apr 9 13:04:24 2010 -0400

   commit from topic

commit cec1eebc8db042b1b8a1002b07767eac56884126
Author: Eugene Sajine <esajine@foo.com>
Date:   Fri Apr 9 13:04:24 2010 -0400

   commit from next

commit 89c5353c9b0cfc476b323db11565ea210341469a
Author: Eugene Sajine <esajine@foo.com>
Date:   Fri Apr 9 13:04:24 2010 -0400

   initial commit

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ git rebase master Current branch topic is up to date. <======= Really? Topic is actually based on next – what does this "up to date" mean??

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git rebase --onto master topic
First, rewinding head to replay your work on top of it...
fatal: Not a range.
Nothing to do.
                <======== topic..HEAD is not a range, agreed

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git rebase --onto master
fatal: Needed a single revision
invalid upstream
              <========= fine

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git rebase --onto master next..topic
fatal: Needed a single revision
invalid upstream next..topic
    <======== strange that this notation is not supported, considering
error above about the range

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ git rebase -i --onto master topic Successfully rebased and updated refs/heads/topic. <=== BUG – here it printed me “noop” in file to edit, when I exited it should do nothing, but it still did something and I double checked it.

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ git rebase --onto master topic Current branch topic is up to date.

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git log
commit 89c5353c9b0cfc476b323db11565ea210341469a
Author: Eugene Sajine <esajine@foo.com>
Date:   Fri Apr 9 13:04:24 2010 -0400

   initial commit
        <====== topic now points to master, last commit from topic is
lost

esajine@ESAJINEWWW /c/git_repos $ git version git version 1.6.4.msysgit.0

Thanks, Eugene

Am 4/12/2010 16:09, schrieb Eugene Sajine:

Why should rebase bother? The difference between master and topic are *two* commits. Since these two are already on top of master in linear history, you get no advantage by doing a rebase operation. Therefore, you see "already up to date".

The fact that the second commit from the tip of topic is also labeled "next" is absolutely irrelevant to rebase. rebase only looks at the refs that you tell it about: master and topic (implicitly pointed to by HEAD), nothing else.

What you really want to do, obviously, is:

   git rebase --onto master next topic

No, there is no shorter form to spell this operation.

Not a bug.

Your command is the same as

   git rebase -i --onto master topic topic

because you are already on branch topic. Since there are no commits in the range topic..topic, rebase -i told you "noop". This word is perhaps poorly chosen, because it does not mean "no operation"[*], but "there are no commits to transfer". But branch 'topic' that you gave as the last argument (or implicitly by being at branch 'topic') is still transferred --onto master. This explains the result that you observed.

Of course, if you do not 'reset --hard topic@{1}' at this point, you will ultimately lose the commits on branch topic.

[*] You can get "no operation" by deleting the line "noop".

-- Hannes

On Mon, Apr 12, 2010 at 11:13 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:

You lost me completely... Rebase means change the base of the commit, change the fork point. Current fork point for topic is next. I want it to be master. What is up to date here??? The message is poorly worded for sure.

I know that the form i have to use is:

git rebase --onto master next topic

but it is just because topic is not direct descendant of master, isn't it?

Come on! Please, please, explain me why it behaves DIFFERENTLY:

esajine@ESAJINEWWW /c/git_repos/test2 (topic)
$ git rebase --onto master topic
First, rewinding head to replay your work on top of it...
fatal: Not a range.
Nothing to do.
                <======== topic..HEAD is not a range, agreed

esajine@ESAJINEWWW /c/git_repos/test2 (topic) $ git rebase -i --onto master topic Successfully rebased and updated refs/heads/topic. <=== BUG – here it printed me “noop” in file to edit, when I exited it should do nothing, but it still did something and I double checked it.

Thanks, Eugene

Am 4/12/2010 17:28, schrieb Eugene Sajine:

Rebase is meant to change the fork point for a history like this:

  --o--o--A--o--o--B  <-- master
           \
            o
             \
              o       <-- topic

topic forks from master at commit A; you use rebase to create history that looks like this:

  --o--o--A--o--o--B     <-- master
                    \
                     o
                      \
                       o <-- topic

(OK, you knew that already.)

You are already in the second situation above. The fact that you labeled one commit "next" in between, like this:

  --o--o--A--o--o--B     <-- master
                    \
                     o   <-- next
                      \
                       o <-- topic

does not change the meaning of the command "git rebase master" in the slightest: topic is "up to date" with respect to master.

Watch out the wording that you use: "descendant of" has a well-defined meaning in git, in particular, topic *is* a direct descendant of master.

What you wanted to say is: "but it is just because I have accidentally begun to commit 'topic' on top of 'next', but I didn't want to do that".

And yes, you are right, because of this you need --onto in the rebase command.

A historical accident, so to say. The implementor of interactive rebase felt the "noop" behavior was useful, and I agree, FWIW.

-- Hannes

IMHO this "noop" behavior seems to be identical to "git reset --hard master" - the result is the same. Frankly, I don't understand what is useful in such thing being HIDDEN in "incorrect call" of interactive rebase...

Thanks, Eugene

Am 4/13/2010 18:30, schrieb Eugene Sajine:

This thing is useful when you 'git rebase -i master topic' and all patches on topic are already in master.

-- Hannes