Opened 11 years ago

Closed 10 years ago

#2339 defect closed fixed (fixed)

twisted/plugins/__init__.py incorrectly considers other installations of Twisted

Reported by: radix Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: therve, itamarst Branch:
Author:

Description

If I have a system install of e.g. Twisted at /usr/lib/python2.4/site-packages/twisted, and a local install at ~/lib/python2.4/site-packages, and I make sure the latter is in my sys.path in such a way that "import twisted" picks up the copy in my home directory, plugin lookups will still honor plugins in the former location, to much sadness. It should probably not consider those plugins, since Python can't even import from that location.

Change History (15)

comment:1 Changed 11 years ago by Glyph

Status: newassigned

comment:2 Changed 11 years ago by Glyph

This is one of the bugs fixed in the #1951 branch.

comment:3 Changed 11 years ago by radix

Owner: changed from Glyph to radix
Status: assignednew
Summary: twisted.plugin considers plugins from packages which Python cannot import (when there are two packages with the same name)twisted/plugins/__init__.py incorrectly considers other installations of Twisted

In fact it is not fixed in #1951.

After further investigation, it has become clear that the problem does not lie in twisted.plugin, but rather twisted.plugin*s*, in its munging of path in init.py. It is adding entries to path from other installations of Twisted, when it should only be adding plugins directories of third-party applications: one (the only?) way to discern whether a plugins directory is a Twisted installation or a third-party application is to check to see if it has an init.py.

  • twisted/plugins/__init__.py

     
    1212"""
    1313
    1414import os, sys
    15 __path__ = [os.path.abspath(os.path.join(x, 'twisted', 'plugins')) for x in sys.path]
     15__path__.extend([os.path.abspath(os.path.join(x, 'twisted', 'plugins')) for x in sys.path if not os.path.exists(os.path.join(x, 'twisted', 'plugins', '__init__.py'))])
    1616
    1717__all__ = []                    # nothing to see here, move along, move along

comment:4 Changed 10 years ago by Jean-Paul Calderone

Priority: normalhigh

comment:5 Changed 10 years ago by Glyph

I think that line has now gotten long enough to warrant a separate, tested function in twisted.python which multiple projects can use.

comment:6 Changed 10 years ago by Jean-Paul Calderone

Owner: changed from radix to Jean-Paul Calderone
Status: newassigned

comment:7 Changed 10 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted
Priority: highhighest
Status: assignednew

/branches/plugin-collision-2339

comment:8 Changed 10 years ago by therve

Cc: therve added
Keywords: review removed
Owner: set to Jean-Paul Calderone

That looks good. pluginPackagePaths would need to have direct tests, to be sure of what is done in there.

AdjacentPackageTests.createDummyPackage is missing a docstring.

comment:9 Changed 10 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

Fixed those things, both. Same branch.

comment:10 Changed 10 years ago by therve

Keywords: review removed
Owner: set to Jean-Paul Calderone

test_plugin.py is missing the Twisted copyright. Once done please merge.

comment:11 Changed 10 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [20920]) Merge plugin-collision-2339

Author: exarkun Reviewer: therve Fixes #2339

Add a function, twisted.plugin.pluginPackagePaths, which returns a list of additional directories to search for plugin modules for a particular plugin package. Change twisted.plugins.init to use this function instead of looping over sys.path directly.

Unlike the loop previously in twisted.plugins.init, pluginPackagePaths will not include any directory which is a Python package. This prevents plugins from being discovered from packages which Python may not actually be able to import and limits plugin discovery to those installed on the version of the package which is in use by an application. It also allows a site installation to co-exist with another installation (for example, a personal user installation) without interfering with each other.

comment:12 Changed 10 years ago by itamarst

Resolution: fixed
Status: closedreopened

Shouldn't the plugins howto also have been updated?

comment:13 Changed 10 years ago by Jean-Paul Calderone

Cc: itamarst added

I don't see anything about a plugin howto in the ticket description.

Want to file another ticket?

comment:14 Changed 10 years ago by itamarst

Resolution: fixed
Status: reopenedclosed

I would argue that, as a rule, howtos should be updated on API changes, in that same ticket. However, in this case I opened a new ticket.

comment:15 Changed 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.