Edgewall Software

Opened 5 years ago

Closed 2 years ago

#12158 closed defect (fixed)

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

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2.3
Component: general Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Components that failed to load due to missing required dependencies would still show on the plugin admin page.

API Changes:
Internal Changes:

Description (last modified by 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.

Attachments (0)

Change History (7)

comment:1 by Jun Omae, 5 years ago

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

    diff --git a/trac/loader.py b/trac/loader.py
    index df65356ba..d3f6bf029 100644
    a b def load_eggs(entry_point_name):  
    6868                entry.load(require=True)
    6969            except Exception, e:
    7070                _log_error(entry, e)
     71                from trac.core import ComponentMeta
     72                for name in entry.attrs:
     73                    for cls in ComponentMeta._components:
     74                        if cls.__name__ == name and \
     75                                cls.__module__ == entry.module_name:
     76                            ComponentMeta._components.remove(cls)
     77                            break
     78                    for classes in ComponentMeta._registry.itervalues():
     79                        for cls in classes:
     80                            if cls.__name__ == name and \
     81                                    cls.__module__ == entry.module_name:
     82                                classes.remove(cls)
     83                                break
    7184            else:
    7285                if os.path.dirname(entry.dist.location) == auto_enable:
    7386                    _enable_plugin(env, entry.module_name)

comment:2 by Ryan J Ollos, 5 years ago

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.

comment:3 by Ryan J Ollos, 2 years ago

Milestone: 1.2.3
Owner: set to Ryan J Ollos
Status: newassigned

comment:4 by Ryan J Ollos, 2 years ago

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

  • trac/core.py

    diff --git a/trac/core.py b/trac/core.py
    index 9731a030e..941d15587 100644
    a b class ComponentMeta(type):  
    168168            for interface in class_.__dict__.get('_implements', ()):
    169169                implementers = cls._registry.get(interface)
    170170                try:
    171                     implementers.remove(class_)
     171                    implementers.remove(component)
    172172                except ValueError:
    173173                    pass

comment:5 by Ryan J Ollos, 2 years ago

Description: modified (diff)

comment:7 by Ryan J Ollos, 2 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.