stop putting pkg_resources.require() into .tac files

[zooko]

Per comment:1:ticket:1159:

One thing that we can do right away to ease this is change create-node to stop putting the call to pkg_resources.require() in the tahoe-client.tac.

[davidsarah]

Agreed.

[zooko]

There are two calls to pkg_resources.require() in the .tac files ​that we currently produce. According to 1aed9fcfa1c45538 the patch that added them, this was necessary to make them importable if they were installed in setuptools's "multiversion mode". I don't think we need to support that. We experimented with it (see #530) and removing multiversion mode from our setup.py did not break any of our unit tests, not even test-with-fake-pkg or test-with-fake-dists.

[davidsarah]

Replying to zooko:

There are two calls to pkg_resources.require() in the .tac files ​that we currently produce. According to 1aed9fcfa1c45538 the patch that added them, this was necessary to make them importable if they were installed in setuptools's "multiversion mode".

Those calls should never have been necessary even for multiversion mode; I think the commit comment was wrong. pkg_resources.require() does not, despite the misleading name and documentation, make a package available that wouldn't already have been available. The set of packages available in the working set is determined at the point of first importing pkg_resources, which happens in the setuptools-generated support script, long before the .tac is executed.

So I guess I'm violently agreeing with your conclusion that these calls aren't necessary ;-)

[warner]

remove pkg_resources calls from generated .tac files

[warner]

something like that?

[davidsarah]

Will review.

Note that import pkg_resources has side effects on sys.path even if pkg_resources.require() is not used; so, removing the import might have an additional effect. I can't immediately remember whether that's desirable or not; need to page this stuff back into my brain.

[zooko]

According to 1aed9fcfa1c45538, I added that so that it would find an allmydata and twisted package tht had been installed in "multi-version mode". I guess that mode of install offered by setuptools doesn't put the installed egg into the sys.path at *all* until a pkg_resources.require() with a matching version number has been invoked. We experimented with it at one point as a way to work around other problems, but I believe we gave up on using it.

[davidsarah]

Obsolete if we accept #1159.

[warner]

I want this, but won't block 1.10 for it. Kicking it and its cousin #1159 into 1.11.

[remyroy]

I'll be doing the review on this one.

[daira]

I would prefer to keep the pkg_resources import just because:

• removing it isn't necessary for the purpose of this patch;
• importing pkg_resources can change sys.path, which can change where allmydata is imported from.

If the .tac file is run via tahoe start or tahoe run, then pkg_resources will already have been imported and therefore importing it again will have no effect -- but this is dependent on the current implementation of tahoe start/run. If the .tac were run directly via twistd or another Python application, then this could have an effect. It is never correct to import allmydata and then have the sys.path change as a result of importing pkg_resources, so it is safer to import it first. (When we switch to not using the contents of the .tac files, it won't matter either way, but we're not doing that yet.)

[remyroy]

Review of 1539-smaller-tac.diff​:

Everything is great except that it breaks the following test: allmydata.test.test_drop_upload.RealTest?.test_drop_upload . The patch should be changed so that no test is broken.

daira has an objection to this change so I'm not sure who I should reassign this to.

[daira]

I will review this together with #1159.

[daira]

This is wontfix because we decided to stop using the contents of .tac files instead (#1159, fixed in 87a6894e6299148d639279fbd909110ba00d0080).