Ticket #2339 defect closed fixed

Opened 7 years ago

Last modified 7 years ago

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

1

Changed 7 years ago by glyph

  • status changed from new to assigned

2

Changed 7 years ago by glyph

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

3

Changed 7 years ago by radix

  • status changed from assigned to new
  • owner changed from glyph to radix
  • 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 

4

Changed 7 years ago by exarkun

  • priority changed from normal to high

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.

6

Changed 7 years ago by exarkun

  • status changed from new to assigned
  • owner changed from radix to exarkun

7

Changed 7 years ago by exarkun

  • keywords review added
  • status changed from assigned to new
  • owner exarkun deleted
  • priority changed from high to highest

/branches/plugin-collision-2339

8

Changed 7 years ago by therve

  • keywords review removed
  • cc therve added
  • 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.

9

Changed 7 years ago by exarkun

  • owner exarkun deleted
  • keywords review added

Fixed those things, both. Same branch.

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.

11

Changed 7 years ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(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.

12

Changed 7 years ago by itamarst

  • status changed from closed to reopened
  • resolution fixed deleted

Shouldn't the plugins howto also have been updated?

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?

14

Changed 7 years ago by itamarst

  • status changed from reopened to closed
  • resolution set to fixed

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.

15

Changed 3 years ago by <automation>

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