[PATCH] pull: prevent duplicated entry in `op.pulledsubset`

[Pierre-Yves David]

On 03/27/2014 10:50 AM, Simon King wrote:
> On Thu, Mar 27, 2014 at 5:18 PM, David Soria Parra <davidsp at fb.com> wrote:
>> pierre-yves.david at ens-lyon.org writes:
>>
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>>> # Date 1395874532 25200
>>> #      Wed Mar 26 15:55:32 2014 -0700
>>> # Node ID 5d859e949ce867db1439448f6ea2bb3745252768
>>> # Parent  8c210b169c694f654414a977a59cb5e4c34eb48f
>>> pull: prevent duplicated entry in `op.pulledsubset`
>>>
>>> In the bare pull case we could add the same node multiple time to the
>>> `pulloperation.pulledsubset`. Beside being a bit wrong this confused the new
>>> revset implementation of `revset._revancestor` into giving bad result.
>>>
>>> This changeset fix the pull operation part. The fix for the revset itself will
>>> come in another changeset.
>>>
>>> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
>>> --- a/mercurial/exchange.py
>>> +++ b/mercurial/exchange.py
>>> @@ -407,11 +407,16 @@ class pulloperation(object):
>>>           """heads of the set of changeset target by the pull"""
>>>           # compute target subset
>>>           if self.heads is None:
>>>               # We pulled every thing possible
>>>               # sync on everything common
>>> -            return self.common + self.rheads
>>> +            c = set(self.common)
>>> +            ret = list(self.common)
>>> +            for n in self.rheads:
>>> +                if n not in c:
>>> +                    ret.append(n)
>>> +            return ret
>>
>> isn't this equivalent to
>>
>>    list(set(self.common + self.rheads))
>>
>> Maybe my python isn't good enough, but are the performance implications
>> for list + set that problematic to do it ourself?
>
> Pierre-Yves' version preserves the order of items in self.common and
> self.rheads, whereas yours would lose that order.
>
> (I've no idea if that's important though...)

What Simon says. It is usually easier to preserve order in our list.

The whole thing should not be to performace critical anyway.