Details

Reviewers

Group Reviewers

Commits

rPHU9e0ea2c27118: Support non-blocking reads for ExecFuture on Windows

Summary

This enables non-blocking reads on Windows for ExecFuture by redirecting the process's standard output and error to temporary files. We then use stream_select on the open files to determine when there is new data to read.

This is primarily to enable setUpdateInterval to work on FutureIterator when running under Windows, which is essential when performing arc unit --everything with lots of tests.

Test Plan

Ran arc diff and arc unit --everything on a project with lots of tests. Saw everything behave correctly and saw setUpdateInterval work as it does on Linux.

Diff Detail

Repository

rPHU libphutil

Branch

master

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 4392
Build 4405: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

hach-que updated this revision to Diff 28128.Feb 5 2015, 3:27 AM

hach-que retitled this revision from to Support non-blocking reads for ExecFuture on Windows.

hach-que updated this object.

hach-que edited the test plan for this revision. (Show Details)

hach-que added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptFeb 5 2015, 3:27 AM

Herald added subscribers: epriestley, Korvin. · View Herald Transcript

Remove commented code

hach-que added a child revision: D11689: Add support for xUnit.NET 2 and improve build / test performance.Feb 5 2015, 6:06 AM

Some inlines. Since this mechanism is certain to be fraught with peril, can we reasonably have an ExecFuture opt in to it for now until we're more confident it works properly? Then only your stuff will break if we hit further Windows weirdness. For example, something like setUseWindowsFileStreams(true).

src/future/exec/ExecFuture.php
588–589	This seems completely crazy -- do feof(), fseek(), ftell(), etc., not work? I'd expect them to work properly since these are standard files.
599	$real_length is only written to and never read, so something is awry in any case.
707–708	These objects are local and not preserved, so I would expect that we'll try to destroy the temporary files immediately upon exiting this function, and fail. In net, I'd expect that we will never remove these files. Do these files get removed? Roughly, I'd expect that you need to save these to `$this->something`. That might also resolve the unlink issue on its own. We should make every reasonable effort to unlink these files. The default permissions of TempFile also aren't as locked-down as they could be, but it seems like these files should be as tightly locked as possible. Not sure if we can reasonably do anything or what `chmod()` translates to under Windows, though.
712–713	These calls should be tested for errors.
735–736	As above.