threads / patch / 23018

patchrebase--interactive: don't enforce valid branch

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

## tl;dr

8 messages between Mar 15, 2010 and Mar 15, 2010. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Dave Olszewski· Mar 15, 2010, 04:48 UTC · lore

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(-)
Show changes to 2 files +7 −2

git-rebase--interactive.sh, t/t3404-rebase-interactive.sh

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
Junio C Hamano· Mar 15, 2010, 05:20 UTC · re: Dave Olszewski · lore

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

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?
Dave Olszewski· Mar 15, 2010, 05:28 UTC · re: Junio C Hamano · lore

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

On Sun, 14 Mar 2010, Junio C Hamano wrote:
Show 8 quoted lines
> 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
Show 10 quoted lines
>> +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.

Junio C Hamano· Mar 15, 2010, 05:42 UTC · re: Dave Olszewski · lore

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

Dave Olszewski <cxreg@pobox.com> writes:
Show 14 quoted lines
>>> +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.

Dave Olszewski· Mar 15, 2010, 05:46 UTC · re: Junio C Hamano · lore

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

On Sun, 14 Mar 2010, Junio C Hamano wrote:
Show 14 quoted lines
>> 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.

Junio C Hamano· Mar 15, 2010, 06:14 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 13 quoted lines
> 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"?

Dave Olszewski· Mar 15, 2010, 08:41 UTC · re: Junio C Hamano · lore

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

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
Show 11 quoted lines
> 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.
Show 7 quoted lines
> 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]
Junio C Hamano· Mar 15, 2010, 17:14 UTC · re: Dave Olszewski · lore

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

Dave Olszewski <cxreg@pobox.com> writes:
Show 16 quoted lines
>> 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.

← back to recent threads