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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 16, 2011 at 5:45 PM, Martin von Zweigbergk
<martin.von.zweigbergk@xxxxxxxxx> wrote:
> On Wed, 16 Feb 2011, Santi B?jar wrote:
>
>> On Wed, Feb 16, 2011 at 3:03 AM, Martin von Zweigbergk
>> <martin.von.zweigbergk@xxxxxxxxx> wrote:
>> > On Tue, 15 Feb 2011, Junio C Hamano wrote:
>> >
>> >> Martin von Zweigbergk <martin.von.zweigbergk@xxxxxxxxx> 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"

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

Yes, 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]