From 0af5ba7e6a08c282eb3bc2bca38b1d4056304efd Mon Sep 17 00:00:00 2001 From: dgw Date: Sat, 7 Apr 2018 10:47:46 -0500 Subject: [PATCH 1/3] reload: Recurse into packaged modules Force-reload all sub-modules of packages recursively. See #1056. --- sopel/modules/reload.py | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/sopel/modules/reload.py b/sopel/modules/reload.py index 1fdcb0451e..1a17ec1a8a 100644 --- a/sopel/modules/reload.py +++ b/sopel/modules/reload.py @@ -16,6 +16,14 @@ import sopel.module import subprocess +try: + from importlib import reload +except ImportError: + try: + from imp import reload + except ImportError: + pass # fallback to builtin if neither module is available + @sopel.module.nickname_commands("reload") @sopel.module.priority("low") @@ -37,14 +45,38 @@ def f_reload(bot, trigger): bot.setup() return bot.reply('done') - if name not in sys.modules: + if (name not in sys.modules and name not in sopel.loader.enumerate_modules(bot.config)): return bot.reply('"%s" not loaded, try the `load` command' % name) + reload_module_tree(bot, name) + + +def reload_module_tree(bot, name, seen=None): + from types import ModuleType + old_module = sys.modules[name] + if seen is None: + seen = {} + if name not in seen: + seen[name] = [] + old_callables = {} for obj_name, obj in iteritems(vars(old_module)): - bot.unregister(obj) + if callable(obj): + bot.unregister(obj) + elif (type(obj) is ModuleType and + obj.__name__.startswith(name + '.') and + obj.__name__ not in sys.builtin_module_names): + # recurse into submodules, see issue 1056 + if obj not in seen[name]: + seen[name].append(obj) + reload(obj) + reload_module_tree(bot, obj.__name__, seen) + + modules = sopel.loader.enumerate_modules(bot.config) + if name not in modules: + return # Only reload the top-level module, once recursion is finished # Also remove all references to sopel callables from top level of the # module, so that they will not get loaded again if reloading the @@ -53,12 +85,10 @@ def f_reload(bot, trigger): delattr(old_module, obj_name) # Also delete the setup function + # Sub-modules shouldn't have setup functions, so do after the recursion check if hasattr(old_module, "setup"): delattr(old_module, "setup") - modules = sopel.loader.enumerate_modules(bot.config) - if name not in modules: - return bot.reply('"%s" not loaded, try the `load` command' % name) path, type_ = modules[name] load_module(bot, name, path, type_) From dca9bd888a7a2b17dd7c08449b9d8f0d745d18f0 Mon Sep 17 00:00:00 2001 From: Humorous Baby <44451911+HumorBaby@users.noreply.github.com> Date: Fri, 26 Oct 2018 23:03:06 -0400 Subject: [PATCH 2/3] reload: fix issue with reloading modules within subpackage sopel-irl#1314 used recursion to reload nested modules. However, in Python 2 (maybe python 3 as well), this approach leads to odd behavior with callables being unloaded/reloaded twice. The loop detects my_module.my_callable and my_callable as two separate modules to reload. The first time it detects it, the rule is added to bot._callables as a string. I combined patched dgw branch 1056-no-dupes onto 1056-fix-with-recursion, to provide a more optimal solution to issue sopel-irc#1056. --- sopel/loader.py | 10 ++++++---- sopel/modules/reload.py | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/sopel/loader.py b/sopel/loader.py index 6dceb0bfe8..320ea23c3d 100644 --- a/sopel/loader.py +++ b/sopel/loader.py @@ -163,10 +163,12 @@ def clean_callable(func, config): func.rule = getattr(func, 'rule', []) for command in getattr(func, 'commands', []): regexp = get_command_regexp(prefix, command) - func.rule.append(regexp) - for command in getattr(func, 'nickname_commands', []): - regexp = get_nickname_command_regexp(nick, command, alias_nicks) - func.rule.append(regexp) + if regexp not in func.rule: + # list.append() ALWAYS adds the value, even if it already exists + # Checking first prevents duplicating triggers on module reloads + # See issue 1056 + # TODO: Maybe func.rule should be a set() instead? + func.rule.append(regexp) if hasattr(func, 'example'): example = func.example[0]["example"] example = example.replace('$nickname', nick) diff --git a/sopel/modules/reload.py b/sopel/modules/reload.py index 1a17ec1a8a..2d264bb932 100644 --- a/sopel/modules/reload.py +++ b/sopel/modules/reload.py @@ -6,7 +6,8 @@ https://sopel.chat """ -from __future__ import unicode_literals, absolute_import, print_function, division +from __future__ import unicode_literals, absolute_import, print_function, \ + division import collections import sys @@ -45,7 +46,8 @@ def f_reload(bot, trigger): bot.setup() return bot.reply('done') - if (name not in sys.modules and name not in sopel.loader.enumerate_modules(bot.config)): + if name not in sys.modules and name not in sopel.loader.enumerate_modules( + bot.config): return bot.reply('"%s" not loaded, try the `load` command' % name) reload_module_tree(bot, name) @@ -63,7 +65,16 @@ def reload_module_tree(bot, name, seen=None): old_callables = {} for obj_name, obj in iteritems(vars(old_module)): - if callable(obj): + # If a callable was imported like 'from .module import *', the recursion + # below will catch the callable twice. The first time around, when the + # arg:name has the format module.submodule, the callable `obj` for some + # weird reason only has strings of the regexp rules, and causes issues + # for any trigger (basically immediately). The second time around, + # arg:name is just the root module name, and now the rule has been + # compiled. Haven't gotten a chance to trace through the code to see + # why this happens, but I know that my change below fixes the problem + # for now. + if callable(obj) and '.' not in name: bot.unregister(obj) elif (type(obj) is ModuleType and obj.__name__.startswith(name + '.') and From b2d805dc6620dc524bfa2c7581eca54969de4250 Mon Sep 17 00:00:00 2001 From: Humorous Baby <44451911+HumorBaby@users.noreply.github.com> Date: Fri, 26 Oct 2018 23:46:45 -0400 Subject: [PATCH 3/3] Fix nickname_commands loading that got lost along the way. The for loop that adds nickname_commands to the func.rules got lost along the way to prevent duplicate regexp rules for a given func. --- sopel/loader.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sopel/loader.py b/sopel/loader.py index 320ea23c3d..906e78b59e 100644 --- a/sopel/loader.py +++ b/sopel/loader.py @@ -169,6 +169,11 @@ def clean_callable(func, config): # See issue 1056 # TODO: Maybe func.rule should be a set() instead? func.rule.append(regexp) + for command in getattr(func, 'nickname_commands', []): + regexp = get_nickname_command_regexp(nick, command, alias_nicks) + if regexp not in func.rule: + # See for-loop immediately above this. + func.rule.append(regexp) if hasattr(func, 'example'): example = func.example[0]["example"] example = example.replace('$nickname', nick)