Merge pull request #75 from asgrim/adjust-timeouts-for-process-invokations

Adjust timeouts for invoked processes

Author: asgrim

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<non-empty-string> $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,
+        );
     }
 }

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);

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,27 +198,23 @@ 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);
     }
 
     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);

src/Util/Process.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Php\Pie\Util;
+
+use Symfony\Component\Process\Exception\ProcessFailedException;
+use Symfony\Component\Process\Process as SymfonyProcess;
+
+use function trim;
+
+final class Process
+{
+    /** @psalm-suppress UnusedConstructor */
+    private function __construct()
+    {
+    }
+
+    /**
+     * Just a helper to invoke a Symfony Process command with a simplified API
+     * for the common invocations we have in PIE.
+     *
+     * Things to note:
+     *  - uses mustRun (i.e. throws exception if command execution fails)
+     *  - very short timeout by default (5 seconds)
+     *  - output is trimmed
+     *
+     * @param list<string> $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());
+    }
+}