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

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Feb 16, 2011, 16:45 UTC
Message-ID: <alpine.DEB.2.00.1102161122350.14950@debian>
In-Reply-To: <AANLkTinmxbYLB-K+VzY50NtOAPwd-q3WwAosAHqKRq_0@mail.gmail.com>

On Wed, 16 Feb 2011, Santi B?jar wrote:

Show 24 quoted lines

> On Wed, Feb 16, 2011 at 3:03 AM, Martin von Zweigbergk
> <martin.von.zweigbergk@gmail.com> wrote:
> > On Tue, 15 Feb 2011, Junio C Hamano wrote:
> >
> >> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> >>
> >> > diff --git a/git-rebase.sh b/git-rebase.sh
> >> > index 5abfeac..1bc0c29 100755
> >> > --- a/git-rebase.sh
> >> > +++ b/git-rebase.sh
> >>       test -n "$upstream_name" &&
> >>         for reflog in $(git rev-list ...)
> >>         do
> >>               ...
> >>       done
> >>
> >> Don't you need to make sure $upstream_name is a branch (or a ref in
> >> general that can have a reflog), or does it not matter because the
> >> "rev-list -g" will die without producing anything and you are discarding
> >> the error message?
> >
> > Exactly as you suspect. Is it too ugly?
> 
> I also prefer Junio's version.

I fixed the test + for loop, if that's what you mean by "Junio's version". Or did you mean "make sure $upstream_name is a branch"? I could do that as well if you like. I have no preference.

Show 26 quoted lines

> >        .-u@{0}
> >       /
> >      .---u@{1}
> >     /
> > x---y-----u@{2}
> >     \
> >      .---u@{3}---b
> >       \
> >        .-u@{4}
> >
> >
> > I have an idea inspired by bisection, Thomas's exponential stride, and
> > what someone (you?) mentioned the other day about virtual merge
> > commits. I haven't tried it out, but let me know what you think. I'll
> > try to explain it using an example only:
> >
> > Exponential stride phase:
> > 1. candidates={ u@{0} }
> >   merge-base b $candidates -> y, _not_ in $candidates
> > 2. candidates={ u@{1} u@{2} }
> >   merge-base b $candidates -> y, _not_ in $candidates
> > 3. candidates={ u@{3} u@{4} u@{5} u@{6} }
> >   merge-base b $candidates -> u@{3}, in $candidates
> 
> Doesn't it indicate that u@{3} is the commit we are looking for? I
> haven't found a counterexample...

Yes, of course. Stupid me ;-). Forget about the other half. (I think that's what I did manually to match the sha1 back to the ref name, but that is of course complete non-sense to do in the script.)

> If this is true the following patch can implement it for git-pull.sh and
> git-rebase.sh (sorry if it is space damaged):

Thanks! Will have a closer look at it later today. If I understand correctly, you simply call merge-base with the _entire_ reflog. I would have thought that would be slow, but it's great if that is fast enough. The resulting code looks very nice and short. Thanks again.

/Martin