[PATCH 2 of 5] commands: start registering command properties with registrar

Gregory Szorc gregory.szorc at gmail.com
Sat Jan 2 13:30:51 CST 2016


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1451757944 28800
#      Sat Jan 02 10:05:44 2016 -0800
# Node ID 932b8e6ce49a4b352b7c08c33d5274e07b64ec02
# Parent  4116b90197f9d7a62c75b79ada0d9932dcf81206
commands: start registering command properties with registrar

Currently, the special command properties "no repo," "optional repo,"
and "infer repo" are tracked in variables in commands.py. Historically,
commands (or extensions) would mutate these variables as appropriate.
The modern way to register these properties is by passing arguments to
the @command decorator.

Under the scenes, @command's implementation (which is defined in
cmdutil.py) imports commands.py in order to modify these variables.
This creates an import cycle, which is avoided by delay importing
"commands." Unfortunately, this hack no longer works with our
modern import convention:

1. commands imports cmdutil
2. @command decorators in commands are processed as the module is loaded
3. cmdutil.command attempts to import command, which results in an
   ImportError because commands isn't full loaded yet

I'm not sure why this doesn't work with our modern convention. I
suspect it has to do with using the "from . import X" style, which
subtly changes when modules are added to sys.modules. We know import
cycles are bad, so let's fix it.

registrar.py was recently introduced. It has minimal dependencies and
can be used to break import cycles.

This patch starts the process of moving the aforementioned special
command properties from commands.py to registrar.py.

These properties are established as sets (as opposed to strings of
space delimited values) in registrar.py. @command is taught to update
variables in registrar.py instead of commands.py.

Making registrar.py the full source of truth will require updating
dispatch.py. This patch stops short of that. So, we modify commands.py
to extract values from registrar at the bottom of the module. We also
change the behavior of @command after commands.py is loaded to not use
the registrar. This is a temporary hack to allow the existing dispatch
logic to continue working without being aware of registrar.

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3249,16 +3249,21 @@ def _performrevert(repo, parents, ctx, a
         normal(f)
 
     copied = copies.pathcopies(repo[parent], ctx)
 
     for f in actions['add'][0] + actions['undelete'][0] + actions['revert'][0]:
         if f in copied:
             repo.dirstate.copy(copied[f], f)
 
+# Whether commands should be registered with the registrar module.
+# TODO remove this once dispatch and extension loading are aware of
+# the registrar.
+commanduseregistrar = True
+
 def command(table):
     """Returns a function object to be used as a decorator for making commands.
 
     This function receives a command table as its argument. The table should
     be a dict.
 
     The returned function can be used as a decorator for adding commands
     to that command table. This function accepts multiple arguments to define
@@ -3292,23 +3297,32 @@ def command(table):
             else:
                 table[name] = func, list(options)
 
             if norepo or optionalrepo or inferrepo:
                 aliases = parsealiases(name)
                 import commands
 
             if norepo:
-                commands.norepo += ' %s' % ' '.join(aliases)
+                if commanduseregistrar:
+                    registrar.commands['norepo'] |= set(aliases)
+                else:
+                    commands.norepo += ' %s' % ' '.join(aliases)
 
             if optionalrepo:
-                commands.optionalrepo += ' %s' % ' '.join(aliases)
+                if commanduseregistrar:
+                    registrar.commands['optionalrepo'] |= set(aliases)
+                else:
+                    commands.optionalrepo += ' %s' % ' '.join(aliases)
 
             if inferrepo:
-                commands.inferrepo += ' %s' % ' '.join(aliases)
+                if commanduseregistrar:
+                    registrar.commands['inferrepo'] |= set(aliases)
+                else:
+                    commands.inferrepo += ' %s' % ' '.join(aliases)
 
             return func
         return decorator
 
     return cmd
 
 # a list of (ui, repo, otherpeer, opts, missing) functions called by
 # commands.outgoing.  "missing" is "missing" of the result of
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -19,33 +19,22 @@ import merge as mergemod
 import minirst, revset, fileset
 import dagparser, context, simplemerge, graphmod, copies
 import random, operator
 import setdiscovery, treediscovery, dagutil, pvec, localrepo, destutil
 import phases, obsolete, exchange, bundle2, repair, lock as lockmod
 import ui as uimod
 import streamclone
 import commandserver
+import registrar
 
 table = {}
 
 command = cmdutil.command(table)
 
-# Space delimited list of commands that don't require local repositories.
-# This should be populated by passing norepo=True into the @command decorator.
-norepo = ''
-# Space delimited list of commands that optionally require local repositories.
-# This should be populated by passing optionalrepo=True into the @command
-# decorator.
-optionalrepo = ''
-# Space delimited list of commands that will examine arguments looking for
-# a repository. This should be populated by passing inferrepo=True into the
-# @command decorator.
-inferrepo = ''
-
 # label constants
 # until 3.5, bookmarks.current was the advertised name, not
 # bookmarks.active, so we must use both to avoid breaking old
 # custom styles
 activebookmarklabel = 'bookmarks.active bookmarks.current'
 
 # common command options
 
@@ -7001,8 +6990,29 @@ def version_(ui):
         vers = []
         for name, module in extensions.extensions():
             names.append(name)
             vers.append(extensions.moduleversion(module))
         if names:
             maxnamelen = max(len(n) for n in names)
             for i, name in enumerate(names):
                 ui.write("  %-*s  %s\n" % (maxnamelen, name, vers[i]))
+
+# TODO make registrar source of truth and pull values from these variables into
+# registrar.
+
+# Space delimited list of commands that don't require local repositories.
+# This should be populated by passing norepo=True into the @command decorator.
+norepo = ' '.join(registrar.commands['norepo'])
+
+# Space delimited list of commands that optionally require local repositories.
+# This should be populated by passing optionalrepo=True into the @command
+# decorator.
+optionalrepo = ' '.join(registrar.commands['optionalrepo'])
+
+# Space delimited list of commands that will examine arguments looking for
+# a repository. This should be populated by passing inferrepo=True into the
+# @command decorator.
+inferrepo = ' '.join(registrar.commands['inferrepo'])
+
+# Dispatch isn't yet aware of the registrar. So have the decorator register to
+# variables in this module for now.
+cmdutil.commanduseregistrar = False
diff --git a/mercurial/registrar.py b/mercurial/registrar.py
--- a/mercurial/registrar.py
+++ b/mercurial/registrar.py
@@ -6,16 +6,22 @@
 # GNU General Public License version 2 or any later version.
 
 from __future__ import absolute_import
 
 from . import (
     util,
 )
 
+commands = {
+    'norepo': set(),
+    'optionalrepo': set(),
+    'inferrepo': set(),
+}
+
 class funcregistrar(object):
     """Base of decorator to register a fuction for specific purpose
 
     The least derived class can be defined by overriding 'table' and
     'formatdoc', for example::
 
         symbols = {}
         class keyword(funcregistrar):


More information about the Mercurial-devel mailing list