[PATCH 3 of 4 V2] changelog: prefilter in headrevs()

[Georges Racinet]

On 2/13/19 2:18 PM, Yuya Nishihara wrote:
> On Wed, 13 Feb 2019 12:47:56 +0100, Georges Racinet wrote:
>> # HG changeset patch
>> # User Georges Racinet <georges.racinet at octobus.net>
>> # Date 1547815081 -3600
>> #      Fri Jan 18 13:38:01 2019 +0100
>> # Node ID eae29e210636ee44851e0caa385097a090c60af8
>> # Parent  956c5b54b4ce3e8decf5243a7f73c6f9a06f1229
>> # EXP-Topic revset.predicates
>> changelog: prefilter in headrevs()
> Queued the first two patches, thanks.
>
>> +    def _checknofilteredgenrevs(self, revs):
>> +        """rewrap 'revs' generator to include check for filtered revisions
>> +
>> +        This does not consume the incoming generator.
>> +        """
>> +        filteredrevs = self.filteredrevs
>> +        for r in revs:
>> +            if r in filteredrevs:
>> +                raise error.FilteredIndexError(r)
>> +            yield r
>> +
>> +    def _checknofilteredinrevs(self, revs):
>> +        """raise the appropriate error if 'revs' contains a filtered revision
>> +
>> +        This returns a version of 'revs' to be used by the caller, that works
>> +        for all cases, including lazy ones
>> +        """
>> +        safehasattr = util.safehasattr
>> +        if safehasattr(revs, '__next__'):
>> +            # Note that inspect.isgenerator() is not true for iterators,
>> +            # and that calling even implicitely iter() on a iterator does not
>> +            # clone it
>> +            return self._checknofilteredgenrevs(revs)
>> +
>> +        filteredrevs = self.filteredrevs
>> +        if safehasattr(revs, 'first'):  # smartset
>> +            offenders = revs & filteredrevs
>> +        else:
>> +            offenders = filteredrevs.intersection(revs)
>> +
>> +        for rev in offenders:
>> +            raise error.FilteredIndexError(rev)
>> +        return revs
> Do we need this complexity to handle various types of 'revs' differently?
>
> IIUC, revlog.headrevs(revs) is just forwarded to dagop.headrevs(revs), where
> 'revs' is first converted to a set, and then iterated. Which means 'revs'
> can't be an iterator, and the order doesn't basically matter.

So to rephrase you're saying that no caller will ever expect an iterator
not to be consumed by calling headrevs() on it, due to filtering,
because that's what happens anyway, even without any filtering. That's
right indeed, I've been overprotective on that one.

I'll make a v3 for this patch and its successor, thanks for the review
and the queueing !

Note: I'll be offline until the 20th, not sure I can make the v3 before
that.