mbox series

[0/2] git rebase: Make sure upstream branch is left alone.

Message ID 20190818095349.3218-1-ben@wijen.net (mailing list archive)
Headers show
Series git rebase: Make sure upstream branch is left alone. | expand

Message

Ben Wijen Aug. 18, 2019, 9:53 a.m. UTC
I found an issue with git rebase --autostash <upstream> <branch> with an dirty worktree/index.
It seems the currently active branch is moved, which is not correct.
The following patches contain both a test and a fix.

Ben Wijen (2):
  t3420: never change upstream branch
  rebase.c: make sure current branch isn't moved when autostashing

 builtin/rebase.c            | 18 ++++++------------
 t/t3420-rebase-autostash.sh | 13 +++++++++----
 2 files changed, 15 insertions(+), 16 deletions(-)

Comments

Phillip Wood Aug. 19, 2019, 9:26 a.m. UTC | #1
Hi Ben

On 18/08/2019 10:53, Ben Wijen wrote:
> I found an issue with git rebase --autostash <upstream> <branch> with an dirty worktree/index.
> It seems the currently active branch is moved, which is not correct.

I'm not sure I understand what you mean by "the currently active branch 
is moved". 'git rebase --autostash <upstream> <branch>' should checkout 
<branch> (presumably stashing any unstaged and uncommitted changes 
first) and then do rebase <upstream> - what's it doing instead?

Best Wishes

Phillip

> The following patches contain both a test and a fix.
> 
> Ben Wijen (2):
>    t3420: never change upstream branch
>    rebase.c: make sure current branch isn't moved when autostashing
> 
>   builtin/rebase.c            | 18 ++++++------------
>   t/t3420-rebase-autostash.sh | 13 +++++++++----
>   2 files changed, 15 insertions(+), 16 deletions(-)
>
Ben Wijen Aug. 19, 2019, 3:33 p.m. UTC | #2
Hi Phillip,

The expected behavior: (as per v2.21.0:/git-legacy-rebase.sh:679)
    AUTOSTASH=$(git stash create autostash)
    git reset --hard
    git checkout master
    git rebase upstream
    git stash apply $AUTOSTASH 


The actual behavior:
    AUTOSTASH=$(git stash create autostash)
    git reset --hard master
    git checkout master
    git rebase upstream
    git stash apply $AUTOSTASH 

So, the problem with the actual behavior is the move of the currently active branch with 'git reset --hard master'


Best regards,

Ben...

On 19-08-2019 11:26, Phillip Wood wrote:
> Hi Ben
> 
> On 18/08/2019 10:53, Ben Wijen wrote:
>> I found an issue with git rebase --autostash <upstream> <branch> with an dirty worktree/index.
>> It seems the currently active branch is moved, which is not correct.
> 
> I'm not sure I understand what you mean by "the currently active branch is moved". 'git rebase --autostash <upstream> <branch>' should checkout <branch> (presumably stashing any unstaged and uncommitted changes first) and then do rebase <upstream> - what's it doing instead?
> 
> Best Wishes
> 
> Phillip
> 
>> The following patches contain both a test and a fix.
>>
>> Ben Wijen (2):
>>    t3420: never change upstream branch
>>    rebase.c: make sure current branch isn't moved when autostashing
>>
>>   builtin/rebase.c            | 18 ++++++------------
>>   t/t3420-rebase-autostash.sh | 13 +++++++++----
>>   2 files changed, 15 insertions(+), 16 deletions(-)
>>
>
Junio C Hamano Aug. 19, 2019, 6:21 p.m. UTC | #3
Ben <ben@wijen.net> writes:

> Hi Phillip,
>
> The expected behavior: (as per v2.21.0:/git-legacy-rebase.sh:679)
>     AUTOSTASH=$(git stash create autostash)
>     git reset --hard
>     git checkout master
>     git rebase upstream
>     git stash apply $AUTOSTASH 
>
>
> The actual behavior:
>     AUTOSTASH=$(git stash create autostash)
>     git reset --hard master
>     git checkout master
>     git rebase upstream
>     git stash apply $AUTOSTASH 
>
> So, the problem with the actual behavior is the move of the currently active branch with 'git reset --hard master'

Your expected and actual behaviour are described above, but it is
not mentioned out of what command (and in what settings) you expect
the above behaviour.

I am guessing that the setup and the command is something along this
line?

    git checkout not-the-master
    work work work
    git rebase --autostash upstream master

That is, you have checked out a branch that is not the 'master'
branch, you have accumulated some work in the working tree, and then
you ask "please stash away the work in progress and then rebase the
'master' branch on top of the upstream branch"?

If so, my expectation for the third step (i.e. the actual "rebase")
aligns with yours.  After stashing away the local change, we want to
get a clean working tree, checkout the master branch and rebase it
on top of the upstream.  With the command sequence you wrote in the
"actual" section, after stashing away the local change, the current
branch that is *not* the master is reset to the tip of 'master',
which would not be what we want.