-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. | 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
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
I'm not sure what weirdness is expected here, I've added a test... |
ping @romainneutron |
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 ? |
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... |
Yep, we could think of a Passed streams should be fclosed the same way they arealready close here 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 |
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()) |
|
The other nice thing you could do with setProcessPipes is have a passthrough, binding to STDIN, STDOUT and STDERR. |
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.