From 6e55cb36d855a585c10588e7afaa7cdf5085c119 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 28 Oct 2022 14:24:55 +0200 Subject: [PATCH] Add support for adding Command classes as scripts, (#11151) * Add support for adding Command classes as scripts, fixes #11134 * Allow all options to be forwarded and allow using references to other scripts with args * Fix build * Add more checks * Ensure exceptions are not swallowed, and remove naming restriction by using a single-command app * Update docs * Add tests, fix issue merging params when combining nested scripts and CLI params --- doc/articles/scripts.md | 64 ++++++++++++++++- src/Composer/Command/ScriptAliasCommand.php | 10 ++- .../EventDispatcher/EventDispatcher.php | 59 +++++++++++++++ .../Test/Command/RunScriptCommandTest.php | 72 +++++++++++++++++++ 4 files changed, 201 insertions(+), 4 deletions(-) diff --git a/doc/articles/scripts.md b/doc/articles/scripts.md index 5337720fe..f83db4072 100644 --- a/doc/articles/scripts.md +++ b/doc/articles/scripts.md @@ -11,6 +11,10 @@ static method) or any command-line executable command. Scripts are useful for executing a package's custom code or package-specific commands during the Composer execution process. +As of Composer 2.5 scripts can also be Symfony Console Command classes, +which allows you to easily run them including passing options. This is +however not recommended for handling events. + > **Note:** Only scripts defined in the root package's `composer.json` are > executed. If a dependency of the root package specifies its own scripts, > Composer does not execute those additional scripts. @@ -94,8 +98,8 @@ For any given event: - Scripts execute in the order defined when their corresponding event is fired. - An array of scripts wired to a single event can contain both PHP callbacks and command-line executable commands. -- PHP classes containing defined callbacks must be autoloadable via Composer's -autoload functionality. +- PHP classes and commands containing defined callbacks must be autoloadable +via Composer's autoload functionality. - Callbacks can only autoload classes from psr-0, psr-4 and classmap definitions. If a defined callback relies on functions defined outside of a class, the callback itself is responsible for loading the file containing these @@ -217,7 +221,9 @@ running `composer test`: ```json { "scripts": { - "test": "phpunit" + "test": "phpunit", + "do-something": "MyVendor\\MyClass::doSomething" + "my-cmd": "MyVendor\\MyCommand" } } ``` @@ -226,11 +232,63 @@ Similar to the `run-script` command you can give additional arguments to scripts e.g. `composer test -- --filter ` will pass `--filter ` along to the `phpunit` script. +Using a PHP method via `composer do-something arg` lets you execute a +`static function doSomething(\Composer\Script\Event $event)` and `arg` becomes +available in `$event->getArguments()`. This however does not let you easily pass +custom options in the form of `--flags`. + +Using a [symfony/console](https://packagist.org/packages/symfony/console) `Command` +class you can define and access arguments and options more easily. + +For example with the command below you can then simply call `composer my-cmd +--arbitrary-flag` without even the need for a `--` separator. To be detected +as symfony/console commands the class name must end with `Command` and extend +symfony's `Command` class. Also note that this will run using Composer's built-in +symfony/console version which may not match the one you have required in your +project, and may change between Composer minor releases. If you need more +safety guarantees you should rather use your own binary file that runs your own +symfony/console version in isolation in its own process then. + +```php +setDefinition([ + new InputOption('arbitrary-flag', null, InputOption::VALUE_NONE, 'Example flag'), + new InputArgument('foo', InputArgument::OPTIONAL, 'Optional arg'), + ]); + } + + public function execute(InputInterface $input, OutputInterface $output): int + { + if ($input->getOption('arbitrary-flag')) { + $output->writeln('The flag was used') + } + + return 0; + } +} +``` + > **Note:** Before executing scripts, Composer's bin-dir is temporarily pushed > on top of the PATH environment variable so that binaries of dependencies > are directly accessible. In this example no matter if the `phpunit` binary is > actually in `vendor/bin/phpunit` or `bin/phpunit` it will be found and executed. + +## Managing the process timeout + Although Composer is not intended to manage long-running processes and other such aspects of PHP projects, it can sometimes be handy to disable the process timeout on custom commands. This timeout defaults to 300 seconds and can be diff --git a/src/Composer/Command/ScriptAliasCommand.php b/src/Composer/Command/ScriptAliasCommand.php index 34a41e058..303aab452 100644 --- a/src/Composer/Command/ScriptAliasCommand.php +++ b/src/Composer/Command/ScriptAliasCommand.php @@ -12,6 +12,7 @@ namespace Composer\Command; +use Composer\Pcre\Preg; use Symfony\Component\Console\Input\InputInterface; use Composer\Console\Input\InputOption; use Composer\Console\Input\InputArgument; @@ -32,6 +33,8 @@ class ScriptAliasCommand extends BaseCommand $this->script = $script; $this->description = $description ?? 'Runs the '.$script.' script as defined in composer.json'; + $this->ignoreValidationErrors(); + parent::__construct(); } @@ -63,6 +66,11 @@ EOT $args = $input->getArguments(); - return $composer->getEventDispatcher()->dispatchScript($this->script, $input->getOption('dev') || !$input->getOption('no-dev'), $args['args']); + // TODO remove for Symfony 6+ as it is then in the interface + if (!method_exists($input, '__toString')) { // @phpstan-ignore-line + throw new \LogicException('Expected an Input instance that is stringable, got '.get_class($input)); + } + + return $composer->getEventDispatcher()->dispatchScript($this->script, $input->getOption('dev') || !$input->getOption('no-dev'), $args['args'], ['script-alias-input' => Preg::replace('{^\S+ ?}', '', $input->__toString(), 1)]); } } diff --git a/src/Composer/EventDispatcher/EventDispatcher.php b/src/Composer/EventDispatcher/EventDispatcher.php index c6547e0d7..26ea4aa30 100644 --- a/src/Composer/EventDispatcher/EventDispatcher.php +++ b/src/Composer/EventDispatcher/EventDispatcher.php @@ -14,6 +14,8 @@ namespace Composer\EventDispatcher; use Composer\DependencyResolver\Transaction; use Composer\Installer\InstallerEvent; +use Composer\IO\BufferIO; +use Composer\IO\ConsoleIO; use Composer\IO\IOInterface; use Composer\Composer; use Composer\PartialComposer; @@ -27,6 +29,10 @@ use Composer\Installer\BinaryInstaller; use Composer\Util\ProcessExecutor; use Composer\Script\Event as ScriptEvent; use Composer\Autoload\ClassLoader; +use Symfony\Component\Console\Application; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\StringInput; +use Symfony\Component\Console\Output\ConsoleOutput; use Symfony\Component\Process\PhpExecutableFinder; use Symfony\Component\Process\ExecutableFinder; @@ -207,6 +213,11 @@ class EventDispatcher $args = array_merge($script, $event->getArguments()); $flags = $event->getFlags(); + if (isset($flags['script-alias-input'])) { + $argsString = implode(' ', array_map(static function ($arg) { return ProcessExecutor::escape($arg); }, $script)); + $flags['script-alias-input'] = $argsString . ' ' . $flags['script-alias-input']; + unset($argsString); + } if (strpos($callable, '@composer ') === 0) { $exec = $this->getPhpExecCommand() . ' ' . ProcessExecutor::escape(Platform::getEnv('COMPOSER_BINARY')) . ' ' . implode(' ', $args); if (0 !== ($exitCode = $this->executeTty($exec))) { @@ -249,6 +260,46 @@ class EventDispatcher $this->io->writeError(''.sprintf($message, $callable, $event->getName()).'', true, IOInterface::QUIET); throw $e; } + } elseif ($this->isCommandClass($callable)) { + $className = $callable; + if (!class_exists($className)) { + $this->io->writeError('Class '.$className.' is not autoloadable, can not call '.$event->getName().' script', true, IOInterface::QUIET); + continue; + } + if (!is_a($className, Command::class, true)) { + $this->io->writeError('Class '.$className.' does not extend '.Command::class.', can not call '.$event->getName().' script', true, IOInterface::QUIET); + continue; + } + if (defined('Composer\Script\ScriptEvents::'.str_replace('-', '_', strtoupper($event->getName())))) { + $this->io->writeError('You cannot bind '.$event->getName().' to a Command class, use a non-reserved name', true, IOInterface::QUIET); + continue; + } + + $app = new Application(); + $app->setCatchExceptions(false); + $app->setAutoExit(false); + $cmd = new $className($event->getName()); + $app->add($cmd); + $app->setDefaultCommand((string) $cmd->getName(), true); + try { + $args = implode(' ', array_map(static function ($arg) { return ProcessExecutor::escape($arg); }, $event->getArguments())); + // reusing the output from $this->io is mostly needed for tests, but generally speaking + // it does not hurt to keep the same stream as the current Application + if ($this->io instanceof ConsoleIO) { + $reflProp = new \ReflectionProperty($this->io, 'output'); + if (PHP_VERSION_ID < 80100) { + $reflProp->setAccessible(true); + } + $output = $reflProp->getValue($this->io); + } else { + $output = new ConsoleOutput(); + } + $return = $app->run(new StringInput($event->getFlags()['script-alias-input'] ?? $args), $output); + } catch (\Exception $e) { + $message = "Script %s handling the %s event terminated with an exception"; + $this->io->writeError(''.sprintf($message, $callable, $event->getName()).'', true, IOInterface::QUIET); + throw $e; + } } else { $args = implode(' ', array_map(['Composer\Util\ProcessExecutor', 'escape'], $event->getArguments())); @@ -507,6 +558,14 @@ class EventDispatcher return false === strpos($callable, ' ') && false !== strpos($callable, '::'); } + /** + * Checks if string given references a command class + */ + protected function isCommandClass(string $callable): bool + { + return str_contains($callable, '\\') && !str_contains($callable, ' ') && str_ends_with($callable, 'Command'); + } + /** * Checks if string given references a composer run-script */ diff --git a/tests/Composer/Test/Command/RunScriptCommandTest.php b/tests/Composer/Test/Command/RunScriptCommandTest.php index 13f1b5f52..01e56fab1 100644 --- a/tests/Composer/Test/Command/RunScriptCommandTest.php +++ b/tests/Composer/Test/Command/RunScriptCommandTest.php @@ -109,6 +109,78 @@ class RunScriptCommandTest extends TestCase $this->assertStringContainsString('Run the codestyle fixer', $output, 'The custom description for the fix-cs script should be printed'); } + public function testExecutionOfCustomSymfonyCommand(): void + { + $this->initTempComposer([ + 'scripts' => [ + 'test-direct' => 'Test\\MyCommand', + 'test-ref' => ['@test-direct --inneropt innerarg'], + ], + 'autoload' => [ + 'psr-4' => [ + 'Test\\' => '', + ], + ], + ]); + + file_put_contents('MyCommand.php', <<<'TEST' +setDefinition([ + new InputArgument('req-arg', InputArgument::REQUIRED, 'Required arg.'), + new InputArgument('opt-arg', InputArgument::OPTIONAL, 'Optional arg.'), + new InputOption('inneropt', null, InputOption::VALUE_NONE, 'Option.'), + new InputOption('outeropt', null, InputOption::VALUE_OPTIONAL, 'Optional option.'), + ]); + } + + public function execute(InputInterface $input, OutputInterface $output): int + { + $output->writeln($input->getArgument('req-arg')); + $output->writeln((string) $input->getArgument('opt-arg')); + $output->writeln('inneropt: '.($input->getOption('inneropt') ? 'set' : 'unset')); + $output->writeln('outeropt: '.($input->getOption('outeropt') ? 'set' : 'unset')); + + return 2; + } +} + +TEST +); + + $appTester = $this->getApplicationTester(); + $appTester->run(['command' => 'test-direct', '--outeropt' => true, 'req-arg' => 'lala']); + + self::assertSame('lala + +inneropt: unset +outeropt: set +', $appTester->getDisplay(true)); + self::assertSame(2, $appTester->getStatusCode()); + + $appTester = $this->getApplicationTester(); + $appTester->run(['command' => 'test-ref', '--outeropt' => true, 'req-arg' => 'lala']); + + self::assertSame('innerarg +lala +inneropt: set +outeropt: set +', $appTester->getDisplay(true)); + self::assertSame(2, $appTester->getStatusCode()); + } + /** @return bool[][] **/ public function getDevOptions(): array {