Merge lp:~lifeless/bzr/Commands.hooks into lp:~bzr/bzr/trunk-old

[Robert Collins]

[Robert Collins (lifeless)]

This makes command lookup fully hookified.

Why? It makes the core logic much clearer IMO, and also makes it
possible to skin bzrlib more completely for things like loggerhead and
olive want to have their own command (not subcommand) but still want to
use bzr's Command and option handling infrastructure.

-Rob

[Ian Clatworthy (ian-clatworthy)]

This was reviewed (via BB) last week and my review comments haven't been actioned yet. See https://lists.ubuntu.com/archives/bazaar/2009q2/058746.html.

review: Needs Fixing

=== modified file 'NEWS'
--- NEWS	2009-06-12 08:39:44 +0000
+++ NEWS	2009-06-15 08:36:49 +0000
@@ -184,9 +184,19 @@
 Internals
 *********
 
+<<<<<<< TREE
 * Remove ``weave.py`` script for accessing internals of old weave-format
   repositories.  (Martin Pool)
 
+=======
+* Command lookup has had hooks added. ``bzrlib.Command.hooks`` has
+  three new hook points: ``get_command``, ``get_missing_command`` and
+  ``list_commands``, which allow just-in-time command name provision
+  rather than requiring all command names be known a-priori.
+  (Robert Collins)
+
+
+>>>>>>> MERGE-SOURCE
 Testing
 *******
 

=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py	2009-06-10 03:56:49 +0000
+++ bzrlib/commands.py	2009-06-15 08:36:49 +0000
@@ -49,10 +49,11 @@
     )
 """)
 
-from bzrlib import registry
-# Compatibility
 from bzrlib.hooks import HookPoint, Hooks
+# Compatibility - Option used to be in commands.
 from bzrlib.option import Option
+from bzrlib import registry
+from bzrlib.symbol_versioning import deprecated_function, deprecated_in
 
 
 class CommandInfo(object):
@@ -133,98 +134,181 @@
 
 def _builtin_commands():
     import bzrlib.builtins
+    return _scan_module_for_commands(bzrlib.builtins)
+
+
+def _scan_module_for_commands(module):
     r = {}
-    builtins = bzrlib.builtins.__dict__
-    for name in builtins:
+    for name, obj in module.__dict__.iteritems():
         if name.startswith("cmd_"):
             real_name = _unsquish_command_name(name)
-            r[real_name] = builtins[name]
+            r[real_name] = obj
     return r
 
 
+def _list_bzr_commands(names):
+    """Return a list of all the registered commands.
+
+    This searches plugins and the core.
+    """
+    # to eliminate duplicates
+    names.update(builtin_command_names())
+    names.update(plugin_command_names())
+    return names
+
+
+def all_command_names():
+    """Return a list of all command names."""
+    names = set()
+    for hook in Command.hooks['list_commands']:
+        new_names = hook(names)
+        if new_names is None:
+            raise AssertionError(
+                'hook %s returned None' % Command.hooks.get_hook_name(hook))
+        names = new_names
+    return names
+
+
 def builtin_command_names():
-    """Return list of builtin command names."""
+    """Return list of builtin command names.
+    
+    Use of all_command_names() is encouraged rather than builtin_command_names
+    and/or plugin_command_names.
+    """
     return _builtin_commands().keys()
 
 
 def plugin_command_names():
+    """Returns command names from commands registered by plugins."""
     return plugin_cmds.keys()
 
 
-def _get_cmd_dict(plugins_override=True):
-    """Return name->class mapping for all commands."""
+@deprecated_function(deprecated_in((1, 16, 0)))
+def get_all_cmds():
+    """Return canonical name and class for most commands.
+    
+    NB: This does not return all commands since the introduction of
+    command hooks, and returning the class is not sufficient to 
+    get correctly setup commands, which is why it is deprecated.
+
+    Use 'all_command_names' + 'get_cmd_object' instead.
+    """
     d = _builtin_commands()
     if plugins_override:
         d.update(plugin_cmds.iteritems())
-    return d
-
-
-def get_all_cmds(plugins_override=True):
-    """Return canonical name and class for all registered commands."""
-    for k, v in _get_cmd_dict(plugins_override=plugins_override).iteritems():
+    for k, v in d.iteritems():
         yield k,v
 
 
 def get_cmd_object(cmd_name, plugins_override=True):
-    """Return the canonical name and command class for a command.
+    """Return the command object for a command.
 
     plugins_override
         If true, plugin commands can override builtins.
     """
     try:
-        cmd = _get_cmd_object(cmd_name, plugins_override)
-        # Allow plugins to extend commands
-        for hook in Command.hooks['extend_command']:
-            hook(cmd)
-        return cmd
+        return _get_cmd_object(cmd_name, plugins_override)
     except KeyError:
         raise errors.BzrCommandError('unknown command "%s"' % cmd_name)
 
 
 def _get_cmd_object(cmd_name, plugins_override=True):
-    """Worker for get_cmd_object which raises KeyError rather than BzrCommandError."""
-    from bzrlib.externalcommand import ExternalCommand
+    """Get a command object.
 
+    :param cmd_name: The name of the command.
+    :param plugins_override: Allow plugins to override builtins.
+    :return: A Command object instance
+    :raises: KeyError if no command is found.
+    """
     # We want only 'ascii' command names, but the user may have typed
     # in a Unicode name. In that case, they should just get a
     # 'command not found' error later.
     # In the future, we may actually support Unicode command names.
-
-    # first look up this command under the specified name
-    if plugins_override:
+    cmd = None
+    # Get a command
+    for hook in Command.hooks['get_command']:
+        cmd = hook(cmd, cmd_name)
+        if cmd is not None and not plugins_override:
+            # We've found a non-plugin command, don't permit it to be
+            # overridden.
+            if not cmd.plugin_name():
+                break
+    if cmd is None:
+        for hook in Command.hooks['get_missing_command']:
+            cmd = hook(cmd_name)
+            if cmd is not None:
+                break
+    if cmd is None:
+        # No command found.
+        raise KeyError
+    # Allow plugins to extend commands
+    for hook in Command.hooks['extend_command']:
+        hook(cmd)
+    return cmd
+
+
+def _try_plugin_provider(cmd_name):
+    """Probe for a plugin provider having cmd_name."""
+    try:
+        plugin_metadata, provider = probe_for_provider(cmd_name)
+        raise errors.CommandAvailableInPlugin(cmd_name,
+            plugin_metadata, provider)
+    except errors.NoPluginAvailable:
+        pass
+
+
+def probe_for_provider(cmd_name):
+    """Look for a provider for cmd_name.
+
+    :param cmd_name: The command name.
+    :return: plugin_metadata, provider for getting cmd_name.
+    :raises NoPluginAvailable: When no provider can supply the plugin.
+    """
+    # look for providers that provide this command but aren't installed
+    for provider in command_providers_registry:
         try:
-            return plugin_cmds.get(cmd_name)()
-        except KeyError:
+            return provider.plugin_for_command(cmd_name), provider
+        except errors.NoPluginAvailable:
             pass
-    cmds = _get_cmd_dict(plugins_override=False)
+    raise errors.NoPluginAvailable(cmd_name)
+
+
+def _get_bzr_command(cmd_or_None, cmd_name):
+    """Get a command from bzr's core."""
+    cmds = _builtin_commands()
     try:
         return cmds[cmd_name]()
     except KeyError:
         pass
-    if plugins_override:
-        for key in plugin_cmds.keys():
-            info = plugin_cmds.get_info(key)
-            if cmd_name in info.aliases:
-                return plugin_cmds.get(key)()
     # look for any command which claims this as an alias
     for real_cmd_name, cmd_class in cmds.iteritems():
         if cmd_name in cmd_class.aliases:
             return cmd_class()
-
+    return cmd_or_None
+
+
+def _get_external_command(cmd_or_None, cmd_name):
+    """Lookup a command that is a shell script."""
+    # Only do external command lookups when no command is found so far.
+    if cmd_or_None is not None:
+        return cmd_or_None
+    from bzrlib.externalcommand import ExternalCommand
     cmd_obj = ExternalCommand.find_command(cmd_name)
     if cmd_obj:
         return cmd_obj
 
-    # look for plugins that provide this command but aren't installed
-    for provider in command_providers_registry:
-        try:
-            plugin_metadata = provider.plugin_for_command(cmd_name)
-        except errors.NoPluginAvailable:
-            pass
-        else:
-            raise errors.CommandAvailableInPlugin(cmd_name,
-                                                  plugin_metadata, provider)
-    raise KeyError
+
+def _get_plugin_command(cmd_or_None, cmd_name):
+    """Get a command from bzr's plugins."""
+    try:
+        return plugin_cmds.get(cmd_name)()
+    except KeyError:
+        pass
+    for key in plugin_cmds.keys():
+        info = plugin_cmds.get_info(key)
+        if cmd_name in info.aliases:
+            return plugin_cmds.get(key)()
+    return cmd_or_None
 
 
 class Command(object):
@@ -608,6 +692,23 @@
             "Called after creating a command object to allow modifications "
             "such as adding or removing options, docs etc. Called with the "
             "new bzrlib.commands.Command object.", (1, 13), None))
+        self.create_hook(HookPoint('get_command',
+            "Called when creating a single command. Called with "
+            "(cmd_or_None, command_name). get_command should either return "
+            "the cmd_or_None parameter, or a replacement Command object that "
+            "should be used for the command.", (1, 16), None))
+        self.create_hook(HookPoint('get_missing_command',
+            "Called when creating a single command if no command could be "
+            "found. Called with (command_name). get_missing_command should "
+            "either return None, or a Command object to be used for the "
+            "command.", (1, 16), None))
+        self.create_hook(HookPoint('list_commands',
+            "Called when enumerating commands. Called with a dict of "
+            "cmd_name: cmd_class tuples for all the commands found "
+            "so far. This dict is safe to mutate - to remove a command or "
+            "to replace it with another (eg plugin supplied) version. "
+            "list_commands should return the updated dict of commands.",
+            (1, 16), None))
 
 Command.hooks = CommandHooks()
 
@@ -952,6 +1053,20 @@
     return ignore_pipe
 
 
+def install_bzr_command_hooks():
+    """Install the hooks to supply bzr's own commands."""
+    Command.hooks.install_named_hook("list_commands", _list_bzr_commands,
+        "bzr commands")
+    Command.hooks.install_named_hook("get_command", _get_bzr_command,
+        "bzr commands")
+    Command.hooks.install_named_hook("get_command", _get_plugin_command,
+        "bzr plugin commands")
+    Command.hooks.install_named_hook("get_command", _get_external_command,
+        "bzr external command lookup")
+    Command.hooks.install_named_hook("get_missing_command", _try_plugin_provider,
+        "bzr plugin-provider-db check")
+
+
 def main(argv=None):
     """Main entry point of command-line interface.
 
@@ -968,7 +1083,6 @@
 
     # Is this a final release version? If so, we should suppress warnings
     if bzrlib.version_info[3] == 'final':
-        from bzrlib import symbol_versioning
         symbol_versioning.suppress_deprecation_warnings(override=False)
     if argv is None:
         argv = osutils.get_unicode_argv()
@@ -984,6 +1098,7 @@
         except UnicodeDecodeError:
             raise errors.BzrError("argv should be list of unicode strings.")
         argv = new_argv
+    install_bzr_command_hooks()
     ret = run_bzr_catch_errors(argv)
     trace.mutter("return code %d", ret)
     return ret

=== modified file 'bzrlib/help.py'
--- bzrlib/help.py	2009-03-23 14:59:43 +0000
+++ bzrlib/help.py	2009-06-15 08:36:49 +0000
@@ -73,8 +73,7 @@
         hidden = True
     else:
         hidden = False
-    names = set(_mod_commands.builtin_command_names()) # to eliminate duplicates
-    names.update(_mod_commands.plugin_command_names())
+    names = list(_mod_commands.all_command_names())
     commands = ((n, _mod_commands.get_cmd_object(n)) for n in names)
     shown_commands = [(n, o) for n, o in commands if o.hidden == hidden]
     max_name = max(len(n) for n, o in shown_commands)

=== modified file 'bzrlib/shellcomplete.py'
--- bzrlib/shellcomplete.py	2009-03-23 14:59:43 +0000
+++ bzrlib/shellcomplete.py	2009-06-15 08:36:49 +0000
@@ -64,15 +64,16 @@
         outfile = sys.stdout
 
     cmds = []
-    for cmdname, cmdclass in commands.get_all_cmds():
-        cmds.append((cmdname, cmdclass))
-        for alias in cmdclass.aliases:
-            cmds.append((alias, cmdclass))
+    for cmdname in commands.all_command_names():
+        cmd = commands.get_cmd_object(cmdname)))
+        cmds.append((cmdname, cmd))
+        for alias in cmd.aliases:
+            cmds.append((alias, cmd))
     cmds.sort()
-    for cmdname, cmdclass in cmds:
-        if cmdclass.hidden:
+    for cmdname, cmd in cmds:
+        if cmd.hidden:
             continue
-        doc = getdoc(cmdclass)
+        doc = getdoc(cmd)
         if doc is None:
             outfile.write(cmdname + '\n')
         else:

=== modified file 'bzrlib/tests/test_commands.py'
--- bzrlib/tests/test_commands.py	2009-04-07 17:13:51 +0000
+++ bzrlib/tests/test_commands.py	2009-06-15 08:36:49 +0000
@@ -163,6 +163,7 @@
         del sys.modules['bzrlib.tests.fake_command']
         global lazy_command_imported
         lazy_command_imported = False
+        commands.install_bzr_command_hooks()
 
     @staticmethod
     def remove_fake():
@@ -205,6 +206,7 @@
         # commands are registered).
         # when they are simply created.
         hook_calls = []
+        commands.install_bzr_command_hooks()
         commands.Command.hooks.install_named_hook(
             "extend_command", hook_calls.append, None)
         # create a command, should not fire
@@ -237,3 +239,87 @@
             self.assertEqual([cmd], hook_calls)
         finally:
             commands.plugin_cmds.remove('fake')
+
+
+class TestGetCommandHook(tests.TestCase):
+
+    def test_fires_on_get_cmd_object(self):
+        # The get_command(cmd) hook fires when commands are delivered to the
+        # ui.
+        commands.install_bzr_command_hooks()
+        hook_calls = []
+        class ACommand(commands.Command):
+            """A sample command."""
+        def get_cmd(cmd_or_None, cmd_name):
+            hook_calls.append(('called', cmd_or_None, cmd_name))
+            if cmd_name in ('foo', 'info'):
+                return ACommand()
+        commands.Command.hooks.install_named_hook(
+            "get_command", get_cmd, None)
+        # create a command directly, should not fire
+        cmd = ACommand()
+        self.assertEqual([], hook_calls)
+        # ask by name, should fire and give us our command
+        cmd = commands.get_cmd_object('foo')
+        self.assertEqual([('called', None, 'foo')], hook_calls)
+        self.assertIsInstance(cmd, ACommand)
+        del hook_calls[:]
+        # ask by a name that is supplied by a builtin - the hook should still
+        # fire and we still get our object, but we should see the builtin
+        # passed to the hook.
+        cmd = commands.get_cmd_object('info')
+        self.assertIsInstance(cmd, ACommand)
+        self.assertEqual(1, len(hook_calls))
+        self.assertEqual('info', hook_calls[0][2])
+        self.assertIsInstance(hook_calls[0][1], builtins.cmd_info)
+
+
+class TestGetMissingCommandHook(tests.TestCase):
+
+    def test_fires_on_get_cmd_object(self):
+        # The get_missing_command(cmd) hook fires when commands are delivered to the
+        # ui.
+        hook_calls = []
+        class ACommand(commands.Command):
+            """A sample command."""
+        def get_missing_cmd(cmd_name):
+            hook_calls.append(('called', cmd_name))
+            if cmd_name in ('foo', 'info'):
+                return ACommand()
+        commands.Command.hooks.install_named_hook(
+            "get_missing_command", get_missing_cmd, None)
+        # create a command directly, should not fire
+        cmd = ACommand()
+        self.assertEqual([], hook_calls)
+        # ask by name, should fire and give us our command
+        cmd = commands.get_cmd_object('foo')
+        self.assertEqual([('called', 'foo')], hook_calls)
+        self.assertIsInstance(cmd, ACommand)
+        del hook_calls[:]
+        # ask by a name that is supplied by a builtin - the hook should not
+        # fire and we still get our object.
+        commands.install_bzr_command_hooks()
+        cmd = commands.get_cmd_object('info')
+        self.assertNotEqual(None, cmd)
+        self.assertEqual(0, len(hook_calls))
+
+
+class TestListCommandHook(tests.TestCase):
+
+    def test_fires_on_all_command_names(self):
+        # The list_commands() hook fires when all_command_names() is invoked.
+        hook_calls = []
+        commands.install_bzr_command_hooks()
+        def list_my_commands(cmd_names):
+            hook_calls.append('called')
+            cmd_names.update(['foo', 'bar'])
+            return cmd_names
+        commands.Command.hooks.install_named_hook(
+            "list_commands", list_my_commands, None)
+        # Get a command, which should not trigger the hook.
+        cmd = commands.get_cmd_object('info')
+        self.assertEqual([], hook_calls)
+        # Get all command classes (for docs and shell completion).
+        cmds = list(commands.all_command_names())
+        self.assertEqual(['called'], hook_calls)
+        self.assertSubset(['foo', 'bar'], cmds)

=== modified file 'bzrlib/tests/test_options.py'
--- bzrlib/tests/test_options.py	2009-04-03 20:05:25 +0000
+++ bzrlib/tests/test_options.py	2009-06-15 08:36:49 +0000
@@ -324,8 +324,8 @@
 
     def get_builtin_command_options(self):
         g = []
-        for cmd_name, cmd_class in sorted(commands.get_all_cmds()):
-            cmd = cmd_class()
+        for cmd_name in sorted(commands.all_command_names()):
+            cmd = commands.get_cmd_object(cmd_name)
             for opt_name, opt in sorted(cmd.options().items()):
                 g.append((cmd_name, opt))
         return g
@@ -338,14 +338,15 @@
         g = dict(option.Option.OPTIONS.items())
         used_globals = {}
         msgs = []
-        for cmd_name, cmd_class in sorted(commands.get_all_cmds()):
-            for option_or_name in sorted(cmd_class.takes_options):
+        for cmd_name in sorted(commands.all_command_names()):
+            cmd = commands.get_cmd_object(cmd_name)
+            for option_or_name in sorted(cmd.takes_options):
                 if not isinstance(option_or_name, basestring):
                     self.assertIsInstance(option_or_name, option.Option)
                 elif not option_or_name in g:
                     msgs.append("apparent reference to undefined "
                         "global option %r from %r"
-                        % (option_or_name, cmd_class))
+                        % (option_or_name, cmd))
                 else:
                     used_globals.setdefault(option_or_name, []).append(cmd_name)
         unused_globals = set(g.keys()) - set(used_globals.keys())