can't test on buildslaves that we don't depend on pywin32

[davidsarah]

#1274 removed Tahoe's direct and indirect dependencies on pywin32.

However, it appears that buildbot depends on pywin32 on Windows, so we can't uninstall it on the Windows buildslaves to automatically test that it isn't needed (and avoid regressions).

[davidsarah]

After looking at buildbot's dependencies on pywin32 (including via Twisted, for process control), I doubt this will be fixed any time soon. We'll just have to live with having to run the test suite manually in order to check that we don't depend on pywin32.

[davidsarah]

Replying to davidsarah:

After looking at buildbot's dependencies on pywin32 (including via Twisted, for process control), I doubt this will be fixed any time soon. We'll just have to live with having to run the test suite manually in order to check that we don't depend on pywin32.

Oh hang on, maybe it's not necessary that pywin32 not be installed in order for us to check that we don't import it. We can have a test that runs last and does self.failIfIn("pywin32", sys.modules).

[davidsarah]

Replying to davidsarah:

Oh hang on, maybe it's not necessary that pywin32 not be installed in order for us to check that we don't import it. We can have a test that runs last and does self.failIfIn("pywin32", sys.modules).

Unfortunately it wasn't as simple as that. Several of our dependencies (for example, Twisted) will try to import pywin32 modules, even though they tolerate unavailability of those modules. So we have to block loading of pywin32 (and do so fairly early, before we've tried to import any such dependencies) in order to be able to test that we don't need it.

[davidsarah]

Prevent pywin32 modules from loading, and test at the end of the test suite that they haven't. refs #1274. fixes #1334'

[davidsarah]

It's really ugly that attachment:pywin32-just-say-no.darcs.patch​ adds to the complexity of src/allmydata/__init__.py, but I couldn't find any other way to do it (in particular, src/allmydata/windows/fixups.py​ is too late because pywin32 modules would already have been loaded by then).

[zooko]

I'm not sure if it is worth it to add complexity to src/allmydata/__init__.py​ in order to test this. We could also test it by having a buildslave with no pywin32 on it, right? Maybe also in concert with a "desert island build" test which goes red if the build system downloads anything.

In general I've recently been trying to find ways to test code without altering the code to add test hooks, although if this is worth it in specific then I don't object.

[davidsarah]

Replying to zooko:

I'm not sure if it is worth it to add complexity to src/allmydata/__init__.py​ in order to test this. We could also test it by having a buildslave with no pywin32 on it, right?

No, because the part of buildbot that runs on the buildslaves depends on pywin32. The only other way to test this, I think, would be to make pywin32 available to the buildbot code without being installed, and without being available to subprocesses. There are two ways to do that that I can think of:

1. add an entry for pywin32 to sys.path in the startup script for the buildslave;
2. add an entry for pywin32 to PYTHONPATH when starting the buildslave (or globally), but then in the buildbot step that runs the Tahoe test suite, remove that entry from the PYTHONPATH passed to the subprocess.

[david-sarah@…]

In 6e0607f4e05b159c:

misc/build_helpers/run_trial.py: ensure that pywin32 is not on the sys.path when running the test suite. Includes some temporary debugging printouts that will be removed. refs #1334

[david-sarah@…]

In 91c7cf9007ae5b5d:

misc/build_helpers/run_trial.py: undo change to block pywin32 (it didn't work because run_trial.py is no longer used). refs #1334

[davidsarah]

I no longer favour the approach used in the patch of blocking loading of pywin32 in allmydata/__init__.py. It's too ugly and intrusive to install an import hook just to avoid loading a library that we don't want to have to care about any more.

I have suggested to our win32 buildslave operators, Freestorm and Dcoder, the following changes to their set up, that should achieve a similar effect of only allowing pywin32 to be used by the initial buildslave process:

---

Assume that the Python directory is C:\Python27 (if not then adjust the paths below). Stop the buildslave, then do:

ren C:\Python27\Lib\site-packages\pywin32.pth pywin32.pth.disabled

Edit C:\Python27\Scripts\buildbot (using an editor that can handle Unix line endings). Add the following lines just after the !# line:

import os, sys
site = os.path.join(os.path.basename(sys.executable), 'Lib', 'site-packages')
sys.path += [os.path.join(site, d) for d in
             ['win32', 'win32\\lib', 'Pythonwin']]

Similarly, in C:\Python27\Scripts\buildbot_service.py, after 'import threading' but before 'import pywintypes', add:

site = os.path.join(os.path.basename(sys.executable), 'Lib', 'site-packages')
sys.path += [os.path.join(site, d) for d in
             ['win32', 'win32\\lib', 'Pythonwin']]

(i.e. the same thing without the 'import os, sys').

The names of the buildbot and buildbot_service.py scripts might be different depending on the version of Buildbot. (There are some references to a 'buildslave' script in the docs which wasn't present in my copy, but if it exists in yours then it will need a similar modification.) You may want to back up the old versions of the scripts before editing them.

To check that pywin32 can't be imported by code other than buildbot, do:

python -c "import win32api; print win32api"

That should give an ImportError. Now restart the buildslave and check that it still works.

---

Freestorm and Dcoder haven't got back to me yet.

[zooko]

For future reference, the ​exocet tool from Allen Short is an importer (which I think means it replaces the thing that currently implements the import keyword) which can do things like blacklist modules. Might ease the sort of hack that we're currently deciding not to do to the longsuffering src/allmydata/__init__.py.

[davidsarah]

Replying to zooko:

For future reference, the ​exocet tool from Allen Short is an importer (which I think means it replaces the thing that currently implements the import keyword) which can do things like blacklist modules.

Well, that's only 6 lines in pywin32-just-say-no.darcs.patch​:

class BlockPywin32Finder(object):
    def find_module(self, fullname, path=None):
        if fullname.split('.')[0] in pywin32modules:
            raise ImportError(fullname)
        return None
sys.meta_path.insert(0, BlockPywin32Finder())

If 6 lines is too complicated, then dependency on another tool certainly is!

[davidsarah]

Not to mention that ​exocet tries to import modules from pywin32 :-)

[zooko]

I didn't mean we would use exocet solely to avoid those six lines. It may be a thing that we prefer to use in general, if avoiding these six lines turns out to be one of several benefits that it brings. I've opened use exocet instead of the builtin Python module loader: #1443.

[davidsarah]

#1464 is a generalization of this ticket to other libraries (it also affects Twisted).

[davidsarah]

I just spotted the --without-module= argument to trial, which might be sufficient to fix this.

[warner]

We now *do* depend on pywin32 (specifically pypiwin32), so this is now a WONTFIX.