From 6e527a2932c7bfcdc83a2988541f2d4ee5153382 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 24 Oct 2024 10:39:06 +0100 Subject: [PATCH] Adjust timeouts for invoked processes, especially make which can take time --- src/Building/UnixBuild.php | 30 ++++++++----- src/Installing/UnixInstall.php | 12 ++++-- src/Platform/TargetPhp/PhpBinaryPath.php | 54 ++++++++---------------- src/Util/Process.php | 38 +++++++++++++++++ 4 files changed, 84 insertions(+), 50 deletions(-) create mode 100644 src/Util/Process.php diff --git a/src/Building/UnixBuild.php b/src/Building/UnixBuild.php index b965f8b..2673280 100644 --- a/src/Building/UnixBuild.php +++ b/src/Building/UnixBuild.php @@ -7,8 +7,8 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\Platform\TargetPhp\PhpizePath; use Php\Pie\Platform\TargetPlatform; +use Php\Pie\Util\Process; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\Process\Process; use function count; use function file_exists; @@ -18,6 +18,10 @@ /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ final class UnixBuild implements Build { + private const PHPIZE_TIMEOUT_SECS = 60; // 1 minute + private const CONFIGURE_TIMEOUT_SECS = 120; // 2 minutes + private const MAKE_TIMEOUT_SECS = 600; // 10 minutes + /** {@inheritDoc} */ public function __invoke( DownloadedPackage $downloadedPackage, @@ -72,23 +76,29 @@ public function __invoke( private function phpize(PhpizePath $phpize, DownloadedPackage $downloadedPackage): string { - return (new Process([$phpize->phpizeBinaryPath], $downloadedPackage->extractedSourcePath)) - ->mustRun() - ->getOutput(); + return Process::run( + [$phpize->phpizeBinaryPath], + $downloadedPackage->extractedSourcePath, + self::PHPIZE_TIMEOUT_SECS, + ); } /** @param list $configureOptions */ private function configure(DownloadedPackage $downloadedPackage, array $configureOptions = []): string { - return (new Process(['./configure', ...$configureOptions], $downloadedPackage->extractedSourcePath)) - ->mustRun() - ->getOutput(); + return Process::run( + ['./configure', ...$configureOptions], + $downloadedPackage->extractedSourcePath, + self::CONFIGURE_TIMEOUT_SECS, + ); } private function make(DownloadedPackage $downloadedPackage): string { - return (new Process(['make'], $downloadedPackage->extractedSourcePath)) - ->mustRun() - ->getOutput(); + return Process::run( + ['make'], + $downloadedPackage->extractedSourcePath, + self::MAKE_TIMEOUT_SECS, + ); } } diff --git a/src/Installing/UnixInstall.php b/src/Installing/UnixInstall.php index f354cb9..4194180 100644 --- a/src/Installing/UnixInstall.php +++ b/src/Installing/UnixInstall.php @@ -7,9 +7,9 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionType; use Php\Pie\Platform\TargetPlatform; +use Php\Pie\Util\Process; use RuntimeException; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\Process\Process; use function array_unshift; use function file_exists; @@ -19,6 +19,8 @@ /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ final class UnixInstall implements Install { + private const MAKE_INSTALL_TIMEOUT_SECS = 60; // 1 minute + public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): string { $targetExtensionPath = $targetPlatform->phpBinaryPath->extensionPath(); @@ -44,9 +46,11 @@ public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $t array_unshift($makeInstallCommand, 'sudo'); } - $makeInstallOutput = (new Process($makeInstallCommand, $downloadedPackage->extractedSourcePath)) - ->mustRun() - ->getOutput(); + $makeInstallOutput = Process::run( + $makeInstallCommand, + $downloadedPackage->extractedSourcePath, + self::MAKE_INSTALL_TIMEOUT_SECS, + ); if ($output->isVeryVerbose()) { $output->writeln($makeInstallOutput); diff --git a/src/Platform/TargetPhp/PhpBinaryPath.php b/src/Platform/TargetPhp/PhpBinaryPath.php index 8f99f0d..d9b1e02 100644 --- a/src/Platform/TargetPhp/PhpBinaryPath.php +++ b/src/Platform/TargetPhp/PhpBinaryPath.php @@ -8,9 +8,9 @@ use Composer\Util\Platform; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; +use Php\Pie\Util\Process; use RuntimeException; use Symfony\Component\Process\PhpExecutableFinder; -use Symfony\Component\Process\Process; use Webmozart\Assert\Assert; use function array_combine; @@ -58,9 +58,7 @@ private static function assertValidLookingPhpBinary(string $phpBinaryPath): void // This is somewhat of a rudimentary check that the target PHP really is a PHP instance; not sure why you // WOULDN'T want to use a real PHP, but this should stop obvious hiccups at least (rather than for security) - $testOutput = trim((new Process([$phpBinaryPath, '-r', 'echo "PHP";'])) - ->mustRun() - ->getOutput()); + $testOutput = Process::run([$phpBinaryPath, '-r', 'echo "PHP";']); if ($testOutput !== 'PHP') { throw Exception\InvalidPhpBinaryPath::fromInvalidPhpBinary($phpBinaryPath); @@ -118,7 +116,7 @@ public function extensionPath(): string */ public function extensions(): array { - $extVersionsRawJson = trim((new Process([ + $extVersionsList = Process::run([ $this->phpBinaryPath, '-r', <<<'PHP' @@ -141,13 +139,11 @@ static function ($k, $v) { $extVersions )); PHP, - ])) - ->mustRun() - ->getOutput()); + ]); $pairs = array_map( static fn (string $row) => explode(':', $row), - explode("\n", $extVersionsRawJson), + explode("\n", $extVersionsList), ); return array_combine( @@ -158,13 +154,11 @@ static function ($k, $v) { public function operatingSystem(): OperatingSystem { - $winOrNot = trim((new Process([ + $winOrNot = Process::run([ $this->phpBinaryPath, '-r', 'echo \\defined(\'PHP_WINDOWS_VERSION_BUILD\') ? \'win\' : \'not\';', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($winOrNot, 'Could not determine PHP version'); return $winOrNot === 'win' ? OperatingSystem::Windows : OperatingSystem::NonWindows; @@ -173,13 +167,11 @@ public function operatingSystem(): OperatingSystem /** @return non-empty-string */ public function version(): string { - $phpVersion = trim((new Process([ + $phpVersion = Process::run([ $this->phpBinaryPath, '-r', 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION . "." . PHP_RELEASE_VERSION;', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($phpVersion, 'Could not determine PHP version'); // normalizing the version will throw an exception if it is not a valid version @@ -191,13 +183,11 @@ public function version(): string /** @return non-empty-string */ public function majorMinorVersion(): string { - $phpVersion = trim((new Process([ + $phpVersion = Process::run([ $this->phpBinaryPath, '-r', 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION;', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($phpVersion, 'Could not determine PHP version'); // normalizing the version will throw an exception if it is not a valid version @@ -208,13 +198,11 @@ public function majorMinorVersion(): string public function machineType(): Architecture { - $phpMachineType = trim((new Process([ + $phpMachineType = Process::run([ $this->phpBinaryPath, '-r', 'echo php_uname("m");', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($phpMachineType, 'Could not determine PHP machine type'); return Architecture::parseArchitecture($phpMachineType); @@ -222,13 +210,11 @@ public function machineType(): Architecture public function phpIntSize(): int { - $phpIntSize = trim((new Process([ + $phpIntSize = Process::run([ $this->phpBinaryPath, '-r', 'echo PHP_INT_SIZE;', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($phpIntSize, 'Could not fetch PHP_INT_SIZE'); Assert::same($phpIntSize, (string) (int) $phpIntSize, 'PHP_INT_SIZE was not an integer processed %2$s from %s'); @@ -238,12 +224,10 @@ public function phpIntSize(): int /** @return non-empty-string */ public function phpinfo(): string { - $phpInfo = trim((new Process([ + $phpInfo = Process::run([ $this->phpBinaryPath, '-i', - ])) - ->mustRun() - ->getOutput()); + ]); Assert::stringNotEmpty($phpInfo, sprintf('Could not run phpinfo using %s', $this->phpBinaryPath)); @@ -264,9 +248,7 @@ public function phpConfigPath(): string|null /** @param non-empty-string $phpConfig */ public static function fromPhpConfigExecutable(string $phpConfig): self { - $phpExecutable = trim((new Process([$phpConfig, '--php-binary'])) - ->mustRun() - ->getOutput()); + $phpExecutable = Process::run([$phpConfig, '--php-binary']); Assert::stringNotEmpty($phpExecutable, 'Could not find path to PHP executable.'); self::assertValidLookingPhpBinary($phpExecutable); diff --git a/src/Util/Process.php b/src/Util/Process.php new file mode 100644 index 0000000..8c9ef18 --- /dev/null +++ b/src/Util/Process.php @@ -0,0 +1,38 @@ + $command + * + * @throws ProcessFailedException + */ + public static function run(array $command, string|null $workingDirectory = null, int|null $timeout = 5): string + { + return trim((new SymfonyProcess($command, $workingDirectory, timeout: $timeout)) + ->mustRun() + ->getOutput()); + } +}