Ticket #3707 (closed regression: fixed)

Opened 17 months ago

Last modified 17 months ago

Twisted 8.2.0 requires pywin32

Reported by: amaury Owned by: exarkun
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/skip-without-pywin32-3707
Author: exarkun, Christian Long Launchpad Bug:

Description

On Windows, if pywin32 is not installed, it is impossible to import Twisted:

Python 2.6.1 (r261:69718, Feb 17 2009, 15:28:19) [MSC v.1400 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from twisted.internet import defer
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\prod\python\lib\site-packages\twisted-8.2.0-py2.6-win32.egg\twisted\internet\defer.py", l
ine 17, in <module>
    from twisted.python import log, failure, lockfile
  File "c:\prod\python\lib\site-packages\twisted-8.2.0-py2.6-win32.egg\twisted\python\lockfile.py",
line 28, in <module>
    from win32api import OpenProcess
ImportError: No module named win32api

This is unfortunate for simple applications, which don't use the DeferredFilesystemLock. And this makes Twisted usage by pypy much harder...

I suggest to make this dependency optional. For example, if pywin32 cannot be imported, we could assume that the other process is still running. I attach a patch along this idea.

Attachments

lockfile-win32.diff Download (2.0 KB) - added by amaury 17 months ago.
test_lockfile-win32.diff Download (1.0 KB) - added by christianmlong 17 months ago.
patch 3707.diff Download (10.1 KB) - added by christianmlong 17 months ago.
Added some skips for cases when we are on windows, but pywin32 is not available

Change History

Changed 17 months ago by amaury

Changed 17 months ago by christianmlong

  • keywords review added
  • owner changed from glyph to christianmlong
  • branch_author set to Christian Long

I reviewed the patch. It applied cleanly, and it fixed the Import error. I added some skips to twisted.test.test_lockfile.py. See attached patch for test_lockfile.py.

Changed 17 months ago by christianmlong

  • owner christianmlong deleted

Changed 17 months ago by christianmlong

Changed 17 months ago by amaury

Is it possible to setup a buildbot to run tests with the following option?

    --without-module=pywintypes,win32api,win32file,win32pipe,win32process,win32com

Changed 17 months ago by exarkun

We're pretty short on Windows slaves, unfortunately. :/ Setting up a new builder is pretty easy, but we don't have any place to put one right now.

Changed 17 months ago by exarkun

  • owner set to christianmlong
  • keywords review removed

Patches mostly look good. The unit tests accidentally cause skips on non-Windows platforms, though. I suppose they need to check something like twisted.python.runtime.platform.isWindows() before trying to do the imports.

Changed 17 months ago by exarkun

Hmm, one further comment. I think the test suite needs to expand a little bit to exercise the new if kill is not None: check. It just needs to verify that when kill does end up as None, it doesn't end up being called and raising a TypeError.

Changed 17 months ago by christianmlong

  • owner christianmlong deleted
  • keywords review added

Changed 17 months ago by christianmlong

Added some skips for cases when we are on windows, but pywin32 is not available

Changed 17 months ago by exarkun

  • branch set to branches/skip-without-pywin32-3707
  • branch_author changed from Christian Long to exarkun, Christian Long

(In [26632]) Branching to 'skip-without-pywin32-3707'

Changed 17 months ago by exarkun

(In [26633]) Apply patch from amaury and christian long

refs #3707

Changed 17 months ago by exarkun

  • keywords review removed
  • owner set to exarkun

This looks good to me. I hope we can set up a builder to test this configuration soon. I think we could also improve our overall skip factoring. Seeing all the duplicates added at once, instead of one at a time over a long period of time, makes it obvious just how much duplication we have. I'm thinking that in the future we might have a twisted.test.skips (or something) which does all the detection and defines skips which can then be imported into test modules and set onto things.

Changed 17 months ago by exarkun

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

(In [26634]) Merge skip-without-pywin32-3707

Author: amaury, christianmlong Reviewer: exarkun Fixes: #3707

Skip all the tests which require process support on Windows if pywin32 is not installed (and thus IReactorProcess won't work).

Changed 17 months ago by amaury

Thanks for the correction!

I think that the change description should mention somewhere that Twisted could not work at all if pywin32 is not installed.

Note: See TracTickets for help on using tickets.