[Wilds] On Windows: always use stdout/stderr files and always "bypass_shell"
Summary:
Ref T13209. I'll walk through this a bit inline, but two major ideas here:
First, always use bypass_shell when calling proc_open(). The effect of this parameter is to use a "raw" CreateProcessW() Windows API call to spawn the subprocess instead of wrapping it in cmd.exe /c ..., which is roughly equivalent to sh -c.
The major advantage of adding cmd.exe /c ... is that you can use shell features like > outfile.txt.
The major disadvantage of adding cmd.exe /c ... is that cmd.exe is a terrible shell and escaping becomes absurdly complicated and position-dependent. It does not appear to be possible to escape some characters, like "\n", in argument lists passed through cmd.exe. It is unclear if we can safely escape environmental variables ("%APPDATA%"). PHP gives up on this and Python defaults to a bypass_shell equivalent with warnings about how using shell=True is dangerous, although they appear to understate the danger.
Since we don't currently rely on any shell features, don't plan to rely on shell features, and making program behavior generally sane should be easier without needing to fight against cmd.exe, always bypass it for now. We may later find that we need it in some cases (e.g. remote execution, or arc alias shell commands, or whatever else). If so, we can support it at that time with an elective mode on CommandString, but bypassing it seems to clearly be the far saner default.
Second, Windows doesn't have nonblocking pipes so if you try to read output from a subprocess you block until it exits. Most of the time that's not great but ends up working out okay, but when it's not fine you likely just deadlocked and hung and that's the end of you.
We can avoid this by using files instead of pipes. We've supported this mode since D11688, but never used it in the upstream. Ostensibly, it works. Make it the only mode, then rearrange it a bit so that resource management is a little tighter (for example, previously we could try to use $pipes[0] even after proc_open() failed, which meant that $pipes would not be populated).
It appears to actually work, in the sense that all the tests pass, which is promising.
Test Plan: With further changes, no tests fail on Windows, which is at least promising.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13209
Differential Revision: https://secure.phabricator.com/D19727