Plugin auto-enable is too eager for plugins with sub-packages

[Remy Blank]

Plugins located in $ENV/plugins are auto-enabled in _enable_plugin()​ if there is no entry for each module in [components]. However, the check is too precise, and prevents e.g. disabling a whole plugin containing sub-packages with a single line in [components].

For example, adding the following line to [components] doesn't disable ​AccountManagerPlugin if it is installed in $ENV/plugins:

acct_mgr.* = disabled

Instead, all the various sub-packages have to be listed explicitly:

acct_mgr.admin.* = disabled
acct_mgr.api.* = disabled
acct_mgr.db.* = disabled
acct_mgr.htfile.* = disabled
acct_mgr.http.* = disabled
acct_mgr.notification.* = disabled
acct_mgr.pwhash.* = disabled
acct_mgr.svnserve.* = disabled
acct_mgr.web_ui.* = disabled

I suggest extending the check to include all parent packages:

trac/loader.py

@@ -25 +25 @@
 
 from trac import __version__ as TRAC_VERSION
 from trac.util import get_doc, get_module_path, get_pkginfo
+from trac.util.compat import any
 from trac.util.text import exception_to_unicode, to_unicode
 
 __all__ = ['load_components']
@@ -45 +46 @@
 def _enable_plugin(env, module):
     """Enable the given plugin module by adding an entry to the configuration.
     """
-    if module + '.*' not in env.config['components']:
+    parts = module.split('.')
+    if not any('.'.join(parts[:i + 1]) + '.*' in env.config['components']
+               for i in range(len(parts))):
         env.config['components'].set(module + '.*', 'enabled')
 
 def load_eggs(entry_point_name):

Thoughts?

[Christian Boos]

Use env.is_component_enabled(module)?

[Remy Blank]

Replying to cboos:

Use env.is_component_enabled(module)?

You mean, instead of updating the config? I'll have to try that. I also thought that modifying the config was a bit of a hack.

[Christian Boos]

Ah, no I meant doing if not env.is_component_enabled() instead of the if not any(...) - but that won't work alone as the env._rules will then get out of sync of the change done with config.set. But we could add an env.enable_component(module) method that will add that entry to env._rules.

[Christian Boos]

Replying to cboos:

… add an env.enable_component(module) method that will add that entry to env._rules.

… and use that instead of config.set, i.e.

def _enable_plugin(env, module): 
    """Enable the given plugin module by adding an entry to the configuration. 
    """ 
    if not env.is_component_enabled(module):
        env.enable_component(module)

[Remy Blank]

Patch using the component rule cache.

[Remy Blank]

Using the cache is a good idea, but _enable_plugin() should only enable a plugin if it wasn't disabled explicitly. env.is_component_enabled() currently doesn't provide this information. In the patch above, I made it return None if a component was neither enabled, nor disabled through a rule in [components] (and therefore is disabled implicitly).

This seems to work fine, but I'm not completely sure if this has other implications.

[Remy Blank]

Patch applied (with some docstring updates) in [9035].