From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, santi@agolina.net, Johannes.Schindelin@gmx.de,
Elijah Newren <newren@gmail.com>
Subject: [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches
Date: Wed, 11 Aug 2010 23:56:09 -0600 [thread overview]
Message-ID: <1281592569-740-3-git-send-email-newren@gmail.com> (raw)
In-Reply-To: <1281592569-740-1-git-send-email-newren@gmail.com>
Prior to c85c79279df2c8a583d95449d1029baba41f8660, 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 c85c79279df2c8a583d95449d1029baba41f8660
and d44e71261f91d3cc81293e0976bb40daa8abb583. 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.")
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.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
git-pull.sh | 34 ++++++++++++++++++++++------------
t/t5520-pull.sh | 4 ++--
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index a09a44e..54da07b 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -206,18 +206,6 @@ test true = "$rebase" && {
git diff-index --ignore-submodules --cached --quiet HEAD -- ||
die "refusing to pull with rebase: your working tree is not up-to-date"
fi
- oldremoteref= &&
- . git-parse-remote &&
- remoteref="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
- oldremoteref="$(git rev-parse -q --verify "$remoteref")" &&
- for reflog in $(git rev-list -g $remoteref 2>/dev/null)
- do
- if test "$reflog" = "$(git merge-base $reflog $curr_branch)"
- then
- oldremoteref="$reflog"
- break
- fi
- done
}
orig_head=$(git rev-parse -q --verify HEAD)
git fetch $verbosity $progress $dry_run --update-head-ok "$@" || exit 1
@@ -273,6 +261,28 @@ then
exit
fi
+if test true = "$rebase"
+then
+ oldremoteref= &&
+ . git-parse-remote &&
+ remoteref="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
+ oldremoteref="$(git rev-parse -q --verify "$remoteref")" &&
+ for reflog in $(git rev-list -g $remoteref 2>/dev/null)
+ do
+ if test "$reflog" = "$(git merge-base $reflog $curr_branch)"
+ then
+ oldremoteref="$reflog"
+ break
+ fi
+ done
+
+ o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)
+ if test "$oldremoteref" = "$o"
+ then
+ unset oldremoteref
+ fi
+fi
+
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
case "$rebase" in
true)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 85a6b23..0b489f5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -186,7 +186,7 @@ test_expect_success 'setup for detecting upstreamed changes' '
)
'
-test_expect_failure 'git pull --rebase detects upstreamed changes' '
+test_expect_success 'git pull --rebase detects upstreamed changes' '
(cd dst &&
git pull --rebase &&
test -z "$(git ls-files -u)"
@@ -215,7 +215,7 @@ test_expect_success 'setup for avoiding reapplying old patches' '
)
'
-test_expect_failure 'git pull --rebase does not reapply old patches' '
+test_expect_success 'git pull --rebase does not reapply old patches' '
(cd dst &&
test_must_fail git pull --rebase &&
test 1 = $(find .git/rebase-apply -name "000*" | wc -l)
--
1.7.2.1.43.gbae63
next prev parent reply other threads:[~2010-08-12 5:48 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-12 5:56 [PATCHv4 0/2] Fix spurious conflicts with pull --rebase Elijah Newren
2010-08-12 5:56 ` [PATCHv4 1/2] t5520-pull: Add testcases showing spurious conflicts from git " Elijah Newren
2010-08-12 5:56 ` Elijah Newren [this message]
2010-08-12 13:34 ` [PATCHv4 2/2] pull --rebase: Avoid spurious conflicts and reapplying unnecessary patches Santi Béjar
2010-08-12 14:37 ` Santi Béjar
2010-08-12 14:40 ` Santi Béjar
2010-08-12 21:02 ` Elijah Newren
2010-08-12 22:51 ` Santi Béjar
2010-08-12 20:19 ` Elijah Newren
2010-08-12 22:08 ` Santi Béjar
2010-08-12 23:17 ` Santi Béjar
2010-08-12 22:29 ` Junio C Hamano
2010-08-13 1:47 ` Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1281592569-740-3-git-send-email-newren@gmail.com \
--to=newren@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=santi@agolina.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).