{"thread":{"id":"25734","subject":"[PATCH] Use reflog in 'pull --rebase . foo'","startedAt":"2010-11-12T19:38:28Z","lastAt":"2010-11-13T23:10:55Z","messageCount":6,"participants":["Martin von Zweigbergk","Andreas Schwab","Santi Béjar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"155815","messageId":"1289590708-11064-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"25734","inReplyTo":null,"subject":"[PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-12T19:38:28Z","receivedAt":"2010-11-12T19:38:28Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since c85c792 (pull --rebase: be cleverer with rebased upstream\nbranches, 2008-01-26), \"git pull --rebase\" has used the reflog to try to\nrebase from the old upstream onto the new upstream.\n\nMake this work if the local repository is explicitly passed on the\ncommand line as in 'git pull --rebase . foo'.\n---\nI removed some seemingly unnecessary variables. I couldn't find any uses\nof them in the callers' code either, so it should be safe...\n\nI apparently introduced a call to sed as well. I think I remember seeing\nsome guideslines about the use of sed in Git, but I can't remember what.\nI couldn't find anything in the CodingGuidelines either. Is the code\nbelow ok?\n\n git-parse-remote.sh |   24 ++++++++++--------------\n t/t5520-pull.sh     |    7 +++++++\n 2 files changed, 17 insertions(+), 14 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 5f47b18..0565edd 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -63,33 +63,29 @@ get_default_remote () {\n get_remote_merge_branch () {\n \tcase \"$#\" in\n \t0|1)\n-\t    origin=\"$1\"\n-\t    default=$(get_default_remote)\n-\t    test -z \"$origin\" && origin=$default\n \t    curr_branch=$(git symbolic-ref -q HEAD)\n-\t    [ \"$origin\" = \"$default\" ] &&\n+\t    test -z \"$1\" || test \"$1\" = $(get_default_remote) &&\n \t    echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n \t    ;;\n \t*)\n \t    repo=$1\n \t    shift\n-\t    ref=$1\n \t    # FIXME: It should return the tracking branch\n \t    #        Currently only works with the default mapping\n-\t    case \"$ref\" in\n-\t    +*)\n-\t\tref=$(expr \"z$ref\" : 'z+\\(.*\\)')\n-\t\t;;\n-\t    esac\n-\t    expr \"z$ref\" : 'z.*:' >/dev/null || ref=\"${ref}:\"\n-\t    remote=$(expr \"z$ref\" : 'z\\([^:]*\\):')\n+\t    remote=$(echo \"$1\" | sed -e 's|+\\?\\([^:]*\\):\\?|\\1|g')\n \t    case \"$remote\" in\n \t    '' | HEAD ) remote=HEAD ;;\n \t    heads/*) remote=${remote#heads/} ;;\n \t    refs/heads/*) remote=${remote#refs/heads/} ;;\n \t    refs/* | tags/* | remotes/* ) remote=\n \t    esac\n-\n-\t    [ -n \"$remote\" ] && echo \"refs/remotes/$repo/$remote\"\n+\t    [ -n \"$remote\" ] && case \"$repo\" in\n+\t\t.)\n+\t\t    echo \"refs/heads/$remote\"\n+\t\t    ;;\n+\t\t*)\n+\t\t    echo \"refs/remotes/$repo/$remote\"\n+\t\t    ;;\n+\t    esac\n \tesac\n }\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 0b489f5..0470a81 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -222,4 +222,11 @@ test_expect_success 'git pull --rebase does not reapply old patches' '\n \t)\n '\n \n+test_expect_success 'git pull --rebase against local branch' '\n+\tgit checkout -b copy2 to-rebase-orig &&\n+\tgit pull --rebase . to-rebase &&\n+\ttest \"conflicting modification\" = \"$(cat file)\" &&\n+\ttest file = \"$(cat file2)\"\n+'\n+\n test_done\n-- \n1.7.3.2.167.ga361b\n"},{"id":"155821","messageId":"m2pqu98urp.fsf@whitebox.home","threadId":"25734","inReplyTo":"1289590708-11064-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2010-11-13T08:58:34Z","receivedAt":"2010-11-13T08:58:34Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> +\t    remote=$(echo \"$1\" | sed -e 's|+\\?\\([^:]*\\):\\?|\\1|g')\n\n\\? is not portable.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"155823","messageId":"AANLkTiniwpk7ao=M4=5vuahczk6FscQYJE1rZx=wuBk_@mail.gmail.com","threadId":"25734","inReplyTo":"m2pqu98urp.fsf@whitebox.home","subject":"Re: [PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-13T12:09:43Z","receivedAt":"2010-11-13T12:09:43Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"I see, maybe just dropping the middle part of the patch would be better.\nI think it looks like I have lost a '.*' in there as well (test cases\npass though, so I'm not sure). Anyway, better be safe than sorry. The\nfollowing seems better.\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 5f47b18..2e1661d 100644\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -63,11 +63,8 @@ get_default_remote () {\n get_remote_merge_branch () {\n \tcase \"$#\" in\n \t0|1)\n-\t    origin=\"$1\"\n-\t    default=$(get_default_remote)\n-\t    test -z \"$origin\" && origin=$default\n \t    curr_branch=$(git symbolic-ref -q HEAD)\n-\t    [ \"$origin\" = \"$default\" ] &&\n+\t    test -z \"$1\" || test \"$1\" = $(get_default_remote) &&\n \t    echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n \t    ;;\n \t*)\n@@ -89,7 +86,13 @@ get_remote_merge_branch () {\n \t    refs/heads/*) remote=${remote#refs/heads/} ;;\n \t    refs/* | tags/* | remotes/* ) remote=\n \t    esac\n-\n-\t    [ -n \"$remote\" ] && echo \"refs/remotes/$repo/$remote\"\n+\t    [ -n \"$remote\" ] && case \"$repo\" in\n+\t\t.)\n+\t\t    echo \"refs/heads/$remote\"\n+\t\t    ;;\n+\t\t*)\n+\t\t    echo \"refs/remotes/$repo/$remote\"\n+\t\t    ;;\n+\t    esac\n \tesac\n }\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 0b489f5..0470a81 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -222,4 +222,11 @@ test_expect_success 'git pull --rebase does not\nreapply old patches' '\n \t)\n '\n\n+test_expect_success 'git pull --rebase against local branch' '\n+\tgit checkout -b copy2 to-rebase-orig &&\n+\tgit pull --rebase . to-rebase &&\n+\ttest \"conflicting modification\" = \"$(cat file)\" &&\n+\ttest file = \"$(cat file2)\"\n+'\n+\n test_done\n-- \n1.7.3.2.167.ga361b\n"},{"id":"155826","messageId":"AANLkTikmJVj9BM=K2oSYJvPV27Uzn_9SQhsLc9EQm8zQ@mail.gmail.com","threadId":"25734","inReplyTo":"AANLkTiniwpk7ao=M4=5vuahczk6FscQYJE1rZx=wuBk_@mail.gmail.com","subject":"Re: [PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-11-13T17:20:47Z","receivedAt":"2010-11-13T17:20:47Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Sat, Nov 13, 2010 at 1:09 PM, Martin von Zweigbergk\n<martin.von.zweigbergk@gmail.com> wrote:\n> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n> index 5f47b18..2e1661d 100644\n> --- a/git-parse-remote.sh\n> +++ b/git-parse-remote.sh\n> @@ -63,11 +63,8 @@ get_default_remote () {\n>  get_remote_merge_branch () {\n>        case \"$#\" in\n>        0|1)\n> -           origin=\"$1\"\n> -           default=$(get_default_remote)\n> -           test -z \"$origin\" && origin=$default\n>            curr_branch=$(git symbolic-ref -q HEAD)\n> -           [ \"$origin\" = \"$default\" ] &&\n> +           test -z \"$1\" || test \"$1\" = $(get_default_remote) &&\n>            echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n>            ;;\n>        *)\n\nThey are not equivalent, the last line (echo $(git for-each-ref...))\nis always executed, not only when ask for the default remote. When\n$origin != $default the last line does not return the correct answer.\nIt should return nothing, it is not well defined. Or maybe it should\nreturn the branch pointed by $origin/HEAD (I cannot test right now\nwhat 'git pull $remote-not-the-default' merges).\n\nAnd it has nothing to do with letting 'pull --rebase . for\" work.\n\n> @@ -89,7 +86,13 @@ get_remote_merge_branch () {\n>            refs/heads/*) remote=${remote#refs/heads/} ;;\n>            refs/* | tags/* | remotes/* ) remote=\n>            esac\n> -\n> -           [ -n \"$remote\" ] && echo \"refs/remotes/$repo/$remote\"\n> +           [ -n \"$remote\" ] && case \"$repo\" in\n> +               .)\n> +                   echo \"refs/heads/$remote\"\n> +                   ;;\n> +               *)\n> +                   echo \"refs/remotes/$repo/$remote\"\n> +                   ;;\n> +           esac\n>        esac\n>  }\n> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\n> index 0b489f5..0470a81 100755\n> --- a/t/t5520-pull.sh\n> +++ b/t/t5520-pull.sh\n> @@ -222,4 +222,11 @@ test_expect_success 'git pull --rebase does not\n> reapply old patches' '\n>        )\n>  '\n>\n> +test_expect_success 'git pull --rebase against local branch' '\n> +       git checkout -b copy2 to-rebase-orig &&\n> +       git pull --rebase . to-rebase &&\n> +       test \"conflicting modification\" = \"$(cat file)\" &&\n> +       test file = \"$(cat file2)\"\n> +'\n> +\n>  test_done\n\nFor the rest I think they are OK, and make sense.\n\nHTH,\nSanti\n"},{"id":"155829","messageId":"AANLkTineaFVawXbJKHKv39bwPco7JHcmpUwXtZhscaFs@mail.gmail.com","threadId":"25734","inReplyTo":"AANLkTikmJVj9BM=K2oSYJvPV27Uzn_9SQhsLc9EQm8zQ@mail.gmail.com","subject":"Re: [PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-13T17:52:06Z","receivedAt":"2010-11-13T17:52:06Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sat, Nov 13, 2010 at 12:20 PM, Santi Béjar <santi@agolina.net> wrote:\n> On Sat, Nov 13, 2010 at 1:09 PM, Martin von Zweigbergk\n> <martin.von.zweigbergk@gmail.com> wrote:\n>> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n>> index 5f47b18..2e1661d 100644\n>> --- a/git-parse-remote.sh\n>> +++ b/git-parse-remote.sh\n>> @@ -63,11 +63,8 @@ get_default_remote () {\n>>  get_remote_merge_branch () {\n>>        case \"$#\" in\n>>        0|1)\n>> -           origin=\"$1\"\n>> -           default=$(get_default_remote)\n>> -           test -z \"$origin\" && origin=$default\n>>            curr_branch=$(git symbolic-ref -q HEAD)\n>> -           [ \"$origin\" = \"$default\" ] &&\n>> +           test -z \"$1\" || test \"$1\" = $(get_default_remote) &&\n>>            echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n>>            ;;\n>>        *)\n>\n> They are not equivalent, the last line (echo $(git for-each-ref...))\n> is always executed, not only when ask for the default remote. When\n> $origin != $default the last line does not return the correct answer.\n> It should return nothing, it is not well defined. Or maybe it should\n> return the branch pointed by $origin/HEAD (I cannot test right now\n> what 'git pull $remote-not-the-default' merges).\n>\n> And it has nothing to do with letting 'pull --rebase . for\" work.\n\nYou are right, of course. I think I was modifying the code to try to\nunderstand how it behaved and it should not have been part of the patch.\nSorry about that.\n\nJunio, will you just exclude that hunk?\n"},{"id":"155835","messageId":"AANLkTi=QQZUjYtvRRiunrsCXMOX9+85K_nZXwsrT30bM@mail.gmail.com","threadId":"25734","inReplyTo":"AANLkTineaFVawXbJKHKv39bwPco7JHcmpUwXtZhscaFs@mail.gmail.com","subject":"Re: [PATCH] Use reflog in 'pull --rebase . foo'","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-11-13T23:10:55Z","receivedAt":"2010-11-13T23:10:55Z","isPatch":true,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Sat, Nov 13, 2010 at 6:52 PM, Martin von Zweigbergk\n<martin.von.zweigbergk@gmail.com> wrote:\n> On Sat, Nov 13, 2010 at 12:20 PM, Santi Béjar <santi@agolina.net> wrote:\n>> On Sat, Nov 13, 2010 at 1:09 PM, Martin von Zweigbergk\n>> <martin.von.zweigbergk@gmail.com> wrote:\n>>> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n>>> index 5f47b18..2e1661d 100644\n>>> --- a/git-parse-remote.sh\n>>> +++ b/git-parse-remote.sh\n>>> @@ -63,11 +63,8 @@ get_default_remote () {\n>>>  get_remote_merge_branch () {\n>>>        case \"$#\" in\n>>>        0|1)\n>>> -           origin=\"$1\"\n>>> -           default=$(get_default_remote)\n>>> -           test -z \"$origin\" && origin=$default\n>>>            curr_branch=$(git symbolic-ref -q HEAD)\n>>> -           [ \"$origin\" = \"$default\" ] &&\n>>> +           test -z \"$1\" || test \"$1\" = $(get_default_remote) &&\n>>>            echo $(git for-each-ref --format='%(upstream)' $curr_branch)\n>>>            ;;\n>>>        *)\n>>\n>> They are not equivalent, the last line (echo $(git for-each-ref...))\n>> is always executed, not only when ask for the default remote. When\n>> $origin != $default the last line does not return the correct answer.\n>> It should return nothing, it is not well defined. Or maybe it should\n>> return the branch pointed by $origin/HEAD (I cannot test right now\n>> what 'git pull $remote-not-the-default' merges).\n\nAfter thinking (and testing) this again, I see now that I was wrong,\nthe behavior is identical. But I find it more difficult to follow, but\nmaybe it is because I wrote the original.\n\n>>\n>> And it has nothing to do with letting 'pull --rebase . for\" work.\n>\n> You are right, of course. I think I was modifying the code to try to\n> understand how it behaved and it should not have been part of the patch.\n> Sorry about that.\n>\n> Junio, will you just exclude that hunk?\n\nBetter send it again, and add my\n\nAck-by: Santi Béjar <santi@agolina.net>\n\nSanti\n"}]}