Patchwork [2,of,4,V3] revset: disable compat with legacy compat for internal revsets API (API)

login
register
mail settings
Submitter Boris Feld
Date April 11, 2018, 9:50 a.m.
Message ID <00090d394d4e0a7c2637.1523440215@FB>
Download mbox | patch
Permalink /patch/30675/
State Superseded
Headers show

Comments

Boris Feld - April 11, 2018, 9:50 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1520412972 18000
#      Wed Mar 07 03:56:12 2018 -0500
# Node ID 00090d394d4e0a7c2637f161493353530dcf739d
# Parent  5c1a0d784a26d4e8659dcec80503c8764432a303
# EXP-Topic noname
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 00090d394d4e
revset: disable compat with legacy compat for internal revsets API (API)

In theory, there should be no user input going directly to "repo.revs" and
"repo.set" so we can adjust the behavior in all cases. Avoiding name lookup
during revsets parsing provide important speedup when some namespaces are slow
to load.
Yuya Nishihara - April 11, 2018, 3:23 p.m.
On Wed, 11 Apr 2018 11:50:15 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1520412972 18000
> #      Wed Mar 07 03:56:12 2018 -0500
> # Node ID 00090d394d4e0a7c2637f161493353530dcf739d
> # Parent  5c1a0d784a26d4e8659dcec80503c8764432a303
> # EXP-Topic noname
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 00090d394d4e
> revset: disable compat with legacy compat for internal revsets API (API)
> 
> In theory, there should be no user input going directly to "repo.revs" and
> "repo.set" so we can adjust the behavior in all cases. Avoiding name lookup
> during revsets parsing provide important speedup when some namespaces are slow
> to load.
> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -832,7 +832,7 @@ class localrepository(object):
>          that contains integer revisions.
>          '''
>          expr = revsetlang.formatspec(expr, *args)
> -        m = revset.match(None, expr)
> +        m = revset.match(None, expr, legacycompat=False)
>          return m(self)

> -def matchany(ui, specs, repo=None, localalias=None):
> +def matchany(ui, specs, repo=None, localalias=None, legacycompat=True):
>      """Create a matcher that will include any revisions matching one of the
>      given specs
>  
> @@ -2191,7 +2191,7 @@ def matchany(ui, specs, repo=None, local
>      if not all(specs):
>          raise error.ParseError(_("empty query"))
>      lookup = None
> -    if repo:
> +    if repo and legacycompat:
>          lookup = lookupfn(repo)

So legacycompat=False is identical to repo=None?
Boris FELD - April 13, 2018, 12:53 p.m.
On 11/04/2018 17:23, Yuya Nishihara wrote:
> On Wed, 11 Apr 2018 11:50:15 +0200, Boris Feld wrote:
>> # HG changeset patch
>> # User Boris Feld <boris.feld@octobus.net>
>> # Date 1520412972 18000
>> #      Wed Mar 07 03:56:12 2018 -0500
>> # Node ID 00090d394d4e0a7c2637f161493353530dcf739d
>> # Parent  5c1a0d784a26d4e8659dcec80503c8764432a303
>> # EXP-Topic noname
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 00090d394d4e
>> revset: disable compat with legacy compat for internal revsets API (API)
>>
>> In theory, there should be no user input going directly to "repo.revs" and
>> "repo.set" so we can adjust the behavior in all cases. Avoiding name lookup
>> during revsets parsing provide important speedup when some namespaces are slow
>> to load.
>>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -832,7 +832,7 @@ class localrepository(object):
>>           that contains integer revisions.
>>           '''
>>           expr = revsetlang.formatspec(expr, *args)
>> -        m = revset.match(None, expr)
>> +        m = revset.match(None, expr, legacycompat=False)
>>           return m(self)
>> -def matchany(ui, specs, repo=None, localalias=None):
>> +def matchany(ui, specs, repo=None, localalias=None, legacycompat=True):
>>       """Create a matcher that will include any revisions matching one of the
>>       given specs
>>   
>> @@ -2191,7 +2191,7 @@ def matchany(ui, specs, repo=None, local
>>       if not all(specs):
>>           raise error.ParseError(_("empty query"))
>>       lookup = None
>> -    if repo:
>> +    if repo and legacycompat:
>>           lookup = lookupfn(repo)
> So legacycompat=False is identical to repo=None?

Almost, there is also the `posttreebuilthook` hook that takes repo as a 
parameter.

It is (was?) used at least by the directaccess extension.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - April 13, 2018, 1:03 p.m.
On Fri, 13 Apr 2018 14:53:57 +0200, Feld Boris wrote:
> On 11/04/2018 17:23, Yuya Nishihara wrote:
> > On Wed, 11 Apr 2018 11:50:15 +0200, Boris Feld wrote:
> >> # HG changeset patch
> >> # User Boris Feld <boris.feld@octobus.net>
> >> # Date 1520412972 18000
> >> #      Wed Mar 07 03:56:12 2018 -0500
> >> # Node ID 00090d394d4e0a7c2637f161493353530dcf739d
> >> # Parent  5c1a0d784a26d4e8659dcec80503c8764432a303
> >> # EXP-Topic noname
> >> # Available At https://bitbucket.org/octobus/mercurial-devel/
> >> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 00090d394d4e
> >> revset: disable compat with legacy compat for internal revsets API (API)
> >>
> >> In theory, there should be no user input going directly to "repo.revs" and
> >> "repo.set" so we can adjust the behavior in all cases. Avoiding name lookup
> >> during revsets parsing provide important speedup when some namespaces are slow
> >> to load.
> >>
> >> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> >> --- a/mercurial/localrepo.py
> >> +++ b/mercurial/localrepo.py
> >> @@ -832,7 +832,7 @@ class localrepository(object):
> >>           that contains integer revisions.
> >>           '''
> >>           expr = revsetlang.formatspec(expr, *args)
> >> -        m = revset.match(None, expr)
> >> +        m = revset.match(None, expr, legacycompat=False)
> >>           return m(self)
> >> -def matchany(ui, specs, repo=None, localalias=None):
> >> +def matchany(ui, specs, repo=None, localalias=None, legacycompat=True):
> >>       """Create a matcher that will include any revisions matching one of the
> >>       given specs
> >>   
> >> @@ -2191,7 +2191,7 @@ def matchany(ui, specs, repo=None, local
> >>       if not all(specs):
> >>           raise error.ParseError(_("empty query"))
> >>       lookup = None
> >> -    if repo:
> >> +    if repo and legacycompat:
> >>           lookup = lookupfn(repo)
> > So legacycompat=False is identical to repo=None?
> 
> Almost, there is also the `posttreebuilthook` hook that takes repo as a 
> parameter.

Got it. Then, can you rephrase the commit message? The legacy lookup is
disabled for internal API from the beginning.

> It is (was?) used at least by the directaccess extension.

Pulkit, do we still need the posttreebuildhook? I think the only user was
the directaccess, and it's in core.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -832,7 +832,7 @@  class localrepository(object):
         that contains integer revisions.
         '''
         expr = revsetlang.formatspec(expr, *args)
-        m = revset.match(None, expr)
+        m = revset.match(None, expr, legacycompat=False)
         return m(self)
 
     def set(self, expr, *args):
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2173,11 +2173,11 @@  def posttreebuilthook(tree, repo):
 def lookupfn(repo):
     return lambda symbol: scmutil.isrevsymbol(repo, symbol)
 
-def match(ui, spec, repo=None):
+def match(ui, spec, repo=None, legacycompat=True):
     """Create a matcher for a single revision spec"""
-    return matchany(ui, [spec], repo=repo)
+    return matchany(ui, [spec], repo=repo, legacycompat=legacycompat)
 
-def matchany(ui, specs, repo=None, localalias=None):
+def matchany(ui, specs, repo=None, localalias=None, legacycompat=True):
     """Create a matcher that will include any revisions matching one of the
     given specs
 
@@ -2191,7 +2191,7 @@  def matchany(ui, specs, repo=None, local
     if not all(specs):
         raise error.ParseError(_("empty query"))
     lookup = None
-    if repo:
+    if repo and legacycompat:
         lookup = lookupfn(repo)
     if len(specs) == 1:
         tree = revsetlang.parse(specs[0], lookup)