git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Santi Béjar" <santi@agolina.net>
To: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] rebase: be cleverer with rebased upstream branches
Date: Tue, 15 Feb 2011 11:28:09 +0000	[thread overview]
Message-ID: <AANLkTi=1WkZXBtQu71mELTBc6F7XrfBi+NWNWy-AxS79@mail.gmail.com> (raw)
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:
> 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.

>
> 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>
> ---

[...]

>
>    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

  reply	other threads:[~2011-02-15 11:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-14 13:51 [PATCH] rebase: be cleverer with rebased upstream branches Martin von Zweigbergk
2011-02-15 11:28 ` Santi Béjar [this message]
2011-02-16  1:37   ` Martin von Zweigbergk
2011-02-16  9:26     ` Santi Béjar
2011-02-15 20:30 ` Junio C Hamano
2011-02-16  3:03   ` Martin von Zweigbergk
2011-02-16 12:10     ` Santi Béjar
2011-02-16 13:22       ` Santi Béjar
2011-02-16 19:07         ` Junio C Hamano
2011-02-16 21:16           ` Santi Béjar
2011-02-16 16:45       ` Martin von Zweigbergk
2011-02-17 10:24         ` Santi Béjar
2011-03-12 21:15           ` Martin von Zweigbergk
2011-03-12 23:51             ` Santi Béjar
2011-03-13  1:32               ` Martin von Zweigbergk
2011-03-13  3:14                 ` Martin von Zweigbergk
2011-03-13 22:57                   ` Junio C Hamano
2011-03-13 23:42                     ` Santi Béjar
2011-03-13 23:09             ` Santi Béjar

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='AANLkTi=1WkZXBtQu71mELTBc6F7XrfBi+NWNWy-AxS79@mail.gmail.com' \
    --to=santi@agolina.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.von.zweigbergk@gmail.com \
    /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.
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 NNTP newsgroup(s).