{"thread":{"id":"24715","subject":"[PATCHv4 1/2] t5520-pull: Add testcases showing spurious conflicts from git pull --rebase","startedAt":"2010-08-12T05:56:07Z","lastAt":"2010-08-13T01:47:11Z","messageCount":13,"participants":["Elijah Newren","Santi Béjar","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"147872","messageId":"1281592569-740-1-git-send-email-newren@gmail.com","threadId":"24715","inReplyTo":null,"subject":"[PATCHv4 0/2] Fix spurious conflicts with pull --rebase","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-12T05:56:07Z","receivedAt":"2010-08-12T05:56:07Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Changes since v1 (since v3, only Junio's testcase style issue was fixed):\n * Wording changes in the commit log\n * Address style and format issues for the testcase, raised by Ævar\n Arnfjörð Bjarmason, Junio, and Hannes.\n\nThis patch series fixes spurious conflict issues with git pull\n--rebase for the case where the upstream repository is *not* rebased.\n(There is no change in the case where the upstream repository is\nrebased.)\n\nIn c85c79279d and d44e71261f, the call to git-rebase was modified in\nan effort to reduce the number of commits being reapplied, by trying\nto avoid commits that upstream already had or has. It was\nspecifically designed with an upstream that is rebased in mind.\nUnfortunately, it had two side effects for the non-rebased upstream\ncase: (1) it prevented detecting if \"local\" patches are already\nupstream, and (2) it could in some cases cause more patches known to\nbe upstream to be reapplied rather than less. This series fixes both\nof these issues for the non-rebased upstream case. See the commit\nmessage of the second patch for details.\n\nIt's worth noting that issue (1) above also affects the case where the\nupstream repository has been rebased, which this patch series does not\naddress. As far as I can tell, fixing it would require changes\n(including new syntax) to format-patch to allow it to be told what\n'upstream' is, and some changes to git-pull.sh/git-rebase.sh to pass\nit this information.\n\nElijah Newren (2):\n t5520-pull: Add testcases showing spurious conflicts from git pull\n --rebase\n pull --rebase: Avoid spurious conflicts and reapplying unnecessary\n patches\n\n git-pull.sh | 34 +++++++++++++++++++----------\n t/t5520-pull.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 84 insertions(+), 12 deletions(-)\n\n-- \n1.7.2.1.43.gbae63\n"},{"id":"147870","messageId":"1281592569-740-2-git-send-email-newren@gmail.com","threadId":"24715","inReplyTo":"1281592569-740-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 1/2] t5520-pull: Add testcases showing spurious conflicts from git pull --rebase","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-12T05:56:08Z","receivedAt":"2010-08-12T05:56:08Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"\nSigned-off-by: Elijah Newren \n---\n t/t5520-pull.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 62 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 319e389..85a6b23 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -4,6 +4,11 @@ test_description='pulling into void'\n \n . ./test-lib.sh\n \n+modify () {\n+\tsed -e \"$1\" <\"$2\" >\"$2.x\" &&\n+\tmv \"$2.x\" \"$2\"\n+}\n+\n D=`pwd`\n \n test_expect_success setup '\n@@ -160,4 +165,61 @@ test_expect_success 'pull --rebase works on branch yet to be born' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'setup for detecting upstreamed changes' '\n+\tmkdir src &&\n+\t(cd src &&\n+\t git init &&\n+\t printf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\n\" > stuff &&\n+\t git add stuff &&\n+\t git commit -m \"Initial revision\"\n+\t) &&\n+\tgit clone src dst &&\n+\t(cd src &&\n+\t modify s/5/43/ stuff &&\n+\t git commit -a -m \"5->43\" &&\n+\t modify s/6/42/ stuff &&\n+\t git commit -a -m \"Make it bigger\"\n+\t) &&\n+\t(cd dst &&\n+\t modify s/5/43/ stuff &&\n+\t git commit -a -m \"Independent discovery of 5->43\"\n+\t)\n+'\n+\n+test_expect_failure 'git pull --rebase detects upstreamed changes' '\n+\t(cd dst &&\n+\t git pull --rebase &&\n+\t test -z \"$(git ls-files -u)\"\n+\t)\n+'\n+\n+test_expect_success 'setup for avoiding reapplying old patches' '\n+\t(cd dst &&\n+\t test_might_fail git rebase --abort &&\n+\t git reset --hard origin/master\n+\t) &&\n+\tgit clone --bare src src-replace.git &&\n+\trm -rf src &&\n+\tmv src-replace.git src &&\n+\t(cd dst &&\n+\t modify s/2/22/ stuff &&\n+\t git commit -a -m \"Change 2\" &&\n+\t modify s/3/33/ stuff &&\n+\t git commit -a -m \"Change 3\" &&\n+\t modify s/4/44/ stuff &&\n+\t git commit -a -m \"Change 4\" &&\n+\t git push &&\n+\n+\t modify s/44/55/ stuff &&\n+\t git commit --amend -a -m \"Modified Change 4\"\n+\t)\n+'\n+\n+test_expect_failure 'git pull --rebase does not reapply old patches' '\n+\t(cd dst &&\n+\t test_must_fail git pull --rebase &&\n+\t test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n+\t)\n+'\n+\n test_done\n-- \n1.7.2.1.43.gbae63\n"},{"id":"147871","messageId":"1281592569-740-3-git-send-email-newren@gmail.com","threadId":"24715","inReplyTo":"1281592569-740-1-git-send-email-newren@gmail.com","subject":"[PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-12T05:56:09Z","receivedAt":"2010-08-12T05:56:09Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Prior to c85c79279df2c8a583d95449d1029baba41f8660, pull --rebase would run\n git rebase $merge_head\nwhich resulted in a call to\n git format-patch ... --ignore-if-in-upstream $merge_head..$cur_branch\n\nThis resulted in patches from $merge_head..$cur_branch being applied, as\nlong as they did not already exist in $cur_branch..$merge_head.\nUnfortunately, when upstream is rebased, $merge_head..$cur_branch also\nrefers to \"old\" commits that have already been rebased upstream, meaning\nthat many patches that were already fixed upstream would be reapplied.\nThis could result in many spurious conflicts, as well as reintroduce\npatches that were intentionally dropped upstream.\n\nSo the algorithm was changed in c85c79279df2c8a583d95449d1029baba41f8660\nand d44e71261f91d3cc81293e0976bb40daa8abb583. Defining $old_remote_ref to\nbe the most recent entry in the reflog for @{upstream} that is an ancestor\nof $cur_branch, pull --rebase was changed to run\n git rebase --onto $merge_head $old_remote_ref\nwhich results in a call to\n git format-patch ... --ignore-if-in-upstream $old_remote_ref..$cur_branch\n\nThe whole point of this change was to reduce the number of commits being\nreapplied, by avoiding commits that upstream already has or had.\n\nIn the rebased upstream case, this change achieved that purpose. It is\nworth noting, though, that since $old_remote_ref is always an ancestor of\n$cur_branch (by its definition), format-patch will not know what upstream\nis and thus will not be able to determine if any patches are already\nupstream; they will all be reapplied.\n\nIn the non-rebased upstream case, this new form is usually the same as the\noriginal code but in some cases $old_remote_ref can be an ancestor of\n $(git merge-base $merge_head $cur_branch)\nmeaning that instead of avoiding reapplying commits that upstream already\nhas, it actually includes more such commits. Combined with the fact that\nformat-patch can no longer detect commits that are already upstream (since\nit is no longer told what upstream is), results in lots of confusion for\nusers (e.g. \"git is giving me lots of conflicts in stuff I didn't even\nchange since my last push.\")\n\nFix the non-rebased upstream case by ignoring $old_remote_ref whenever it\nis contained in $(git merge-base $merge_head $cur_branch). This should\nhave no affect on the rebased upstream case.\n\nSigned-off-by: Elijah Newren \n---\n git-pull.sh | 34 ++++++++++++++++++++++------------\n t/t5520-pull.sh | 4 ++--\n 2 files changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex a09a44e..54da07b 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -206,18 +206,6 @@ test true = \"$rebase\" && {\n \t\tgit diff-index --ignore-submodules --cached --quiet HEAD -- ||\n \t\tdie \"refusing to pull with rebase: your working tree is not up-to-date\"\n \tfi\n-\toldremoteref= &&\n-\t. git-parse-remote &&\n-\tremoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n-\toldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n-\tfor reflog in $(git rev-list -g $remoteref 2>/dev/null)\n-\tdo\n-\t\tif test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n-\t\tthen\n-\t\t\toldremoteref=\"$reflog\"\n-\t\t\tbreak\n-\t\tfi\n-\tdone\n }\n orig_head=$(git rev-parse -q --verify HEAD)\n git fetch $verbosity $progress $dry_run --update-head-ok \"$@\" || exit 1\n@@ -273,6 +261,28 @@ then\n \texit\n fi\n \n+if test true = \"$rebase\"\n+then\n+\toldremoteref= &&\n+\t. git-parse-remote &&\n+\tremoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n+\toldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n+\tfor reflog in $(git rev-list -g $remoteref 2>/dev/null)\n+\tdo\n+\t\tif test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n+\t\tthen\n+\t\t\toldremoteref=\"$reflog\"\n+\t\t\tbreak\n+\t\tfi\n+\tdone\n+\n+\to=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)\n+\tif test \"$oldremoteref\" = \"$o\"\n+\tthen\n+\t\tunset oldremoteref\n+\tfi\n+fi\n+\n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n case \"$rebase\" in\n true)\ndiff --git a/t/t5520-pull.sh b/t/t5520-pull.sh\nindex 85a6b23..0b489f5 100755\n--- a/t/t5520-pull.sh\n+++ b/t/t5520-pull.sh\n@@ -186,7 +186,7 @@ test_expect_success 'setup for detecting upstreamed changes' '\n \t)\n '\n \n-test_expect_failure 'git pull --rebase detects upstreamed changes' '\n+test_expect_success 'git pull --rebase detects upstreamed changes' '\n \t(cd dst &&\n \t git pull --rebase &&\n \t test -z \"$(git ls-files -u)\"\n@@ -215,7 +215,7 @@ test_expect_success 'setup for avoiding reapplying old patches' '\n \t)\n '\n \n-test_expect_failure 'git pull --rebase does not reapply old patches' '\n+test_expect_success 'git pull --rebase does not reapply old patches' '\n \t(cd dst &&\n \t test_must_fail git pull --rebase &&\n \t test 1 = $(find .git/rebase-apply -name \"000*\" | wc -l)\n-- \n1.7.2.1.43.gbae63\n"},{"id":"147904","messageId":"AANLkTimHiYUPyNTtT4SwapqN8YZGB1wjxJPwTTaPZhEa@mail.gmail.com","threadId":"24715","inReplyTo":"1281592569-740-3-git-send-email-newren@gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T13:34:15Z","receivedAt":"2010-08-12T13:34:15Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Thu, Aug 12, 2010 at 7:56 AM, Elijah Newren wrote:\n> Prior to c85c79279df2c8a583d95449d1029baba41f8660, pull --rebase would run\n>  git rebase $merge_head\n> which resulted in a call to\n>  git format-patch ... --ignore-if-in-upstream $merge_head..$cur_branch\n>\n> This resulted in patches from $merge_head..$cur_branch being applied, as\n> long as they did not already exist in $cur_branch..$merge_head.\n> Unfortunately, when upstream is rebased, $merge_head..$cur_branch also\n> refers to \"old\" commits that have already been rebased upstream, meaning\n> that many patches that were already fixed upstream would be reapplied.\n> This could result in many spurious conflicts, as well as reintroduce\n> patches that were intentionally dropped upstream.\n>\n> So the algorithm was changed in c85c79279df2c8a583d95449d1029baba41f8660\n> and d44e71261f91d3cc81293e0976bb40daa8abb583.  Defining $old_remote_ref to\n> be the most recent entry in the reflog for @{upstream} that is an ancestor\n> of $cur_branch, pull --rebase was changed to run\n>  git rebase --onto $merge_head $old_remote_ref\n> which results in a call to\n>  git format-patch ... --ignore-if-in-upstream $old_remote_ref..$cur_branch\n>\n> The whole point of this change was to reduce the number of commits being\n> reapplied, by avoiding commits that upstream already has or had.\n>\n> In the rebased upstream case, this change achieved that purpose.  It is\n> worth noting, though, that since $old_remote_ref is always an ancestor of\n> $cur_branch (by its definition), format-patch will not know what upstream\n> is and thus will not be able to determine if any patches are already\n> upstream; they will all be reapplied.\n>\n> In the non-rebased upstream case, this new form is usually the same as the\n> original code but in some cases $old_remote_ref can be an ancestor of\n>   $(git merge-base $merge_head $cur_branch)\n> meaning that instead of avoiding reapplying commits that upstream already\n> has, it actually includes more such commits.  Combined with the fact that\n> format-patch can no longer detect commits that are already upstream (since\n> it is no longer told what upstream is), results in lots of confusion for\n> users (e.g. \"git is giving me lots of conflicts in stuff I didn't even\n> change since my last push.\")\n>\n> Fix the non-rebased upstream case by ignoring $old_remote_ref whenever it\n> is contained in $(git merge-base $merge_head $cur_branch).  This should\n> have no affect on the rebased upstream case.\n\nAll this makes sense.\n\nBut can you explain when it happens? One possibility is when you don't\nfork from the tracking branch as in:\n\nSubject: Difference between pull --rebase and fetch+rebase \t\t\t\t\t\t\nMessage-ID: <27059158.post@talk.nabble.com>\nFrom: martinvz \n\nand this patch should also fix martinvz's issue (I've CC martinvz, can\nyou test this patch? Thanks).\n\nCan you refer to commits with something like this?\n\nc85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26)\n\nalias.one !git --no-pager show -s --pretty='tformat:%h (%s, %ad)' --date=short\n\n>\n> Signed-off-by: Elijah Newren \n> ---\n>  git-pull.sh     |   34 ++++++++++++++++++++++------------\n>  t/t5520-pull.sh |    4 ++--\n>  2 files changed, 24 insertions(+), 14 deletions(-)\n>\n> diff --git a/git-pull.sh b/git-pull.sh\n> index a09a44e..54da07b 100755\n> --- a/git-pull.sh\n> +++ b/git-pull.sh\n> @@ -206,18 +206,6 @@ test true = \"$rebase\" && {\n>                git diff-index --ignore-submodules --cached --quiet HEAD -- ||\n>                die \"refusing to pull with rebase: your working tree is not up-to-date\"\n>        fi\n> -       oldremoteref= &&\n> -       . git-parse-remote &&\n> -       remoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n> -       oldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n> -       for reflog in $(git rev-list -g $remoteref 2>/dev/null)\n> -       do\n> -               if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n> -               then\n> -                       oldremoteref=\"$reflog\"\n> -                       break\n> -               fi\n> -       done\n>  }\n>  orig_head=$(git rev-parse -q --verify HEAD)\n>  git fetch $verbosity $progress $dry_run --update-head-ok \"$@\" || exit 1\n> @@ -273,6 +261,28 @@ then\n>        exit\n>  fi\n>\n> +if test true = \"$rebase\"\n> +then\n> +       oldremoteref= &&\n> +       . git-parse-remote &&\n> +       remoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n> +       oldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n> +       for reflog in $(git rev-list -g $remoteref 2>/dev/null)\n> +       do\n> +               if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n> +               then\n> +                       oldremoteref=\"$reflog\"\n> +                       break\n> +               fi\n> +       done\n> +\n> +       o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)\n> +       if test \"$oldremoteref\" = \"$o\"\n> +       then\n> +               unset oldremoteref\n> +       fi\n> +fi\n> +\n\nYou've moved all the lines after the call to \"git fetch\". It changes\nthe behavior when the reflog is not enabled, as the old value of\nremoteref is lost.\n\nThanks\nSanti\n"},{"id":"147906","messageId":"AANLkTikLhe9+6ovLT99qG9wwDjXVmT8_81xuN8P31eve@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTimHiYUPyNTtT4SwapqN8YZGB1wjxJPwTTaPZhEa@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T14:37:25Z","receivedAt":"2010-08-12T14:37:25Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Thu, Aug 12, 2010 at 3:34 PM, Santi Béjar wrote:\n> On Thu, Aug 12, 2010 at 7:56 AM, Elijah Newren wrote:\n\n[...]\n\n>>\n>> Signed-off-by: Elijah Newren \n>> ---\n>>  git-pull.sh     |   34 ++++++++++++++++++++++------------\n>>  t/t5520-pull.sh |    4 ++--\n>>  2 files changed, 24 insertions(+), 14 deletions(-)\n>>\n>> diff --git a/git-pull.sh b/git-pull.sh\n>> index a09a44e..54da07b 100755\n>> --- a/git-pull.sh\n>> +++ b/git-pull.sh\n>> @@ -206,18 +206,6 @@ test true = \"$rebase\" && {\n>>                git diff-index --ignore-submodules --cached --quiet HEAD -- ||\n>>                die \"refusing to pull with rebase: your working tree is not up-to-date\"\n>>        fi\n>> -       oldremoteref= &&\n>> -       . git-parse-remote &&\n>> -       remoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n>> -       oldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n>> -       for reflog in $(git rev-list -g $remoteref 2>/dev/null)\n>> -       do\n>> -               if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n>> -               then\n>> -                       oldremoteref=\"$reflog\"\n>> -                       break\n>> -               fi\n>> -       done\n>>  }\n>>  orig_head=$(git rev-parse -q --verify HEAD)\n>>  git fetch $verbosity $progress $dry_run --update-head-ok \"$@\" || exit 1\n>> @@ -273,6 +261,28 @@ then\n>>        exit\n>>  fi\n>>\n>> +if test true = \"$rebase\"\n>> +then\n>> +       oldremoteref= &&\n>> +       . git-parse-remote &&\n>> +       remoteref=\"$(get_remote_merge_branch \"$@\" 2>/dev/null)\" &&\n>> +       oldremoteref=\"$(git rev-parse -q --verify \"$remoteref\")\" &&\n>> +       for reflog in $(git rev-list -g $remoteref 2>/dev/null)\n>> +       do\n>> +               if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n>> +               then\n>> +                       oldremoteref=\"$reflog\"\n>> +                       break\n>> +               fi\n>> +       done\n>> +\n>> +       o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)\n>> +       if test \"$oldremoteref\" = \"$o\"\n>> +       then\n>> +               unset oldremoteref\n>> +       fi\n>> +fi\n>> +\n>\n> You've moved all the lines after the call to \"git fetch\". It changes\n> the behavior when the reflog is not enabled, as the old value of\n> remoteref is lost.\n\nSomething like this?\n\nFix the non-rebased upstream case by only setting $old_remote_ref\nwith commits not ancestors of $remoteref or $merge_head. This should\nhave no affect on the rebased upstream case.\n\ndiff --git c/git-pull.sh w/git-pull.sh\nindex a09a44e..c1617d5 100755\n--- c/git-pull.sh\n+++ w/git-pull.sh\n@@ -214,7 +214,10 @@ test true = \"$rebase\" && {\n do\n if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n then\n- oldremoteref=\"$reflog\"\n+ if test \"$reflog\" != $(git merge-base $reflog\n$remoteref)\n+ then\n+ oldremoteref=\"$reflog\"\n+ fi\n break\n fi\n done\n@@ -273,6 +276,14 @@ then\n exit\n fi\n\n+if test true = \"$rebase\"\n+then\n+ if test \"$oldremoteref\" = $(git merge-base $oldremoteref $merge_head)\n+ then\n+ unset oldremoteref\n+ fi\n+fi\n+\n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n case \"$rebase\" in\n true)\n\n(also attached because of whitespace damage)\n\nHTH,\nSanti\n"},{"id":"147907","messageId":"AANLkTi=a51EOCmLCbJTbLg5bcN=96HR5yWMruRjVnM-J@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTikLhe9+6ovLT99qG9wwDjXVmT8_81xuN8P31eve@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T14:40:20Z","receivedAt":"2010-08-12T14:40:20Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"And now the attachment.\n\nSanti\n\n\ndiff --git c/git-pull.sh w/git-pull.sh\nindex a09a44e..c1617d5 100755\n--- c/git-pull.sh\n+++ w/git-pull.sh\n@@ -214,7 +214,10 @@ test true = \"$rebase\" && {\n \tdo\n \t\tif test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n \t\tthen\n-\t\t\toldremoteref=\"$reflog\"\n+\t\t\tif test \"$reflog\" != $(git merge-base $reflog $remoteref)\n+\t\t\tthen\n+\t\t\t\toldremoteref=\"$reflog\"\n+\t\t\tfi\n \t\t\tbreak\n \t\tfi\n \tdone\n@@ -273,6 +276,14 @@ then\n \texit\n fi\n \n+if test true = \"$rebase\"\n+then\n+\tif test \"$oldremoteref\" = $(git merge-base $oldremoteref $merge_head)\n+\tthen\n+\t\tunset oldremoteref\n+\tfi\n+fi\n+\n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n case \"$rebase\" in\n true)\n"},{"id":"147937","messageId":"AANLkTimoB=__-FmuFYa_rKqMjoYwWQS9hzziuk9Jseyx@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTimHiYUPyNTtT4SwapqN8YZGB1wjxJPwTTaPZhEa@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-12T20:19:55Z","receivedAt":"2010-08-12T20:19:55Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nThanks for the review and comments!\n\nOn Thu, Aug 12, 2010 at 7:34 AM, Santi Béjar wrote:\n\n> All this makes sense.\n>\n> But can you explain when it happens? One possibility is when you don't\n> fork from the tracking branch as in:\n\nThat's one possibility. Patch 1/2 in this thread contains testcases\nfor two others. Another possibility is having your patches get\nupstream by some alternative route (e.g. pulling your changes to a\nthird machine, pushing from there, and then going back to your\noriginal machine and trying to pull --rebase).\n\n> Subject: Difference between pull --rebase and fetch+rebase\n> Message-ID: <27059158.post@talk.nabble.com>\n> From: martinvz \n>\n> and this patch should also fix martinvz's issue (I've CC martinvz, can\n> you test this patch? Thanks).\n\nSince you've cc'd martinvz, I'll note for his benefit that in the\nthread you reference above, you stated,\n\n\"By the way, when Git tries to apply these two commits it should detect\nthat they are already applied so it should do nothing, isn't it?\"\n\nThe answer is no, it won't detect they are already applied, as\nexplained in the commit message that started the current thread. (If\ngit did detect the changes were already applied, this bug would have\nbeen innocuous.)\n\n> Can you refer to commits with something like this?\n>\n> c85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26)\n\nSure, I'll start doing that.\n\n> You've moved all the lines after the call to \"git fetch\". It changes\n> the behavior when the reflog is not enabled, as the old value of\n> remoteref is lost.\n\nDoh. That's what I get for trying to 'clean up' some code and put all\nthe references to setting oldremoteref together. I should have stuck\nwith my original 7-line addition patch. I'll resend the simplified\npatch.\n\nElijah\n"},{"id":"147946","messageId":"AANLkTinqW4h-SG21ZEAvhKEPJ46WfBRyYboUsfXjy7Zn@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTikLhe9+6ovLT99qG9wwDjXVmT8_81xuN8P31eve@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-12T21:02:48Z","receivedAt":"2010-08-12T21:02:48Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Thu, Aug 12, 2010 at 8:37 AM, Santi Béjar wrote:\n> diff --git c/git-pull.sh w/git-pull.sh\n> index a09a44e..c1617d5 100755\n> --- c/git-pull.sh\n> +++ w/git-pull.sh\n> @@ -214,7 +214,10 @@ test true = \"$rebase\" && {\n>        do\n>                if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n>                then\n> -                       oldremoteref=\"$reflog\"\n> +                       if test \"$reflog\" != $(git merge-base $reflog\n> $remoteref)\n> +                       then\n> +                               oldremoteref=\"$reflog\"\n> +                       fi\n\nHow does this help? I've been trying to scratch my head trying to\nfigure out a case where it could affect the outcome, and am struggling\nto come up with one.\n\n> @@ -273,6 +276,14 @@ then\n>        exit\n>  fi\n>\n> +if test true = \"$rebase\"\n> +then\n> +       if test \"$oldremoteref\" = $(git merge-base $oldremoteref $merge_head)\n> +       then\n> +               unset oldremoteref\n> +       fi\n> +fi\n> +\n\nThis was indeed my original patch, then I just (incorrectly, as you\npointed out) moved other bits of code to be with this so that the\ndetermination of oldremoteref was all in one place.\n"},{"id":"147956","messageId":"AANLkTikt6kRZRGW5Y=0qFf41P2HVLe97qGH=5ya5gcbw@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTimoB=__-FmuFYa_rKqMjoYwWQS9hzziuk9Jseyx@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T22:08:23Z","receivedAt":"2010-08-12T22:08:23Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Thu, Aug 12, 2010 at 10:19 PM, Elijah Newren wrote:\n> Hi,\n>\n> Thanks for the review and comments!\n>\n> On Thu, Aug 12, 2010 at 7:34 AM, Santi Béjar wrote:\n> \n>> All this makes sense.\n>>\n>> But can you explain when it happens? One possibility is when you don't\n>> fork from the tracking branch as in:\n>\n> That's one possibility.  Patch 1/2 in this thread contains testcases\n> for two others.  Another possibility is having your patches get\n> upstream by some alternative route (e.g. pulling your changes to a\n> third machine, pushing from there, and then going back to your\n> original machine and trying to pull --rebase).\n\nI think this is commit message material.\n\n>\n>> Subject: Difference between pull --rebase and fetch+rebase\n>> Message-ID: <27059158.post@talk.nabble.com>\n>> From: martinvz \n>>\n>> and this patch should also fix martinvz's issue (I've CC martinvz, can\n>> you test this patch? Thanks).\n>\n> Since you've cc'd martinvz, I'll note for his benefit that in the\n> thread you reference above, you stated,\n>\n> \"By the way, when Git tries to apply these two commits it should detect\n> that they are already applied so it should do nothing, isn't it?\"\n>\n> The answer is no, it won't detect they are already applied, as\n> explained in the commit message that started the current thread.  (If\n> git did detect the changes were already applied, this bug would have\n> been innocuous.)\n\nThanks, you are right.\n\nSanti\n"},{"id":"147963","messageId":"7vhbizpirg.fsf@alter.siamese.dyndns.org","threadId":"24715","inReplyTo":"AANLkTimoB=__-FmuFYa_rKqMjoYwWQS9hzziuk9Jseyx@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-12T22:29:23Z","receivedAt":"2010-08-12T22:29:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> Can you refer to commits with something like this?\n>>\n>> c85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26)\n>\n> Sure, I'll start doing that.\n>\n>> You've moved all the lines after the call to \"git fetch\". It changes\n>> the behavior when the reflog is not enabled, as the old value of\n>> remoteref is lost.\n>\n> Doh. That's what I get for trying to 'clean up' some code and put all\n> the references to setting oldremoteref together. I should have stuck\n> with my original 7-line addition patch. I'll resend the simplified\n> patch.\n\nThen I'll wait for the re-roll of 2/2 with updated message and\ncorrected code; thanks.\n"},{"id":"147968","messageId":"AANLkTinJK3YZ0sojnpiH0KZvFFC38X8LFr3U1ZrU_UXG@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTinqW4h-SG21ZEAvhKEPJ46WfBRyYboUsfXjy7Zn@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T22:51:59Z","receivedAt":"2010-08-12T22:51:59Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Thu, Aug 12, 2010 at 11:02 PM, Elijah Newren wrote:\n> Hi,\n>\n> On Thu, Aug 12, 2010 at 8:37 AM, Santi Béjar wrote:\n>> diff --git c/git-pull.sh w/git-pull.sh\n>> index a09a44e..c1617d5 100755\n>> --- c/git-pull.sh\n>> +++ w/git-pull.sh\n>> @@ -214,7 +214,10 @@ test true = \"$rebase\" && {\n>>        do\n>>                if test \"$reflog\" = \"$(git merge-base $reflog $curr_branch)\"\n>>                then\n>> -                       oldremoteref=\"$reflog\"\n>> +                       if test \"$reflog\" != $(git merge-base $reflog\n>> $remoteref)\n>> +                       then\n>> +                               oldremoteref=\"$reflog\"\n>> +                       fi\n>\n> How does this help?  I've been trying to scratch my head trying to\n> figure out a case where it could affect the outcome, and am struggling\n> to come up with one.\n\nMe too :)\n\nThe only case where it makes a difference, it is wrong. I was trying\nto be extra cautious, but ... too much in this case.\n\n>\n>> @@ -273,6 +276,14 @@ then\n>>        exit\n>>  fi\n>>\n>> +if test true = \"$rebase\"\n>> +then\n>> +       if test \"$oldremoteref\" = $(git merge-base $oldremoteref $merge_head)\n>> +       then\n>> +               unset oldremoteref\n>> +       fi\n>> +fi\n>> +\n>\n> This was indeed my original patch, then I just (incorrectly, as you\n> pointed out) moved other bits of code to be with this so that the\n> determination of oldremoteref was all in one place.\n\nOK. Then for your original patch:\n\nAcked-by: Santi Béjar \n\nThanks,\nSanti\n"},{"id":"147970","messageId":"AANLkTim6Tg7fNqxFYGU=bxQAn24ipkNaiENU5waCyQ4y@mail.gmail.com","threadId":"24715","inReplyTo":"AANLkTikt6kRZRGW5Y=0qFf41P2HVLe97qGH=5ya5gcbw@mail.gmail.com","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Santi Béjar","fromEmail":"santi@agolina.net","sentAt":"2010-08-12T23:17:32Z","receivedAt":"2010-08-12T23:17:32Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"On Fri, Aug 13, 2010 at 12:08 AM, Santi Béjar wrote:\n> On Thu, Aug 12, 2010 at 10:19 PM, Elijah Newren wrote:\n>> Hi,\n>>\n>> Thanks for the review and comments!\n>>\n>> On Thu, Aug 12, 2010 at 7:34 AM, Santi Béjar wrote:\n>> \n>>> All this makes sense.\n>>>\n>>> But can you explain when it happens? One possibility is when you don't\n>>> fork from the tracking branch as in:\n>>\n>> That's one possibility.  Patch 1/2 in this thread contains testcases\n>> for two others.  Another possibility is having your patches get\n>> upstream by some alternative route (e.g. pulling your changes to a\n>> third machine, pushing from there, and then going back to your\n>> original machine and trying to pull --rebase).\n>\n> I think this is commit message material.\n\nI just want to add one thing, thanks for your great commit message.\nAlthough the patch itself if 7 lines it is fantastic to have a commit\nmessage explaining all the historical context and this level of\ndetail.\n\nThanks,\nSanti\n"},{"id":"147984","messageId":"AANLkTi=hJMCVP4sfSr6Ru82aXVoj6OcLxKaj-cV_1GJ1@mail.gmail.com","threadId":"24715","inReplyTo":"7vhbizpirg.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-08-13T01:47:11Z","receivedAt":"2010-08-13T01:47:11Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, Aug 12, 2010 at 4:29 PM, Junio C Hamano wrote:\n> Elijah Newren writes:\n>\n>>> Can you refer to commits with something like this?\n>>>\n>>> c85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26)\n>>\n>> Sure, I'll start doing that.\n>>\n>>> You've moved all the lines after the call to \"git fetch\". It changes\n>>> the behavior when the reflog is not enabled, as the old value of\n>>> remoteref is lost.\n>>\n>> Doh.  That's what I get for trying to 'clean up' some code and put all\n>> the references to setting oldremoteref together.  I should have stuck\n>> with my original 7-line addition patch.  I'll resend the simplified\n>> patch.\n>\n> Then I'll wait for the re-roll of 2/2 with updated message and\n> corrected code; thanks.\n\nOk, sent.\n"}]}