From f1b9d439d4155cb1bd790683e91e755f09e08f27 Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: May 30 2019 16:42:17 +0000
Subject: [PATCH 1/4] cli: also load plugins from ~/.koji/plugins


- add plugin_path in koji.conf
- add `--plugin-paths` in cli arguments

fixes: #887

---

diff --git a/cli/koji b/cli/koji
index 5e153a7..c38d08f 100755
--- a/cli/koji
+++ b/cli/koji
@@ -65,20 +65,24 @@ def register_plugin(plugin):
                 globals()[name] = v
 
 
-def load_plugins(options, path):
+def load_plugins(options, paths):
     """Load plugins specified by our configuration plus system plugins. Order
     is that system plugins are first, so they can be overridden by
     user-specified ones with same name."""
     logger = logging.getLogger('koji.plugins')
-    if os.path.exists(path):
-        tracker = koji.plugin.PluginTracker(path=path)
-        for name in sorted(os.listdir(path)):
-            if not name.endswith('.py'):
-                continue
-            name = name[:-3]
-            logger.info('Loading plugin: %s', name)
-            tracker.load(name)
-            register_plugin(tracker.get(name))
+    tracker = koji.plugin.PluginTracker(path=paths)
+    names = set()
+    for path in paths:
+        if os.path.exists(path):
+            for name in sorted(os.listdir(path)):
+                if not name.endswith('.py'):
+                    continue
+                name = name[:-3]
+                names.add(name)
+    for name in names:
+        logger.info('Loading plugin: %s', name)
+        tracker.load(name)
+        register_plugin(tracker.get(name))
 
 
 def get_options():
@@ -124,6 +128,9 @@ def get_options():
     parser.add_option("--weburl", help=_("url of the Koji web interface"))
     parser.add_option("--topurl", help=_("url for Koji file access"))
     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"))
     parser.add_option("--help-commands", action="store_true", default=False, help=_("list commands"))
     (options, args) = parser.parse_args()
 
@@ -163,9 +170,21 @@ def get_options():
             else:
                 warn("Warning: The pkgurl option is obsolete, please use topurl instead")
 
-    plugins_path = '%s/lib/python%s.%s/site-packages/koji_cli_plugins' % \
-                   (sys.prefix, sys.version_info[0], sys.version_info[1])
-    load_plugins(options, plugins_path)
+    # update plugin_paths to list
+    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 options.help_commands:
         list_commands()
diff --git a/cli/koji.conf b/cli/koji.conf
index addd4e3..70aa543 100644
--- a/cli/koji.conf
+++ b/cli/koji.conf
@@ -37,6 +37,11 @@
 ;certificate of the CA that issued the HTTP server certificate
 ;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
+;plugin_paths = ~/.koji/plugins
+
+;[not_implemented_yet]
 ;enabled plugins for CLI, runroot and save_failed_tree are available
 ;plugins =
 
diff --git a/koji/__init__.py b/koji/__init__.py
index 4388e8b..12aaf17 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -1699,7 +1699,8 @@ def read_config(profile_name, user_config=None):
         'authtype': None,
         'debug': False,
         'debug_xmlrpc': False,
-        'pyver' : None,
+        'pyver': None,
+        'plugin_paths': None,
     }
 
     result = config_defaults.copy()
diff --git a/tests/test_cli/data/plugins2/plugin2.py b/tests/test_cli/data/plugins2/plugin2.py
new file mode 100644
index 0000000..336aaf4
--- /dev/null
+++ b/tests/test_cli/data/plugins2/plugin2.py
@@ -0,0 +1,5 @@
+from koji.plugin import export_cli
+
+@export_cli
+def foo5():
+    pass
diff --git a/tests/test_cli/data/plugins2/plugin3.py b/tests/test_cli/data/plugins2/plugin3.py
new file mode 100644
index 0000000..304db20
--- /dev/null
+++ b/tests/test_cli/data/plugins2/plugin3.py
@@ -0,0 +1,7 @@
+from koji.plugin import export_cli, export_as
+
+@export_as('foo6')
+@export_cli
+def foo():
+    pass
+
diff --git a/tests/test_cli/test_load_plugins.py b/tests/test_cli/test_load_plugins.py
index 4747d63..f9368b9 100644
--- a/tests/test_cli/test_load_plugins.py
+++ b/tests/test_cli/test_load_plugins.py
@@ -1,23 +1,28 @@
 from __future__ import absolute_import
-import mock
+
 import os
 try:
     import unittest2 as unittest
 except ImportError:
     import unittest
 
+import mock
+
 from . import loadcli
 cli = loadcli.cli
 
 
 class TestLoadPlugins(unittest.TestCase):
-
     @mock.patch('logging.getLogger')
     def test_load_plugins(self, getLogger):
         options = mock.MagicMock()
-        cli.load_plugins(options, os.path.dirname(__file__) + '/data/plugins')
+        cli.load_plugins(options, [os.path.dirname(__file__) + '/data/plugins',
+                                   os.path.dirname(
+                                       __file__) + '/data/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.assertFalse(hasattr(cli, 'sth'))

From 671499d970107cdd7730e34f778170d7d0676dc5 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Jun 03 2019 05:09:20 +0000
Subject: [PATCH 2/4] adjust cli plugin config description


---

diff --git a/cli/koji b/cli/koji
index c38d08f..21a4198 100755
--- a/cli/koji
+++ b/cli/koji
@@ -65,24 +65,40 @@ def register_plugin(plugin):
                 globals()[name] = v
 
 
-def load_plugins(options, paths):
-    """Load plugins specified by our configuration plus system plugins. Order
-    is that system plugins are first, so they can be overridden by
-    user-specified ones with same name."""
+def load_plugins(plugin_paths):
+    """Load plugins specified by input paths, ~/.koji/plugins, system plugins.
+    Loading order is descending, so they can be overridden by user-specified
+    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)
-    names = set()
+    paths = []
+    # 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)
-    for name in names:
-        logger.info('Loading plugin: %s', name)
-        tracker.load(name)
-        register_plugin(tracker.get(name))
+                logger.info('Loading plugin: %s', fullname)
+                register_plugin(tracker.load(name, path=path, reload=True))
 
 
 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
-    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)
+    load_plugins(options.plugin_paths)
 
     if options.help_commands:
         list_commands()
diff --git a/cli/koji.conf b/cli/koji.conf
index 70aa543..0a27810 100644
--- a/cli/koji.conf
+++ b/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]
diff --git a/tests/test_cli/data/cli_plugins1/not_plugin.omg b/tests/test_cli/data/cli_plugins1/not_plugin.omg
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/test_cli/data/cli_plugins1/not_plugin.omg
diff --git a/tests/test_cli/data/cli_plugins1/plugin1.py b/tests/test_cli/data/cli_plugins1/plugin1.py
new file mode 100644
index 0000000..12622a8
--- /dev/null
+++ b/tests/test_cli/data/cli_plugins1/plugin1.py
@@ -0,0 +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
+
diff --git a/tests/test_cli/data/cli_plugins1/plugin2.py b/tests/test_cli/data/cli_plugins1/plugin2.py
new file mode 100644
index 0000000..58648d6
--- /dev/null
+++ b/tests/test_cli/data/cli_plugins1/plugin2.py
@@ -0,0 +1 @@
+sth = 123
\ No newline at end of file
diff --git a/tests/test_cli/data/cli_plugins2/plugin2.py b/tests/test_cli/data/cli_plugins2/plugin2.py
new file mode 100644
index 0000000..38a2e41
--- /dev/null
+++ b/tests/test_cli/data/cli_plugins2/plugin2.py
@@ -0,0 +1,6 @@
+from koji.plugin import export_cli
+
+
+@export_cli
+def foo5():
+    pass
diff --git a/tests/test_cli/data/cli_plugins2/plugin3.py b/tests/test_cli/data/cli_plugins2/plugin3.py
new file mode 100644
index 0000000..253be59
--- /dev/null
+++ b/tests/test_cli/data/cli_plugins2/plugin3.py
@@ -0,0 +1,8 @@
+from koji.plugin import export_cli, export_as
+
+
+@export_as('foo6')
+@export_cli
+def foo():
+    pass
+
diff --git a/tests/test_cli/data/plugins/not_plugin.omg b/tests/test_cli/data/plugins/not_plugin.omg
deleted file mode 100644
index e69de29..0000000
--- a/tests/test_cli/data/plugins/not_plugin.omg
+++ /dev/null
diff --git a/tests/test_cli/data/plugins/plugin1.py b/tests/test_cli/data/plugins/plugin1.py
deleted file mode 100644
index 41d876e..0000000
--- a/tests/test_cli/data/plugins/plugin1.py
+++ /dev/null
@@ -1,20 +0,0 @@
-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
-
diff --git a/tests/test_cli/data/plugins/plugin2.py b/tests/test_cli/data/plugins/plugin2.py
deleted file mode 100644
index 58648d6..0000000
--- a/tests/test_cli/data/plugins/plugin2.py
+++ /dev/null
@@ -1 +0,0 @@
-sth = 123
\ No newline at end of file
diff --git a/tests/test_cli/data/plugins2/plugin2.py b/tests/test_cli/data/plugins2/plugin2.py
deleted file mode 100644
index 336aaf4..0000000
--- a/tests/test_cli/data/plugins2/plugin2.py
+++ /dev/null
@@ -1,5 +0,0 @@
-from koji.plugin import export_cli
-
-@export_cli
-def foo5():
-    pass
diff --git a/tests/test_cli/data/plugins2/plugin3.py b/tests/test_cli/data/plugins2/plugin3.py
deleted file mode 100644
index 304db20..0000000
--- a/tests/test_cli/data/plugins2/plugin3.py
+++ /dev/null
@@ -1,7 +0,0 @@
-from koji.plugin import export_cli, export_as
-
-@export_as('foo6')
-@export_cli
-def foo():
-    pass
-
diff --git a/tests/test_cli/test_load_plugins.py b/tests/test_cli/test_load_plugins.py
index f9368b9..7e7e7b1 100644
--- a/tests/test_cli/test_load_plugins.py
+++ b/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):
-        options = mock.MagicMock()
-        cli.load_plugins(options, [os.path.dirname(__file__) + '/data/plugins',
-                                   os.path.dirname(
-                                       __file__) + '/data/plugins2'])
+    @mock.patch('os.path.isdir')
+    def test_load_plugins(self, isdir, getLogger):
+        # skip system path and default user plugin directory check
+        isdir.side_effect = lambda path: False if path.startswith('/usr') \
+                            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'))

From 43942cc860110bc34a47b61e7cc010f1ac1bde02 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jul 09 2019 17:37:50 +0000
Subject: [PATCH 3/4] fix whitespace


---

diff --git a/tests/test_cli/data/cli_plugins1/plugin1.py b/tests/test_cli/data/cli_plugins1/plugin1.py
index 12622a8..4d562ca 100644
--- a/tests/test_cli/data/cli_plugins1/plugin1.py
+++ b/tests/test_cli/data/cli_plugins1/plugin1.py
@@ -22,4 +22,3 @@ foo4 = 'foo4'
 
 class bar():
     pass
-
diff --git a/tests/test_cli/data/cli_plugins2/plugin3.py b/tests/test_cli/data/cli_plugins2/plugin3.py
index 253be59..6dec166 100644
--- a/tests/test_cli/data/cli_plugins2/plugin3.py
+++ b/tests/test_cli/data/cli_plugins2/plugin3.py
@@ -5,4 +5,3 @@ from koji.plugin import export_cli, export_as
 @export_cli
 def foo():
     pass
-

From 600ed9af9c9f6ba2e58538ff12675a8c940cca4b Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jul 10 2019 12:05:44 +0000
Subject: [PATCH 4/4] shorten help text


---

diff --git a/cli/koji b/cli/koji
index 21a4198..e6bacd6 100755
--- a/cli/koji
+++ b/cli/koji
@@ -145,9 +145,7 @@ def get_options():
     parser.add_option("--topurl", help=_("url for Koji file access"))
     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_cli_plugins module and '~/.koji/plugins' are always loaded in advance, "
-                             "and then overridden by this option."))
+            help=_("specify additional plugin paths (colon separated)"))
     parser.add_option("--help-commands", action="store_true", default=False, help=_("list commands"))
     (options, args) = parser.parse_args()
 

