Ticket #2830 (closed defect: fixed)

Opened 2 years ago

Last modified 1 year ago

setup.py incorrectly declares twisted.plugins to be a package

Reported by: zooko Assigned to: exarkun
Priority: normal Milestone:
Component: Nevow Severity: normal
Keywords: Cc: zooko, zooko@zooko.com
Estimated Completion (YYYY/MM/DD): Branch: branches/nevow-package-warning-2830-3
Author: exarkun, zooko

Description

It looks like in this comment I suggested hacking setup.py to call "twisted.plugins" a package:

http://divmod.org/trac/ticket/2527#comment:24

and in this comment I realized there was a cleaner way to get twisted/plugins/ installed without calling it a package:

http://divmod.org/trac/ticket/2527#comment:26

and in this comment I updated my patch to be against the current trunk and accidentally included that line from the earlier attempt:

http://divmod.org/trac/ticket/2527#comment:27

Or, hm, perhaps exarkun somehow applied the wrong patch? It looks like the attachment that I attached at that time is correct:

http://divmod.org/trac/attachment/ticket/2527/patch.txt

But that the svn revision linked from my comment is incorrect:

http://divmod.org/trac/changeset/16896

(Because it says setupdict['packages'].append('twisted.plugins').)

What I don't understand is how my comment #27: http://divmod.org/trac/ticket/2527#comment:27 has a hyperlink to [16896] in it, which was committed by exarkun, not by me. Did exarkun edit my comment after committing [16896] to make my comment link to [16896]?

Anyway, I noticed this because declaring twisted.plugins to be a package causes irritating warnings to be emitted by setuptools/pkg_resources, which is currently causing a test to fail on the tahoe-lafs buildbot. Also it might cause other problems.

Attachments

x.patch.txt (1.3 kB) - added by zooko on 01/24/09 10:59:27.
patch from comment:4
x.patch.2.txt (1.3 kB) - added by zooko on 01/24/09 10:59:30.
patch from comment:4

Change History

01/23/09 23:24:13 changed by zooko

added a link to this ticket to http://bugs.python.org/setuptools/issue36 (warning about "module was already imported from")

01/24/09 00:07:38 changed by exarkun

Well, you've managed to thoroughly confuse me, too.

What's the thrust of this ticket? That the append('twisted.plugins') should be deleted and everything will continue to work awesomely? Work even better, in fact?

01/24/09 10:38:16 changed by zooko

Yes. The main thing is that when I deleted append('twisted.plugins') from my local version of Nevow and reinstalled, then everything that I do continued to work, plus the warning message stopped happening.

Then there's the other aspect to this ticket, which is that I'm dying to know what the heck happened here...

Oh, I see what happened. What happened was that I put that append('twisted.plugins') *back* again in this patch: [16898]. The comment says:

""" List the "twisted.plugins" package (?) as an explicit package to be included It is detected neither by setuptools's "include all packages you can find" feature nor by its "include all files under revision control" feature. Why, I don't know. """

Hm, I see that this is (still) true. By removing that line append('twisted.plugins'), then the resulting installation of nevow doesn't have a twisted/plugins directory. Hm.

01/24/09 10:58:32 changed by zooko

Okay, here is a patch which convinces setuptools (and presumably distutils) to include a "twisted/plugins" directory in the installation but 'not' to think that "twisted" is a package. It uses the distutils data_files feature for this, and that feature requires that there is an actual file involved -- it won't install an empty directory -- so I threw in nevow_widget.py.

Index: setup.py
===================================================================
--- setup.py    (revision 17166)
+++ setup.py    (working copy)
@@ -8,15 +8,17 @@
     setuptools = None
 
 import os
-docs=[]
+data_files=[]
 for (dirpath, dirnames, filenames) in os.walk("doc"):
     if ".svn" in dirnames:
         del dirnames[dirnames.index(".svn")]
     thesedocs = []
     for fname in filenames:
         thesedocs.append(os.path.join(dirpath, fname))
-    docs.append((dirpath, thesedocs))
+    data_files.append((dirpath, thesedocs))
 
+data_files.append((os.path.join('twisted', 'plugins'), [os.path.join('twisted', 'plugins', 'nevow_widget.py')]))
+
 setupdict = {
     'name': 'Nevow', 
     'version': version,
@@ -32,7 +34,7 @@
         "Development Status :: 4 - Beta",
         "Topic :: Internet :: WWW/HTTP :: Dynamic Content"],
     'scripts': ['bin/nevow-xmlgettext', 'bin/nit'],
-    'data_files': docs,
+    'data_files': data_files,
     'package_data': {
             'formless': [
                 'freeform-default.css'
@@ -79,7 +81,6 @@
     from setuptools import setup, find_packages
 
     setupdict['packages'] = find_packages()
-    setupdict['packages'].append("twisted.plugins")
     setupdict['include_package_data'] = True
 else:
     # No setuptools -- decide where the data files should go and explicitly list

01/24/09 10:59:27 changed by zooko

  • attachment x.patch.txt added.

patch from comment:4

01/24/09 10:59:30 changed by zooko

  • attachment x.patch.2.txt added.

patch from comment:4

01/26/09 18:35:58 changed by zooko

I added a test to allmydata-tahoe for the way that this issue effects tahoe:

http://allmydata.org/trac/tahoe/changeset/20090126233046-92b7f-191bd2856dea8f477eea529e8b986890fec11106

You can see the results on the allmydata-tahoe buildbot:

http://allmydata.org/buildbot/waterfall?show_events=false

04/12/09 12:07:37 changed by zooko

This issue makes it so that setuptools prints out a warning whenever Nevow is imported. The warning is:

/usr/lib/python2.6/site-packages/zope/__init__.py:3: UserWarning: Module twisted was already imported from /usr/lib/python2.6/site-packages/twisted/__init__.pyc, but /usr/lib/python2.6/site-packages/Nevow-0.9.32-py2.6.egg is being added to sys.path

This is because Nevow in v0.9.33 and in current trunk declares that twisted.plugins is a package that is being installed. That's not right -- it isn't a package. It's just a directory.

The patch in this comment changes Nevow's setup.py to declare that twisted/plugins is a directory (or more precisely that twisted/plugins/nevow_widget.py is a file) that should be installed without being a package.

This issue is being tested by the Tahoe buildbot -- the issue causes unsightly warnings to appear whenever Tahoe command-line commands are executed, and the Tahoe unit tests assert that Tahoe commands are not allowed to be unnecessarily noisy. So we have had to mark those tests as "TODO" until this issue is fixed in Nevow:

    test_client_no_noise.todo = "We submitted a patch to Nevow to silence this warning: http://divmod.org/trac/ticket/2830"

If you want to have a test in Nevow's test suite itself, then simply install Nevow using setuptools, and then import Nevow, and assert that no warning messages are printed during the import.

04/12/09 12:10:28 changed by zooko

  • cc set to zooko, zooko@zooko.com.

Adding Cc: me, but due to #2698 (please mail me when my tickets change) I don't actually receive mail.

06/01/09 02:28:17 changed by zooko

A new user of Tahoe reported a bug about this:

http://allmydata.org/trac/tahoe/ticket/718

Is there something I could do to help get this patch into Nevow? I think it requires adding a buildstep so that the warning message such as the following:

http://buildbot.divmod.org/builders/linux32-py2.5-nevowinstall/builds/145/steps/select_2/logs/stdio

/usr/lib/python2.5/site-packages/zope/__init__.py:19: UserWarning: Module twisted was already imported from /usr/lib/python2.5/site-packages/twisted/__init__.pyc, but /var/lib/buildbot/twisted-trunk/q-nevowinstall/build/installdir/lib/python2.5/site-packages/Nevow-0.9.33_r17369-py2.5.egg is being added to sys.path

Are treated as a test failure.

06/01/09 16:09:41 changed by exarkun

  • branch set to branches/nevow-package-warning-2830.
  • branch_author set to exarkun.

(In [17413]) Branching to 'nevow-package-warning-2830'

06/01/09 16:11:31 changed by exarkun

(In [17414]) Apply zooko's patch to reshuffle random junk

refs #2830

06/01/09 16:45:07 changed by exarkun

I think I made the necessary change to the buildmaster configuration for this to be reported as a problem. However, it will be hard to know if I succeeded or not until the existing test failures on the -nevowinstall builders are fixed. There's a branch associated with #2884 which should fix those, but it needs to be reviewed.

06/01/09 16:47:09 changed by exarkun

  • owner changed from amir to exarkun.
  • component changed from Addressbook to Nevow.

06/01/09 16:57:55 changed by zooko

I looked at this patch:

http://divmod.org/trac/changeset?new=branches%2Fnewflat-filenames-2884%4017370&old=branches%2Fnewflat-filenames-2884%4017365

I would delete the work "Arbitrarily".

I don't understand the HERE construct. Oh, I get it, you want to delay evaluation so that it gets the .func_code from the current.. Uh. I don't really understand it. How about replacing HERE with a function which takes a module object and returns a string? Or adding documentation explaining why this lambda thing is good and what it does.

06/01/09 16:58:07 changed by zooko

s/work/word/

06/04/09 07:53:05 changed by exarkun

Okay, #2884 is resolved. The Nevow builders are all green now. And apparently what I did to the buildmaster did not work. I tried using the warnOnWarnings feature, but despite there being warnings, the builds are still "green" instead of "orange". I'll investigate further soon, but if anyone has any suggestions, they'd be helpful.

06/05/09 10:38:09 changed by exarkun

  • branch changed from branches/nevow-package-warning-2830 to branches/nevow-package-warning-2830-2.

(In [17440]) Branching to 'nevow-package-warning-2830-2'

06/05/09 10:41:32 changed by exarkun

  • branch_author changed from exarkun to zooko.

Okay, I made the builder treat all UserWarnings? as errors (this was the only one). So now trunk fails, and the branch succeeds. I think that means this is a good change.

06/05/09 10:45:25 changed by exarkun

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

(In [17442]) Merge nevow-package-warning-2830-2

Author: zooko Reviewer: exarkun Fixes: #2830

Change Nevow's setup.py to install the Twisted plugin using the distutils "data_files" mechanism instead of the "packages" mechanism. This eliminates a (spurious) warning from setuptools at nevow import time.

06/08/09 15:55:18 changed by zooko

Thank you!

06/22/09 14:25:59 changed by exarkun

  • status changed from closed to reopened.
  • resolution deleted.

(In [17526]) Revert r17442 - it broke "setup.py sdist"

Reopens #2830

With this change, the sdist tarball is missing the twisted/plugins/ files.

06/22/09 16:44:58 changed by exarkun

  • branch changed from branches/nevow-package-warning-2830-2 to branches/nevow-package-warning-2830-3.
  • branch_author changed from zooko to exarkun, zooko.

(In [17527]) Branching to 'nevow-package-warning-2830-3'

06/22/09 17:00:59 changed by zooko

Here is a patch which fixes it to include twisted/plugins, without marking twisted/plugins as being a package, by adding twisted/plugins to the MANIFEST.in, as suggested by exarkun:

Index: MANIFEST.in
===================================================================
--- MANIFEST.in (revision 17528)
+++ MANIFEST.in (working copy)
@@ -20,6 +20,7 @@
 include extras/xhtml-nevow.rnc
 include nevow/Canvas.fla
 include nevow/canvas.as
+recursive-include twisted *
 include win32/*
 prune */.svn
 prune doc/html/*.html

06/22/09 17:09:52 changed by exarkun

(In [17529]) put the top of the twisted plugins directory into MANIFEST.in so it gets found by setuptools sdist

refs #2830

06/22/09 17:14:46 changed by exarkun

  • keywords set to review.
  • owner deleted.
  • status changed from reopened to new.

Added a recursive-include to the branch. I also configured buildbot to test sdist both with and without setuptools installed.

06/23/09 00:24:16 changed by glyph

  • keywords deleted.
  • owner set to exarkun.

This looks OK to merge, to me. Unfortunately I can't see the buildbot, but I'll trust it's there and passing. Which buildbot is it?

I am curious though; why 'recursive-include' and not the apparently simpler 'graft', which is what Axiom does?

06/23/09 09:01:41 changed by exarkun

linux32-py2.4-nevowinstall and linux32-py2.5-nevowinstall are covering these cases now. They each test four different ways Nevow can be installed now. I didn't feel like the waterfall would survive having six new Nevow columns.

I used recursive-include instead of graft because the documentation for recursive-include comes slightly before the documentation for graft. Now that I compare the two, I still think recursive-include is better, since it won't put pycs or the dropin.cache into the package.

06/23/09 09:14:15 changed by exarkun

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

(In [17531]) Merge nevow-package-warning-2830-3

Author: exarkun, zooko Reviewer: glyph Fixes: #2830

Change Nevow's setup.py so that it does not declare its twisted/plugins/ contribution to be a package if setuptools is being used. This suppresses a runtime warning about "twisted" already having been imported.

jethro@divmod.org