Ticket #3707 regression closed fixed

Opened 4 years ago

Last modified 4 years ago

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

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

Change History

Changed 4 years ago by amaury

1

Changed 4 years ago by christianmlong

  • owner changed from glyph to christianmlong
  • keywords review added
  • 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.

2

Changed 4 years ago by christianmlong

  • owner christianmlong deleted

Changed 4 years ago by christianmlong

3

Changed 4 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

4

Changed 4 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.

5

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

6

Changed 4 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.

7

Changed 4 years ago by christianmlong

  • keywords review added
  • owner christianmlong deleted

Changed 4 years ago by christianmlong

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

8

Changed 4 years 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'

9

Changed 4 years ago by exarkun

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

refs #3707

10

Changed 4 years ago by exarkun

  • owner set to exarkun
  • keywords review removed

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.

11

Changed 4 years 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).

12

Changed 4 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.

13

Changed 2 years ago by <automation>

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