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

From: Santi Béjar <santi@agolina.net>
Date: Feb 15, 2011, 11:28 UTC
Message-ID: <AANLkTi=1WkZXBtQu71mELTBc6F7XrfBi+NWNWy-AxS79@mail.gmail.com>
In-Reply-To: <1297691481-3308-1-git-send-email-martin.von.zweigbergk@gmail.com>

On Mon, Feb 14, 2011 at 1:51 PM, Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> wrote:

Show 12 quoted lines

> Since c85c792 (pull --rebase: be cleverer with rebased upstream
> branches, 2008-01-26), 'git pull --rebase' has used the reflog to try
> to rebase from the old upstream onto the new upstream.
>
> However, if, instead of 'git pull --rebase', the user were to do 'git
> fetch' followed by 'git rebase', the reflog would not be walked. This
> patch teaches "git rebase" the same reflog-walking tricks that 'git
> pull --rebase' already knows.
>
> This may be useful for rebasing one branch against another local
> branch that has been rebased. Currently, you would have to do that
> using 'git rebase --onto' or by configuring it on the branch.

It make sense.

Show 12 quoted lines

>
> It might seem like most of the related code in git-pull.sh can be
> removed once git-rebase.sh supports reflog walking. Unfortunately, not
> much of it can be removed, though. The reason is that git-pull.sh
> simulates one step of "reflog walking" by keeping track of the
> position of the remote-tracking branch before and after the fetch
> operation. This does not rely on reflogs. There are at least two cases
> where the reflog is not used: a) when it is disabled, b) when the
> remote branch was specified on the command line (as in 'git pull
> --rebase origin master').  In both of these cases, git-pull.sh
> remembers the position of the reference before the fetch and uses that
> as a kind of '$upstream@{1}'.

I don't agree with point b). In line 190:

	remoteref="$(get_remote_merge_branch "$@" 2>/dev/null)" &&

It returns the local tracking branch for repo=origin and branch=master and uses its reflog.

The end result is the same, there is one case where you need the old value of the tracking branch, so it should be done in git-pull.

But I wonder if it is possible to write a function shared by git-pull.sh and git-rebase.sh that computes the branch forking points, the number of arguments could detect if it has the old-remote-hash or not.

>
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> ---

[...]

Show 32 quoted lines

>
>    HOWEVER, this causes a very noticable delay in some cases. With this
>    patch, 'git rebase' walks the reflog of the upstream ref until it
>    finds a commit that the branch-to-rebase contains. If the upstream ref
>    has moved a lot since the branch was last rebased, there may be quite
>    a few commits to test before the old upstream commit is found.
>
>    The same thing can already occur with 'git pull --rebase' for exactly
>    the same reasons. For example, assume that your upstream remote branch
>    changes quite frequently and that you often fetch from the remote so
>    that your origin/master gets a long reflog. If you then checkout some
>    branch you had not been working on for a while, and run 'git pull',
>    you get into the same situation. The delay is probably less likely to
>    be noticed in the case of 'git pull --rebase', however, since most
>    users will probably assume it is a problem with the network or the
>    server.
>
>    Of course, 'git pull --rebase' can also be used with a local branch
>    configured as upstream. In this case, the behavior today is just like
>    what this patch introduces for 'git rebase'.
>
>    What do you think? I think it's a useful feature, but how do we handle
>    the delay problem? Maybe simply by making it configurable?
>
>    Should such a configuration variable apply to 'git pull --rebase' as
>    well? It would seem inconsistent otherwise, but maybe that's ok since
>    'git pull --rebase' is usually used with remote-tracking branches,
>    which probably change less frequently. Btw, is this a correct
>    assumption? It is definitely true for my own work on git, but I
>    actually think it's the other way around for my work at $dayjob. Am I
>    missing some part to the puzzle that explains why I had not noticed
>    the delay until I started using this patch?

I agree with you that it may add a long delay in some cases. I normally rebase branches based on an upstream branch and this may explain why I haven't seen the delay (or maybe I thought it was a network delay).

I think the delay could be much shorter if the computation was not in shell, but in C. Or maybe change the algorithm. So I don't think a configuration item is the answer here.

Other than that I think the change make sense it include docs and tests and works, thanks.

Santi