Skipped classes can be loaded, activated and displayed on plugin admin page

[Ryan J Ollos]

Issue was found when making changes to ​trac-github plugin. The entry_points configuration for the plugin was changed to:

    extras_require={'oauth': ['requests_oauthlib >= 0.5']},
    entry_points={'trac.plugins': [
        'github.browser = tracext.github:GitHubBrowser',
        'github.loginmodule = tracext.github:GitHubLoginModule[oauth]',
        'github.postcommithook = tracext.github:GitHubPostCommitHook',
    ]},

The GitHubLoginModule still shows on the Plugins admin page when the dependency is not satisfied. The component can be activated when requests-oauthlib is not installed even though it was claimed to be skipped during environment startup.

02:22:27 AM Trac[loader] DEBUG: Skipping "github.auth = tracext.github:GitHubLoginModule [oauth]": ("DistributionNotFound: requests-oauthlib>=0.5" not found)
02:22:27 AM Trac[loader] DEBUG: Loading github.browser from /home/user/Workspace/trac-github-dev/pve/lib/python2.7/site-packages/trac_github-2.1.3-py2.7.egg
02:22:27 AM Trac[loader] DEBUG: Loading github.postcommithook from /home/user/Workspace/trac-github-dev/pve/lib/python2.7/site-packages/trac_github-2.1.3-py2.7.egg

The issue seems to be that since the dependency is a ​method-scoped import, the GitHubLoginModule class is added to the Component registry when other classes in the module are loaded: tags/trac-1.0.8/trac/loader.py@:68#L36​. Primarily it would be nice to not show the Component on the plugin admin page when it is claimed to be skipped.

I don't know if there's a reasonable solution to this, but I just thought I would raise the issue to see if there are any ideas.

[Jun Omae]

Proof-of-concept code: when dependencies are not solved, the GitHubLoginModule would be removed from _components and _registry in ComponentMeta class. Or, it would be good to mark as disabled for such classes to prevent enable components from admin panel.

trac/loader.py

@@ -68 +68 @@
                 entry.load(require=True)
             except Exception, e:
                 _log_error(entry, e)
+                from trac.core import ComponentMeta
+                for name in entry.attrs:
+                    for cls in ComponentMeta._components:
+                        if cls.__name__ == name and \
+                                cls.__module__ == entry.module_name:
+                            ComponentMeta._components.remove(cls)
+                            break
+                    for classes in ComponentMeta._registry.itervalues():
+                        for cls in classes:
+                            if cls.__name__ == name and \
+                                    cls.__module__ == entry.module_name:
+                                classes.remove(cls)
+                                break
             else:
                 if os.path.dirname(entry.dist.location) == auto_enable:
                     _enable_plugin(env, entry.module_name)

[Ryan J Ollos]

The patch works well. It seems I was wrong about the mechanism then. Since your patch is effective, the class must be created and added to the registry even when an exception occurs during entry.load.

[Ryan J Ollos]

r15777 added ComponentMeta.deregister, which could be useful here. It appears there is an error in that implementation:

trac/core.py

@@ -168 +168 @@
             for interface in class_.__dict__.get('_implements', ()):
                 implementers = cls._registry.get(interface)
                 try:
-                    implementers.remove(class_)
+                    implementers.remove(component)
                 except ValueError:
                     pass

[Ryan J Ollos]

Proposed changes: log:rjollos.git:t12158_remove_components_from_registry.

[Ryan J Ollos]

Committed to 1.2-stable in r16493, merged to trunk in r16494.