console support for stdio in Windows

[synapsis]

This patch allows twisted.internet.stdio to work when connected to a console, in Windows.

I have done a small refactoring in twisted.internet._pollingfile. I think pipe support should go in twisted.internet._dumbwin32proc and the console support in a new twisted.internet._conio module.

I have also written a simple test suite.

Console support is still very limited, but now stdiodemo.py and stdin.py examples works.

In the near future I hope to have a working conch.

[synapsis]

patch for console support

[synapsis]

test suite for console support

[Jean-Paul Calderone]

Some comments on the attached code:

• classic to new-style class conversion: good (but unrelated)
• generalization of _PollableReadPipe and _PollableWritePipe - good
• _PollableReadConsole - looks like a good start at the low-levels of dealing with a windows console, but the factoring needs to be cleaned up. Dealing with the win32-specific events should not be handled by the same class which implements echoing and so forth. As a long term goal, providing a win32 implementation of the insults ITerminalTransport/ITerminalProtocol interfaces would seem to make sense (and it may make sense to make some adjustment to these interfaces based on what win32 is capable of). This further suggests splitting the transport logic (ie, WriteConsole calls) from the protocol logic (ie, should an input be echoed?), since insults is part of Twisted Conch, but the core of win32 conio support belongs in Twisted Core.
• test suite - looks good. Adding docstrings explaining the purpose of each test method would be even better.

I don't know much about the Win32 APIs involved here, so I can't comment much on the correctness of most of this code, but it looks like a nice start in the right direction.

[synapsis]

Replying to exarkun:

Some comments on the attached code:

• classic to new-style class conversion: good (but unrelated)

Yes. I have done some modification on _pollingfile for private use, forgetting to revert them.

Shold I remove them?

• generalization of _PollableReadPipe and _PollableWritePipe - good

• _PollableReadConsole - looks like a good start at the low-levels of dealing with a windows console, but the factoring needs to be cleaned up. Dealing with the win32-specific events should not be handled by the same class which implements echoing and so forth.

One possible (but complex) solution is to provide file like objects for console stdin/stdout that are POSIX compatible (like done by Cygwin).

As a long term goal, providing a win32 implementation of the insults ITerminalTransport/ITerminalProtocol interfaces would seem to make sense (and it may make sense to make some adjustment to these interfaces based on what win32 is capable of).

IPython contains a pyreadline package that supports ANSI terminal escape characters.

This further suggests splitting the transport logic (ie, WriteConsole calls) from the protocol logic (ie, should an input be echoed?), since insults is part of Twisted Conch, but the core of win32 conio support belongs in Twisted Core.

• test suite - looks good. Adding docstrings explaining the purpose of each test method would be even better.

I don't know much about the Win32 APIs involved here, so I can't comment much on the correctness of most of this code, but it looks like a nice start in the right direction.

Some comments of what needs to be done:

1) pipe support should (IMHO) be moved from _pollingfile to _dumbwin32proc, console support should be moved to a new _conio module

2) Since a Window process can have only one console and a console can have only on input buffer, the _PollableReadConsole should be a singleton/borg.

The _conio module should provide globals stdin/stdout/stderr objects

3) Since in a child process can happen(?) that stdin is attached to a console and stdout to a pipe (or viceversa) _win32stdio should use isatty on both file descriptors

4) In _PollableReadConsole I convert '\r' to '\r\n'.

I have done so to be able to send empty lines to a LineReceiver protocol. Maybe '\r' should be simply converted to '\n'?

5) I really don't know if _PollableWriteConsole can block.

I have just copied the implementation of _PollableWritePipe, using WriteConsole instead of WriteFile

Can I implement the changes from 1)?

[PenguinOfDoom]

Please make the IO mechanism pluggable. For example, a reactor that can wait on win32 handles (such as win32eventreactor or, potentially, iocpreactor) could turn off mouse and focus events and use WaitForMultipleObjects to find out when ReadConsole wouldn't block.

At the very least, StandardIO should not be inheriting from _PollingTimer.

Also, the standard API for closing a file handle is its loseConnection() method. I don't know why _pollingfile uses close().

[Cory Dodt]

I only have a vague idea about the design details you guys are throwing around here, but I applaud this effort and I hold out hope that the win32 platform will be greatly improved by this patch. So I'm only going to make a simple plea. There seem to be an awful lot of modules and classes that start with underscores. Please review the API at least once and think hard about whether so many things need to be private/untouchable, and which things can be available to applications.

Thanks.

[synapsis]

Replying to PenguinOfDoom:

Please make the IO mechanism pluggable. For example, a reactor that can wait on win32 handles (such as win32eventreactor or, potentially, iocpreactor) could turn off mouse and focus events and use WaitForMultipleObjects to find out when ReadConsole wouldn't block.

This is a good idea. I'm planning to write a file like object that allows a non blocking read from a console.

At the very least, StandardIO should not be inheriting from _PollingTimer.

I'm not sure about this. When running the win32eventreactor we can receive notifications about input available. But what about _PollableWriteConsole?

Moreover I really don't know if WriteConsole can block.

Also, the standard API for closing a file handle is its loseConnection() method. I don't know why _pollingfile uses close().

[synapsis]

This is a new set of patches for a better implementation of console support in twisted.

I have created a separate patch for _pollingfile so that this can be moved to a new ticket.

Notes:

1) I have changed _pollingfile to allow it to be reusable. More things can be done, like moving pipe support to _dumbwin32process and refactoring _PollingTimer to a more reactor like interface

2) implementation of conio needs to be reviewed. I have to do some tests with a real POSIX implementation (on Linux).

3) support for IConsumer and IProducer (in _win32stdio) needs to be reviewed

[synapsis]

patch _pollingfile.py, version 2

[synapsis]

patch _pollingfile.py, version 2

[synapsis]

patch for _win32stdio.py, version 2

[synapsis]

console support for Twisted, version 2

[synapsis]

test suite for console support, version 2

[synapsis]

Starting with this year I no more use Windows, so I have no more much interest in this patch.

However if this patch is still useful and there is someone that can do a review, I can try to complete it.

[rotund]

Adding "windows" keyword.

[czzl]

Duplicate of #2135. I kept this one open because it has a lot more stuff on it.

[khorn]

Unified diff of above patches against r27422

[khorn]

Consolidated patches into a single unified diff against the current trunk (r27422).

Hopefully this will make it easier to move this forward.

With this patch, tests pass, and the stdiodemo.py example works.

The stdin.py demo does NOT work (no output), at least on my machine (WinXP).

[khorn]

Found the problem with the stdin.py demo. It uses os.linesep as a delimiter (which on Win32 is '\r\n'), while the stdiodemo.py example uses a hardcoded '\n'.

Changing to a hardcoded '\n' in stdin.py makes it work with this patch. I'm sure there's something deeper going on here which probably needs further investigation.

[Thijs Triemstra]

(In [27426]) Branching to 'win-stdio-2157'

[Thijs Triemstra]

(In [27427]) Apply win32_console_io.2.patch by khorn, refs #2157

[Thijs Triemstra]

(In [27428]) Apply coding standard, refs #2157

[Thijs Triemstra]

I slapped the patch in a branch and applied some coding standards, up for review.

[therve]

(In [27500]) Branching to 'win-stdio-2157-2'

[TimAllen]

I don't have a Windows machine here to properly test this code, but here's some comments:

1. There's a lot of "XXX" and "TODO" comments in this code. The coding standard says "Comments marked with XXX or TODO must contain a reference to the associated ticket". Hopefully the questions and unhandled conditions marked by these strings will be answered and handled before this branch is merged to trunk, and no tickets will need to be filed. :)
2. t/i/_pollingfile.py, line 239 in _PollableReadPipe.checkWork(): Commented-out code should be fixed if it ought to work, or deleted otherwise.
3. t/i/_win32stdio.py, line 34 in _PollableReadConsole.checkWork(): Rather than explain the magic value 2000 as "80x25", why not just write it as "80 * 25"? Also, does this break if the console is a different size (such as 80×50)?
4. t/i/_win32stdio.py, line 36 in _PollableReadConsole.checkWork(): If you do get an unexpected IOError, it would be more helpful to raise it than to raise an unrelated AssertionError.
5. t/i/_win32stdio.py, in _PollableWriteConsole.checkWork(): Variables numBytesWritten and nBytesWritten in the same method? There's got to be better names than these.
6. t/i/_win32stdio.py, line 90: Is there a reason that ConsoleReader doesn't have a leading underscore, where _PollableReadConsole (presumably exactly the same thing but for other reactors) does not?
7. t/i/_win32stdio.py, line 90: You have ConsoleReader for win32eventreactor and _PollableReadConsole otherwise; but _PollableWriteConsole works with all reactors. Why not drop ConsoleReader and just use _PollableReadConsole all the time, too?
8. t/i/_win32stdio.py, line 92 in ConsoleReader.__init__(): This is a terrible docstring. It doesn't describe what the class represents or how it's intended to be used.
9. t/i/_win32stdio.py, line 106 in ConsoleReader.doRead(): This looks exactly like _PollableReadConsole.checkWork(). Maybe ConsoleReader and _PollableReadConsole should inherit from some common base class or mixin? Otherwise, all the comments for _PollableReadConsole.checkWork() apply here too.
10. t/i/_win32stdio.py, line 169 in StandardIO.__init__(): You say "for stdfd in range(0, 1, 2)", but I don't think that does what you intend. How about "for stdfd in (0, 1, 2)"?
11. t/i/_win32stdio.py, line 176 in StandardIO.__init__(): win32Enabled is an unfortunate name - it's quite possible to be using Win32 with the select reactor or IOCP reactor. How about calling it eventReactorInstalled?
12. t/i/_win32stdio.py, line 210 in StandardIO.__init__(): I feel it would be more readable to paste "self._addPollableResource(self.stdin)" into both places where it's required than to do the little if dance afterward.
13. t/i/conio.py: Since this is just console IO for Windows rather than console IO in general, perhaps this module should be called win32conio.py?
14. t/t/test_conio.py: Why is this in twisted/test/ and not twisted/internet/test/, since it's testing things in twisted/internet/?
15. t/t/test_conio.py: This test suite explodes horrible on non-Win32 platforms (or presumably Win32 platforms without the Win32 Extensions package installed) because it imports win32console. See t/i/t/test_pollingfile.py for an example of how to write platform-dependent tests that don't explode on other platforms.

I'm afraid I ran out of time before I could properly review conio.py and test_conio.py, but the issues above should be enough to start with.

It looks like khorn has the most involvement with this code at the moment, so assigning back to them to respond to these issues.

[khorn]

Fair warning: I didn't write any of this code to begin with. I just unified the existing patches and made sure they worked with trunk.

That said, I'm happy to give this one a shot, as long as everyone else is happy to wait long enough for me to more fully understand the code. :)

I'm a little perplexed at how I should proceed though. There are now two branches, and the newer one doesn't seem to have all (any?) of the changes in the patches for this ticket. Should I be submitting patches against one of the branches? Which branch?

[khorn]

Anyone? Suggestions?

[therve]

Unless I'm missing something, the latest branch should have all the changes submitted. Do you see anything missing?

[khorn]

OK, I think I know what happened now. When I looked at the branches before, I was browsing the code in Trac, and several files were missing from the latest branch. They all seem to be there now. Maybe some sort of caching issue with Trac?

At any rate, seems fine now, my fault for not thinking of caching issues earlier.

Sorry. :)

[khorn]

(In [28083]) Change line delimiter in doc/core/examples/stdin.py to work on Win32.

Refs: #2157

[khorn]

(In [28085]) Address review comments:

• Removed extraneous commented line from _pollingfile.py
• fixed incorrect loop in _win32stdio.py
• renamed win32Enabled to win32EventReactorInstalled
• renamed conio.py to win32conio.py
• fixed test_conio.py so that the tests don't blow up on non-win32 platforms

Refs: #2157

[khorn]

Replying to TimAllen:

changes in [28085] address some of the review comments

1. There's a lot of "XXX" and "TODO" comments in this code. The coding standard says "Comments marked with XXX or TODO must contain a reference to the associated ticket". Hopefully the questions and unhandled conditions marked by these strings will be answered and handled before this branch is merged to trunk, and no tickets will need to be filed. :)

Let's see if we can fix up the other stuff first...

2. t/i/_pollingfile.py, line 239 in _PollableReadPipe.checkWork(): Commented-out code should be fixed if it ought to work, or deleted otherwise.

Removed the offending line.

3. t/i/_win32stdio.py, line 34 in _PollableReadConsole.checkWork(): Rather than explain the magic value 2000 as "80x25", why not just write it as "80 * 25"? Also, does this break if the console is a different size (such as 80×50)?

Well, it adds one more operation every time the loop runs, so maybe this was a premature optimization?

I currently have no idea if this breaks when/if the console size is different, but I don't see why it would. For that matter I'm not sure why this value was chosen at all...

4. t/i/_win32stdio.py, line 36 in _PollableReadConsole.checkWork(): If you do get an unexpected IOError, it would be more helpful to raise it than to raise an unrelated AssertionError.

Are we sure that the AssertionError would be raised in all cases? I'm not sure why it does what it's doing, but I also don't really know what the correct behavior should be, so...

5. t/i/_win32stdio.py, in _PollableWriteConsole.checkWork(): Variables numBytesWritten and nBytesWritten in the same method? There's got to be better names than these.

Probably so, but it follows the example of t.i._pollingfile.py (before this branch), which does the same thing in several places.

6. t/i/_win32stdio.py, line 90: Is there a reason that ConsoleReader doesn't have a leading underscore, where _PollableReadConsole (presumably exactly the same thing but for other reactors) does not?

7. t/i/_win32stdio.py, line 90: You have ConsoleReader for win32eventreactor and _PollableReadConsole otherwise; but _PollableWriteConsole works with all reactors. Why not drop ConsoleReader and just use _PollableReadConsole all the time, too?

8. t/i/_win32stdio.py, line 92 in ConsoleReader.__init__(): This is a terrible docstring. It doesn't describe what the class represents or how it's intended to be used.

9. t/i/_win32stdio.py, line 106 in ConsoleReader.doRead(): This looks exactly like _PollableReadConsole.checkWork(). Maybe ConsoleReader and _PollableReadConsole should inherit from some common base class or mixin? Otherwise, all the comments for _PollableReadConsole.checkWork() apply here too.

re: 6-9 -- I really am not sure what ConsoleReader was intended to do...if anyone has any ideas, I'd like to hear them.

10. t/i/_win32stdio.py, line 169 in StandardIO.__init__(): You say "for stdfd in range(0, 1, 2)", but I don't think that does what you intend. How about "for stdfd in (0, 1, 2)"?

Yikes! Fixed as suggested.

Not sure why it worked before though...

11. t/i/_win32stdio.py, line 176 in StandardIO.__init__(): win32Enabled is an unfortunate name - it's quite possible to be using Win32 with the select reactor or IOCP reactor. How about calling it eventReactorInstalled?

Renamed to win32EventReactorInstalled.

12. t/i/_win32stdio.py, line 210 in StandardIO.__init__(): I feel it would be more readable to paste "self._addPollableResource(self.stdin)" into both places where it's required than to do the little if dance afterward.

I don't understand what is meant here. Can you clarify?

13. t/i/conio.py: Since this is just console IO for Windows rather than console IO in general, perhaps this module should be called win32conio.py?

Agreed. Fixed.

However, it looks as though this may have been intended to grow into a platform-independent module. See: ​http://twistedmatrix.com/trac/ticket/2157#comment:6

???

14. t/t/test_conio.py: Why is this in twisted/test/ and not twisted/internet/test/, since it's testing things in twisted/internet/?

I'm guessing the original author put it there because test_stdio is also in twisted/test. Should it be moved? Also, should it be renamed to something like test_win32conio.py?

15. t/t/test_conio.py: This test suite explodes horrible on non-Win32 platforms (or presumably Win32 platforms without the Win32 Extensions package installed) because it imports win32console. See t/i/t/test_pollingfile.py for an example of how to write platform-dependent tests that don't explode on other platforms.

I think this is fixed now, and the tests should be skipped on non-Win32. Please test on non-Win32 platforms to make sure.

I've also noticed something a bit strange when running the tests. Some of the tests in test_conio.py send output to the console which is running trial. So it ends up sending out:

  ConInRawTestCase
    testFlush ...                                                          [OK]
    testRead ...                                                           [OK]
    testReadMultiple ...                                                   [OK]
    testReadWithBuffer ...                                                 [OK]
    testReadWithDelete ...                                                 [OK]
  ConInTestCase
    testBuffer ... hello
[OK]
    testDeleteBoundary ...w
                                                [OK]
    testDeleteFullBoundary ... hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
hhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhh
w
                                            [OK]
    testFlush ... hello

Somehow I don't think is intentional. :)

I'm not sure if this is because of something trial is doing (or mis-assuming), or if it has to do with the code that writes to the console input buffer.

It seems to happen when WriteConsoleInput() is called.

The tests pass regardless...so is this really a problem with functionality? I don't know.

[jesstess]

Thanks for all this work, khorn!

​builds

Unfortunately there are both testing and markup issues.

If you're interested in running the buildbot tests yourself, a starting point is UsingBuildbot.

[khorn]

(In [29541]) fix typo to skip tests properly on non-win32

refs: #2157

[khorn]

(In [29542]) Docstring format fix

refs: #2157

[khorn]

the only test that's failing on all the windows builders (aside from the glib2reactor and gtkreactor failures, which are unrelated)

[ERROR]: twisted.test.test_conio

Traceback (most recent call last):
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\trial\runner.py", line 563, in loadPackage
    module = modinfo.load()
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\python\modules.py", line 381, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\python\reflect.py", line 462, in namedAny
    topLevelPackage = _importAndCheckStack(trialname)
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\python\reflect.py", line 398, in _importAndCheckStack
    return __import__(importName)
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\test\test_conio.py", line 17, in <module>
    from twisted.internet import win32conio, _win32stdio as stdio
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\internet\win32conio.py", line 337, in <module>
    stdin = ConIn(win32console.GetStdHandle(win32console.STD_INPUT_HANDLE))
  File "C:\twistedbot\winxp32-py2.6-select\Twisted\twisted\internet\win32conio.py", line 57, in __init__
    defaultMode = handle.GetConsoleMode()
pywintypes.error: (6, 'GetConsoleMode', 'The handle is invalid.')

I can't replicate this locally. It looks like it's unable to get a proper handle to the console's std_in buffer. Some possible causes (guessing):

1. Running under the buildbot makes it so that there's no handle to get. How does buildbot run it's jobs? In a separate process maybe? Or does something else already have a hold on the stdin buffer?
2. Some kind of mismatch between the version of pywin32 on the buildslaves and what's on my local machine?
3. Any other ideas?

[khorn]

(In [29543]) Branching to 'win-stdio-2157-3'

[khorn]

Merged forward and the tests still pass on my machine...still need to fix buildbot errors.

[khorn]

(In [29544]) * Merge forward

Refs: #2157

[khorn]

After some research, it appears that buildslaves usually (always?) run as Windows Services on Win32. This _might_ mean that the build user would need the "Allow service to interact with desktop" right in order to create a console. Which _might_ have something to do with the above test failure.

Still can't replicate the failure locally.

[khorn]

Success!

Finally replicated the error using:

>>> from subprocess import Popen
>>> outfile = open('outfile.txt', 'w')
>>> Popen([r'c:\python26\scripts\trial.bat', 'twisted.test.test_conio'], stdin=outfile)

I get the exact error above. So now I guess it's time to figure out how to build a console or something in the test code...

[Chris Wilson]

From MSDN:

GUI applications are initialized without a console. Console applications are initialized with a console, unless they are created as detached processes (by calling the CreateProcess function with the DETACHED_PROCESS flag).

I.e. your process probably doesn't have a console if you run it in the background or start it as a system service. Then all console-related Win32 APIs will fail.

You can call AllocConsole to get yourself a console. It might appear on the user's screen, at least temporarily, but that might be OK for a buildbot. I'm not sure how you could get any input into it in this state, so it might not be useful for testing, but at least you'd get past GetConsoleMode() failing.

[John Popplewell]

I've been working on improved windows support for StandardIO() and found this branch to be a useful starting point. I've managed to get the (slightly modified) cftp.py and conch.py scripts working and solved the build-bot missing console problem. Improvements include being able to see the console cursor and scroll-back that doesn't keep 'pulling' you back to the active console line. I also tried the imap4client.py example program which worked with no changes.

I had to setup a buildbot-server for testing and have a windows 7 buildbot client (others to follow). I've tested my patches on XP/7/Linux and they seem to work OK. They are against the current trunk 38158.

Unfortunately, the patch is huge (90k) plus 3 new files (8k each) so I'm not sure exactly what to do with it.

For now I'm going to break it up into windows stdio support, tests for that support, conch/cftp changes, tests for that, and finally other unrelated issues I encountered whilst trying to get as many windows tests to pass as possible (6999).

Should I add them here when I'm ready?

[Jean-Paul Calderone]

Hi johnnypops. I think it would be best if you raised this on the mailing list. It would probably also be useful if you shared the code at the same time. It's possible someone will be able to take a look at it and give you some ideas about the way to proceed which has the greatest chance of getting your code into Twisted quickly and easily. Thanks!

[John Popplewell]

_pollingfile, _dumbwin32proc, _win32stdio, test_stdio, test_pollingfile

[John Popplewell]

twisted.internet.win32conio

[John Popplewell]

twisted.internet.test.test_win32conio

[John Popplewell]

twiste.conch.windows

[John Popplewell]

patches for cftp, conch scripts plus conch client, ssh, stdio, tap, python.win32

[John Popplewell]

lots of minor tweaks to the conch tests

[John Popplewell]

Fluffed it. Fixed version.

[John Popplewell]

Fluffed it. Fixed version.

[John Popplewell]

I foolishly forgot to test with the stdiodemo.py and stdin.py examples mentioned in the original post. Attached fixed versions of win32conio.py and _win32stdio.py.

stdin.py worked as-is but stdiodemo.py needs the delimiter in WebCheckerCommandProtocol() fixing:

class WebCheckerCommandProtocol(basic.LineReceiver):
    from os import linesep as delimiter

    def connectionMade(self):

It also revealed that connectionLost() wasn't being called correctly when using the console.

[John Popplewell]

Ignore the attached _win32stdio, this fixed patch includes those changes

[lvh]

This ticket isn't up for review, but it looks like perhaps it should be. Did you intend to put it up for review?

[Glyph]

I suspect that johnnypops may not be paying attention any more, so I'm going to attach the review keyword. (If the author doesn't reply, please consider me the author on this now...)

[Glyph]

(In [42962]) Branching to win-stdio-2157-4.

[Glyph]

johnnypops, thanks very much for your contribution. I'm sorry it got lost in the shuffle.

I had a look at the attached patch and attempted to apply it to a branch. It doesn't apply cleanly to trunk any more, and the patch that's there is missing a lot of stuff. I couldn't really tell which files I was supposed to apply and which I wasn't. It also randomly deleted a bunch of docstrings, changed whitespace to violate the coding standard in various places, and added a bunch of methods whose purpose wasn't clear.

All the new classes, Channel, ChannelReadPipe, ChannelWritePipe, ChannelConsole, _PollableReadConsole, and so on, need docstrings.

Also I don't quite understand tradeoffs involved in using SetHandleInformation vs. DuplicateHandle/CloseHandle, if that affects the way Twisted will behave at all.

I really hope someone will pick this back up, I'll try to review it more promptly in the future :).

[khorn]

I'll try to give this one another shot.

[Glyph]

Thanks khorn! Looking forward to it! :)

[khorn]

I've managed to get johnnypops' patches merged, but am having some trouble pushing them back up to SVN. I'll try to get it pushed up tomorrow.

[Glyph]

Replying to khorn:

I've managed to get johnnypops' patches merged, but am having some trouble pushing them back up to SVN. I'll try to get it pushed up tomorrow.

What kinds of issues are you having? When git takes a long time to do things with git-svn, sometimes it's a clue that it's preparing to do some enormously destructive operation that will mangle the whole repository, so I'm very keen to see the output of git svn dcommit -n or somesuch now :).

[khorn]

First I had an issue with git-svn finding my ssh binary (plink.exe in this case). This was a matter of setting env vars correctly so it would find it, which sounds simpler than it actually was. :/

then I got something like this:

>git svn dcommit --dry-run
Committing to svn+ssh://svn.twistedmatrix.com/svn/Twisted/branches/win-stdio-2157-4 ...
diff-tree 2f0571134babb9902c7dc0976fde3913be50c705~1 2f0571134babb9902c7dc0976fde3913be50c705

which I think means it should work fine (the hash is indeed the commit I want to push). When I ran it without the --dry-run flag, though, it just sat there forever, giving me no feedback. I figured it was up to no good, so I killed it after several minutes.

I'll investigate further presently...if you have any suggestions though I'd be glad to hear them.

[khorn]

(In [43039]) Applied patches from johnnypops to recent branch

Refs: #2157

[khorn]

Updated the latest branch with modified patches. Tests seem to pass in my environment, and the stdin.py and stdiodemo.py examples work.

[Glyph]

Thanks again for looking at this, khorn.

Yes, that git-svn output looks correct.