Patchwork [1,of,4] revset: make repo.anyrevs accept customized alias override

login
register
mail settings
Submitter Jun Wu
Date June 24, 2017, 5:04 a.m.
Message ID <a8a42cc07de6538e92ba.1498280680@x1c>
Download mbox | patch
Permalink /patch/21658/
State Changes Requested
Headers show

Comments

Jun Wu - June 24, 2017, 5:04 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1498279695 25200
#      Fri Jun 23 21:48:15 2017 -0700
# Node ID a8a42cc07de6538e92bae498882144f9b8296c98
# Parent  66117dae87f98789a217e90deb86908927967630
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6
revset: make repo.anyrevs accept customized alias override

Previously repo.anyrevs only expand aliases in [revsetalias] config. This
patch makes it more flexible to accept a customized dict defining aliases
without having to couple with ui.

revsetlang.expandaliases now have the signature (tree, aliases, warn=None)
which is more consistent with templater.expandaliases. revsetlang.py is now
free from "ui", which seems to be a good thing.
Yuya Nishihara - June 24, 2017, 8:40 a.m.
On Fri, 23 Jun 2017 22:04:40 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1498279695 25200
> #      Fri Jun 23 21:48:15 2017 -0700
> # Node ID a8a42cc07de6538e92bae498882144f9b8296c98
> # Parent  66117dae87f98789a217e90deb86908927967630
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6
> revset: make repo.anyrevs accept customized alias override
> 
> Previously repo.anyrevs only expand aliases in [revsetalias] config. This
> patch makes it more flexible to accept a customized dict defining aliases
> without having to couple with ui.
> 
> revsetlang.expandaliases now have the signature (tree, aliases, warn=None)
> which is more consistent with templater.expandaliases. revsetlang.py is now
> free from "ui", which seems to be a good thing.

Yeah, sounds good. One nit.

> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -622,14 +622,15 @@ class localrepository(object):
>              yield self[r]
>  
> -    def anyrevs(self, specs, user=False):
> +    def anyrevs(self, specs, user=False, local=None):
>          '''Find revisions matching one of the given revsets.
>  
>          Revset aliases from the configuration are not expanded by default. To
> -        expand user aliases, specify ``user=True``.
> +        expand user aliases, specify ``user=True``. To provide some local
> +        definitions, set local to {name: definition}.
>          '''
>          if user:
> -            m = revset.matchany(self.ui, specs, repo=self)
> +            m = revset.matchany(self.ui, specs, repo=self, local=local)
>          else:
> -            m = revset.matchany(None, specs)
> +            m = revset.matchany(None, specs, local=local)

Can you rename 'local' to 'localaliases' or something in V2? It seems too
ambiguous. 'user' might not be a good name, but at least it means processing
"user revset."

If V2 isn't planned, I can rename it in flight. (I don't follow the IRC
discussion.)
Jun Wu - June 24, 2017, 4 p.m.
Excerpts from Yuya Nishihara's message of 2017-06-24 17:40:14 +0900:
> Can you rename 'local' to 'localaliases' or something in V2? It seems too
> ambiguous. 'user' might not be a good name, but at least it means
> processing "user revset."

Sure. The name was "inspired" by "code.interact(local=objects)". I didn't
realize it's not clear whether "local" is bool or not given "user" is bool.

> If V2 isn't planned, I can rename it in flight. (I don't follow the IRC
> discussion.)

This series was not written lightly, like "defineparents" has been rewritten
3 times taking me 2 full days to reach the elegance level that satisfies me.
Therefore, although it's a non-trivial change, it's less like RFC but more
of a thoughtful refactoring. I did wish it to be accepted first time. But
since there are 2 comments already and I have discovered another small nit,
I'll send V2 next Tuesday.
via Mercurial-devel - June 27, 2017, 11:33 p.m.
On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1498279695 25200
> #      Fri Jun 23 21:48:15 2017 -0700
> # Node ID a8a42cc07de6538e92bae498882144f9b8296c98
> # Parent  66117dae87f98789a217e90deb86908927967630
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6
> revset: make repo.anyrevs accept customized alias override

I still plan to review this series. The last few days have just been
very busy. I still have many more hours of other work that needs to
get done first, but I'll try to get to this as soon as I can.
Jun Wu - June 28, 2017, 12:07 a.m.
Excerpts from Martin von Zweigbergk's message of 2017-06-27 16:33:52 -0700:
> On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote:
> > # HG changeset patch
> > # User Jun Wu <quark@fb.com>
> > # Date 1498279695 25200
> > #      Fri Jun 23 21:48:15 2017 -0700
> > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98
> > # Parent  66117dae87f98789a217e90deb86908927967630
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r a8a42cc07de6
> > revset: make repo.anyrevs accept customized alias override
> 
> I still plan to review this series. The last few days have just been
> very busy. I still have many more hours of other work that needs to
> get done first, but I'll try to get to this as soon as I can.

Thanks! The series could be simplified a bit after "scmutil.cleanupnodes"
and "drawdag" chagnes. Therefore it might be easier to get some conclusion
on those changes first (which I think are relatively strightfoward).
via Mercurial-devel - June 28, 2017, 5:20 p.m.
On Tue, Jun 27, 2017 at 5:07 PM, Jun Wu <quark@fb.com> wrote:
> Excerpts from Martin von Zweigbergk's message of 2017-06-27 16:33:52 -0700:
>> On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote:
>> > # HG changeset patch
>> > # User Jun Wu <quark@fb.com>
>> > # Date 1498279695 25200
>> > #      Fri Jun 23 21:48:15 2017 -0700
>> > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98
>> > # Parent  66117dae87f98789a217e90deb86908927967630
>> > # Available At https://bitbucket.org/quark-zju/hg-draft
>> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r a8a42cc07de6
>> > revset: make repo.anyrevs accept customized alias override
>>
>> I still plan to review this series. The last few days have just been
>> very busy. I still have many more hours of other work that needs to
>> get done first, but I'll try to get to this as soon as I can.
>
> Thanks! The series could be simplified a bit after "scmutil.cleanupnodes"
> and "drawdag" chagnes. Therefore it might be easier to get some conclusion
> on those changes first (which I think are relatively strightfoward).

The cleanupnodes itself is already in. Do you want the patch to make
rebase use it in before this series too?

Also, what are the drawdag changes?
Jun Wu - June 28, 2017, 6:14 p.m.
Excerpts from Martin von Zweigbergk's message of 2017-06-28 10:20:42 -0700:
> The cleanupnodes itself is already in. Do you want the patch to make
> rebase use it in before this series too?

I was referring to use "cleanupnodes" in rebase:

  rebase: use scmutil.cleanupnodes (issue5606) (BC)  2017-06-26
  scmutil: make cleanupnodes delete divergent bookmarks  2017-06-26
  scmutil: make cleanupnodes handle filtered node  2017-06-26

That will remove bookmark movement code for example which is also changed by
the multi-dest series.

> Also, what are the drawdag changes?

Seems I forgot to send them or didn't check the error code of "hg email".
They will be:

  drawdag: support obsmarker creation in comments  2017-06-27
  test-drawdag: add a test for drawdag.py  2017-06-26

That makes it easier to create repos with obsmarkers.

Patch

diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -1922,7 +1922,9 @@  def debugrevspec(ui, repo, expr, **opts)
     one. Returns 1 if the optimized result differs.
     """
+    aliases = ui.configitems('revsetalias')
     stages = [
         ('parsed', lambda tree: tree),
-        ('expanded', lambda tree: revsetlang.expandaliases(ui, tree)),
+        ('expanded', lambda tree: revsetlang.expandaliases(tree, aliases,
+                                                           ui.warn)),
         ('concatenated', revsetlang.foldconcat),
         ('analyzed', revsetlang.analyze),
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -622,14 +622,15 @@  class localrepository(object):
             yield self[r]
 
-    def anyrevs(self, specs, user=False):
+    def anyrevs(self, specs, user=False, local=None):
         '''Find revisions matching one of the given revsets.
 
         Revset aliases from the configuration are not expanded by default. To
-        expand user aliases, specify ``user=True``.
+        expand user aliases, specify ``user=True``. To provide some local
+        definitions, set local to {name: definition}.
         '''
         if user:
-            m = revset.matchany(self.ui, specs, repo=self)
+            m = revset.matchany(self.ui, specs, repo=self, local=local)
         else:
-            m = revset.matchany(None, specs)
+            m = revset.matchany(None, specs, local=local)
         return m(self)
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1976,5 +1976,5 @@  def match(ui, spec, repo=None, order=def
     return matchany(ui, [spec], repo=repo, order=order)
 
-def matchany(ui, specs, repo=None, order=defineorder):
+def matchany(ui, specs, repo=None, order=defineorder, local=None):
     """Create a matcher that will include any revisions matching one of the
     given specs
@@ -1982,4 +1982,7 @@  def matchany(ui, specs, repo=None, order
     If order=followorder, a matcher takes the ordering specified by the input
     set.
+
+    If local is not None, it is an dict {name: definitionstring} which will
+    be used. It takes precedence over [revsetalias] config section.
     """
     if not specs:
@@ -1998,6 +2001,13 @@  def matchany(ui, specs, repo=None, order
                 ('list',) + tuple(revsetlang.parse(s, lookup) for s in specs))
 
+    aliases = []
+    warn = None
     if ui:
-        tree = revsetlang.expandaliases(ui, tree)
+        aliases.extend(ui.configitems('revsetalias'))
+        warn = ui.warn
+    if local:
+        aliases.extend(local.items())
+    if aliases:
+        tree = revsetlang.expandaliases(tree, aliases, warn=warn)
     tree = revsetlang.foldconcat(tree)
     tree = revsetlang.analyze(tree, order)
diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py
--- a/mercurial/revsetlang.py
+++ b/mercurial/revsetlang.py
@@ -562,12 +562,14 @@  class _aliasrules(parser.basealiasrules)
             return tree[1][1], getlist(tree[2])
 
-def expandaliases(ui, tree):
-    aliases = _aliasrules.buildmap(ui.configitems('revsetalias'))
+def expandaliases(tree, aliases, warn=None):
+    """Expand aliases in a tree, aliases is a list of (name, value) tuples"""
+    aliases = _aliasrules.buildmap(aliases)
     tree = _aliasrules.expand(aliases, tree)
     # warn about problematic (but not referred) aliases
-    for name, alias in sorted(aliases.iteritems()):
-        if alias.error and not alias.warned:
-            ui.warn(_('warning: %s\n') % (alias.error))
-            alias.warned = True
+    if warn is not None:
+        for name, alias in sorted(aliases.iteritems()):
+            if alias.error and not alias.warned:
+                warn(_('warning: %s\n') % (alias.error))
+                alias.warned = True
     return tree
 
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -4070,3 +4070,25 @@  loading it
   [255]
 
+Test repo.anyrevs with customized revset overrides
+
+  $ cat > $TESTTMP/printprevset.py <<EOF
+  > from mercurial import encoding
+  > def reposetup(ui, repo):
+  >     local = {}
+  >     p = encoding.environ.get('P')
+  >     if p:
+  >         local['P'] = p
+  >     ui.write('P=%r' % list(repo.anyrevs(['P'], user=True, local=local)))
+  > EOF
+
+  $ cat >> .hg/hgrc <<EOF
+  > custompredicate = !
+  > printprevset = $TESTTMP/printprevset.py
+  > EOF
+
+  $ hg --config revsetalias.P=1 log -r . -T '\n'
+  P=[1]
+  $ P=3 hg --config revsetalias.P=2 log -r . -T '\n'
+  P=[3]
+
   $ cd ..