Opened 8 years ago

Closed 7 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: Launchpad Bug:

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 8 years ago by glyph

  • Status changed from new to assigned

comment:2 Changed 8 years ago by glyph

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

comment:3 Changed 8 years ago by radix

  • Owner changed from glyph to radix
  • Status changed from assigned to new
  • Summary changed from twisted.plugin considers plugins from packages which Python cannot import (when there are two packages with the same name) to 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 7 years ago by exarkun

  • Priority changed from normal to high

comment:5 Changed 7 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 7 years ago by exarkun

  • Owner changed from radix to exarkun
  • Status changed from new to assigned

comment:7 Changed 7 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted
  • Priority changed from high to highest
  • Status changed from assigned to new

/branches/plugin-collision-2339

comment:8 Changed 7 years ago by therve

  • Cc therve added
  • Keywords review removed
  • Owner set to exarkun

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 7 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

Fixed those things, both. Same branch.

comment:10 Changed 7 years ago by therve

  • Keywords review removed
  • Owner set to exarkun

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

comment:11 Changed 7 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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 7 years ago by itamarst

  • Resolution fixed deleted
  • Status changed from closed to reopened

Shouldn't the plugins howto also have been updated?

comment:13 Changed 7 years ago by exarkun

  • Cc itamarst added

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

Want to file another ticket?

comment:14 Changed 7 years ago by itamarst

  • Resolution set to fixed
  • Status changed from reopened to closed

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 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.