From 50644e72ca8e6090fd4b65e2e7fdf66a056a4f72 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 27 Feb 2026 14:14:43 +0000 Subject: [PATCH] 517: fix inability to provide sudo prompt when using "pie install" on a PHP project Previously, the `pie install` would invoke another instance of `pie`. However, since this would always be run in a non-interactive shell, `sudo` prompts will never work. I've changed this instead to use `InvokeSubCommand` which copies the input params into the sub command, so it is all run in the same PIE process and therefore retains the interactivity (or lack thereof) of the shell it was invoked from. --- phpstan-baseline.neon | 18 ----- .../InstallExtensionsForProjectCommand.php | 26 ++++--- src/Command/InvokeSubCommand.php | 17 +++- .../RequestedPackageAndVersion.php | 9 +++ .../InstallSelectedPackage.php | 78 ++++++------------- src/Util/OutputFormatterWithPrefix.php | 38 +++++++++ test/assets/fake-pie-cli/happy.bat | 5 -- test/assets/fake-pie-cli/happy.sh | 4 - ...InstallExtensionsForProjectCommandTest.php | 15 +++- test/unit/Command/InvokeSubCommandTest.php | 75 ++++++++++++++++++ .../RequestedPackageAndVersionTest.php | 29 +++++++ .../InstallSelectedPackageTest.php | 62 +++++++-------- 12 files changed, 249 insertions(+), 127 deletions(-) create mode 100644 src/Util/OutputFormatterWithPrefix.php delete mode 100644 test/assets/fake-pie-cli/happy.bat delete mode 100755 test/assets/fake-pie-cli/happy.sh create mode 100644 test/unit/Command/InvokeSubCommandTest.php create mode 100644 test/unit/DependencyResolver/RequestedPackageAndVersionTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5df0ca82..775159ab 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -270,18 +270,6 @@ parameters: count: 2 path: src/Installing/InstallForPhpProject/FindMatchingPackages.php - - - message: '#^Parameter \#2 \$callback of function array_walk expects callable\(array\|float\|int\|string\|true, int\|string\)\: mixed, Closure\(string, string\)\: void given\.$#' - identifier: argument.type - count: 1 - path: src/Installing/InstallForPhpProject/InstallSelectedPackage.php - - - - message: '#^Parameter \#2 \$workingDirectory of static method Php\\Pie\\Util\\Process\:\:run\(\) expects string\|null, string\|false given\.$#' - identifier: argument.type - count: 1 - path: src/Installing/InstallForPhpProject/InstallSelectedPackage.php - - message: '#^Negated boolean expression is always false\.$#' identifier: booleanNot.alwaysFalse @@ -480,12 +468,6 @@ parameters: count: 1 path: test/unit/Installing/Ini/StandardSinglePhpIniTest.php - - - message: '#^Parameter \#1 \$originalCwd of class Php\\Pie\\File\\FullPathToSelf constructor expects string, string\|false given\.$#' - identifier: argument.type - count: 1 - path: test/unit/Installing/InstallForPhpProject/InstallSelectedPackageTest.php - - message: '#^Method Php\\PieUnitTest\\SelfManage\\Verify\\FallbackVerificationUsingOpenSslTest\:\:prepareCertificateAndSignature\(\) should return array\{string, string\} but returns array\{mixed, mixed\}\.$#' identifier: return.type diff --git a/src/Command/InstallExtensionsForProjectCommand.php b/src/Command/InstallExtensionsForProjectCommand.php index 0d0303a5..511d43bb 100644 --- a/src/Command/InstallExtensionsForProjectCommand.php +++ b/src/Command/InstallExtensionsForProjectCommand.php @@ -12,6 +12,7 @@ use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieJsonEditor; +use Php\Pie\DependencyResolver\RequestedPackageAndVersion; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; use Php\Pie\Installing\InstallForPhpProject\ComposerFactoryForProject; @@ -28,6 +29,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Throwable; +use Webmozart\Assert\Assert; use function array_column; use function array_key_exists; @@ -268,20 +270,26 @@ static function (array $match): string { $selectedPackageName = $matches[0]['name']; } - $requestInstallConstraint = ''; - if ($linkRequiresConstraint !== '*') { - $requestInstallConstraint = ':' . $linkRequiresConstraint; - } + assert($selectedPackageName !== ''); + $requestedPackageAndVersion = new RequestedPackageAndVersion( + $selectedPackageName, + $linkRequiresConstraint === '*' || $linkRequiresConstraint === '' ? null : $linkRequiresConstraint, + ); try { $this->io->write( - sprintf('Invoking pie install of %s%s', $selectedPackageName, $requestInstallConstraint), + sprintf('Invoking pie install of %s', $requestedPackageAndVersion->prettyNameAndVersion()), verbosity: IOInterface::VERBOSE, ); - $this->installSelectedPackage->withPieCli( - $selectedPackageName . $requestInstallConstraint, - $input, - $this->io, + Assert::same( + 0, + $this->installSelectedPackage->withSubCommand( + ExtensionName::normaliseFromString($link->getTarget()), + $requestedPackageAndVersion, + $this, + $input, + ), + 'Non-zero exit code %s whilst installing ' . $requestedPackageAndVersion->package, ); } catch (Throwable $t) { $anyErrorsHappened = true; diff --git a/src/Command/InvokeSubCommand.php b/src/Command/InvokeSubCommand.php index 35977b1e..f1a5b4d3 100644 --- a/src/Command/InvokeSubCommand.php +++ b/src/Command/InvokeSubCommand.php @@ -5,6 +5,7 @@ namespace Php\Pie\Command; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -29,6 +30,7 @@ public function __invoke( Command $command, array $subCommandInput, InputInterface $originalCommandInput, + OutputFormatter|null $formatter = null, ): int { $originalSuppliedOptions = array_filter($originalCommandInput->getOptions()); $installForProjectInput = new ArrayInput(array_merge( @@ -42,6 +44,19 @@ public function __invoke( $application = $command->getApplication(); Assert::notNull($application); - return $application->doRun($installForProjectInput, $this->output); + if ($formatter instanceof OutputFormatter) { + $oldFormatter = $this->output->getFormatter(); + $this->output->setFormatter($formatter); + } + + try { + $result = $application->doRun($installForProjectInput, $this->output); + } finally { + if ($formatter instanceof OutputFormatter) { + $this->output->setFormatter($oldFormatter); + } + } + + return $result; } } diff --git a/src/DependencyResolver/RequestedPackageAndVersion.php b/src/DependencyResolver/RequestedPackageAndVersion.php index 6b252e90..a5aac5b1 100644 --- a/src/DependencyResolver/RequestedPackageAndVersion.php +++ b/src/DependencyResolver/RequestedPackageAndVersion.php @@ -27,4 +27,13 @@ public function __construct( throw InvalidPackageName::fromMissingForwardSlash($this); } } + + public function prettyNameAndVersion(): string + { + if ($this->version === null) { + return $this->package; + } + + return $this->package . ':' . $this->version; + } } diff --git a/src/Installing/InstallForPhpProject/InstallSelectedPackage.php b/src/Installing/InstallForPhpProject/InstallSelectedPackage.php index 86e682df..97d64207 100644 --- a/src/Installing/InstallForPhpProject/InstallSelectedPackage.php +++ b/src/Installing/InstallForPhpProject/InstallSelectedPackage.php @@ -4,71 +4,37 @@ namespace Php\Pie\Installing\InstallForPhpProject; -use Composer\IO\IOInterface; -use Php\Pie\Command\CommandHelper; -use Php\Pie\File\FullPathToSelf; -use Php\Pie\Util\Process; +use Php\Pie\Command\InvokeSubCommand; +use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\ExtensionName; +use Php\Pie\Util\OutputFormatterWithPrefix; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; -use function array_filter; -use function array_walk; -use function getcwd; -use function in_array; - -use const ARRAY_FILTER_USE_BOTH; - /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ class InstallSelectedPackage { - public function __construct(private readonly FullPathToSelf $fullPathToSelf) - { + public function __construct( + private readonly InvokeSubCommand $invokeSubCommand, + ) { } - public function withPieCli(string $selectedPackage, InputInterface $input, IOInterface $io): void - { - $process = [ - ($this->fullPathToSelf)(), - 'install', - $selectedPackage, + public function withSubCommand( + ExtensionName $ext, + RequestedPackageAndVersion $selectedPackage, + Command $command, + InputInterface $input, + ): int { + $params = [ + 'command' => 'install', + 'requested-package-and-version' => $selectedPackage->prettyNameAndVersion(), ]; - $phpPathOptions = array_filter( - $input->getOptions(), - static function (mixed $value, string|int $key): bool { - return $value !== null - && $value !== false - && in_array( - $key, - [ - CommandHelper::OPTION_WITH_PHP_CONFIG, - CommandHelper::OPTION_WITH_PHP_PATH, - CommandHelper::OPTION_WITH_PHPIZE_PATH, - ], - ); - }, - ARRAY_FILTER_USE_BOTH, - ); - - array_walk( - $phpPathOptions, - static function (string $value, string $key) use (&$process): void { - $process[] = '--' . $key; - $process[] = $value; - }, - ); - - Process::run( - $process, - getcwd(), - outputCallback: static function (string $outOrErr, string $message) use ($io): void { - if ($outOrErr === \Symfony\Component\Process\Process::ERR) { - $io->writeError(' > ' . $message); - - return; - } - - $io->write(' > ' . $message); - }, + return ($this->invokeSubCommand)( + $command, + $params, + $input, + OutputFormatterWithPrefix::newWithPrefix(' ' . $ext->name() . '> '), ); } } diff --git a/src/Util/OutputFormatterWithPrefix.php b/src/Util/OutputFormatterWithPrefix.php new file mode 100644 index 00000000..b8e3c61c --- /dev/null +++ b/src/Util/OutputFormatterWithPrefix.php @@ -0,0 +1,38 @@ +linePrefix . $formatted; + } +} diff --git a/test/assets/fake-pie-cli/happy.bat b/test/assets/fake-pie-cli/happy.bat deleted file mode 100644 index 8cb8a887..00000000 --- a/test/assets/fake-pie-cli/happy.bat +++ /dev/null @@ -1,5 +0,0 @@ -@echo off - -echo Params passed: %* - -exit /b 0 diff --git a/test/assets/fake-pie-cli/happy.sh b/test/assets/fake-pie-cli/happy.sh deleted file mode 100755 index 8c1777f4..00000000 --- a/test/assets/fake-pie-cli/happy.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/usr/bin/env bash - -echo "Params passed: ${*}" -exit 0 diff --git a/test/integration/Command/InstallExtensionsForProjectCommandTest.php b/test/integration/Command/InstallExtensionsForProjectCommandTest.php index 7e1b7676..cad216ca 100644 --- a/test/integration/Command/InstallExtensionsForProjectCommandTest.php +++ b/test/integration/Command/InstallExtensionsForProjectCommandTest.php @@ -17,6 +17,8 @@ use Php\Pie\ComposerIntegration\PieJsonEditor; use Php\Pie\ComposerIntegration\QuieterConsoleIO; use Php\Pie\Container; +use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; use Php\Pie\Installing\InstallForPhpProject\ComposerFactoryForProject; use Php\Pie\Installing\InstallForPhpProject\DetermineExtensionsRequired; @@ -125,8 +127,15 @@ public function testInstallingExtensionsForPhpProject(): void $this->questionHelper->method('ask')->willReturn('vendor1/foobar: The official foobar implementation'); $this->installSelectedPackage->expects(self::once()) - ->method('withPieCli') - ->with('vendor1/foobar:^1.2'); + ->method('withSubCommand') + ->with( + ExtensionName::normaliseFromString('foobar'), + new RequestedPackageAndVersion( + 'vendor1/foobar', + '^1.2', + ), + ) + ->willReturn(0); $this->commandTester->execute( ['--allow-non-interactive-project-install' => true], @@ -173,7 +182,7 @@ public function testInstallingExtensionsForPhpProjectWithMultipleMatches(): void $this->questionHelper->method('ask')->willReturn('vendor1/foobar: The official foobar implementation'); $this->installSelectedPackage->expects(self::never()) - ->method('withPieCli'); + ->method('withSubCommand'); $this->commandTester->execute( ['--allow-non-interactive-project-install' => true], diff --git a/test/unit/Command/InvokeSubCommandTest.php b/test/unit/Command/InvokeSubCommandTest.php new file mode 100644 index 00000000..ca3bb046 --- /dev/null +++ b/test/unit/Command/InvokeSubCommandTest.php @@ -0,0 +1,75 @@ +addOption(new InputOption('verbose', 'v', InputOption::VALUE_NONE, 'Verbose option')); + $input = new ArrayInput(['--verbose' => true], $inputDefinition); + + $output = new BufferedOutput(); + + $application = $this->createMock(Application::class); + $application->expects(self::once()) + ->method('doRun') + ->willReturnCallback(static function (ArrayInput $newInput, OutputInterface $output) { + self::assertSame('foo --verbose=1', (string) $newInput); + $output->writeln('command output here'); + + return 0; + }); + + $command = $this->createMock(Command::class); + $command->method('getApplication')->willReturn($application); + + $invoker = new InvokeSubCommand($output); + self::assertSame(0, ($invoker)($command, ['command' => 'foo'], $input)); + self::assertSame('command output here', trim($output->fetch())); + } + + public function testInvokeWithPrefixOutputFormatterRunsSubCommand(): void + { + $inputDefinition = new InputDefinition(); + $inputDefinition->addOption(new InputOption('verbose', 'v', InputOption::VALUE_NONE, 'Verbose option')); + $input = new ArrayInput(['--verbose' => true], $inputDefinition); + + $output = new BufferedOutput(); + + $application = $this->createMock(Application::class); + $application->expects(self::once()) + ->method('doRun') + ->willReturnCallback(static function (ArrayInput $newInput, OutputInterface $output) { + self::assertSame('foo --verbose=1', (string) $newInput); + $output->writeln('command output here'); + + return 0; + }); + + $command = $this->createMock(Command::class); + $command->method('getApplication')->willReturn($application); + + $invoker = new InvokeSubCommand($output); + self::assertSame(0, ($invoker)($command, ['command' => 'foo'], $input, new OutputFormatterWithPrefix('prefix> '))); + self::assertSame('prefix> command output here', trim($output->fetch())); + } +} diff --git a/test/unit/DependencyResolver/RequestedPackageAndVersionTest.php b/test/unit/DependencyResolver/RequestedPackageAndVersionTest.php new file mode 100644 index 00000000..b0bb7d2d --- /dev/null +++ b/test/unit/DependencyResolver/RequestedPackageAndVersionTest.php @@ -0,0 +1,29 @@ +prettyNameAndVersion(), + ); + } + + public function testPrettyNameAndVersionWithoutVersion(): void + { + self::assertSame( + 'foo/foo', + (new RequestedPackageAndVersion('foo/foo', null))->prettyNameAndVersion(), + ); + } +} diff --git a/test/unit/Installing/InstallForPhpProject/InstallSelectedPackageTest.php b/test/unit/Installing/InstallForPhpProject/InstallSelectedPackageTest.php index 44623dab..1d74a7f1 100644 --- a/test/unit/Installing/InstallForPhpProject/InstallSelectedPackageTest.php +++ b/test/unit/Installing/InstallForPhpProject/InstallSelectedPackageTest.php @@ -4,46 +4,46 @@ namespace Php\PieUnitTest\Installing\InstallForPhpProject; -use Composer\IO\BufferIO; -use Composer\Util\Platform; -use Php\Pie\File\FullPathToSelf; +use Php\Pie\Command\InvokeSubCommand; +use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\ExtensionName; use Php\Pie\Installing\InstallForPhpProject\InstallSelectedPackage; +use Php\Pie\Util\OutputFormatterWithPrefix; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; -use Symfony\Component\Console\Input\ArrayInput; -use Symfony\Component\Console\Input\InputDefinition; -use Symfony\Component\Console\Input\InputOption; - -use function getcwd; -use function trim; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; #[CoversClass(InstallSelectedPackage::class)] final class InstallSelectedPackageTest extends TestCase { - private const FAKE_HAPPY_SH = __DIR__ . '/../../../assets/fake-pie-cli/happy.sh'; - private const FAKE_HAPPY_BAT = __DIR__ . '/../../../assets/fake-pie-cli/happy.bat'; - - public function testWithPieCli(): void + public function testSubCommandIsInvoked(): void { - $_SERVER['PHP_SELF'] = Platform::isWindows() ? self::FAKE_HAPPY_BAT : self::FAKE_HAPPY_SH; - - $input = new ArrayInput( - ['--with-php-config' => '/path/to/php/config'], - new InputDefinition([ - new InputOption('with-php-config', null, InputOption::VALUE_REQUIRED), - ]), - ); - $io = new BufferIO(); - - (new InstallSelectedPackage(new FullPathToSelf(getcwd())))->withPieCli( - 'foo/bar', + $command = $this->createMock(Command::class); + $input = $this->createMock(InputInterface::class); + $invoker = $this->createMock(InvokeSubCommand::class); + $invoker->expects(self::once()) + ->method('__invoke') + ->with( + $command, + [ + 'command' => 'install', + 'requested-package-and-version' => 'foo/foo:^1.0', + ], + $input, + self::isInstanceOf(OutputFormatterWithPrefix::class), + ) + ->willReturn(0); + + $installer = new InstallSelectedPackage($invoker); + $installer->withSubCommand( + ExtensionName::normaliseFromString('foo'), + new RequestedPackageAndVersion( + 'foo/foo', + '^1.0', + ), + $command, $input, - $io, - ); - - self::assertSame( - '> Params passed: install foo/bar --with-php-config /path/to/php/config', - trim($io->getOutput()), ); } }