symfony / flex

Composer plugin for Symfony
MIT License
4.15k stars 181 forks source link

fix: Correctly inherit the Composer restarted process settings #995

Closed theofidry closed 9 months ago

theofidry commented 10 months ago

[!WARNING] Note: this PR is more of draft. I am not confident I understand fully the issue neither that this is the right fix (and I could not test it). Do NOT assume this code just works.

After a lot of digging in https://github.com/box-project/box/issues/988, @maartendekeizer could identify the root of the issue.

My understanding is that Flex tries to detect if the current PHP process was restarted by Composer and forwards its restarted settings to the sub-processes it is going to launch. There is currently two things done:

As mentioned in the warning, I could not test this, not even run the tests locally, so I would be careful about this PR. I just wanted to give a base about a potential fix with context about the original issue encountered.

On my side to not have to wait on Flex and avoid the users to have to update it to have the fix, Box launches its Composer commands with COMPOSER_ORIGINAL_INIS='' to avoid Flex to trigger the problematic piece of code.

maartendekeizer commented 10 months ago

A side note about the original code.

The COMPOSER_ORIGINAL_INIS will contain values like: /etc/php/8.2/cli/php.ini:/etc/php/8.2/cli/conf.d/10-mysqlnd.ini:/etc/php/8.2/cli/conf.d/10-opcache.ini

By exploding this string and take the first item (in this example /etc/php/8.2/cli/php.ini) and then find the directory of the file (/etc/php/8.2/cli) and using this directory as ini scan dir we still not finding other locations. If the env is for example /etc/php/8.2/cli/php.ini:/etc/php/8.2/cli/conf.d/10-mysqlnd.ini:/etc/php/8.2/cli/conf.d/10-opcache.ini:/home/website/config/99-usersettings.ini the /home/website/config directory is never scanned for the symfony-cmd command.

nicolas-grekas commented 10 months ago

Please let us know when you're more confident with the proposed changes.

theofidry commented 10 months ago

Pew I have no idea. Part of the issue is, I'm not sure how you test this. How have you tested it so far?

theofidry commented 9 months ago

Closing as actually this is working as intended. Flex wants to execute a PHP Process as if it was the original command, not the restarted process (if restarted) with modified settings. So if Composer disables Xdebug, the process that flex starts should have Xdebug on.

The current code could be refactored to use XdebugHandler still instead of the environment variable directly. But it is not a requirement.