From de526e0fdaf3fd60ec3bd8f6a272b5ecc014ee8c Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Wed, 9 Sep 2026 09:48:02 +0000 Subject: [PATCH] Spell out the ini files, `sys_temp_dir` and `xdebug.mode` of the current process on spawned PHP command lines - Add `PHPStan\Process\InheritedPhpConfig`, which resolves the PHP CLI options a child process needs to run with the PHP configuration of the process starting it: `-n` when this process read no additional ini files, `-c` for the loaded php.ini, `-d sys_temp_dir=` and `-d xdebug.mode=`. - `ProcessHelper::getWorkerCommand()` builds the worker command from it instead of passing `-c php_ini_loaded_file()` alone, so a worker no longer re-reads the ini scan directory the main process was started without, and no longer loses an `xdebug.mode` set on the command line. - `TurboProcessRestarter` had the same gap on the process it re-executes - extracted `resolveRestartArgs()` and prepended the inherited options there, so `php -d xdebug.mode=off vendor/bin/phpstan` no longer restarts into a process with Xdebug active again (and `-d sys_temp_dir=` no longer gets lost across the restart, which only the worker command used to repeat). - Same fix for the two other PHP processes PHPStan starts: the PHPStan Pro process in `FixerApplication` (which passed no `-c` at all) and the phar of each `bisect` step in `BisectCommand`. - `phpstan diagnose` prints the resulting php options for spawned workers. --- bin/phpstan | 1 + src/Command/BisectCommand.php | 11 +- src/Command/FixerApplication.php | 9 +- src/Parallel/ForkParallelChecker.php | 8 +- src/Process/InheritedPhpConfig.php | 102 ++++++++++++++++ src/Process/ProcessHelper.php | 25 ++-- src/Turbo/TurboProcessRestarter.php | 42 +++++-- .../Process/InheritedPhpConfigTest.php | 115 ++++++++++++++++++ tests/PHPStan/Process/ProcessHelperTest.php | 18 +++ .../PHPStan/Process/data/print-php-config.php | 31 +++++ .../Turbo/TurboProcessRestarterTest.php | 43 +++++++ 11 files changed, 375 insertions(+), 30 deletions(-) create mode 100644 src/Process/InheritedPhpConfig.php create mode 100644 tests/PHPStan/Process/InheritedPhpConfigTest.php create mode 100644 tests/PHPStan/Process/data/print-php-config.php diff --git a/bin/phpstan b/bin/phpstan index 3c415401328..458871545a0 100755 --- a/bin/phpstan +++ b/bin/phpstan @@ -25,6 +25,7 @@ use Symfony\Component\Console\Helper\ProgressBar; define('__PHPSTAN_RUNNING__', true); + require_once __DIR__ . '/../src/Process/InheritedPhpConfig.php'; require_once __DIR__ . '/../src/Turbo/TurboExtensionEnabler.php'; require_once __DIR__ . '/../src/Turbo/TurboExtensionSelector.php'; require_once __DIR__ . '/../src/Turbo/TurboProcessRestarter.php'; diff --git a/src/Command/BisectCommand.php b/src/Command/BisectCommand.php index 9c1e3932d0d..dd8bd041d80 100644 --- a/src/Command/BisectCommand.php +++ b/src/Command/BisectCommand.php @@ -10,6 +10,7 @@ use PHPStan\Command\Bisect\BinarySearch; use PHPStan\File\FileReader; use PHPStan\Internal\HttpClientFactory; +use PHPStan\Process\InheritedPhpConfig; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputArgument; @@ -19,6 +20,7 @@ use Symfony\Component\Console\Style\SymfonyStyle; use Throwable; use function array_filter; +use function array_map; use function array_merge; use function array_values; use function chmod; @@ -416,9 +418,16 @@ public function buildAnalyseArgs(InputInterface $input): string private function runAnalysis(string $pharPath, string $analyseArgs): int { + // every bisect step is a full analysis of its own, and a child process + // inherits nothing of our command line - without this each of them + // would run with an Xdebug the user turned off for us, see + // InheritedPhpConfig + $phpArgs = implode(' ', array_map(static fn (string $arg): string => escapeshellarg($arg), InheritedPhpConfig::getArgs())); + $command = sprintf( - '%s %s analyse %s', + '%s %s %s analyse %s', escapeshellarg(PHP_BINARY), + $phpArgs, escapeshellarg($pharPath), $analyseArgs, ); diff --git a/src/Command/FixerApplication.php b/src/Command/FixerApplication.php index f9b6f60d628..b01f25b0721 100644 --- a/src/Command/FixerApplication.php +++ b/src/Command/FixerApplication.php @@ -27,6 +27,7 @@ use PHPStan\Parallel\ForkParallelChecker; use PHPStan\PhpDoc\StubFilesProvider; use PHPStan\Process\ForkedProcessPromise; +use PHPStan\Process\InheritedPhpConfig; use PHPStan\Process\ProcessCanceledException; use PHPStan\Process\ProcessCrashedException; use PHPStan\Process\ProcessHelper; @@ -43,12 +44,14 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Throwable; +use function array_map; use function array_merge; use function count; use function defined; use function escapeshellarg; use function get_class; use function http_build_query; +use function implode; use function ini_get; use function is_file; use function parse_url; @@ -302,7 +305,11 @@ private function getFixerProcess(OutputInterface $output, int $serverPort): Proc } } - return new Process(sprintf('%s -d memory_limit=%s %s --port %d', escapeshellarg(PHP_BINARY), escapeshellarg(ini_get('memory_limit')), escapeshellarg($pharPath), $serverPort), env: $env, fds: []); + // the PHPStan Pro process is a child like a worker is - it inherits + // nothing of our command line either, see InheritedPhpConfig + $phpArgs = implode(' ', array_map(static fn (string $arg): string => escapeshellarg($arg), InheritedPhpConfig::getArgs())); + + return new Process(sprintf('%s %s -d memory_limit=%s %s --port %d', escapeshellarg(PHP_BINARY), $phpArgs, escapeshellarg(ini_get('memory_limit')), escapeshellarg($pharPath), $serverPort), env: $env, fds: []); } /** diff --git a/src/Parallel/ForkParallelChecker.php b/src/Parallel/ForkParallelChecker.php index 87f95feede6..5699a7559ba 100644 --- a/src/Parallel/ForkParallelChecker.php +++ b/src/Parallel/ForkParallelChecker.php @@ -6,11 +6,13 @@ use PHPStan\Command\Output; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Diagnose\DiagnoseExtension; +use PHPStan\Process\InheritedPhpConfig; use PHPStan\Process\ProcessHelper; use PHPStan\Turbo\TurboExtensionEnabler; use PHPStan\Turbo\TurboProcessRestarter; use function function_exists; use function getmypid; +use function implode; use function opcache_get_status; use function sprintf; use function str_starts_with; @@ -68,8 +70,10 @@ public function print(Output $output): void $output->writeLineFormatted('Mechanism: spawn (react/child-process)'); $output->writeLineFormatted(sprintf('Reason fork not used: %s', $reason)); - // what a spawned worker's command line adds on top of the php.ini - // (see ProcessHelper); the extension path is on the turbo lines + // what a spawned worker's command line spells out for it (see + // ProcessHelper); the extension path is on the turbo lines + $output->writeLineFormatted(sprintf('Worker php options: %s', implode(' ', InheritedPhpConfig::getArgs()))); + $parentPid = getmypid(); $output->writeLineFormatted('Worker -d entries:'); foreach (ProcessHelper::resolveWorkerIniEntries(TurboProcessRestarter::getOpcacheArgs(), PHP_OS_FAMILY, $parentPid === false ? 0 : $parentPid, 1) as $iniEntry) { diff --git a/src/Process/InheritedPhpConfig.php b/src/Process/InheritedPhpConfig.php new file mode 100644 index 00000000000..96e4d8c159a --- /dev/null +++ b/src/Process/InheritedPhpConfig.php @@ -0,0 +1,102 @@ + + */ + public static function getArgs(): array + { + return self::resolveArgs(php_ini_loaded_file(), php_ini_scanned_files(), sys_get_temp_dir(), self::getXdebugMode()); + } + + /** + * The xdebug.mode in effect, or false when nothing set it. + * + * ini_get() answers only for a loaded Xdebug - for any other PHP the + * directive is not registered and only the raw ini entry exists, which is + * what a child loading Xdebug when we do not would be configured by. + */ + private static function getXdebugMode(): string|false + { + $mode = ini_get('xdebug.mode'); + if ($mode !== false) { + return $mode; + } + + $mode = get_cfg_var('xdebug.mode'); + + return is_string($mode) ? $mode : false; + } + + /** + * @param string|false $loadedIniFile php_ini_loaded_file() of the spawning process + * @param string|false $scannedIniFiles php_ini_scanned_files() of the spawning process + * @param string $tempDir sys_get_temp_dir() of the spawning process + * @param string|false $xdebugMode see getXdebugMode() + * @return list + */ + public static function resolveArgs(string|false $loadedIniFile, string|false $scannedIniFiles, string $tempDir, string|false $xdebugMode): array + { + $args = []; + if ($scannedIniFiles === false || trim($scannedIniFiles) === '') { + // -n only suppresses the scan directory here: an explicit -c is + // still honored next to it, the way xdebug-handler restarts + $args[] = '-n'; + } + if ($loadedIniFile !== false && $loadedIniFile !== '') { + $args[] = '-c'; + $args[] = $loadedIniFile; + } + $args[] = '-d'; + // quote value so PHP will parse it as a string when the path contains a bitwise operator like ~ + $args[] = "sys_temp_dir='" . $tempDir . "'"; + if ($xdebugMode !== false) { + $args[] = '-d'; + $args[] = 'xdebug.mode=' . $xdebugMode; + } + + return $args; + } + +} diff --git a/src/Process/ProcessHelper.php b/src/Process/ProcessHelper.php index 2af7feadfd7..907bc2f2602 100644 --- a/src/Process/ProcessHelper.php +++ b/src/Process/ProcessHelper.php @@ -12,9 +12,7 @@ use function implode; use function ini_get; use function is_bool; -use function php_ini_loaded_file; use function sprintf; -use function sys_get_temp_dir; use const PHP_BINARY; use const PHP_OS_FAMILY; @@ -23,11 +21,11 @@ * and SpawnedProcessPromise). * * Besides the worker command and its options it spells out the PHP - * configuration the worker runs with. The php.ini is inherited through - * `-c`, but command-line `-d` entries are not, so whatever the spawning - * process got that way - the turbo extension and the OPcache setup of the - * TurboProcessRestarter restart - is repeated here; see - * resolveWorkerIniEntries() for the set and the reasoning. + * configuration the worker runs with. Nothing of a command line is inherited + * by a child process, so whatever the spawning process got that way is + * repeated here: the php.ini situation it runs with (InheritedPhpConfig), and + * the turbo extension and the OPcache setup of the TurboProcessRestarter + * restart - see resolveWorkerIniEntries() for that set and the reasoning. */ final class ProcessHelper { @@ -46,15 +44,10 @@ public static function getWorkerCommand( InputInterface $input, ): string { - $phpIni = php_ini_loaded_file(); - $phpCmd = $phpIni === false ? escapeshellarg(PHP_BINARY) : sprintf('%s -c %s', escapeshellarg(PHP_BINARY), escapeshellarg($phpIni)); - - $processCommandArray = [ - $phpCmd, - '-d', - // quote value so PHP will parse it as a string when the path contains a bitwise operator like ~ - 'sys_temp_dir=' . escapeshellarg("'" . sys_get_temp_dir() . "'"), - ]; + $processCommandArray = [escapeshellarg(PHP_BINARY)]; + foreach (InheritedPhpConfig::getArgs() as $inheritedArg) { + $processCommandArray[] = escapeshellarg($inheritedArg); + } if ($input->getOption('memory-limit') === null) { $processCommandArray[] = '-d'; diff --git a/src/Turbo/TurboProcessRestarter.php b/src/Turbo/TurboProcessRestarter.php index b1580b3874e..c490349e517 100644 --- a/src/Turbo/TurboProcessRestarter.php +++ b/src/Turbo/TurboProcessRestarter.php @@ -2,6 +2,7 @@ namespace PHPStan\Turbo; +use PHPStan\Process\InheritedPhpConfig; use function explode; use function extension_loaded; use function function_exists; @@ -11,7 +12,6 @@ use function is_string; use function max; use function pcntl_exec; -use function php_ini_loaded_file; use function strtolower; use function trim; use const PHP_BINARY; @@ -129,14 +129,37 @@ public static function restartIfSuitable(array $argv): void return; } - $args = []; - $phpIni = php_ini_loaded_file(); - if ($phpIni !== false) { - $args[] = '-c'; - $args[] = $phpIni; - } + pcntl_exec(PHP_BINARY, self::resolveRestartArgs( + InheritedPhpConfig::getArgs(), + $opcacheArgs, + $extensionPath, + ini_get('memory_limit'), + $argv, + )); + // pcntl_exec() returns only on failure — continue as we are + } + + /** + * The whole command line of the restarted process, php options first. + * + * The restart replaces the process, so everything the current command line + * gave it and a child process does not inherit has to be spelled out again + * - the php.ini situation and the Xdebug mode of InheritedPhpConfig just as + * much as the OPcache setup this restart exists for. Without the former, + * `php -d xdebug.mode=off vendor/bin/phpstan` restarted into a process with + * Xdebug active again, which xdebug-handler then had to restart a second + * time. + * + * @param list $inheritedArgs InheritedPhpConfig::getArgs() + * @param list $opcacheArgs getOpcacheArgs() + * @param list $argv the current $_SERVER['argv'], php options already stripped from it + * @return list + */ + public static function resolveRestartArgs(array $inheritedArgs, array $opcacheArgs, ?string $extensionPath, string $memoryLimit, array $argv): array + { + $args = $inheritedArgs; $args[] = '-d'; - $args[] = 'memory_limit=' . ini_get('memory_limit'); + $args[] = 'memory_limit=' . $memoryLimit; foreach ($opcacheArgs as $opcacheArg) { $args[] = '-d'; $args[] = $opcacheArg; @@ -153,8 +176,7 @@ public static function restartIfSuitable(array $argv): void $args[] = $arg; } - pcntl_exec(PHP_BINARY, $args); - // pcntl_exec() returns only on failure — continue as we are + return $args; } /** diff --git a/tests/PHPStan/Process/InheritedPhpConfigTest.php b/tests/PHPStan/Process/InheritedPhpConfigTest.php new file mode 100644 index 00000000000..9a350f7bd9b --- /dev/null +++ b/tests/PHPStan/Process/InheritedPhpConfigTest.php @@ -0,0 +1,115 @@ +}> + */ + public static function dataResolveArgs(): iterable + { + yield 'php.ini and a scan directory' => [ + '/etc/php/php.ini', + '/etc/php/conf.d/10-opcache.ini,/etc/php/conf.d/20-xdebug.ini', + false, + ['-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'"], + ]; + yield 'no scanned ini files' => [ + '/etc/php/php.ini', + false, + false, + ['-n', '-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'"], + ]; + yield 'an empty scan directory' => [ + '/etc/php/php.ini', + "\n", + false, + ['-n', '-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'"], + ]; + yield 'no php.ini at all' => [ + false, + false, + false, + ['-n', '-d', "sys_temp_dir='/tmp'"], + ]; + yield 'an empty php.ini path' => [ + '', + '/etc/php/conf.d/20-xdebug.ini', + false, + ['-d', "sys_temp_dir='/tmp'"], + ]; + yield 'Xdebug turned off on the command line' => [ + '/etc/php/php.ini', + '/etc/php/conf.d/20-xdebug.ini', + // what the ini parser makes of -d xdebug.mode=off + '', + ['-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'", '-d', 'xdebug.mode='], + ]; + yield 'Xdebug left on' => [ + '/etc/php/php.ini', + '/etc/php/conf.d/20-xdebug.ini', + 'debug,develop', + ['-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'", '-d', 'xdebug.mode=debug,develop'], + ]; + yield 'Xdebug turned off with no ini file to inherit' => [ + false, + false, + '', + ['-n', '-d', "sys_temp_dir='/tmp'", '-d', 'xdebug.mode='], + ]; + } + + /** + * @param string|false $loadedIniFile + * @param string|false $scannedIniFiles + * @param string|false $xdebugMode + * @param list $expected + */ + #[DataProvider('dataResolveArgs')] + public function testResolveArgs($loadedIniFile, $scannedIniFiles, $xdebugMode, array $expected): void + { + $this->assertSame($expected, InheritedPhpConfig::resolveArgs($loadedIniFile, $scannedIniFiles, '/tmp', $xdebugMode)); + } + + /** + * @return iterable}> + */ + public static function dataChildProcessRepeatsThePhpConfigurationOfItsParent(): iterable + { + yield 'as started' => [[]]; + // https://github.com/phpstan/phpstan/issues/15189 - Xdebug stayed + // enabled in the child processes because -d entries are not inherited + yield 'with Xdebug turned off' => [['-d', 'xdebug.mode=off']]; + yield 'without the ini scan directory' => [['-n']]; + } + + /** + * @param list $phpOptions + */ + #[DataProvider('dataChildProcessRepeatsThePhpConfigurationOfItsParent')] + public function testChildProcessRepeatsThePhpConfigurationOfItsParent(array $phpOptions): void + { + $command = implode(' ', array_map( + static fn (string $arg): string => escapeshellarg($arg), + [PHP_BINARY, ...$phpOptions, __DIR__ . '/data/print-php-config.php'], + )); + exec($command, $outputLines, $exitCode); + $this->assertSame(0, $exitCode, implode("\n", $outputLines)); + + $result = json_decode(implode('', $outputLines), true); + $this->assertIsArray($result); + $this->assertSame($result['parent'], $result['child']); + } + +} diff --git a/tests/PHPStan/Process/ProcessHelperTest.php b/tests/PHPStan/Process/ProcessHelperTest.php index 20f1fcf07b4..6cfd19748d2 100644 --- a/tests/PHPStan/Process/ProcessHelperTest.php +++ b/tests/PHPStan/Process/ProcessHelperTest.php @@ -9,9 +9,12 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputDefinition; use Symfony\Component\Console\Input\InputOption; +use function array_map; use function escapeshellarg; +use function implode; use function preg_match; use function sprintf; +use const PHP_BINARY; use const PHP_OS_FAMILY; final class ProcessHelperTest extends TestCase @@ -40,6 +43,21 @@ public function testResolveWorkerIniEntries(array $opcacheArgs, string $osFamily $this->assertSame($expected, ProcessHelper::resolveWorkerIniEntries($opcacheArgs, $osFamily, 4242, 7)); } + public function testWorkerCommandRepeatsThePhpConfigurationOfTheSpawningProcess(): void + { + // nothing of a command line is inherited by a child process, so the + // php.ini situation and the Xdebug mode of this process have to be + // spelled out again - https://github.com/phpstan/phpstan/issues/15189 + $command = ProcessHelper::getWorkerCommand('bin/phpstan', 'worker', null, ['--port', '1234'], $this->createInput()); + + $expectedPrefix = implode(' ', array_map( + static fn (string $arg): string => escapeshellarg($arg), + [PHP_BINARY, ...InheritedPhpConfig::getArgs()], + )); + + $this->assertStringStartsWith($expectedPrefix . ' ', $command); + } + public function testWorkerCommandCarriesTheIniEntries(): void { $command = ProcessHelper::getWorkerCommand('bin/phpstan', 'worker', null, ['--port', '1234'], $this->createInput()); diff --git a/tests/PHPStan/Process/data/print-php-config.php b/tests/PHPStan/Process/data/print-php-config.php new file mode 100644 index 00000000000..f76895d60ae --- /dev/null +++ b/tests/PHPStan/Process/data/print-php-config.php @@ -0,0 +1,31 @@ + [ + 'loadedIniFile' => php_ini_loaded_file(), + 'scannedIniFiles' => php_ini_scanned_files() !== false, + 'xdebugMode' => get_cfg_var('xdebug.mode'), + 'tempDir' => sys_get_temp_dir(), +]; + +if (($argv[1] ?? '') === 'child') { + echo json_encode($readConfig()); + + return; +} + +$command = implode(' ', array_merge( + [escapeshellarg(PHP_BINARY)], + array_map(static fn (string $arg): string => escapeshellarg($arg), PHPStan\Process\InheritedPhpConfig::getArgs()), + [escapeshellarg(__FILE__), 'child'], +)); + +echo json_encode([ + 'parent' => $readConfig(), + 'child' => json_decode((string) shell_exec($command), true), +]); diff --git a/tests/PHPStan/Turbo/TurboProcessRestarterTest.php b/tests/PHPStan/Turbo/TurboProcessRestarterTest.php index d4de751402e..f9a4a683aa6 100644 --- a/tests/PHPStan/Turbo/TurboProcessRestarterTest.php +++ b/tests/PHPStan/Turbo/TurboProcessRestarterTest.php @@ -177,4 +177,47 @@ public function testResolveOpcacheRestartNeeded(array $currentIniValues, bool $e $this->assertSame($expected, TurboProcessRestarter::resolveOpcacheRestartNeeded(self::STOCK_ARGS, $currentIniValues)); } + public function testRestartArgsCarryThePhpConfigurationOfTheCurrentProcess(): void + { + // the restart replaces the process, so what the command line gave it - + // the ini files it reads and the Xdebug mode - has to be repeated + $inheritedArgs = ['-n', '-c', '/etc/php/php.ini', '-d', "sys_temp_dir='/tmp'", '-d', 'xdebug.mode=']; + $args = TurboProcessRestarter::resolveRestartArgs($inheritedArgs, ['opcache.enable_cli=1'], null, '256M', ['bin/phpstan', 'analyse']); + + $this->assertSame([ + '-n', + '-c', + '/etc/php/php.ini', + '-d', + "sys_temp_dir='/tmp'", + '-d', + 'xdebug.mode=', + '-d', + 'memory_limit=256M', + '-d', + 'opcache.enable_cli=1', + '-d', + 'phpstan.restarted=1', + 'bin/phpstan', + 'analyse', + ], $args); + } + + public function testRestartArgsCarryTheTurboExtension(): void + { + $args = TurboProcessRestarter::resolveRestartArgs([], [], '/tmp/phpstan_turbo.so', '-1', ['bin/phpstan']); + + $this->assertSame([ + '-d', + 'memory_limit=-1', + '-d', + 'extension=/tmp/phpstan_turbo.so', + '-d', + 'phpstan.turboExtensionPath=/tmp/phpstan_turbo.so', + '-d', + 'phpstan.restarted=1', + 'bin/phpstan', + ], $args); + } + }