child-process icon indicating copy to clipboard operation
child-process copied to clipboard

support $cmd as array in Process

Open staabm opened this issue 11 months ago • 4 comments

I recently started optimizing phpunit process isolation. while doing research I found out, that proc_open can start processes with a lot less overhead when using a array argument.

would be great if we could use Process with a array argument, so we can start processes with less overhead in e.g. PHPStan

staabm avatar Mar 24 '24 07:03 staabm

Just a random thought, but if it's faster, why not internally change it to an array and always have that performance gain?

WyriHaximus avatar Mar 24 '24 08:03 WyriHaximus

passing the arg as array has a few more implications. escaping of the command is done at php-src level and no longer in userland.

since the command is no longer run in a shell with array-notation it might depend also on the command whether we can apply array-style arg. e.g. stderr->stdout redirection needs to be done differently via pipespec

it might make sense to look into the symfony process component for inspiration

staabm avatar Mar 24 '24 08:03 staabm

Hey @staabm, thanks for the input on this, seems like a reasonable change :+1:

If this works as well as you described it above, than I agree that this is a great addition to the project. Will have a look at this when time allows it, but until then we're more than happy about contributions for this. If someone has interest in opening a PR, it's important for us is that the pull request should not only focus on the implementation side of things, but also make sure that any new additions are also covered by our test suite (unit tests) to confirm the suggested changes work as expected.

Additionally, I don't think this is a bug ticket as nothing is currently broken; I think this is more of a feature suggestion. We recommend opening bug tickets only when you can provide evidence of something being broken, just as a friendly reminder for future tickets :)

SimonFrings avatar Apr 18 '24 12:04 SimonFrings