Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Process] Redirect output without a shell#9726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Currently processes that output large amounts of data will block. Ifthe output is not important then this becomes an issue. It can be workedaround by redirecting the output to > /dev/null but this requires aninstance of /bin/sh to do the work.This patch adds the ability to set the processPipes, and a newprocessPipe that redirects to /dev/null.| Q | A| ------------- | ---| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony#9007| License | MIT| Doc PR |symfony/symfony-docs#3303
stof commentedDec 9, 2013
This patch currently allows to change the ProcessPipes at any time. But this will break if you change it after the process is started. A safeguard should be added to prevent it. And I think this should be added to the ProcessBuilder as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
extra line break
vektah commentedDec 9, 2013
I'm not sure what weirdness is expected here, I've added a test... |
fabpot commentedDec 11, 2013
ping@romainneutron |
romainneutron commentedDec 11, 2013
I started to work on the same ticket few weeks ago,here's my implementation (missing doc blocks and more tests, not yet finished) As far as I remember, this implementation has issue on windows platform and mine too. I've planned to check this on saturday hackday in SymfonyCon, will you be there@vektah ? |
vektah commentedDec 11, 2013
No I wont =( That seems like a fairly solid fix for this bug, though it wouldn't allow for other custom pipes - ie it might be possible to redirect one process's output into anothers input, again without a shell... Will $nullstream = fopen(defined('PHP_WINDOWS_VERSION_BUILD') ? 'NUL' : '/dev/null', 'c'); leave dangling file handles? I don't see an fclose... |
romainneutron commentedDec 12, 2013
Yep, we could think of a Passed streams should be fclosed the same way they arealready closehere About both implementation we have, outside the troubles it could bring on Windows (I'm gonna test it this week end) I've to admit I'd rather prefer easy natural method name on the Process class rather than manipulate |
vektah commentedDec 12, 2013
Why not both? disableOutput() is a nicer interface Set processPipes leaves the class open for extension but closed to modification. It's a pain when you need a feature not to be able to add it without changing base classes. disableOutput could internally setProcessPipes(new NullPipes()) |
stof commentedDec 12, 2013
|
vektah commentedDec 13, 2013
The other nice thing you could do with setProcessPipes is have a passthrough, binding to STDIN, STDOUT and STDERR. |
fabpot commentedMar 12, 2014
closed in favor of#10425 |
Currently processes that output large amounts of data will block. If
the output is not important then this becomes an issue. It can be worked
around by redirecting the output to > /dev/null but this requires an
instance of /bin/sh to do the work.
This patch adds the ability to set the processPipes, and a new
processPipe that redirects to /dev/null.