#3707 release blocker: 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
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | exarkun, Christian Long |
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)
Change History (16)
Changed 12 years ago by
Attachment: | lockfile-win32.diff added |
---|
comment:1 Changed 12 years ago by
Author: | → Christian Long |
---|---|
Keywords: | review added |
Owner: | changed from Glyph to christianmlong |
comment:2 Changed 12 years ago by
Owner: | christianmlong deleted |
---|
Changed 12 years ago by
Attachment: | test_lockfile-win32.diff added |
---|
comment:3 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
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 12 years ago by
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 12 years ago by
Keywords: | review added |
---|---|
Owner: | christianmlong deleted |
Changed 12 years ago by
Attachment: | patch 3707.diff added |
---|
Added some skips for cases when we are on windows, but pywin32 is not available
comment:8 Changed 12 years ago by
Author: | Christian Long → exarkun, Christian Long |
---|---|
Branch: | → branches/skip-without-pywin32-3707 |
(In [26632]) Branching to 'skip-without-pywin32-3707'
comment:9 Changed 12 years ago by
comment:10 Changed 12 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Jean-Paul Calderone |
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 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:12 Changed 12 years ago by
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 10 years ago by
Owner: | Jean-Paul Calderone deleted |
---|
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.