Skip to content

Conversation

@crazywhalecc
Copy link
Owner

What does this PR do?

Fix #854 .

The proc_open does not work with windows cmd very well. The popen could avoid deadlock and infinite loop. For other OS, keeping proc_open is better.

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php or *.json, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:sort-config
  • If it's an extension or dependency update, please ensure the following:
    • Add your test combination to src/globals/test-extensions.php.
    • If adding new or fixing bugs, add commit message containing extension test or test extensions to trigger full test suite.

@henderkes
Copy link
Collaborator

henderkes commented Aug 16, 2025

Have you tried setting

proc_open(..., options: ['bypass_shell' => true]);

?

Base automatically changed from fix/exception-reflection to main August 16, 2025 11:33
@crazywhalecc
Copy link
Owner Author

crazywhalecc commented Aug 16, 2025

proc_open with bypass_shell=true does not accept cd /d "\dir" && $cmd like this. And if we wrap cmd /c it performs exactly the same.

@henderkes
Copy link
Collaborator

Oh, indeed. I will look at this tomorrow. In Udon for the weekend.

Copy link
Collaborator

@henderkes henderkes left a comment

Choose a reason for hiding this comment

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

yeah with the directory change in the shell, I don't see a better way for now

@crazywhalecc crazywhalecc merged commit cbddb26 into main Aug 18, 2025
1 check passed
@crazywhalecc crazywhalecc deleted the fix/windows-cmd branch August 18, 2025 04:07
@crazywhalecc
Copy link
Owner Author

@henderkes Well, I should noticed that I can use search engine to solve the problem.

php/php-src#5777

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows weekly build unexpectedly timed out

3 participants