Patchwork [1,of,2,RFC,V2] largefiles: changed overridelog to work with graphlog

login
register
mail settings
Submitter Lucas Moscovicz
Date March 6, 2014, 12:10 a.m.
Message ID <2367ddb8b07acef2ea7f.1394064649@dev1037.prn2.facebook.com>
Download mbox | patch
Permalink /patch/3867/
State Superseded
Headers show

Comments

Lucas Moscovicz - March 6, 2014, 12:10 a.m.
# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1394063709 28800
#      Wed Mar 05 15:55:09 2014 -0800
# Node ID 2367ddb8b07acef2ea7f72936c0ba46f47908205
# Parent  ef75710021e1c1dd2ef192c21b274313698186d4
largefiles: changed overridelog to work with graphlog

Log for largefiles was failing for graphlog since it was overriding match
instead of matchandpats.

Tests for graphlog to be added in future patches.
Mads Kiilerich - March 6, 2014, 6:26 p.m.
On 03/06/2014 01:10 AM, Lucas Moscovicz wrote:
> # HG changeset patch
> # User Lucas Moscovicz <lmoscovicz@fb.com>
> # Date 1394063709 28800
> #      Wed Mar 05 15:55:09 2014 -0800
> # Node ID 2367ddb8b07acef2ea7f72936c0ba46f47908205
> # Parent  ef75710021e1c1dd2ef192c21b274313698186d4
> largefiles: changed overridelog to work with graphlog

You have my sincere respect and sympathy for looking into this ...

> Log for largefiles was failing for graphlog since it was overriding match
> instead of matchandpats.

Did it already not work and this is a bugfix? Or is it a change of 
strategy that will make it work with the refactoring in the next changeset?

> Tests for graphlog to be added in future patches.

Why not introduce the tests in a patch before this or in this patch so 
we can see that it improves (or at least doesn't break)?

> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -8,7 +8,7 @@
>   
>   '''Overridden Mercurial commands and functions for the largefiles extension'''
>   
> -import os
> +import os, re, glob
>   import copy
>   
>   from mercurial import hg, commands, util, cmdutil, scmutil, match as match_, \
> @@ -47,6 +47,15 @@
>       scmutil.match = f
>       return oldmatch
>   
> +def installmatchandpatsfn(f):
> +    oldmatchandpats = scmutil.matchandpats
> +    setattr(f, 'oldmatchandpats', oldmatchandpats)
> +    scmutil.matchandpats = f
> +    return oldmatchandpats
> +
> +def installfilelogfn(f):
> +    revset.symbols['filelog'] = f

"usually" we store the old function and make sure we restore the right 
one when we are done. Shouldn't we do the same here?

> +
>   def restorematchfn():
>       '''restores scmutil.match to what it was before installnormalfilesmatchfn
>       was called.  no-op if scmutil.match is its original function.
> @@ -55,6 +64,20 @@
>       restore matchfn to reverse'''
>       scmutil.match = getattr(scmutil.match, 'oldmatch', scmutil.match)
>   
> +def restorematchandpatsfn():
> +    '''restores scmutil.matchandpats to what it was before
> +    installnormalfilesmatchandpatsfn was called.  no-op if scmutil.matchandpats
> +    is its original function.
> +
> +    Note that n calls to installnormalfilesmatchandpatsfn will require n calls
> +    to restore matchfn to reverse'''
> +    scmutil.matchandpats = getattr(scmutil.matchandpats, 'oldmatchandpats',
> +            scmutil.matchandpats)
> +
> +def restorefilelogfn():
> +    '''restores revset.filelog to what it was before.'''
> +    revset.symbols['filelog'] = revset.filelog
> +
>   def addlargefiles(ui, repo, *pats, **opts):
>       large = opts.pop('large', None)
>       lfsize = lfutil.getminsize(
> @@ -241,15 +264,15 @@
>           repo._repo.lfstatus = False
>   
>   def overridelog(orig, ui, repo, *pats, **opts):
> -    def overridematch(ctx, pats=[], opts={}, globbed=False,
> +    def overridematchandpats(ctx, pats=[], opts={}, globbed=False,
>               default='relpath'):
>           """Matcher that merges root directory with .hglf, suitable for log.
>           It is still possible to match .hglf directly.
>           For any listed files run log on the standin too.
>           matchfn tries both the given filename and with .hglf stripped.
>           """
> -        match = oldmatch(ctx, pats, opts, globbed, default)
> -        m = copy.copy(match)
> +        matchandpats = oldmatchandpats(ctx, pats, opts, globbed, default)
> +        m, p = copy.copy(matchandpats)
>           for i in range(0, len(m._files)):
>               standin = lfutil.standin(m._files[i])
>               if standin in repo[ctx.node()]:
> @@ -264,14 +287,81 @@
>               r = origmatchfn(f)
>               return r
>           m.matchfn = lfmatchfn
> -        return m
> -    oldmatch = installmatchfn(overridematch)
> +
> +        def expandpats(pats):
> +            ret = []
> +            for p in pats:
> +                kind, name = match_._patsplit(p, None)
> +                if kind is None:
> +                    try:
> +                        globbed = glob.glob(name)
> +                    except re.error:
> +                        globbed = [name]
> +                    if globbed:
> +                        ret.extend(globbed)
> +                        for g in globbed:
> +                            if not lfutil.isstandin(g):
> +                                ret.append(lfutil.standin(g))

I would expect that it only should use the standin if the matched file 
itself not is tracked directly but is a largefile - which probably 
approximately means that it is in lfdirstate or the standin exists in 
the filesystem or revision. I also wonder if the standins should be used 
instead of the matched file ...

BUT this is for log and across multi revisions so I guess it is less 
wrong to include both and see what they match? That might deserve a comment.

> +                        continue
> +                ret.append(p)
> +            return ret

This expandpats function seems to be an almost exact copy of 
scmutil.expandpats, only with some post processing? It is only used once 
- a few lines below? Why not just call scmutil.expandpats and 
postprocess the result?

> +
> +        if pats == ("",):
> +            pats = []
> +        else:
> +            pats = expandpats(pats or [])
> +
> +        for f in m.files():
> +            if f not in pats:

pats is a list. Could we use sets instead?

> +                pats.append(f)
> +                if not lfutil.isstandin(f):
> +                    slf = lfutil.standin(f)
> +                else:
> +                    slf = lfutil.splitstandin(f)

(minor it: un-negate the expression and swap the true and false branches 
to get more readable code?)

> +                if not slf in pats:
> +                    pats.append(slf)
> +        return m, pats
> +
> +    def overridefilelog(repo, subset, x):

In what way is this different from the real filelog function and why? 
That might deserve a comment.

> +        """``filelog(pattern)``
> +        Changesets connected to the specified filelog.
> +
> +        For performance reasons, ``filelog()`` does not show every changeset
> +        that affects the requested file(s). See :hg:`help log` for details. For
> +        a slower, more accurate result, use ``file()``.
> +
> +        The pattern without explicit kind like ``glob:`` is expected to be
> +        relative to the current directory and match against a file exactly
> +        for efficiency.
> +        """
> +
> +        # i18n: "filelog" is a keyword
> +        pat = revset.getstring(x, _("filelog requires a pattern"))
> +        s = set()
> +
> +        if not match_.patkind(pat):
> +            fl = repo.file(pat)
> +            for fr in fl:
> +                s.add(fl.linkrev(fr))
> +        else:
> +            m = match_.match(repo.root, repo.getcwd(), [pat], ctx=repo[None])
> +            for f in repo[None]:
> +                if m(f):
> +                    fl = repo.file(f)
> +                    for fr in fl:
> +                        s.add(fl.linkrev(fr))
> +
> +        return subset.filter(lambda r: r in s)
> +
> +    oldmatchandpats = installmatchandpatsfn(overridematchandpats)
> +    installfilelogfn(overridefilelog)
>       try:
>           repo.lfstatus = True
>           return orig(ui, repo, *pats, **opts)
>       finally:
>           repo.lfstatus = False
> -        restorematchfn()
> +        restorematchandpatsfn()
> +        restorefilelogfn()
>   
>   def overrideverify(orig, ui, repo, *pats, **opts):
>       large = opts.pop('large', False)
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -428,7 +428,7 @@
>           '''Return a list of revisions matching the given revset'''
>           expr = revset.formatspec(expr, *args)
>           m = revset.match(None, expr)
> -        return revset.baseset([r for r in m(self, revset.baseset(self))])
> +        return m(self, revset.spanset(self))

Unrelated change?

/Mads
Lucas Moscovicz - March 6, 2014, 6:54 p.m.
On 3/6/14, 10:26 AM, "Mads Kiilerich" <mads@kiilerich.com> wrote:

>On 03/06/2014 01:10 AM, Lucas Moscovicz wrote:
>> # HG changeset patch
>> # User Lucas Moscovicz <lmoscovicz@fb.com>
>> # Date 1394063709 28800
>> #      Wed Mar 05 15:55:09 2014 -0800
>> # Node ID 2367ddb8b07acef2ea7f72936c0ba46f47908205
>> # Parent  ef75710021e1c1dd2ef192c21b274313698186d4
>> largefiles: changed overridelog to work with graphlog
>
>You have my sincere respect and sympathy for looking into this ...
>
>> Log for largefiles was failing for graphlog since it was overriding
>>match
>> instead of matchandpats.
>
>Did it already not work and this is a bugfix? Or is it a change of
>strategy that will make it work with the refactoring in the next
>changeset?

This was not working already for graph log. In fact, if you try most of
the tests in test-largefiles.t with -G option you get a different output.
This is supposed to fix those tests and leave everything working for the
next patch where I change the log code to also use the graph log option
parser.

>
>> Tests for graphlog to be added in future patches.
>
>Why not introduce the tests in a patch before this or in this patch so
>we can see that it improves (or at least doesn't break)?

Tests for graph log will break before this patch. I am not sure whether I
am supposed to add them and send this patches together but as I understand
the flow for this should include this patch first and the tests later. Is
there another way to get around this?

>
>> diff --git a/hgext/largefiles/overrides.py
>>b/hgext/largefiles/overrides.py
>> --- a/hgext/largefiles/overrides.py
>> +++ b/hgext/largefiles/overrides.py
>> @@ -8,7 +8,7 @@
>>   
>>   '''Overridden Mercurial commands and functions for the largefiles
>>extension'''
>>   
>> -import os
>> +import os, re, glob
>>   import copy
>>   
>>   from mercurial import hg, commands, util, cmdutil, scmutil, match as
>>match_, \
>> @@ -47,6 +47,15 @@
>>       scmutil.match = f
>>       return oldmatch
>>   
>> +def installmatchandpatsfn(f):
>> +    oldmatchandpats = scmutil.matchandpats
>> +    setattr(f, 'oldmatchandpats', oldmatchandpats)
>> +    scmutil.matchandpats = f
>> +    return oldmatchandpats
>> +
>> +def installfilelogfn(f):
>> +    revset.symbols['filelog'] = f
>
>"usually" we store the old function and make sure we restore the right
>one when we are done. Shouldn't we do the same here?

I didn¹t do that since I am not currently overwriting the function, just
changing the entry on the symbols table. Anyways, I can store the function
anyways just to make sure we don¹t break anything else.

>
>> +
>>   def restorematchfn():
>>       '''restores scmutil.match to what it was before
>>installnormalfilesmatchfn
>>       was called.  no-op if scmutil.match is its original function.
>> @@ -55,6 +64,20 @@
>>       restore matchfn to reverse'''
>>       scmutil.match = getattr(scmutil.match, 'oldmatch', scmutil.match)
>>   
>> +def restorematchandpatsfn():
>> +    '''restores scmutil.matchandpats to what it was before
>> +    installnormalfilesmatchandpatsfn was called.  no-op if
>>scmutil.matchandpats
>> +    is its original function.
>> +
>> +    Note that n calls to installnormalfilesmatchandpatsfn will require
>>n calls
>> +    to restore matchfn to reverse'''
>> +    scmutil.matchandpats = getattr(scmutil.matchandpats,
>>'oldmatchandpats',
>> +            scmutil.matchandpats)
>> +
>> +def restorefilelogfn():
>> +    '''restores revset.filelog to what it was before.'''
>> +    revset.symbols['filelog'] = revset.filelog
>> +
>>   def addlargefiles(ui, repo, *pats, **opts):
>>       large = opts.pop('large', None)
>>       lfsize = lfutil.getminsize(
>> @@ -241,15 +264,15 @@
>>           repo._repo.lfstatus = False
>>   
>>   def overridelog(orig, ui, repo, *pats, **opts):
>> -    def overridematch(ctx, pats=[], opts={}, globbed=False,
>> +    def overridematchandpats(ctx, pats=[], opts={}, globbed=False,
>>               default='relpath'):
>>           """Matcher that merges root directory with .hglf, suitable
>>for log.
>>           It is still possible to match .hglf directly.
>>           For any listed files run log on the standin too.
>>           matchfn tries both the given filename and with .hglf stripped.
>>           """
>> -        match = oldmatch(ctx, pats, opts, globbed, default)
>> -        m = copy.copy(match)
>> +        matchandpats = oldmatchandpats(ctx, pats, opts, globbed,
>>default)
>> +        m, p = copy.copy(matchandpats)
>>           for i in range(0, len(m._files)):
>>               standin = lfutil.standin(m._files[i])
>>               if standin in repo[ctx.node()]:
>> @@ -264,14 +287,81 @@
>>               r = origmatchfn(f)
>>               return r
>>           m.matchfn = lfmatchfn
>> -        return m
>> -    oldmatch = installmatchfn(overridematch)
>> +
>> +        def expandpats(pats):
>> +            ret = []
>> +            for p in pats:
>> +                kind, name = match_._patsplit(p, None)
>> +                if kind is None:
>> +                    try:
>> +                        globbed = glob.glob(name)
>> +                    except re.error:
>> +                        globbed = [name]
>> +                    if globbed:
>> +                        ret.extend(globbed)
>> +                        for g in globbed:
>> +                            if not lfutil.isstandin(g):
>> +                                ret.append(lfutil.standin(g))
>
>I would expect that it only should use the standin if the matched file
>itself not is tracked directly but is a largefile - which probably
>approximately means that it is in lfdirstate or the standin exists in
>the filesystem or revision. I also wonder if the standins should be used
>instead of the matched file ...
>
>BUT this is for log and across multi revisions so I guess it is less
>wrong to include both and see what they match? That might deserve a
>comment.
>
>> +                        continue
>> +                ret.append(p)
>> +            return ret
>
>This expandpats function seems to be an almost exact copy of
>scmutil.expandpats, only with some post processing? It is only used once
>- a few lines below? Why not just call scmutil.expandpats and
>postprocess the result?
>
>> +
>> +        if pats == ("",):
>> +            pats = []
>> +        else:
>> +            pats = expandpats(pats or [])
>> +
>> +        for f in m.files():
>> +            if f not in pats:
>
>pats is a list. Could we use sets instead?
>
>> +                pats.append(f)
>> +                if not lfutil.isstandin(f):
>> +                    slf = lfutil.standin(f)
>> +                else:
>> +                    slf = lfutil.splitstandin(f)
>
>(minor it: un-negate the expression and swap the true and false branches
>to get more readable code?)
>
>> +                if not slf in pats:
>> +                    pats.append(slf)
>> +        return m, pats
>> +
>> +    def overridefilelog(repo, subset, x):
>
>In what way is this different from the real filelog function and why?
>That might deserve a comment.

The only difference here is dropping the pathutil.canonpath(Š) in order to
use the relative path when matching the files in .hglf/ I¹ll add a comment
for that.

>
>> +        """``filelog(pattern)``
>> +        Changesets connected to the specified filelog.
>> +
>> +        For performance reasons, ``filelog()`` does not show every
>>changeset
>> +        that affects the requested file(s). See :hg:`help log` for
>>details. For
>> +        a slower, more accurate result, use ``file()``.
>> +
>> +        The pattern without explicit kind like ``glob:`` is expected
>>to be
>> +        relative to the current directory and match against a file
>>exactly
>> +        for efficiency.
>> +        """
>> +
>> +        # i18n: "filelog" is a keyword
>> +        pat = revset.getstring(x, _("filelog requires a pattern"))
>> +        s = set()
>> +
>> +        if not match_.patkind(pat):
>> +            fl = repo.file(pat)
>> +            for fr in fl:
>> +                s.add(fl.linkrev(fr))
>> +        else:
>> +            m = match_.match(repo.root, repo.getcwd(), [pat],
>>ctx=repo[None])
>> +            for f in repo[None]:
>> +                if m(f):
>> +                    fl = repo.file(f)
>> +                    for fr in fl:
>> +                        s.add(fl.linkrev(fr))
>> +
>> +        return subset.filter(lambda r: r in s)
>> +
>> +    oldmatchandpats = installmatchandpatsfn(overridematchandpats)
>> +    installfilelogfn(overridefilelog)
>>       try:
>>           repo.lfstatus = True
>>           return orig(ui, repo, *pats, **opts)
>>       finally:
>>           repo.lfstatus = False
>> -        restorematchfn()
>> +        restorematchandpatsfn()
>> +        restorefilelogfn()
>>   
>>   def overrideverify(orig, ui, repo, *pats, **opts):
>>       large = opts.pop('large', False)
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -428,7 +428,7 @@
>>           '''Return a list of revisions matching the given revset'''
>>           expr = revset.formatspec(expr, *args)
>>           m = revset.match(None, expr)
>> -        return revset.baseset([r for r in m(self,
>>revset.baseset(self))])
>> +        return m(self, revset.spanset(self))
>
>Unrelated change?

Oops, missed that one.

>
>/Mads
>
Mads Kiilerich - March 6, 2014, 7:10 p.m.
On 03/06/2014 07:54 PM, Lucas Moscovicz wrote:
> On 3/6/14, 10:26 AM, "Mads Kiilerich" <mads@kiilerich.com> wrote:
>> Did it already not work and this is a bugfix? Or is it a change of
>> strategy that will make it work with the refactoring in the next
>> changeset?
> This was not working already for graph log. In fact, if you try most of
> the tests in test-largefiles.t with -G option you get a different output.
> This is supposed to fix those tests and leave everything working for the
> next patch where I change the log code to also use the graph log option
> parser.

Ok. It would be nice to have that spelled out in the commit message.

>>
>>> Tests for graphlog to be added in future patches.
>> Why not introduce the tests in a patch before this or in this patch so
>> we can see that it improves (or at least doesn't break)?
> Tests for graph log will break before this patch. I am not sure whether I
> am supposed to add them and send this patches together but as I understand
> the flow for this should include this patch first and the tests later. Is
> there another way to get around this?

If it is broken as in 'gives slightly incorrect result' then I would 
prefer to first have a patch that introduce test coverage of this area 
and documents how it is. Obviously, that test should pass. Next, this 
patch would update the test result and it would be very clear what this 
patch would change.

AFAIK Matt doesn't like that approach very much so usually bugfixes 
(like this one) also introduces a test that makes sure we don't get 
regressions later on but doesn't show what changed.

/Mads

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -8,7 +8,7 @@ 
 
 '''Overridden Mercurial commands and functions for the largefiles extension'''
 
-import os
+import os, re, glob
 import copy
 
 from mercurial import hg, commands, util, cmdutil, scmutil, match as match_, \
@@ -47,6 +47,15 @@ 
     scmutil.match = f
     return oldmatch
 
+def installmatchandpatsfn(f):
+    oldmatchandpats = scmutil.matchandpats
+    setattr(f, 'oldmatchandpats', oldmatchandpats)
+    scmutil.matchandpats = f
+    return oldmatchandpats
+
+def installfilelogfn(f):
+    revset.symbols['filelog'] = f
+
 def restorematchfn():
     '''restores scmutil.match to what it was before installnormalfilesmatchfn
     was called.  no-op if scmutil.match is its original function.
@@ -55,6 +64,20 @@ 
     restore matchfn to reverse'''
     scmutil.match = getattr(scmutil.match, 'oldmatch', scmutil.match)
 
+def restorematchandpatsfn():
+    '''restores scmutil.matchandpats to what it was before
+    installnormalfilesmatchandpatsfn was called.  no-op if scmutil.matchandpats
+    is its original function.
+
+    Note that n calls to installnormalfilesmatchandpatsfn will require n calls
+    to restore matchfn to reverse'''
+    scmutil.matchandpats = getattr(scmutil.matchandpats, 'oldmatchandpats',
+            scmutil.matchandpats)
+
+def restorefilelogfn():
+    '''restores revset.filelog to what it was before.'''
+    revset.symbols['filelog'] = revset.filelog
+
 def addlargefiles(ui, repo, *pats, **opts):
     large = opts.pop('large', None)
     lfsize = lfutil.getminsize(
@@ -241,15 +264,15 @@ 
         repo._repo.lfstatus = False
 
 def overridelog(orig, ui, repo, *pats, **opts):
-    def overridematch(ctx, pats=[], opts={}, globbed=False,
+    def overridematchandpats(ctx, pats=[], opts={}, globbed=False,
             default='relpath'):
         """Matcher that merges root directory with .hglf, suitable for log.
         It is still possible to match .hglf directly.
         For any listed files run log on the standin too.
         matchfn tries both the given filename and with .hglf stripped.
         """
-        match = oldmatch(ctx, pats, opts, globbed, default)
-        m = copy.copy(match)
+        matchandpats = oldmatchandpats(ctx, pats, opts, globbed, default)
+        m, p = copy.copy(matchandpats)
         for i in range(0, len(m._files)):
             standin = lfutil.standin(m._files[i])
             if standin in repo[ctx.node()]:
@@ -264,14 +287,81 @@ 
             r = origmatchfn(f)
             return r
         m.matchfn = lfmatchfn
-        return m
-    oldmatch = installmatchfn(overridematch)
+
+        def expandpats(pats):
+            ret = []
+            for p in pats:
+                kind, name = match_._patsplit(p, None)
+                if kind is None:
+                    try:
+                        globbed = glob.glob(name)
+                    except re.error:
+                        globbed = [name]
+                    if globbed:
+                        ret.extend(globbed)
+                        for g in globbed:
+                            if not lfutil.isstandin(g):
+                                ret.append(lfutil.standin(g))
+                        continue
+                ret.append(p)
+            return ret
+
+        if pats == ("",):
+            pats = []
+        else:
+            pats = expandpats(pats or [])
+
+        for f in m.files():
+            if f not in pats:
+                pats.append(f)
+                if not lfutil.isstandin(f):
+                    slf = lfutil.standin(f)
+                else:
+                    slf = lfutil.splitstandin(f)
+                if not slf in pats:
+                    pats.append(slf)
+        return m, pats
+
+    def overridefilelog(repo, subset, x):
+        """``filelog(pattern)``
+        Changesets connected to the specified filelog.
+
+        For performance reasons, ``filelog()`` does not show every changeset
+        that affects the requested file(s). See :hg:`help log` for details. For
+        a slower, more accurate result, use ``file()``.
+
+        The pattern without explicit kind like ``glob:`` is expected to be
+        relative to the current directory and match against a file exactly
+        for efficiency.
+        """
+
+        # i18n: "filelog" is a keyword
+        pat = revset.getstring(x, _("filelog requires a pattern"))
+        s = set()
+
+        if not match_.patkind(pat):
+            fl = repo.file(pat)
+            for fr in fl:
+                s.add(fl.linkrev(fr))
+        else:
+            m = match_.match(repo.root, repo.getcwd(), [pat], ctx=repo[None])
+            for f in repo[None]:
+                if m(f):
+                    fl = repo.file(f)
+                    for fr in fl:
+                        s.add(fl.linkrev(fr))
+
+        return subset.filter(lambda r: r in s)
+
+    oldmatchandpats = installmatchandpatsfn(overridematchandpats)
+    installfilelogfn(overridefilelog)
     try:
         repo.lfstatus = True
         return orig(ui, repo, *pats, **opts)
     finally:
         repo.lfstatus = False
-        restorematchfn()
+        restorematchandpatsfn()
+        restorefilelogfn()
 
 def overrideverify(orig, ui, repo, *pats, **opts):
     large = opts.pop('large', False)
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -428,7 +428,7 @@ 
         '''Return a list of revisions matching the given revset'''
         expr = revset.formatspec(expr, *args)
         m = revset.match(None, expr)
-        return revset.baseset([r for r in m(self, revset.baseset(self))])
+        return m(self, revset.spanset(self))
 
     def set(self, expr, *args):
         '''