Patchwork [1,of,2,RESEND] extensions: peek command table of disabled extensions without importing

login
register
mail settings
Submitter Yuya Nishihara
Date May 24, 2018, 1:57 p.m.
Message ID <a9b271c7d3c095c381da.1527170257@mimosa>
Download mbox | patch
Permalink /patch/31835/
State Accepted
Headers show

Comments

Yuya Nishihara - May 24, 2018, 1:57 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1525340282 -32400
#      Thu May 03 18:38:02 2018 +0900
# Node ID a9b271c7d3c095c381da92be0ad7bfd2e9555ac9
# Parent  c3960c7e66fad7436f1a550ef1de693a635faa3a
extensions: peek command table of disabled extensions without importing

With chg where demandimport disabled, and if disk cache not warm, it took
more than 5 seconds to get "unknown command" error when you typo a command
name. This is horrible UX.

The new implementation is less accurate than the original one as Python
can do anything at import time and cmdtable may be imported from another
module, but I think it's good enough.

Note that the new implementation has to parse .py files, which is slightly
slower than executing .pyc if demandimport is enabled.
via Mercurial-devel - May 26, 2018, 5:06 a.m.
On Thu, May 24, 2018 at 6:57 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1525340282 -32400
> #      Thu May 03 18:38:02 2018 +0900
> # Node ID a9b271c7d3c095c381da92be0ad7bfd2e9555ac9
> # Parent  c3960c7e66fad7436f1a550ef1de693a635faa3a
> extensions: peek command table of disabled extensions without importing
>

Queuing this, thanks.

We once had a bug where `hg help` would crash because it ended up importing
all the extensions we ship with core even though they were not enabled. The
narrow extension that ships with core (which we had not enabled) did stuff
at import time that conflicted with what the external narrowhg extension
(which we had enabled). They both tried registered a revlog flag processor
and it's an error to register two processors for the same flag. I suspect
this patch would prevent such things from happening again.

Patch

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -7,6 +7,8 @@ 
 
 from __future__ import absolute_import
 
+import ast
+import collections
 import functools
 import imp
 import inspect
@@ -655,34 +657,67 @@  def disabledext(name):
     if name in paths:
         return _disabledhelp(paths[name])
 
+def _walkcommand(node):
+    """Scan @command() decorators in the tree starting at node"""
+    todo = collections.deque([node])
+    while todo:
+        node = todo.popleft()
+        if not isinstance(node, ast.FunctionDef):
+            todo.extend(ast.iter_child_nodes(node))
+            continue
+        for d in node.decorator_list:
+            if not isinstance(d, ast.Call):
+                continue
+            if not isinstance(d.func, ast.Name):
+                continue
+            if d.func.id != r'command':
+                continue
+            yield d
+
+def _disabledcmdtable(path):
+    """Construct a dummy command table without loading the extension module
+
+    This may raise IOError or SyntaxError.
+    """
+    with open(path, 'rb') as src:
+        root = ast.parse(src.read(), path)
+    cmdtable = {}
+    for node in _walkcommand(root):
+        if not node.args:
+            continue
+        a = node.args[0]
+        if isinstance(a, ast.Str):
+            name = pycompat.sysbytes(a.s)
+        elif pycompat.ispy3 and isinstance(a, ast.Bytes):
+            name = a.s
+        else:
+            continue
+        cmdtable[name] = (None, [], b'')
+    return cmdtable
+
 def _finddisabledcmd(ui, cmd, name, path, strict):
     try:
-        mod = loadpath(path, 'hgext.%s' % name)
-    except Exception:
+        cmdtable = _disabledcmdtable(path)
+    except (IOError, SyntaxError):
         return
     try:
-        aliases, entry = cmdutil.findcmd(cmd,
-            getattr(mod, 'cmdtable', {}), strict)
+        aliases, entry = cmdutil.findcmd(cmd, cmdtable, strict)
     except (error.AmbiguousCommand, error.UnknownCommand):
         return
-    except Exception:
-        ui.warn(_('warning: error finding commands in %s\n') % path)
-        ui.traceback()
-        return
     for c in aliases:
         if c.startswith(cmd):
             cmd = c
             break
     else:
         cmd = aliases[0]
-    doc = gettext(pycompat.getdoc(mod))
+    doc = _disabledhelp(path)
     return (cmd, name, doc)
 
 def disabledcmd(ui, cmd, strict=False):
-    '''import disabled extensions until cmd is found.
+    '''find cmd from disabled extensions without importing.
     returns (cmdname, extname, doc)'''
 
-    paths = _disabledpaths(strip_init=True)
+    paths = _disabledpaths()
     if not paths:
         raise error.UnknownCommand(cmd)
 
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -1229,9 +1229,14 @@  Broken disabled extension and command:
 
   $ cat > hgext/forest.py <<EOF
   > cmdtable = None
+  > @command()
+  > def f():
+  >     pass
+  > @command(123)
+  > def g():
+  >     pass
   > EOF
   $ hg --config extensions.path=./path.py help foo > /dev/null
-  warning: error finding commands in $TESTTMP/hgext/forest.py
   abort: no such help topic: foo
   (try 'hg help --keyword foo')
   [255]