Re: [PATCH] rebase: be cleverer with rebased upstream branches
- From
- Santi Béjar <santi@agolina.net>
- Date
- Feb 17, 2011, 10:24 UTC
- Message-ID
- <AANLkTik-JGZFCE+m7g__mwfQhRReOM=Qe_EO3otw50XC@mail.gmail.com>
- In-Reply-To
- <alpine.DEB.2.00.1102161122350.14950@debian>
On Wed, Feb 16, 2011 at 5:45 PM, Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> wrote:
Show 30 quoted lines
> On Wed, 16 Feb 2011, Santi B?jar wrote: > >> 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.
I meant the test + loop, but I would also "make sure $upstream_name is a branch", as done in git-pull.sh with:
git rev-parse -q --verify "$remoteref"
Show 37 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. IYes, that is the idea (plus the old remote hash in case of git-pull)
> would have thought that would be slow, but it's great if that is fast > enough.
Yes, I think it is fast enough in the normal case. Even feeding the entire git.git's master, ~25000 revisions, it takes around 2-4 seconds only:
$ git rev-list origin/master | wc -l 24380 $ time git merge-base $(git rev-list origin/master) 9971d6d52c5afeb8ba60ae6ddcffb34af23eeadd
real 0m4.014s user 0m1.520s sys 0m2.284s
(2.5GHz CPU)
But, as Junio showed, it has problems when the reflog lenght is too large. Maybe git-merge-base can learn the --stdin flag, or we could process the reflog in batches of 1000 (?) entries, ... but the nice property of using the entire reflog is that the output is what you are looking for, if you take the first 1000 entries you have to check if the output is one of these entries.
> The resulting code looks very nice and short. Thanks again.
No, thanks to you for the nice idea!
HTH, Santi