Should "pull --rebase" try to be a little cleverer?

Should "pull --rebase" try to be a little cleverer?

[Johannes Schindelin]

Hi,

last night an idea hit me: if I "git pull --rebase blabla master", it 
could be that "blabla" decided to rebase "master" already, and ATM this 
would lead to quite a few conflicts, since commits that were not mine were 
rewritten.

However, if we already have refs/remotes/blabla/master, we could DWIM the 
--rebase call to

	git rebase --onto FETCH_HEAD refs/remotes/blabla/master

Of course, this would mean that git-pull would need to source 
git-parse-remote again...

Ciao,
Dscho

Re: Should "pull --rebase" try to be a little cleverer?

[Nicolas Pitre]

On Mon, 21 Jan 2008, Johannes Schindelin wrote:

> Hi,
> 
> last night an idea hit me: if I "git pull --rebase blabla master", it 
> could be that "blabla" decided to rebase "master" already, and ATM this 
> would lead to quite a few conflicts, since commits that were not mine were 
> rewritten.

Indeed.

> However, if we already have refs/remotes/blabla/master, we could DWIM the 
> --rebase call to
> 
> 	git rebase --onto FETCH_HEAD refs/remotes/blabla/master

I was believing this was already the case.  Mind you, I never verified 
it.

I think the best is really to have the equivalent of:

	git fetch blabla/master
	git rebase --onto blabla/master blabla/master@{1} HEAD


Nicolas

Re: Should "pull --rebase" try to be a little cleverer?

[Johannes Schindelin]

Hi,

On Mon, 21 Jan 2008, Nicolas Pitre wrote:

> On Mon, 21 Jan 2008, Johannes Schindelin wrote:
> 
> > last night an idea hit me: if I "git pull --rebase blabla master", it 
> > could be that "blabla" decided to rebase "master" already, and ATM 
> > this would lead to quite a few conflicts, since commits that were not 
> > mine were rewritten.
> 
> Indeed.
> 
> > However, if we already have refs/remotes/blabla/master, we could DWIM 
> > the --rebase call to
> > 
> > 	git rebase --onto FETCH_HEAD refs/remotes/blabla/master
> 
> I was believing this was already the case.  Mind you, I never verified 
> it.
> 
> I think the best is really to have the equivalent of:
> 
> 	git fetch blabla/master
> 	git rebase --onto blabla/master blabla/master@{1} HEAD

Not exactly... what would you do with this:

	git pull --rebase git://git.kernel.org/... master

Hm?

So I think that _only_ in the case that a remote ref is updated, _and_ 
that there is already one, can we do the DWIMery.

Ciao,
Dscho

Re: Should "pull --rebase" try to be a little cleverer?

[Nicolas Pitre]

On Mon, 21 Jan 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 21 Jan 2008, Nicolas Pitre wrote:
> 
> > On Mon, 21 Jan 2008, Johannes Schindelin wrote:
> > 
> > > last night an idea hit me: if I "git pull --rebase blabla master", it 
> > > could be that "blabla" decided to rebase "master" already, and ATM 
> > > this would lead to quite a few conflicts, since commits that were not 
> > > mine were rewritten.
> > 
> > Indeed.
> > 
> > > However, if we already have refs/remotes/blabla/master, we could DWIM 
> > > the --rebase call to
> > > 
> > > 	git rebase --onto FETCH_HEAD refs/remotes/blabla/master
> > 
> > I was believing this was already the case.  Mind you, I never verified 
> > it.
> > 
> > I think the best is really to have the equivalent of:
> > 
> > 	git fetch blabla/master
> > 	git rebase --onto blabla/master blabla/master@{1} HEAD
> 
> Not exactly... what would you do with this:
> 
> 	git pull --rebase git://git.kernel.org/... master
> 
> Hm?
> 
> So I think that _only_ in the case that a remote ref is updated, _and_ 
> that there is already one, can we do the DWIMery.

Well, sure.  That's the case I care about.


Nicolas

Re: Should "pull --rebase" try to be a little cleverer?

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> However, if we already have refs/remotes/blabla/master, we could DWIM the 
> --rebase call to
>
> 	git rebase --onto FETCH_HEAD refs/remotes/blabla/master

FWIW, I like it, and I think that is what users expect it to do.

[PATCH] pull --rebase: be cleverer with rebased upstream branches

[Johannes Schindelin]

When the upstream branch is tracked, we can detect if that branch
was rebased since it was last fetched.  Teach git to use that
information to rebase from the old remote head onto the new remote head.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Mon, 21 Jan 2008, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > However, if we already have refs/remotes/blabla/master, we 
	> > could DWIM the --rebase call to
	> >
	> > 	git rebase --onto FETCH_HEAD refs/remotes/blabla/master
	> 
	> FWIW, I like it, and I think that is what users expect it to do.

	Sorry, took me quite some time.

	But I had a problem with the documentation: how am I supposed to 
	do the continuation of an indented paragraph, without using that 
	ugly "+" and unindenting the next paragraph?

 Documentation/git-pull.txt |    6 +++++-
 git-pull.sh                |   12 +++++++++++-
 t/t5520-pull.sh            |   17 +++++++++++++++++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index f9f1e0d..4cc633a 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -35,7 +35,11 @@ include::urls-remotes.txt[]
 include::merge-strategies.txt[]
 
 \--rebase::
-	Instead of a merge, perform a rebase after fetching.
+	Instead of a merge, perform a rebase after fetching.  If
+	there is a remote ref for the upstream branch, and this branch
+	was rebased since last fetched, the rebase uses that information
+	to avoid rebasing non-local changes.
+
 	*NOTE:* This is a potentially _dangerous_ mode of operation.
 	It rewrites history, which does not bode well when you
 	published that history already.  Do *not* use this option
diff --git a/git-pull.sh b/git-pull.sh
index fa97b0f..46da0f4 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -106,6 +106,15 @@ error_on_no_merge_candidates () {
 	exit 1
 }
 
+test true = "$rebase" && {
+	. git-parse-remote &&
+	origin="$1"
+	test -z "$origin" && origin=$(get_default_remote)
+	reflist="$(get_remote_refs_for_fetch "$@" 2>/dev/null |
+		sed "s|refs/heads/\(.*\):|\1|")" &&
+	oldremoteref="$(git rev-parse --verify \
+		"refs/remotes/$origin/$reflist" 2>/dev/null)"
+}
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
 git-fetch --update-head-ok "$@" || exit 1
 
@@ -164,6 +173,7 @@ then
 fi
 
 merge_name=$(git fmt-merge-msg <"$GIT_DIR/FETCH_HEAD") || exit
-test true = "$rebase" && exec git-rebase $merge_head
+test true = "$rebase" &&
+	exec git-rebase --onto $merge_head ${oldremoteref:-$merge_head}
 exec git-merge $no_summary $no_commit $squash $no_ff $strategy_args \
 	"$merge_name" HEAD $merge_head
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 52b3a0c..9484129 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -71,8 +71,25 @@ test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
 	git config branch.to-rebase.rebase 1 &&
 	git pull . copy &&
+	git config branch.to-rebase.rebase 0 &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
+test_expect_success '--rebase with rebased upstream' '
+
+	git remote add -f me . &&
+	git checkout copy &&
+	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 pull --rebase me copy &&
+	test "conflicting modification" = "$(cat file)" &&
+	test file = $(cat file2)
+
+'
+
 test_done
-- 
1.5.4.rc4.36.g6e122

Re: [PATCH] pull --rebase: be cleverer with rebased upstream branches

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When the upstream branch is tracked, we can detect if that branch
> was rebased since it was last fetched.  Teach git to use that
> information to rebase from the old remote head onto the new remote head.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This is certainly nicer than the original (which is not in
1.5.3, so even this late in the cycle it is "fixing up a new
feature we will be introducing in 1.5.4" category that I will
apply).  But I think a bit of caution and perhaps an
illustration or two in the doucmentation would help.

If you do "git fetch" from the origin since the last time you
ran "pull --rebase" for the remote, we will have the same
issue.  Suppose you have this history:

                   .---x---x---x
                  /
         .---A---B
        /
    ---0---o---o---A'--B'--C'--D'
                \
                 o---o---A''-B''-C''-D''-E''

Originally your upstream had 0---A---B; you built your 'x' on
top of it.  Then the upstream rebases and publishes history that
leads to D' (i.e. A and B are rewritten).  Later, the history is
further rewritten and E'' is the latest upstream tip.

If you haven't done "git fetch" since you started building on
top of B, refs/remotes/ will still say B and using B as base
(and E'' as onto) will give you the right rebase.  Earlier, we
did not use B in the rebase in any way, so your patch is
definitely an improvement.

However, if you have run "git fetch" (say, to peek what the
upstream has been up to), your refs/remote/ may say D'.  Using
that as the base and rebasing onto E'' is not quite optimal,
isn't it?

So it might make sense to make the logic to figure out B, given
your local history that leads from 0 to x's (and nothing else),
a bit cleverer than looking at the tracking branch.  We can look
at reflog for example.  "git log -g --pretty=oneline" may have
entries of this form:

    * branch: Created from B
    * rebase finished <branch> onto B

and the latest (i.e. younguest) entry is where the part of your
current history to be rebased (i.e. base commit) starts.  This
is much more reliable than looking at the tracking branch, whose
answer may or may not match B at all.

I do not mean this comment makes your approach invalid, though.
It is a start in the good direction.

Re: [PATCH] pull --rebase: be cleverer with rebased upstream branches

[Johannes Schindelin]

Hi,

On Sat, 26 Jan 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When the upstream branch is tracked, we can detect if that branch was 
> > rebased since it was last fetched.  Teach git to use that information 
> > to rebase from the old remote head onto the new remote head.
> 
> This is certainly nicer than the original (which is not in 1.5.3, so 
> even this late in the cycle it is "fixing up a new feature we will be 
> introducing in 1.5.4" category that I will apply).  But I think a bit of 
> caution and perhaps an illustration or two in the doucmentation would 
> help.

Well, I half expected it to be post-1.5.4.

> If you do "git fetch" from the origin since the last time you ran "pull 
> --rebase" for the remote, we will have the same issue.

Yes, that occurred to me, too.

> So it might make sense to make the logic to figure out B, given
> your local history that leads from 0 to x's (and nothing else),
> a bit cleverer than looking at the tracking branch.  We can look
> at reflog for example.  "git log -g --pretty=oneline" may have
> entries of this form:
> 
>     * branch: Created from B
>     * rebase finished <branch> onto B
> 
> and the latest (i.e. younguest) entry is where the part of your current 
> history to be rebased (i.e. base commit) starts.  This is much more 
> reliable than looking at the tracking branch, whose answer may or may 
> not match B at all.

I did not want to do that, because it is quite possible that the reflogs 
were disabled, or that the relevant reflogs were already expired.

However, I think that it might be a good approach to try reflogs first, 
and fallback to what I sent if there are no reflogs (or if it is detected 
that the reflog entry is not possibly the correct one, since what was 
rebased to is no longer an ancestor of the current branch).

Ciao,
Dscho