From f1b9d439d4155cb1bd790683e91e755f09e08f27 Mon Sep 17 00:00:00 2001 From: Yuming Zhu 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 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 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 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()