threads / discuss / 19719

[PATCHv2 2/3] parse-remote: support default reflist in get_remote_merge_branch

Subject: [PATCHv2 2/3] parse-remote: support default reflist in get_remote_merge_branch

## tl;dr

3 messages between Jun 7, 2009 and Jun 7, 2009.

replies: 2people: 2as markdown or json

Santi Béjar· Jun 7, 2009, 09:44 UTC · lore

Expand get_remote_merge_branch to compute the tracking branch to merge when called without arguments (or only the remote name). This allows "git pull --rebase" without arguments (default upstream branch) to work with a rebased upstream.

Also add a test to check for this case and another one (failing) to test rebasing two branches on top of a rebased upstream using just 'git pull --rebase'.

Signed-off-by: Santi Béjar <santi@agolina.net>
---
Changes from v1:
  - Hopefully provide a more useful and correct commit message

 git-parse-remote.sh |    8 +++++++-
 t/t5520-pull.sh     |   29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 8b3ba72..5df5689 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -232,7 +232,13 @@ get_remote_refs_for_fetch () {
 get_remote_merge_branch () {
 	case "$#" in
 	0|1)
-	    die "internal error: get-remote-merge-branch." ;;
+	    origin="$1"
+	    default=$(get_default_remote)
+	    test -z "$origin" && origin=$default
+	    curr_branch=$(git symbolic-ref -q HEAD)
+	    [ "$origin" = "$default" ] &&
+	    echo $(git for-each-ref --format='%(upstream)' $curr_branch)
+	    ;;
 	*)
 	    repo=$1
 	    shift
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 725771f..359a3e2 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -92,20 +92,49 @@ test_expect_success '--rebase with rebased upstream' '
 
 	git remote add -f me . &&
 	git checkout copy &&
+	git tag copy-orig &&
 	git reset --hard HEAD^ &&
 	echo conflicting modification > file &&
 	git commit -m conflict file &&
 	git checkout to-rebase &&
 	echo file > file2 &&
 	git commit -m to-rebase file2 &&
+	git tag to-rebase-orig &&
 	git pull --rebase me copy &&
 	test "conflicting modification" = "$(cat file)" &&
 	test file = $(cat file2)
 
 '
 
+test_expect_success '--rebase with rebased default upstream' '
+
+	git update-ref refs/remotes/me/copy copy-orig &&
+	git checkout --track -b to-rebase2 me/copy &&
+	git reset --hard to-rebase-orig &&
+	git pull --rebase &&
+	test "conflicting modification" = "$(cat file)" &&
+	test file = $(cat file2)
+
+'
+
+test_expect_failure '--rebase with rebased upstream and two branches' '
+
+	git update-ref refs/remotes/me/copy copy-orig &&
+	git reset --hard to-rebase-orig &&
+	git checkout --track -b to-rebase3 me/copy &&
+	git reset --hard to-rebase-orig &&
+	git pull --rebase &&
+	test "conflicting modification" = "$(cat file)" &&
+	test file = $(cat file2) &&
+	git checkout to-rebase2 &&
+	git pull --rebase me copy
+
+'
+
 test_expect_success 'pull --rebase dies early with dirty working directory' '
 
+	git rebase --abort &&
+	git checkout to-rebase &&
 	git update-ref refs/remotes/me/copy copy^ &&
 	COPY=$(git rev-parse --verify me/copy) &&
 	git rebase --onto $COPY copy &&

-- 
1.6.3.1.308.g426b5

Junio C Hamano· Jun 7, 2009, 16:59 UTC · re: Santi Béjar · lore

Re: [PATCHv2 2/3] parse-remote: support default reflist in get_remote_merge_branch

Santi Béjar <santi@agolina.net> writes:

> Expand get_remote_merge_branch to compute the tracking branch to merge
> when called without arguments (or only the remote name). This allows
> "git pull --rebase" without arguments (default upstream branch) to
> work with a rebased upstream.

The last sentence leaves readers wondering... "Ok, with this patch, X without Y now works. What about X _with_ Y? Is it left unfixed? Was it already working before this patch? What is going on???"

> Also add a test to check for this case and another one (failing) to
> test rebasing two branches on top of a rebased upstream using just
> 'git pull --rebase'.

"test doing X using just Y" _sounds as if_ you are implying

	Doing X using Z (that is more cumbersome to type than Y) works but
        doing X using Y (that ought to be the equivalent to Z) does not.
	Let's expose this inconsistent breakage.

without saying what Z is, and/or why Y is preferred. So if that is what is going on, please spell these out.

If that is not the case please drop "just"; it is confusing.

Show 29 quoted lines

> +test_expect_success '--rebase with rebased default upstream' '
> +
> +	git update-ref refs/remotes/me/copy copy-orig &&
> +	git checkout --track -b to-rebase2 me/copy &&
> +	git reset --hard to-rebase-orig &&
> +	git pull --rebase &&
> +	test "conflicting modification" = "$(cat file)" &&
> +	test file = $(cat file2)
> +
> +'
> +
> +test_expect_failure '--rebase with rebased upstream and two branches' '
> +
> +	git update-ref refs/remotes/me/copy copy-orig &&
> +	git reset --hard to-rebase-orig &&
> +	git checkout --track -b to-rebase3 me/copy &&
> +	git reset --hard to-rebase-orig &&
> +	git pull --rebase &&
> +	test "conflicting modification" = "$(cat file)" &&
> +	test file = $(cat file2) &&
> +	git checkout to-rebase2 &&
> +	git pull --rebase me copy
> +
> +'
> +
>  test_expect_success 'pull --rebase dies early with dirty working directory' '
>  
> +	git rebase --abort &&
> +	git checkout to-rebase &&

Hmm, saying "--abort" when rebase is not in progress (i.e. after your next patch fixes the above "expect_failure" to pass) does not error out? It is not very nice...

Show 5 quoted lines

>  	git update-ref refs/remotes/me/copy copy^ &&
>  	COPY=$(git rev-parse --verify me/copy) &&
>  	git rebase --onto $COPY copy &&
> -- 
> 1.6.3.1.308.g426b5

Santi Béjar· Jun 7, 2009, 18:02 UTC · re: Junio C Hamano · lore

Re: [PATCHv2 2/3] parse-remote: support default reflist in get_remote_merge_branch

2009/6/7 Junio C Hamano <gitster@pobox.com>:

Show 10 quoted lines

> Santi Béjar <santi@agolina.net> writes:
>
>> Expand get_remote_merge_branch to compute the tracking branch to merge
>> when called without arguments (or only the remote name). This allows
>> "git pull --rebase" without arguments (default upstream branch) to
>> work with a rebased upstream.
>
> The last sentence leaves readers wondering...  "Ok, with this patch, X
> without Y now works.  What about X _with_ Y?  Is it left unfixed?  Was it
> already working before this patch?  What is going on???"

No changes in the "with Y" case, so working.

Maybe add the sentence "( with explicit arguments it already worked)".

Show 13 quoted lines

>
>> Also add a test to check for this case and another one (failing) to
>> test rebasing two branches on top of a rebased upstream using just
>> 'git pull --rebase'.
>
> "test doing X using just Y" _sounds as if_ you are implying
>
>        Doing X using Z (that is more cumbersome to type than Y) works but
>        doing X using Y (that ought to be the equivalent to Z) does not.
>        Let's expose this inconsistent breakage.
>
> without saying what Z is, and/or why Y is preferred.  So if that is what
> is going on, please spell these out.

If you have two branches tracking an upstream that is rebased, currently you have to do:

git checkout branch1 git pull --rebase remote branch git checkout branch2 git pull --rebase remote branch

The second rebase works because the first "git pull --rebase" does not store in the local tracking branch the new value, so the second rebase detects that it is rebased.

I think one should be able to do the same without the explicit arguments to "git pull --rebase", but without arguments it stores the new state of the remote branch so the second "git pull --rebase" does not work.

I just wanted to single out that it does not currently works.

I see two solutions for this: 1) declare that it is not going to work and to do it you have to do the explicit invocation or 2) examine the reflog of the remote tracking branch.

> If that is not the case please drop "just"; it is confusing.

I'll add some of this explanation to the commit message, and keep the "just" (they should be equivalent, but are not).

Show 32 quoted lines

>
>> +test_expect_success '--rebase with rebased default upstream' '
>> +
>> +     git update-ref refs/remotes/me/copy copy-orig &&
>> +     git checkout --track -b to-rebase2 me/copy &&
>> +     git reset --hard to-rebase-orig &&
>> +     git pull --rebase &&
>> +     test "conflicting modification" = "$(cat file)" &&
>> +     test file = $(cat file2)
>> +
>> +'
>> +
>> +test_expect_failure '--rebase with rebased upstream and two branches' '
>> +
>> +     git update-ref refs/remotes/me/copy copy-orig &&
>> +     git reset --hard to-rebase-orig &&
>> +     git checkout --track -b to-rebase3 me/copy &&
>> +     git reset --hard to-rebase-orig &&
>> +     git pull --rebase &&
>> +     test "conflicting modification" = "$(cat file)" &&
>> +     test file = $(cat file2) &&
>> +     git checkout to-rebase2 &&
>> +     git pull --rebase me copy
>> +
>> +'
>> +
>>  test_expect_success 'pull --rebase dies early with dirty working directory' '
>>
>> +     git rebase --abort &&
>> +     git checkout to-rebase &&
>
> Hmm, saying "--abort" when rebase is not in progress

The rebase is in progress (the last test failed)

> (i.e. after your next
> patch fixes the above "expect_failure" to pass) does not error out?  It is
> not very nice...

My next patch does not fix the expect_failure, I only wanted to make this behavior explicit. But if at the end this is the prefered behavior (fail rebasing two branches with a rebased upstream) I'll drop this test.

Santi