[PATCHv5 0/2] Fix spurious conflicts with pull --rebase

Changes since v4
  * Added a little bit of detail to the commit log about cases when
    the change can affect
  * Fixed issue pointed out by Santi with moving 12 lines of code (to
    bring definition of oldupstream variable together); the different
    pieces had important reasons for being where they were.
Changes since v1
  * Changes above, plus other commit log wording changes and lots of
    style and formatting issues for the testcase

This patch series fixes spurious conflict issues with git pull --rebase for the case where the upstream repository is *not* rebased. (There is no change in the case where the upstream repository is rebased.)

In cc85c792 (pull --rebase: be cleverer with rebased upstream branches, 2008-01-26) and d44e712 (pull: support rebased upstream + fetch + pull --rebase, 2009-07-19), the call to git-rebase was modified in an effort to reduce the number of commits being reapplied, by trying to avoid commits that upstream already had or has. It was specifically designed with an upstream that is rebased in mind. Unfortunately, it had two side effects for the non-rebased upstream case: (1) it prevented detecting if "local" patches are already upstream, and (2) it could in some cases cause more patches known to be upstream to be reapplied rather than less. This series fixes both of these issues for the non-rebased upstream case. See the commit message of the second patch for details.

It's worth noting that issue (1) above also affects the case where the upstream repository has been rebased, which this patch series does not address. As far as I can tell, fixing it would require changes (including new syntax) to format-patch to allow it to be told what 'upstream' is, and some changes to git-pull.sh/git-rebase.sh to pass it this information.

Elijah Newren (2):
  t5520-pull: Add testcases showing spurious conflicts from git pull
    --rebase
  pull --rebase: Avoid spurious conflicts and reapplying unnecessary
    patches

 git-pull.sh     |    9 ++++++++
 t/t5520-pull.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

This looks very good, and has a great code/test ratio.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t5520-pull.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

Prior to c85c792 (pull --rebase: be cleverer with rebased upstream
branches, 2008-01-26), pull --rebase would run
  git rebase $merge_head
which resulted in a call to
  git format-patch ... --ignore-if-in-upstream $merge_head..$cur_branch

This resulted in patches from $merge_head..$cur_branch being applied, as long as they did not already exist in $cur_branch..$merge_head. Unfortunately, when upstream is rebased, $merge_head..$cur_branch also refers to "old" commits that have already been rebased upstream, meaning that many patches that were already fixed upstream would be reapplied. This could result in many spurious conflicts, as well as reintroduce patches that were intentionally dropped upstream.

So the algorithm was changed in c85c792 (pull --rebase: be cleverer with
rebased upstream branches, 2008-01-26) and d44e712 (pull: support rebased
upstream + fetch + pull --rebase, 2009-07-19).  Defining $old_remote_ref to
be the most recent entry in the reflog for @{upstream} that is an ancestor
of $cur_branch, pull --rebase was changed to run
  git rebase --onto $merge_head $old_remote_ref
which results in a call to
  git format-patch ... --ignore-if-in-upstream $old_remote_ref..$cur_branch

The whole point of this change was to reduce the number of commits being reapplied, by avoiding commits that upstream already has or had.

In the rebased upstream case, this change achieved that purpose. It is worth noting, though, that since $old_remote_ref is always an ancestor of $cur_branch (by its definition), format-patch will not know what upstream is and thus will not be able to determine if any patches are already upstream; they will all be reapplied.

In the non-rebased upstream case, this new form is usually the same as the
original code but in some cases $old_remote_ref can be an ancestor of
   $(git merge-base $merge_head $cur_branch)
meaning that instead of avoiding reapplying commits that upstream already
has, it actually includes more such commits.  Combined with the fact that
format-patch can no longer detect commits that are already upstream (since
it is no longer told what upstream is), results in lots of confusion for
users (e.g. "git is giving me lots of conflicts in stuff I didn't even
change since my last push.")

Cases where additional commits could be reapplied include forking from a commit other than the tracking branch, or amending/rebasing after pushing. Cases where the inability to detect upstreamed commits cause problems include independent discovery of a fix and having your patches get upstreamed by some alternative route (e.g. pulling your changes to a third machine, pushing from there, and then going back to your original machine and trying to pull --rebase).

Fix the non-rebased upstream case by ignoring $old_remote_ref whenever it is contained in $(git merge-base $merge_head $cur_branch). This should have no affect on the rebased upstream case.

Acked-by: Santi Béjar <santi@agolina.net>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 git-pull.sh     |    9 +++++++++
 t/t5520-pull.sh |    4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)