Opened 12 years ago

Last modified 8 years ago

#2410 defect new

setup.py should generate a twisted.plugin dropin.cache file during install.

Reported by: jknight Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: Glyph, Thijs Triemstra Branch:
Author:

Description

Otherwise, when you install to somewhere only root can write, the cache will never be written.

Change History (10)

comment:1 Changed 12 years ago by Jean-Paul Calderone

autosetup does this:

http://divmod.org/trac/browser/trunk/Epsilon/epsilon/setuphelper.py

However, it's such a trivial operation, there's probably nothing interesting there that can't be duplicated with one or two microseconds of consideration.

comment:2 Changed 12 years ago by Glyph

IMHO, autosetup probably ought to be in Twisted for other reasons as well - but not necessarily all of it to resolve something this trivial.

comment:3 Changed 12 years ago by Glyph

On a related note, there really ought to be an officially mandated, idiomatic, blessed way to generate the cache file at install time. This is important so that programs which create their own Twisted plugins can easily write the system cache after they've installed them. Do you guys think this should be a separate ticket, or rolled into this one?

comment:4 Changed 11 years ago by therve

Owner: changed from Glyph to therve

comment:5 Changed 11 years ago by therve

For the record, I've looked at this issue. The patch done is epsilon doesn't seem to work when setuptools is used (because we can't get the path on the installation on the distribution object). I was handle to get it using some monkeypatching on easy_install, but it's bad.

On the other, the following patch solves the problem for me :

Index: twisted/plugin.py
===================================================================
--- twisted/plugin.py   (revision 23373)
+++ twisted/plugin.py   (working copy)
@@ -159,7 +159,7 @@
             pluginKey = pluginModule.name.split('.')[-1]
             existingKeys[pluginKey] = True
             if ((pluginKey not in dropinDotCache) or
-                (pluginModule.filePath.getModificationTime() >= lastCached)):
+                (pluginModule.filePath.getModificationTime() > lastCached)):
                 needsWrite = True
                 try:
                     provider = pluginModule.load()

But that may be dependant on the fact that the machine is fast enough build everything in the same second.

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

Yea, this fixes the problem (sometimes, as you say) by preventing the cache from being regenerated, but this will skip some instances where the cache does need to be regenerated.

It needs to be >= instead of just > because a cache may be written first and then a dropin changed during the same second. This leaves the cache stale but with a timestamp which is == to the timestamp of the dropin file so with > the cache isn't rewritten and remains stale.

comment:7 Changed 10 years ago by Glyph

Cc: Glyph added

comment:8 Changed 10 years ago by Glyph

Since it recently came up on the mailing list, I feel like I should summarize the problems here:

  • autosetup's approach is not general enough, because it attempts to re-generate the plugin cache unconditionally, even if setup() is being called to do something other than installation (i.e. we may be trying to generate the plugin cache of something that isn't actually installed).
  • setuptools doesn't support post-installation hooks. This may actually be OK, though. The plugin system's insistence on writing everything to the same directory is designed to make both 'import' and plugin lookup really fast, by avoiding the need for listdir() at every point and reducing the number of entries added to sys.path. Eggs already require an absurd explosion of entries on sys.path though, which destroys the performance of __import__, so we might want to just generate a plugin.cache inside eggs of Twisted plugin providers, as we build them. I'm not really sure about this though, I need to do some experimentation to see whether anything needs to be modified about the plugin system itself, and if so what.
  • the post-installation hook needs to be a real post-installation hook in bdist_msi, bdist_wininst, and bdist_rpm, because 'install' isn't invoked for those plugins. This is partially covered by this stackoverflow answer.
  • we have to be careful not to break the debian package when we add this, because historically QA on the debian package has been somewhere between non-existent and very bad, and we don't maintain the debian/ directory. Looking at the way that Debian invokes setup.py, it seems as though we'll need to carefully detect that we are not really installing and do different stuff. It would be nice to provide the post-installation hook in a way that was usable by debian packages in the future though, so that one day we can stop being in this situation.

comment:9 Changed 8 years ago by <automation>

Owner: therve deleted

comment:10 Changed 8 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

Also see #2409.

Note: See TracTickets for help on using tickets.