Skip to content
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

Introducing a new PhpSubprocess handler #48485

Open
wants to merge 1 commit into
base: 6.3
Choose a base branch
from

Conversation

Toflar
Copy link
Contributor

@Toflar Toflar commented Dec 5, 2022

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR will provide as soon as the API is clear

This PR provides a new PhpSubprocess handler which can be used to run PHP commands and taking over their parent PHP configuration. This is not super easy in PHP because the only way to do that requires a temporary ini file.
That's why I see perfect fit for it in the symfony/process component as it solves a common issue.

The code is heavily inspired/copied from composer/xdebug-handler where the requirement is similar - being able to have a child process/subprocess that takes over the config of the parent process. Hence, mentioning @Seldaek here.

Basically, this can be very useful in Symfony when you want to run bin/console other-command from within Symfony.
Let's imagine a general cronjob command that is supposed to delegate to other commands like this (simplified a lot):

#[AsCommand(name: 'run-cronjobs')]
class RunCronJobsCommand extends Command
{
    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $cron1 = new Process(['bin/console', 'cache:pool:prune']);
        $cron2 = new Process(['bin/console', 'something:else']);
        // etc.
    }
}

Now if you run the cronjob command itself with dynamic configuration like so: php -d memory_limit=256M bin/console run-cronjobs, the subprocesses (cache:pool:prune etc.) will not be limited to 256M because they themselves will start a completely new PHP process which will take the default PHP settings again.
This affects everything, not just memory_limit but also runtime, extensions etc. You name it.

Oftentimes, however, that's not what you want. You'd like to have the children have the same restrictions as your parent process.

With this new handler, all you have to do is to use PhpSubprocess instead of Process and it will create a temporary ini file taking over all the settings of the parent process. Done 😊

The current design is pretty straightforward without any extension points just yet (no way to fetch the ini files separately etc.). Also, it could maybe do with some more tests for the ini parsing but at this stage I would like to ask if this would even be a welcomed addition to Symfony or not 😊

use Symfony\Component\Process\PhpSubprocess;
use Symfony\Component\Process\Process;

require \dirname(__DIR__).'/vendor/autoload.php';
Copy link
Contributor Author

@Toflar Toflar Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently causes an issue because that file does not exist on the mono repository. However, we already do have the same code here: https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/Process/Tests/ErrorProcessInitiator.php
That's actually a bug. That test should fail already and get fixed. Not sure how but I've added a an issue here: #48486
I will adapt the PR to the solution found to that ticket :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants