Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#3707 regression closed fixed (fixed)

Twisted 8.2.0 requires pywin32

Reported by: amaury Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/skip-without-pywin32-3707
(diff, github, buildbot, log)
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 (3)

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

Download all attachments as: .zip

Change History (16)

Changed 5 years ago by amaury

comment:1 Changed 5 years ago by christianmlong

  • Author set to Christian Long
  • Keywords review added
  • Owner changed from glyph to christianmlong

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.

comment:2 Changed 5 years ago by christianmlong

  • Owner christianmlong deleted

Changed 5 years ago by christianmlong

comment:3 Changed 5 years 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

comment:4 Changed 5 years 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.

comment:5 Changed 5 years ago by exarkun

  • Keywords review removed
  • Owner set to christianmlong

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.

comment:6 Changed 5 years 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.

comment:7 Changed 5 years ago by christianmlong

  • Keywords review added
  • Owner christianmlong deleted

Changed 5 years ago by christianmlong

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

comment:8 Changed 5 years ago by exarkun

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

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

comment:9 Changed 5 years ago by exarkun

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

refs #3707

comment:10 Changed 5 years 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.

comment:11 Changed 5 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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).

comment:12 Changed 5 years 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.

comment:13 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.