adjust cli plugin config description

cli/koji

@@ -65,24 +65,40 @@ def register_plugin(plugin):
                 globals()[name] = v
 
 
-def load_plugins(options, paths):
+def load_plugins(plugin_paths):
-    """Load plugins specified by our configuration plus system plugins. Order
+    """Load plugins specified by input paths, ~/.koji/plugins, system plugins.
-    is that system plugins are first, so they can be overridden by
+    Loading order is descending, so they can be overridden by user-specified
-    user-specified ones with same name."""
+    ones.
+    Notice that:
+    - plugin file should end with .py extension
+    - non-directory is not acceptable by plugin_paths
+    - all plugin files and the exported handlers inside will be loaded, and
+      handler with the same name will override the one has already been loaded
+      before"""
+
     logger = logging.getLogger('koji.plugins')
-    tracker = koji.plugin.PluginTracker(path=paths)
+    paths = []
-    names = set()
+    # first, always load plugins from koji_cli_plugins module
+    paths.append(
+        '%s/lib/python%s.%s/site-packages/koji_cli_plugins' %
+        (sys.prefix, sys.version_info[0], sys.version_info[1]))
+    # second, always load plugins from ~/.koji/plugins
+    paths.append(os.path.expanduser('~/.koji/plugins'))
+    # finally, update plugin_paths to the list
+    if plugin_paths:
+        if not isinstance(plugin_paths, (list, tuple)):
+            plugin_paths = plugin_paths.split(':')
+        paths.extend([os.path.expanduser(p) for p in reversed(plugin_paths)])
+    tracker = koji.plugin.PluginTracker()
     for path in paths:
-        if os.path.exists(path):
+        if os.path.exists(path) and os.path.isdir(path):
             for name in sorted(os.listdir(path)):
-                if not name.endswith('.py'):
+                fullname = os.path.join(path, name)
+                if not (os.path.isfile(fullname) and name.endswith('.py')):
                     continue
                 name = name[:-3]
-                names.add(name)
+                logger.info('Loading plugin: %s', fullname)
-    for name in names:
+                register_plugin(tracker.load(name, path=path, reload=True))
-        logger.info('Loading plugin: %s', name)
-        tracker.load(name)
-        register_plugin(tracker.get(name))
 
 
 def get_options():
@@ -130,7 +146,8 @@ def get_options():
     parser.add_option("--pkgurl", help=SUPPRESS_HELP)
     parser.add_option("--plugin-paths", metavar='PATHS',
                       help=_("specify plugin paths with format as the same as the shell's PATH, "
-                             "'~/.koji/plugins', koji_cli_plugins module are always appended"))
+                             "koji_cli_plugins module and '~/.koji/plugins' are always loaded in advance, "
+                             "and then overridden by this option."))
     parser.add_option("--help-commands", action="store_true", default=False, help=_("list commands"))
     (options, args) = parser.parse_args()
 
@@ -170,21 +187,7 @@ def get_options():
             else:
                 warn("Warning: The pkgurl option is obsolete, please use topurl instead")
 
-    # update plugin_paths to list
+    load_plugins(options.plugin_paths)
-    plugin_paths = options.plugin_paths
-    if plugin_paths:
-        plugin_paths = [os.path.expanduser(p) for p in
-                        plugin_paths.split(':')]
-    else:
-        plugin_paths = []
-    # always load plugins from ~/.koji/plugins
-    plugin_paths.append(os.path.expanduser('~/.koji/plugins'))
-    # always load plugins from koji_cli_plugins module
-    plugin_paths.append(
-        '%s/lib/python%s.%s/site-packages/koji_cli_plugins' %
-        (sys.prefix, sys.version_info[0], sys.version_info[1]))
-    setattr(options, 'plugin_paths', plugin_paths)
-    load_plugins(options, plugin_paths)
 
     if not args:
         options.help_commands = True

cli/koji.conf

@@ -38,7 +38,8 @@
 ;serverca = ~/.koji/serverca.crt
 
 ;plugin paths, separated by ':' as the same as the shell's PATH
-;~/.koji/plugins and koji_cli_plugins module are always be appended
+;koji_cli_plugins module and ~/.koji/plugins are always loaded in advance,
+;and then be overridden by this option
 ;plugin_paths = ~/.koji/plugins
 
 ;[not_implemented_yet]

tests/test_cli/data/cli_plugins1/plugin1.py

@@ -1,20 +1,25 @@
 from __future__ import absolute_import
 from koji.plugin import export_cli, export_as
 
+
 @export_as('foobar')
 @export_cli
 def foo():
     pass
 
+
 @export_cli
 def foo2():
     pass
 
+
 def foo3():
     pass
 
+
 foo4 = 'foo4'
 
+
 class bar():
     pass
 

tests/test_cli/data/cli_plugins2/plugin2.py

@@ -1,5 +1,6 @@
 from koji.plugin import export_cli
 
+
 @export_cli
 def foo5():
     pass

tests/test_cli/data/cli_plugins2/plugin3.py

@@ -1,5 +1,6 @@
 from koji.plugin import export_cli, export_as
 
+
 @export_as('foo6')
 @export_cli
 def foo():

tests/test_cli/test_load_plugins.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 
 import os
+
 try:
     import unittest2 as unittest
 except ImportError:
@@ -9,20 +10,24 @@ except ImportError:
 import mock
 
 from . import loadcli
+
 cli = loadcli.cli
 
 
 class TestLoadPlugins(unittest.TestCase):
     @mock.patch('logging.getLogger')
-    def test_load_plugins(self, getLogger):
+    @mock.patch('os.path.isdir')
-        options = mock.MagicMock()
+    def test_load_plugins(self, isdir, getLogger):
-        cli.load_plugins(options, [os.path.dirname(__file__) + '/data/plugins',
+        # skip system path and default user plugin directory check
-                                   os.path.dirname(
+        isdir.side_effect = lambda path: False if path.startswith('/usr') \
-                                       __file__) + '/data/plugins2'])
+                            or path == os.path.expanduser("~/.koji/plugins") \
+                            else True
+        cli.load_plugins(os.path.dirname(__file__) + '/data/cli_plugins1:' +
+                         os.path.dirname(__file__) + '/data/cli_plugins2')
         self.assertTrue(callable(cli.foobar))
         self.assertTrue(callable(cli.foo2))
         self.assertTrue(hasattr(cli, 'foo6'))
         self.assertFalse(hasattr(cli, 'foo3'))
         self.assertFalse(hasattr(cli, 'foo4'))
-        self.assertFalse(hasattr(cli, 'foo5'))
+        self.assertTrue(hasattr(cli, 'foo5'))
         self.assertFalse(hasattr(cli, 'sth'))