diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index f0d1a19ad6e..aa99478278d 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -2576,6 +2576,17 @@ jobs: fi php check.php restarted-for=xdebug,opcache xdebug-armed=no xdebug-loaded=yes processes=2+ starts=2 + # xdebug armed in php.ini, disarmed with `php -d xdebug.mode=off`: + # nothing to restart for, and the spawned workers do not inherit the + # -d flag - ProcessHelper has to pass the switch on, or they run with + # the php.ini's xdebug.mode + - operating-system: "windows-latest" + coverage: "xdebug" + extensions: "" + script: | + php -d xdebug.mode=off ../../bin/phpstan analyse --no-progress + php check.php restarted-for= xdebug-armed=no xdebug-loaded=yes processes=2+ starts=1 + # XDEBUG_MODE overrides xdebug.mode, so flagging the ini alone would # be silently overridable - the row exports the env var too - operating-system: "ubuntu-latest" diff --git a/src/Parallel/ForkParallelChecker.php b/src/Parallel/ForkParallelChecker.php index 38da59a37ba..0645576069d 100644 --- a/src/Parallel/ForkParallelChecker.php +++ b/src/Parallel/ForkParallelChecker.php @@ -8,6 +8,7 @@ use PHPStan\Diagnose\DiagnoseExtension; use PHPStan\Process\ProcessHelper; use PHPStan\Process\Restart\OpcacheSettings; +use PHPStan\Process\Restart\RestartExtensionRegistry; use PHPStan\Turbo\TurboExtensionEnabler; use function function_exists; use function getmypid; @@ -72,7 +73,7 @@ public function print(Output $output): void // (see ProcessHelper); the extension path is on the turbo lines $parentPid = getmypid(); $output->writeLineFormatted('Worker -d entries:'); - foreach (ProcessHelper::resolveWorkerIniEntries(OpcacheSettings::getIniEntries(), PHP_OS_FAMILY, $parentPid === false ? 0 : $parentPid, 1) as $iniEntry) { + foreach (ProcessHelper::resolveWorkerIniEntries(OpcacheSettings::getIniEntries(), RestartExtensionRegistry::getStartupSwitches(), PHP_OS_FAMILY, $parentPid === false ? 0 : $parentPid, 1) as $iniEntry) { $output->writeLineFormatted(sprintf( ' %s%s', $iniEntry, diff --git a/src/Process/ProcessHelper.php b/src/Process/ProcessHelper.php index a24975c10f5..460e119b41a 100644 --- a/src/Process/ProcessHelper.php +++ b/src/Process/ProcessHelper.php @@ -4,6 +4,7 @@ use PHPStan\Command\AnalyseCommand; use PHPStan\Process\Restart\OpcacheSettings; +use PHPStan\Process\Restart\RestartExtensionRegistry; use PHPStan\Process\Restart\RestartManager; use PHPStan\Turbo\TurboExtensionSelector; use Symfony\Component\Console\Input\InputInterface; @@ -76,9 +77,10 @@ public static function getWorkerCommand( $parentPid = getmypid(); self::$spawnedWorkerCount++; - foreach (self::resolveWorkerIniEntries(OpcacheSettings::getIniEntries(), PHP_OS_FAMILY, $parentPid === false ? 0 : $parentPid, self::$spawnedWorkerCount) as $iniEntry) { + foreach (self::resolveWorkerIniEntries(OpcacheSettings::getIniEntries(), RestartExtensionRegistry::getStartupSwitches(), PHP_OS_FAMILY, $parentPid === false ? 0 : $parentPid, self::$spawnedWorkerCount) as $iniEntry) { $processCommandArray[] = '-d'; - $processCommandArray[] = $iniEntry; + // xdebug.mode can be a list with spaces in it + $processCommandArray[] = escapeshellarg($iniEntry); } foreach ([$mainScript, $commandName] as $arg) { @@ -154,18 +156,28 @@ public static function getWorkerCommand( * model spawned workers have elsewhere, where the segment is an anonymous * mapping private to the process. * + * The startup switches of xdebug, pcov and uopz are repeated as the + * spawning process has them. Where it got them through `-d` - `php -d + * xdebug.mode=off` with xdebug armed in the php.ini - it had nothing to + * restart for, and the worker would otherwise run with the php.ini's + * armed extension. + * * The restart marker closes the loop: the worker's configuration is * decided here, so the worker must not restart itself. * * @param list $opcacheArgs OpcacheSettings::getIniEntries() of the spawning process + * @param array $startupSwitches RestartExtensionRegistry::getStartupSwitches() of the spawning process * @return list */ - public static function resolveWorkerIniEntries(array $opcacheArgs, string $osFamily, int $parentPid, int $workerNumber): array + public static function resolveWorkerIniEntries(array $opcacheArgs, array $startupSwitches, string $osFamily, int $parentPid, int $workerNumber): array { $entries = $opcacheArgs; if ($entries !== [] && $osFamily === 'Windows') { $entries[] = sprintf('opcache.cache_id=phpstan-%d-%d', $parentPid, $workerNumber); } + foreach ($startupSwitches as $name => $value) { + $entries[] = sprintf('%s=%s', $name, $value); + } $entries[] = RestartManager::RESTARTED_INI . '=1'; return $entries; diff --git a/src/Process/Restart/RestartExtensionRegistry.php b/src/Process/Restart/RestartExtensionRegistry.php index 55fc5e783d8..cf69c49df33 100644 --- a/src/Process/Restart/RestartExtensionRegistry.php +++ b/src/Process/Restart/RestartExtensionRegistry.php @@ -90,15 +90,7 @@ final class RestartExtensionRegistry */ public static function collectReasons(bool $allowXdebug, ?string $pharPath = null): array { - $iniSettings = []; - foreach (['xdebug.mode', 'pcov.enabled', 'uopz.disable'] as $name) { - $value = ini_get($name); - if ($value === false) { - continue; - } - - $iniSettings[$name] = $value; - } + $iniSettings = self::getStartupSwitches(); // XDEBUG_MODE wins over the ini file, so the effective mode - not the // configured one - is what decides. Resolved here rather than in @@ -120,6 +112,30 @@ public static function collectReasons(bool $allowXdebug, ?string $pharPath = nul ); } + /** + * The startup switches of the flag rows (xdebug.mode, pcov.enabled, + * uopz.disable) as the current process has them, for each extension that + * is loaded. A switch set with `-d` - `php -d xdebug.mode=off` - leaves + * nothing to restart for, and is not inherited by spawned workers either, + * so ProcessHelper passes these on. + * + * @return array + */ + public static function getStartupSwitches(): array + { + $iniSettings = []; + foreach (['xdebug.mode', 'pcov.enabled', 'uopz.disable'] as $name) { + $value = ini_get($name); + if ($value === false) { + continue; + } + + $iniSettings[$name] = $value; + } + + return $iniSettings; + } + /** * @param list $loadedExtensions * @param array $iniSettings diff --git a/src/Process/Restart/RestartManager.php b/src/Process/Restart/RestartManager.php index e701548eb7f..1cf45099576 100644 --- a/src/Process/Restart/RestartManager.php +++ b/src/Process/Restart/RestartManager.php @@ -458,6 +458,13 @@ private static function getPassedThroughArguments(): array $arguments[] = '-d'; $arguments[] = 'memory_limit=' . ini_get('memory_limit'); + // `php -d xdebug.mode=off` left nothing to restart for, and the + // restarted process must not get the php.ini's armed xdebug back + foreach (RestartExtensionRegistry::getStartupSwitches() as $name => $value) { + $arguments[] = '-d'; + $arguments[] = $name . '=' . $value; + } + return $arguments; } diff --git a/tests/PHPStan/Process/ProcessHelperTest.php b/tests/PHPStan/Process/ProcessHelperTest.php index 13cd94bd7e4..c063374efa1 100644 --- a/tests/PHPStan/Process/ProcessHelperTest.php +++ b/tests/PHPStan/Process/ProcessHelperTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Process; use PHPStan\Process\Restart\OpcacheSettings; +use PHPStan\Process\Restart\RestartExtensionRegistry; use PHPStan\Process\Restart\RestartManager; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -19,26 +20,33 @@ final class ProcessHelperTest extends TestCase { /** - * @return iterable, string, list}> + * @return iterable, array, string, list}> */ public static function dataResolveWorkerIniEntries(): iterable { $opcacheArgs = ['opcache.enable=1', 'opcache.enable_cli=1']; - yield 'linux' => [$opcacheArgs, 'Linux', ['opcache.enable=1', 'opcache.enable_cli=1', 'phpstan.restarted=1']]; - yield 'darwin' => [$opcacheArgs, 'Darwin', ['opcache.enable=1', 'opcache.enable_cli=1', 'phpstan.restarted=1']]; - yield 'windows' => [$opcacheArgs, 'Windows', ['opcache.enable=1', 'opcache.enable_cli=1', 'opcache.cache_id=phpstan-4242-7', 'phpstan.restarted=1']]; - yield 'linux without OPcache' => [[], 'Linux', ['phpstan.restarted=1']]; - yield 'windows without OPcache' => [[], 'Windows', ['phpstan.restarted=1']]; + yield 'linux' => [$opcacheArgs, [], 'Linux', ['opcache.enable=1', 'opcache.enable_cli=1', 'phpstan.restarted=1']]; + yield 'darwin' => [$opcacheArgs, [], 'Darwin', ['opcache.enable=1', 'opcache.enable_cli=1', 'phpstan.restarted=1']]; + yield 'windows' => [$opcacheArgs, [], 'Windows', ['opcache.enable=1', 'opcache.enable_cli=1', 'opcache.cache_id=phpstan-4242-7', 'phpstan.restarted=1']]; + yield 'linux without OPcache' => [[], [], 'Linux', ['phpstan.restarted=1']]; + yield 'windows without OPcache' => [[], [], 'Windows', ['phpstan.restarted=1']]; + yield 'windows with xdebug and pcov switched off by -d' => [ + $opcacheArgs, + ['xdebug.mode' => 'off', 'pcov.enabled' => '0'], + 'Windows', + ['opcache.enable=1', 'opcache.enable_cli=1', 'opcache.cache_id=phpstan-4242-7', 'xdebug.mode=off', 'pcov.enabled=0', 'phpstan.restarted=1'], + ]; } /** * @param list $opcacheArgs + * @param array $startupSwitches * @param list $expected */ #[DataProvider('dataResolveWorkerIniEntries')] - public function testResolveWorkerIniEntries(array $opcacheArgs, string $osFamily, array $expected): void + public function testResolveWorkerIniEntries(array $opcacheArgs, array $startupSwitches, string $osFamily, array $expected): void { - $this->assertSame($expected, ProcessHelper::resolveWorkerIniEntries($opcacheArgs, $osFamily, 4242, 7)); + $this->assertSame($expected, ProcessHelper::resolveWorkerIniEntries($opcacheArgs, $startupSwitches, $osFamily, 4242, 7)); } public function testWorkerCommandCarriesTheIniEntries(): void @@ -48,12 +56,16 @@ public function testWorkerCommandCarriesTheIniEntries(): void $opcacheArgs = OpcacheSettings::getIniEntries(); foreach ($opcacheArgs as $opcacheArg) { - $this->assertStringContainsString(sprintf(' -d %s ', $opcacheArg), $command); + $this->assertStringContainsString(sprintf(' -d %s ', escapeshellarg($opcacheArg)), $command); + } + + foreach (RestartExtensionRegistry::getStartupSwitches() as $name => $value) { + $this->assertStringContainsString(sprintf(' -d %s ', escapeshellarg($name . '=' . $value)), $command); } // the restart marker is the last -d, right before the script $this->assertStringContainsString( - sprintf(' -d %s=1 %s %s ', RestartManager::RESTARTED_INI, escapeshellarg('bin/phpstan'), escapeshellarg('worker')), + sprintf(' -d %s %s %s ', escapeshellarg(RestartManager::RESTARTED_INI . '=1'), escapeshellarg('bin/phpstan'), escapeshellarg('worker')), $command, ); @@ -63,8 +75,8 @@ public function testWorkerCommandCarriesTheIniEntries(): void } // every Windows worker gets its own OPcache instance - $this->assertSame(1, preg_match('~ -d opcache\.cache_id=(phpstan-\d+-\d+) ~', $command, $matches)); - $this->assertSame(1, preg_match('~ -d opcache\.cache_id=(phpstan-\d+-\d+) ~', $commandAgain, $matchesAgain)); + $this->assertSame(1, preg_match('~ -d [\'"]?opcache\.cache_id=(phpstan-\d+-\d+)[\'"]? ~', $command, $matches)); + $this->assertSame(1, preg_match('~ -d [\'"]?opcache\.cache_id=(phpstan-\d+-\d+)[\'"]? ~', $commandAgain, $matchesAgain)); $this->assertNotSame($matches[1], $matchesAgain[1]); }