[PATCH] rebase--interactive: don't enforce valid branch

[PATCH] rebase--interactive: don't enforce valid branch

[Dave Olszewski]

git rebase allows you to specify a non-branch commit-ish as the "branch"
argument, which leaves HEAD detached when it's finished.  This is
occasionally useful, and this patch brings the same functionality to git
rebase ---interactive.

Signed-off-by: Dave Olszewski <cxreg@pobox.com>
---
 git-rebase--interactive.sh    |    2 --
 t/t3404-rebase-interactive.sh |    7 +++++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..d047dcb 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -783,8 +783,6 @@ first and then run 'git rebase --continue' again."
 
 		if test ! -z "$1"
 		then
-			output git show-ref --verify --quiet "refs/heads/$1" ||
-				die "Invalid branchname: $1"
 			output git checkout "$1" ||
 				die "Could not checkout $1"
 		fi
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4e35137..32ffa15 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -553,4 +553,11 @@ test_expect_success 'reword' '
 	git show HEAD~2 | grep "C changed"
 '
 
+test_expect_success 'rebase while detaching HEAD' '
+	grandparent=$(git rev-parse HEAD~2) &&
+	test_tick &&
+	FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
+	test $grandparent = $(git rev-parse HEAD~2)
+'
+
 test_done
-- 
1.7.0.2.213.gd6898.dirty

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Junio C Hamano]

Dave Olszewski <cxreg@pobox.com> writes:

> git rebase allows you to specify a non-branch commit-ish as the "branch"
> argument, which leaves HEAD detached when it's finished.  This is
> occasionally useful, and this patch brings the same functionality to git
> rebase ---interactive.

Three dashes?

> +test_expect_success 'rebase while detaching HEAD' '
> +	grandparent=$(git rev-parse HEAD~2) &&
> +	test_tick &&
> +	FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&

What's the point of saying this?  You could instead say:

	git rebase -i HEAD~2

no?

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Dave Olszewski]

On Sun, 14 Mar 2010, Junio C Hamano wrote:

> Dave Olszewski <cxreg@pobox.com> writes:
>
>> git rebase allows you to specify a non-branch commit-ish as the "branch"
>> argument, which leaves HEAD detached when it's finished.  This is
>> occasionally useful, and this patch brings the same functionality to git
>> rebase ---interactive.
>
> Three dashes?

Oops, good catch

>> +test_expect_success 'rebase while detaching HEAD' '
>> +	grandparent=$(git rev-parse HEAD~2) &&
>> +	test_tick &&
>> +	FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
>
> What's the point of saying this?  You could instead say:
>
> 	git rebase -i HEAD~2
>
> no?

There's already a test for rebasing on a previously detached HEAD.  The
form "git rebase -i HEAD~2" specifies a non-branch upstream, but doesn't
take the branch argument which is the point of the change.

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Junio C Hamano]

Dave Olszewski <cxreg@pobox.com> writes:

>>> +test_expect_success 'rebase while detaching HEAD' '
>>> +	grandparent=$(git rev-parse HEAD~2) &&
>>> +	test_tick &&
>>> +	FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
>>
>> What's the point of saying this?  You could instead say:
>>
>> 	git rebase -i HEAD~2
>>
>> no?
>
> There's already a test for rebasing on a previously detached HEAD.  The
> form "git rebase -i HEAD~2" specifies a non-branch upstream, but doesn't
> take the branch argument which is the point of the change.

What I meant was that if you prefer to work on a detached HEAD (and I
sometimes do), then your HEAD would likely to be detached already when you
run rebase.  IOW, I would expect that 

	git checkout HEAD^0
        ... perhaps do something, perhaps do nothing, here ...
        git rebase -i HEAD~2

would be a lot more natural thing to do, and in that case you do not need
to say HEAD^0 there.

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Dave Olszewski]

On Sun, 14 Mar 2010, Junio C Hamano wrote:

>> There's already a test for rebasing on a previously detached HEAD.  The
>> form "git rebase -i HEAD~2" specifies a non-branch upstream, but doesn't
>> take the branch argument which is the point of the change.
>
> What I meant was that if you prefer to work on a detached HEAD (and I
> sometimes do), then your HEAD would likely to be detached already when you
> run rebase.  IOW, I would expect that
>
> 	git checkout HEAD^0
>        ... perhaps do something, perhaps do nothing, here ...
>        git rebase -i HEAD~2
>
> would be a lot more natural thing to do, and in that case you do not need
> to say HEAD^0 there.

That functionality already works.  It's certainly possible to do it as
you describe, but why require the extra step?  git-rebase already
supports detaching as part of its behavior, but rebase -i does not.
It may seem nitpicky, but it's asymmetric and occasionally it's handy
syntax.

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Dave Olszewski <cxreg@pobox.com> writes:
>
>>>> +test_expect_success 'rebase while detaching HEAD' '
>>>> +	grandparent=$(git rev-parse HEAD~2) &&
>>>> +	test_tick &&
>>>> +	FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
>>>
>>> What's the point of saying this?  You could instead say:
>>>
>>> 	git rebase -i HEAD~2
>>>
>>> no?
>> ...

Ahh, Ok, the point is that when we start this sequence we are on a branch,
and then you want to end up on a detached HEAD that points at the result
of the branch.

I'll queue it in 'pu', but with a little tweak to the test to make it
clear what is going on, perhaps like this.

    test_expect_success 'rebase while detaching HEAD' '
            git symbolic-ref HEAD &&
            grandparent=$(git rev-parse HEAD~2) &&
            test_tick &&
            FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
            test $grandparent = $(git rev-parse HEAD~2) &&
            test_must_fail git symbolic-ref HEAD
    '

We may need to document this behaviour, by the way, if we make it official
that the extra "branch to be rewritten" parameter can be a non-branch.
Two points are that you can give arbitrary commit, and that you will end
up with a detached HEAD that points at the result if you did so.

Also I did't followed the code, but does it behave sanely when you say
"rebase --abort"?

Re: Re: [PATCH] rebase--interactive: don't enforce valid branch

[Dave Olszewski]

On Sun, 14 Mar 2010, Junio C Hamano wrote:

> Ahh, Ok, the point is that when we start this sequence we are on a branch,
> and then you want to end up on a detached HEAD that points at the result
> of the branch.

Yep, you got it


> I'll queue it in 'pu', but with a little tweak to the test to make it
> clear what is going on, perhaps like this.
>
>    test_expect_success 'rebase while detaching HEAD' '
>            git symbolic-ref HEAD &&
>            grandparent=$(git rev-parse HEAD~2) &&
>            test_tick &&
>            FAKE_LINES="2 1" git rebase -i HEAD~2 HEAD^0 &&
>            test $grandparent = $(git rev-parse HEAD~2) &&
>            test_must_fail git symbolic-ref HEAD
>    '

Good idea.  Thanks.


> We may need to document this behaviour, by the way, if we make it official
> that the extra "branch to be rewritten" parameter can be a non-branch.
> Two points are that you can give arbitrary commit, and that you will end
> up with a detached HEAD that points at the result if you did so.
>
> Also I did't followed the code, but does it behave sanely when you say
> "rebase --abort"?

Good question.  It turns out that both rebase and rebase -i will end
up on the commit specified by <branch>, whether it's a branch or not.
That might be the expected and desired behavior, though:

   [Starting on branch A]
   git rebase origin/B B
   git rebase --abort
   [HEAD is now a symref to B]

   [Starting on branch A]
   git rebase origin/B B^0
   git rebase --abort
   [HEAD is now detached at B^0]

Re: [PATCH] rebase--interactive: don't enforce valid branch

[Junio C Hamano]

Dave Olszewski <cxreg@pobox.com> writes:

>> Also I did't follow the code, but does it behave sanely when you say
>> "rebase --abort"?
>
> Good question.  It turns out that both rebase and rebase -i will end
> up on the commit specified by <branch>, whether it's a branch or not.
> That might be the expected and desired behavior, though:
>
>   [Starting on branch A]
>   git rebase origin/B B
>   git rebase --abort
>   [HEAD is now a symref to B]
>
>   [Starting on branch A]
>   git rebase origin/B B^0
>   git rebase --abort
>   [HEAD is now detached at B^0]

Yup, that is what I would call "behave sanely".  Thanks for clarification.