From 9fb25cc74b1c13cccc6ca2b1e7fd870f1ea5d59d Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Thu, 26 Dec 2024 20:49:50 -0500 Subject: [PATCH 1/4] Remove --drall prefix from all options --drall-verbose is now --verbose --drall-debug is now --debug --drall-group is now --group --drall-filter is now --filter --drall-workers is now --workers --drall-no-execute is now --dry-run --drall-no-progress is now --no-progress --- README.md | 59 +++-- src/Command/BaseCommand.php | 12 +- src/Command/ExecCommand.php | 103 ++++---- src/Command/SiteAliasesCommand.php | 4 +- src/Command/SiteDirectoriesCommand.php | 2 +- src/Command/SiteKeysCommand.php | 4 +- src/Drall.php | 29 ++- src/Model/RawCommand.php | 39 --- test/Integration/Command/ExecCommandTest.php | 231 ++++++++++++------ .../Command/SiteAliasesCommandTest.php | 8 +- .../Command/SiteDirectoriesCommandTest.php | 8 +- .../Command/SiteKeysCommandTest.php | 8 +- test/Unit/DrallTest.php | 15 +- test/Unit/Model/RawCommandTest.php | 21 -- 14 files changed, 282 insertions(+), 261 deletions(-) delete mode 100644 src/Model/RawCommand.php delete mode 100644 test/Unit/Model/RawCommandTest.php diff --git a/README.md b/README.md index bfd512f..2e4a5c4 100644 --- a/README.md +++ b/README.md @@ -173,25 +173,34 @@ cat web/sites/ralph/settings.local.php #### Options -For the `drall exec` command, all Drall parameters must be passed right -after the `drall exec`. Here's an example: +For the `drall exec` command, all Drall options must be set right after +`drall exec`. For example, ```shell -# This will work. -drall exec --drall-workers=4 drush cache:rebuild +# Correct: Drall gets --verbose. +drall exec --verbose drush core:status +# Incorrect: --verbose is ignored. +drall exec drush core:status --verbose +# Correct: Drall and Drush, both --verbose. +drall exec --verbose -- drush --verbose core:status +# Incorrect: Only Drall gets --verbose. +drall exec --verbose drush core:status +``` + +In summary, the syntax is as follows: -# This won't work. -drall exec drush cache:rebuild --drall-workers=4 +```shell +drall exec [DRALL-OPTIONS] -- drush [DRUSH-OPTIONS] ``` Besides the global options, the `exec` command supports the following options. -#### --drall-workers +#### --workers Say you have 100 sites in a Drupal installation. By default, Drall runs commands on these sites one after the other. To speed up the execution, you can ask Drall to execute multiple commands in parallel. You can specify the -number of workers with the `--drall-workers=n` option, where `n` is the +number of workers with the `--workers=n` option, where `n` is the number of processes you want to run in parallel. Please keep in mind that the performance of the workers depends on your @@ -204,11 +213,11 @@ conflict between the Drall workers. The command below launches 3 instances of Drall to run `core:rebuild` command. - drall exec drush core:rebuild --drall-workers=3 + drall exec drush core:rebuild --workers=3 When a worker runs out of work, it terminates automatically. -#### --drall-no-progress +#### --no-progress By default, Drall displays a progress bar that indicates how many sites have been processed and how many are remaining. In verbose mode, this progress @@ -216,13 +225,13 @@ indicator also displays the time elapsed. However, the progress display that can mess with some terminals or scripts which don't handle backspace characters. For these environments, the progress -bar can be disabled using the `--drall-no-progress` option. +bar can be disabled using the `--no-progress` option. ##### Example: Hide progress bar - drall exec --drall-no-progress drush core:rebuild + drall exec --no-progress drush core:rebuild -#### --drall-no-execute +#### --dry-run This option allows you to see what commands will be executed without actually executing them. @@ -230,7 +239,7 @@ executing them. ##### Example: Dry run ```shell -$ drall exec --drall-no-execute --drall-group=bluish core:status +$ drall exec --dry-run --group=bluish core:status drush --uri=donnie core:status drush --uri=leo core:status ``` @@ -319,38 +328,38 @@ done; This section covers some options that are supported by all `drall` commands. -### --drall-group +### --group Specify the target site group. See the section *site groups* for more information on site groups. - drall exec --drall-group=GROUP core:status --field=site + drall exec --group=GROUP core:status --field=site -If `--drall-group` is not set, then the Drall uses the environment variable +If `--group` is not set, then the Drall uses the environment variable `DRALL_GROUP`, if it is set. -### --drall-filter +### --filter Filter placeholder values with an expression. This is helpful for running commands on specific sites. ```shell # Run only on the "leo" site. -drall exec --drall-filter=leo core:status +drall exec --filter=leo core:status # Run only on "leo" and "ralph" sites. -drall exec --drall-filter="leo||ralph" core:status +drall exec --filter="leo||ralph" core:status ``` For more on using filter expressions, refer to the documentation on [consolidation/filter-via-dot-access-data](https://github.com/consolidation/filter-via-dot-access-data). -### --drall-verbose +### --verbose -Whether Drall should display verbose output. +Display verbose output. -### --drall-debug +### --debug -Whether Drall should display debugging output. +Display debug-level output. ## Auto-detect sites @@ -367,7 +376,7 @@ can alter the `$sites` variable based on your requirements. ## Site groups Drall allows you to group your sites so that you can run commands on these -groups using the `--drall-group` option. +groups using the `--group` option. ### Drall groups with site aliases diff --git a/src/Command/BaseCommand.php b/src/Command/BaseCommand.php index fc52f4c..9a5abac 100644 --- a/src/Command/BaseCommand.php +++ b/src/Command/BaseCommand.php @@ -18,15 +18,15 @@ abstract class BaseCommand extends Command { protected function configure() { $this->addOption( - 'drall-group', - NULL, + 'group', + 'g', InputOption::VALUE_OPTIONAL, 'Site group identifier.' ); $this->addOption( - 'drall-filter', - NULL, + 'filter', + 'f', InputOption::VALUE_OPTIONAL, 'Filter sites based on provided expression.' ); @@ -42,7 +42,7 @@ protected function configure() { * Drall group, if any. Otherwise, NULL. */ protected function getDrallGroup(InputInterface $input): ?string { - if ($group = $input->getOption('drall-group')) { + if ($group = $input->getOption('group')) { return $group; } @@ -61,7 +61,7 @@ protected function getDrallGroup(InputInterface $input): ?string { * @see https://packagist.org/packages/consolidation/filter-via-dot-access-data */ protected function getDrallFilter(InputInterface $input): ?string { - return $input->getOption('drall-filter') ?: NULL; + return $input->getOption('filter') ?: NULL; } protected function preExecute(InputInterface $input, OutputInterface $output) { diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index 5d19bce..52ceca3 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -11,7 +11,6 @@ use Drall\Drall; use Drall\Model\EnvironmentId; use Drall\Model\Placeholder; -use Drall\Model\RawCommand; use Drall\Trait\SignalAwareTrait; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputArgument; @@ -54,11 +53,11 @@ protected function configure() { $this->setDescription('Execute a command on multiple Drupal sites.'); $this->addUsage('drush core:status'); $this->addUsage('./vendor/bin/drush core:status'); - $this->addUsage('--drall-group=GROUP drush core:status'); - $this->addUsage('--drall-filter=FILTER drush core:status'); - $this->addUsage('--drall-workers=4 drush cache:rebuild'); + $this->addUsage('--group=GROUP -- drush core:status'); + $this->addUsage('--filter=FILTER -- drush core:status'); + $this->addUsage('--workers=4 -- drush cache:rebuild'); $this->addUsage('ls web/sites/@@dir/settings.php'); - $this->addUsage('echo "Working on @@site" && drush @@site.local core:status'); + $this->addUsage('\'echo "Working on @@site" && drush @@site.local core:status\''); $this->addArgument( 'cmd', @@ -67,22 +66,22 @@ protected function configure() { ); $this->addOption( - 'drall-workers', - NULL, + 'workers', + 'w', InputOption::VALUE_OPTIONAL, 'Number of commands to execute in parallel.', 1, ); $this->addOption( - 'drall-no-execute', - NULL, + 'dry-run', + 'X', InputOption::VALUE_NONE, 'Do not execute commands, only display them.' ); $this->addOption( - 'drall-no-progress', + 'no-progress', NULL, InputOption::VALUE_NONE, 'Do not show a progress bar.' @@ -91,41 +90,10 @@ protected function configure() { $this->ignoreValidationErrors(); } - /** - * Sets an array to be treated as $argv, mostly for testing. - * - * The $argv array contains: - * - Script name as the first parameter, i.e. drall. - * - The Drall command as the second parameter, e.g. exec. - * - Options for the Drall command, e.g. --drall-group=bluish. - * - The Drush command and its arguments, e.g. pmu devel - * - Options for the Drush command, e.g. --fields=site. - * - * @code - * $command->setArgv([ - * '/opt/drall/bin/drall', - * 'exec', - * '--drall-group=bluish', - * 'core:status', - * '--fields=site', - * ]); - * @endcode - * - * @param array $argv - * An array matching the $argv array format. - * - * @return self - * The command. - */ - public function setArgv(array $argv): self { - $this->argv = $argv; - return $this; - } - protected function execute(InputInterface $input, OutputInterface $output): int { $this->preExecute($input, $output); - $command = $this->getCommand(); + $command = $this->getCommand($input); $group = $this->getDrallGroup($input); $filter = $this->getDrallFilter($input); @@ -150,7 +118,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $workers = $this->getWorkerCount($input); // Display commands without executing them. - if ($input->getOption('drall-no-execute')) { + if ($input->getOption('dry-run')) { foreach ($values as $value) { $sCommand = Placeholder::replace([$placeholder->value => $value], $command); $output->writeln("# Item: $value", OutputInterface::VERBOSITY_VERBOSE); @@ -235,22 +203,35 @@ function ($value) use ($command, $placeholder, $output, $progressBar, &$exitCode return $exitCode; } - protected function getCommand(): RawCommand { - // Symfony Console only recognizes options that are defined in the - // ::configure() method. Since our goal is to catch all arguments and - // options and send them to drush, we do it ourselves using $argv. - // - // @todo Is there a way to catch all options from $input? - $command = RawCommand::fromArgv($this->argv); - - if (!str_contains($command, 'drush')) { - return $command; - } + /** + * Extracts the command to be executed by Drall. + * + * All drall-specific components are removed from the command. + * + * @param \Symfony\Component\Console\Input\InputInterface $input + * Console input. + * + * @return string + * The command without Drall elements. + * + * @example + * Input: /path/to/drall exec --verbose -- drush st --fields=site + * Output: drush st --fields=site + */ + protected function getCommand(InputInterface $input): string { + // @todo Force -- for clarity if options are present. + // @todo Throw an error if --drall-* options are present. + // Everything after the first "--" is treated as an argument. All such + // arguments are treated as parts of the command to be executed. + $command = implode(' ', $input->getArguments()['cmd']); + $this->logger->debug("Command: {command}", ['command' => $command]); - // Inject --uri=@@dir for Drush commands without placeholders. - if (!Placeholder::search($command)) { - $sCommand = preg_replace('/\b(drush) /', 'drush --uri=@@dir ', $command, -1); - $command = new RawCommand($sCommand); + if ( + str_contains($command, 'drush') && + !Placeholder::search($command) + ) { + // Inject --uri=@@dir for Drush commands without placeholders. + $command = preg_replace('/\b(drush) /', 'drush --uri=@@dir ', $command, -1); $this->logger->debug('Injected --uri parameter for Drush command.'); } @@ -267,7 +248,7 @@ protected function getCommand(): RawCommand { * Number of workers to be used. */ protected function getWorkerCount(InputInterface $input): int { - $result = $input->getOption('drall-workers'); + $result = $input->getOption('workers'); if ($result > self::WORKER_LIMIT) { $this->logger->warning('Limiting workers to {count}, which is the maximum.', ['count' => self::WORKER_LIMIT]); @@ -284,7 +265,7 @@ protected function getWorkerCount(InputInterface $input): int { /** * Get unique placeholder from a command. */ - private function getUniquePlaceholder(RawCommand $command): ?Placeholder { + private function getUniquePlaceholder(string $command): ?Placeholder { if (!$placeholders = Placeholder::search($command)) { $this->logger->error('The command contains no placeholders. Please run it directly without Drall.'); return NULL; @@ -311,7 +292,7 @@ private function getUniquePlaceholder(RawCommand $command): ?Placeholder { private function isProgressBarHidden(InputInterface $input): bool { if ( Drall::isEnvironment(EnvironmentId::Test) || - $input->getOption('drall-no-progress') + $input->getOption('no-progress') ) { return TRUE; } diff --git a/src/Command/SiteAliasesCommand.php b/src/Command/SiteAliasesCommand.php index a324734..bd8c391 100644 --- a/src/Command/SiteAliasesCommand.php +++ b/src/Command/SiteAliasesCommand.php @@ -17,8 +17,8 @@ protected function configure() { $this->setAliases(['sa']); $this->setDescription('List all Drush site aliases.'); $this->addUsage('site:aliases'); - $this->addUsage('--drall-group=GROUP site:aliases'); - $this->addUsage('--drall-filter=FILTER site:aliases'); + $this->addUsage('--group=GROUP site:aliases'); + $this->addUsage('--filter=FILTER site:aliases'); } protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/src/Command/SiteDirectoriesCommand.php b/src/Command/SiteDirectoriesCommand.php index 37708d8..ab5ec36 100644 --- a/src/Command/SiteDirectoriesCommand.php +++ b/src/Command/SiteDirectoriesCommand.php @@ -17,7 +17,7 @@ protected function configure() { $this->setAliases(['sd']); $this->setDescription('List the values of the $sites array.'); $this->addUsage('site:directories'); - $this->addUsage('--drall-group=GROUP site:directories'); + $this->addUsage('--group=GROUP site:directories'); } protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/src/Command/SiteKeysCommand.php b/src/Command/SiteKeysCommand.php index 962e682..e75501c 100644 --- a/src/Command/SiteKeysCommand.php +++ b/src/Command/SiteKeysCommand.php @@ -17,8 +17,8 @@ protected function configure() { $this->setAliases(['sk']); $this->setDescription('List the keys of the $sites array.'); $this->addUsage('site:keys'); - $this->addUsage('--drall-group=GROUP site:keys'); - $this->addUsage('--drall-filter=FILTER site:keys'); + $this->addUsage('--group=GROUP site:keys'); + $this->addUsage('--filter=FILTER site:keys'); } protected function execute(InputInterface $input, OutputInterface $output): int { diff --git a/src/Drall.php b/src/Drall.php index 4e04b78..d89d7d4 100644 --- a/src/Drall.php +++ b/src/Drall.php @@ -42,15 +42,25 @@ public function __construct() { protected function configureIO(InputInterface $input, OutputInterface $output): void { parent::configureIO($input, $output); - if ($input->hasParameterOption('--drall-debug', TRUE)) { + if ($input->hasParameterOption('--debug', TRUE)) { $output->setVerbosity(OutputInterface::VERBOSITY_DEBUG); } - elseif ($input->hasParameterOption('--drall-verbose', TRUE)) { + elseif ($input->hasParameterOption('--verbose', TRUE)) { $output->setVerbosity(OutputInterface::VERBOSITY_VERY_VERBOSE); } else { $output->setVerbosity(OutputInterface::VERBOSITY_NORMAL); } + + // The parent::configureIO sets verbosity in a SHELL_VERBOSITY. This causes + // other Symfony Console apps to become verbose, for example, Drush. To + // prevent such behavior, we force the SHELL_VERBOSITY to be normal. + $shellVerbosity = 0; + if (\function_exists('putenv')) { + @putenv("SHELL_VERBOSITY=$shellVerbosity"); + } + $_ENV['SHELL_VERBOSITY'] = $shellVerbosity; + $_SERVER['SHELL_VERBOSITY'] = $shellVerbosity; } protected function getDefaultInputDefinition(): InputDefinition { @@ -58,18 +68,15 @@ protected function getDefaultInputDefinition(): InputDefinition { // Remove unneeded options. $options = $definition->getOptions(); - unset($options['verbose'], $options['quiet']); + unset( + $options['quiet'], + $options['no-interaction'], + ); $definition->setOptions($options); $definition->addOption(new InputOption( - 'drall-verbose', - NULL, - InputOption::VALUE_NONE, - 'Display verbose output for Drall.' - )); - $definition->addOption(new InputOption( - 'drall-debug', - NULL, + 'debug', + 'd', InputOption::VALUE_NONE, 'Display debugging output for Drall.' )); diff --git a/src/Model/RawCommand.php b/src/Model/RawCommand.php deleted file mode 100644 index a05cdaa..0000000 --- a/src/Model/RawCommand.php +++ /dev/null @@ -1,39 +0,0 @@ -command = $command; - } - - public function __toString() { - return $this->command; - } - - /** - * Extracts a Drall sub-command from $argv, ignoring parts that are only for Drall. - * - * @param array $argv - * An $argv array. - * - * @return self - * The command without Drall elements. - * - * @example - * Input: ['/path/to/drall', 'exs', 'drush', '--drall-option=foo', 'st', '--fields=site'] - * Output: 'drush st --fields=site' - */ - public static function fromArgv(array $argv): self { - // Ignore the script name and the word "exec". - $parts = array_slice($argv, 2); - // Ignore options with --drall namespace. - $parts = array_filter($parts, fn($w) => !str_starts_with($w, '--drall-')); - - return new RawCommand(implode(' ', $parts)); - } - -} diff --git a/test/Integration/Command/ExecCommandTest.php b/test/Integration/Command/ExecCommandTest.php index 58f1585..2cd0692 100644 --- a/test/Integration/Command/ExecCommandTest.php +++ b/test/Integration/Command/ExecCommandTest.php @@ -11,12 +11,38 @@ */ class ExecCommandTest extends TestCase { + /** + * @testdox Detects commands correctly. + */ + public function testCommandDetection() { + $process = Process::fromShellCommandline( + 'drall exec --debug --dry-run drush st', + static::PATH_DRUPAL, + ); + $process->run(); + $this->assertEquals(<<getOutput()); + } + /** * @testdox With no Drupal installation. */ public function testWithNoDrupal(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush --uri=@@dir core:status', + 'drall exec -- ./vendor/bin/drush --uri=@@dir core:status', static::PATH_NO_DRUPAL, ); $process->run(); @@ -35,7 +61,7 @@ public function testWithNoDrupal(): void { */ public function testWithEmptyDrupal(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush --uri=@@dir core:status', + 'drall exec -- ./vendor/bin/drush --uri=@@dir core:status', static::PATH_EMPTY_DRUPAL, ); $process->run(); @@ -50,9 +76,9 @@ public function testWithEmptyDrupal(): void { } /** - * @testdox With no placeholders. + * @testdox Raises error for non-Drush command with no placeholders. */ - public function testWithNoPlaceholders(): void { + public function testNonDrushWithNoPlaceholders(): void { $process = Process::fromShellCommandline('drall exec foo', static::PATH_DRUPAL); $process->run(); $this->assertOutputEquals( @@ -66,7 +92,7 @@ public function testWithNoPlaceholders(): void { */ public function testWorkingDirectory(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-filter=tmnt "echo \"Site: @@site\" && pwd"', + 'drall exec --filter=tmnt "echo \"Site: @@site\" && pwd"', static::PATH_DRUPAL, ); $process->run(); @@ -83,21 +109,21 @@ public function testWorkingDirectory(): void { */ public function testDrushWithUriPlaceholder(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush --uri=@@dir core:status --fields=site', + 'drall exec -- ./vendor/bin/drush --uri=@@dir core:status --field=site', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<getOutput()); } @@ -107,45 +133,45 @@ public function testDrushWithUriPlaceholder(): void { */ public function testDrushWithSitePlaceholder(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush @@site.local core:status --fields=site', + 'drall exec ./vendor/bin/drush -- @@site.local core:status --field=site', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<getOutput()); } /** - * @testdox With no placeholders. + * @testdox Injects --uri for Drush command with no placeholders. */ public function testDrushWithNoPlaceholders(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush core:status --fields=site', + 'drall exec -- ./vendor/bin/drush core:status --field=site', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<getOutput()); } @@ -266,31 +292,44 @@ public function testWithDirPlaceholder(): void { } /** - * @testdox With --drall-filter. + * @testdox With --filter. */ public function testWithFilter(): void { - $process = Process::fromShellCommandline( - 'drall exec --drall-filter=leo ./vendor/bin/drush st --field=site', + $process1 = Process::fromShellCommandline( + 'drall exec --filter=leo -- ./vendor/bin/drush st --field=site', static::PATH_DRUPAL, ); - $process->run(); + $process1->run(); $this->assertOutputEquals(<<getOutput()); +EOF, $process1->getOutput()); + + // Short form. + $process2 = Process::fromShellCommandline( + 'drall exec -f leo -- ./vendor/bin/drush st --field=site', + static::PATH_DRUPAL, + ); + $process2->run(); + $this->assertOutputEquals(<<getOutput()); } /** - * @testdox With @@dir placeholder and --drall-debug. + * @testdox With @@dir placeholder and --debug. */ public function testWithDirPlaceholderAndDebug(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-debug ls web/sites/@@dir/settings.php', + 'drall exec --debug ls web/sites/@@dir/settings.php', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<run(); + $process1->run(); $this->assertOutputEquals(<<getOutput()); +EOF, $process1->getOutput()); + + // Short form. + $process2 = Process::fromShellCommandline( + 'drall exec -g bluish -- ./vendor/bin/drush st --field=site', + static::PATH_DRUPAL, + ); + $process2->run(); + $this->assertOutputEquals(<<getOutput()); } /** @@ -333,7 +386,7 @@ public function testWithGroup(): void { */ public function testWithGroupEnvVar(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush st --field=site', + 'drall exec -- ./vendor/bin/drush st --field=site', static::PATH_DRUPAL, ['DRALL_GROUP' => 'bluish'], ); @@ -352,7 +405,7 @@ public function testWithGroupEnvVar(): void { */ public function testWithSitePlaceholder(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush @@site.local core:status --fields=site', + 'drall exec ./vendor/bin/drush -- @@site.local core:status --fields=site', static::PATH_DRUPAL, ); $process->run(); @@ -372,15 +425,16 @@ public function testWithSitePlaceholder(): void { } /** - * @testdox With @@site placeholder and --drall-debug. + * @testdox With @@site placeholder and --debug. */ public function testWithSitePlaceholderDebug(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-debug ./vendor/bin/drush @@site.local st --fields=site', + 'drall exec --debug -- ./vendor/bin/drush @@site.local st --fields=site', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<run(); $this->assertOutputEquals(<<getOutput()); } @@ -423,7 +477,7 @@ public function testWithSitePlaceholderAndGroup(): void { */ public function testCatchStdErrOutput(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-filter=default ./vendor/bin/drush --verbose version', + 'drall exec --filter=default -- ./vendor/bin/drush --verbose version', static::PATH_DRUPAL, ); $process->run(); @@ -446,7 +500,7 @@ public function testCatchStdErrOutput(): void { */ public function testWithProgressBarVisible(): void { $process = Process::fromShellCommandline( - 'drall exec ./vendor/bin/drush st --field=site 2>&1', + 'drall exec -- ./vendor/bin/drush st --field=site 2>&1', static::PATH_DRUPAL, ['DRALL_ENVIRONMENT' => 'unknown'], ); @@ -468,11 +522,11 @@ public function testWithProgressBarVisible(): void { } /** - * @testdox With --drall-no-progress. + * @testdox With --no-progress. */ public function testWithProgressBarHidden(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-no-progress ./vendor/bin/drush st --field=site 2>&1', + 'drall exec --no-progress -- ./vendor/bin/drush st --field=site 2>&1', static::PATH_DRUPAL, // The progress bar is always hidden in the "test" environment to avoid // repeating --no-progress in all commands. Thus, for this test, @@ -497,72 +551,99 @@ public function testWithProgressBarHidden(): void { } /** - * @testdox With --drall-no-execute. + * @testdox With --dry-run. */ - public function testWithNoExecute(): void { - $process = Process::fromShellCommandline( - 'drall exec --drall-no-execute ./vendor/bin/drush core:status', + public function testWithDryRun(): void { + $process1 = Process::fromShellCommandline( + 'drall exec --dry-run -- ./vendor/bin/drush st', static::PATH_DRUPAL, ); - $process->run(); + $process1->run(); $this->assertOutputEquals(<<getOutput()); +EOF, $process1->getOutput()); + + // Short form. + $process2 = Process::fromShellCommandline( + 'drall exec -X -- ./vendor/bin/drush st', + static::PATH_DRUPAL, + ); + $process2->run(); + $this->assertOutputEquals(<<getOutput()); } /** - * @testdox With --drall-no-execute --drall-verbose. + * @testdox With --dry-run --verbose. */ - public function testWithNoExecuteVerbose(): void { + public function testWithDryRunVerbose(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-no-execute --drall-verbose drush core:status', + 'drall exec --dry-run --verbose -- drush st', static::PATH_DRUPAL, ); $process->run(); $this->assertOutputEquals(<<getOutput()); } /** - * @testdox With --drall-workers=2. + * @testdox With --workers=2. */ public function testWithWorkers(): void { - $process = Process::fromShellCommandline( - 'drall ex --drall-workers=2 --drall-verbose drush --uri=@@dir core:status --fields=site', + $process1 = Process::fromShellCommandline( + 'drall ex --workers=2 --verbose -- drush --uri=@@dir core:status --fields=site', static::PATH_DRUPAL, ); - $process->run(); + $process1->run(); $this->assertStringStartsWith( '[notice] Using 2 workers.', - $process->getOutput(), + $process1->getOutput(), + ); + + // Short form. + $process2 = Process::fromShellCommandline( + 'drall ex -w2 --verbose -- drush --uri=@@dir core:status --fields=site', + static::PATH_DRUPAL, + ); + $process2->run(); + + $this->assertStringStartsWith( + '[notice] Using 2 workers.', + $process2->getOutput(), ); } /** - * @testdox With --drall-workers=17. + * @testdox With --workers=17. * * Drall caps the maximum workers to a pre-determined limit. */ public function testWorkerLimit(): void { $process = Process::fromShellCommandline( - 'drall ex --drall-workers=17 --drall-verbose drush --uri=@@dir st --fields=site', + 'drall ex --workers=17 --verbose -- drush --uri=@@dir st --fields=site', static::PATH_DRUPAL, ); $process->run(); @@ -577,7 +658,7 @@ public function testWorkerLimit(): void { */ public function testNonZeroExitCode(): void { $process = Process::fromShellCommandline( - 'drall exec --drall-group=bad ./vendor/bin/drush st --field=site', + 'drall exec --group=bad ./vendor/bin/drush st --field=site', ); $process->run(); $this->assertEquals(1, $process->getExitCode()); diff --git a/test/Integration/Command/SiteAliasesCommandTest.php b/test/Integration/Command/SiteAliasesCommandTest.php index f3c56d2..61b0d7d 100644 --- a/test/Integration/Command/SiteAliasesCommandTest.php +++ b/test/Integration/Command/SiteAliasesCommandTest.php @@ -46,11 +46,11 @@ public function testExecute(): void { } /** - * @testdox with --drall-filter. + * @testdox with --filter. */ public function testWithFilter(): void { $process = Process::fromShellCommandline( - 'drall site:aliases --drall-filter="leo||ralph"', + 'drall site:aliases --filter="leo||ralph"', static::PATH_DRUPAL, ); $process->run(); @@ -62,11 +62,11 @@ public function testWithFilter(): void { } /** - * @testdox with --drall-group. + * @testdox with --group. */ public function testWithGroup(): void { $process = Process::fromShellCommandline( - 'drall site:aliases --drall-group=reddish', + 'drall site:aliases --group=reddish', static::PATH_DRUPAL ); $process->run(); diff --git a/test/Integration/Command/SiteDirectoriesCommandTest.php b/test/Integration/Command/SiteDirectoriesCommandTest.php index 739deeb..87cd380 100644 --- a/test/Integration/Command/SiteDirectoriesCommandTest.php +++ b/test/Integration/Command/SiteDirectoriesCommandTest.php @@ -46,10 +46,10 @@ public function testExecute(): void { } /** - * @testdox with --drall-filter. + * @testdox with --filter. */ public function testExecuteWithFilter(): void { - $process = Process::fromShellCommandline('drall site:directories --drall-filter="leo||ralph"', static::PATH_DRUPAL); + $process = Process::fromShellCommandline('drall site:directories --filter="leo||ralph"', static::PATH_DRUPAL); $process->run(); $this->assertOutputEquals(<<run(); $this->assertOutputEquals(<<run(); $this->assertOutputEquals(<<run(); $this->assertOutputEquals(<<getDefinition()->getOptions(); - $this->assertArrayNotHasKey('quiet', $options); - $this->assertArrayNotHasKey('verbose', $options); - $this->assertArrayHasKey('drall-verbose', $options); - $this->assertArrayHasKey('drall-debug', $options); + $this->assertEquals([ + 'help', + 'verbose', + 'version', + 'ansi', + 'debug', + ], array_keys($options)); } public function testOptionVerbosityNormal() { @@ -35,13 +38,13 @@ public function testOptionVerbosityNormal() { public function testOptionVerbosityVerbose() { $tester = new ApplicationTester(new Drall()); - $tester->run(['command' => 'version', '--drall-verbose' => TRUE]); + $tester->run(['command' => 'version', '--verbose' => TRUE]); $this->assertEquals(OutputInterface::VERBOSITY_VERY_VERBOSE, $tester->getOutput()->getVerbosity()); } public function testOptionVerbosityDebug() { $tester = new ApplicationTester(new Drall()); - $tester->run(['command' => 'version', '--drall-debug' => TRUE]); + $tester->run(['command' => 'version', '--debug' => TRUE]); $this->assertEquals(OutputInterface::VERBOSITY_DEBUG, $tester->getOutput()->getVerbosity()); } diff --git a/test/Unit/Model/RawCommandTest.php b/test/Unit/Model/RawCommandTest.php deleted file mode 100644 index 47857f6..0000000 --- a/test/Unit/Model/RawCommandTest.php +++ /dev/null @@ -1,21 +0,0 @@ -assertEquals('hello world', (string) $command); - } - - public function testFromArgv() { - $command = RawCommand::fromArgv(['/path/to/drall', 'exs', 'ls', '-alh']); - $this->assertEquals("ls -alh", (string) $command); - } - -} From a010ec6d8ea7ecde0eedb7aa850a1bb808baaf88 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Thu, 26 Dec 2024 23:04:38 -0500 Subject: [PATCH 2/4] Force -- when options are used; refs #45 --- src/Command/ExecCommand.php | 32 ++++++++++++--- test/Integration/Command/ExecCommandTest.php | 42 ++++++++++++++++++-- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index 52ceca3..3894d66 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -22,7 +22,7 @@ /** * A command to execute a shell command on multiple sites. */ -class ExecCommand extends BaseCommand { +final class ExecCommand extends BaseCommand { use SignalAwareTrait; @@ -93,7 +93,10 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output): int { $this->preExecute($input, $output); - $command = $this->getCommand($input); + if (!$command = $this->getCommand($input, $output)) { + return 1; + } + $group = $this->getDrallGroup($input); $filter = $this->getDrallFilter($input); @@ -210,16 +213,35 @@ function ($value) use ($command, $placeholder, $output, $progressBar, &$exitCode * * @param \Symfony\Component\Console\Input\InputInterface $input * Console input. + * @param \Symfony\Component\Console\Output\OutputInterface $output + * Console output. * - * @return string + * @return string|null * The command without Drall elements. * * @example * Input: /path/to/drall exec --verbose -- drush st --fields=site * Output: drush st --fields=site */ - protected function getCommand(InputInterface $input): string { - // @todo Force -- for clarity if options are present. + private function getCommand(InputInterface $input, OutputInterface $output): ?string { + $rawTokens = $input->getRawTokens(TRUE); + if (!in_array('--', $rawTokens)) { + foreach ($rawTokens as $token) { + if (str_starts_with($token, '-')) { + $output->writeln(<<Incorrect: drall exec --dry-run drush --field=site core:status +Correct: drall exec --dry-run -- drush --field=site core:status + +Notice the `--` between `--dry-run` and the word `drush`. +EOT); + $this->logger->error('Separator "--" must be used when using options.'); + return NULL; + } + } + } + // @todo Throw an error if --drall-* options are present. // Everything after the first "--" is treated as an argument. All such // arguments are treated as parts of the command to be executed. diff --git a/test/Integration/Command/ExecCommandTest.php b/test/Integration/Command/ExecCommandTest.php index 2cd0692..2f80bea 100644 --- a/test/Integration/Command/ExecCommandTest.php +++ b/test/Integration/Command/ExecCommandTest.php @@ -16,7 +16,7 @@ class ExecCommandTest extends TestCase { */ public function testCommandDetection() { $process = Process::fromShellCommandline( - 'drall exec --debug --dry-run drush st', + 'drall exec --debug --dry-run -- drush st', static::PATH_DRUPAL, ); $process->run(); @@ -34,6 +34,42 @@ public function testCommandDetection() { # Item: ralph drush --uri=ralph st +EOT, $process->getOutput()); + } + + /** + * @testdox Works when -- is absent and options are not used. + */ + public function testMissingArgsSeparatorWithNoOptions(): void { + $process = Process::fromShellCommandline( + 'drall exec --dry-run drush st', + static::PATH_DRUPAL, + ); + $process->run(); + $this->assertOutputEquals(<<getOutput()); + } + + /** + * @testdox Shows error when -- is absent but options are used. + */ + public function testMissingArgsSeparatorWithOptions(): void { + $process = Process::fromShellCommandline( + 'drall exec --dry-run drush st', + static::PATH_DRUPAL, + ); + $process->run(); + $this->assertOutputEquals(<<getOutput()); } @@ -92,7 +128,7 @@ public function testNonDrushWithNoPlaceholders(): void { */ public function testWorkingDirectory(): void { $process = Process::fromShellCommandline( - 'drall exec --filter=tmnt "echo \"Site: @@site\" && pwd"', + 'drall exec --filter=tmnt -- "echo \"Site: @@site\" && pwd"', static::PATH_DRUPAL, ); $process->run(); @@ -324,7 +360,7 @@ public function testWithFilter(): void { */ public function testWithDirPlaceholderAndDebug(): void { $process = Process::fromShellCommandline( - 'drall exec --debug ls web/sites/@@dir/settings.php', + 'drall exec --debug -- ls web/sites/@@dir/settings.php', static::PATH_DRUPAL, ); $process->run(); From 87740596ca2f64eace4883548eaf199ed9deb9f9 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Fri, 27 Dec 2024 00:18:02 -0500 Subject: [PATCH 3/4] Exit with non-zero code when applicable --- bin/drall | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/drall b/bin/drall index f546fd3..e2e1465 100755 --- a/bin/drall +++ b/bin/drall @@ -26,11 +26,11 @@ use Drall\Drall; try { $drall = new Drall(); - $drall->run(); + $exitCode = $drall->run(); } catch (Exception $e) { echo "ERROR {$e->getCode()}: {$e->getMessage()}"; exit(1); } -exit(0); +exit($exitCode); From 2d1d40dd143c109cc7671f469aced517c84a28c5 Mon Sep 17 00:00:00 2001 From: Jerry Radwick Date: Fri, 27 Dec 2024 00:18:58 -0500 Subject: [PATCH 4/4] Show error when --drall-* options are detected --- src/Command/BaseCommand.php | 12 ++++++++---- src/Command/ExecCommand.php | 17 +++++++++++++++++ test/Integration/DrallTest.php | 23 ++++++++++++++++++++--- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/Command/BaseCommand.php b/src/Command/BaseCommand.php index 9a5abac..2299e6e 100644 --- a/src/Command/BaseCommand.php +++ b/src/Command/BaseCommand.php @@ -32,6 +32,14 @@ protected function configure() { ); } + protected function initialize(InputInterface $input, OutputInterface $output): void { + if (!$this->logger) { + $this->logger = new ConsoleLogger($output); + } + + parent::initialize($input, $output); + } + /** * Gets the active Drall group. * @@ -65,10 +73,6 @@ protected function getDrallFilter(InputInterface $input): ?string { } protected function preExecute(InputInterface $input, OutputInterface $output) { - if (!$this->logger) { - $this->logger = new ConsoleLogger($output); - } - if (!$this->hasSiteDetector()) { $this->setSiteDetector(new SiteDetector()); } diff --git a/src/Command/ExecCommand.php b/src/Command/ExecCommand.php index 3894d66..28a9b4c 100644 --- a/src/Command/ExecCommand.php +++ b/src/Command/ExecCommand.php @@ -90,6 +90,23 @@ protected function configure() { $this->ignoreValidationErrors(); } + protected function initialize(InputInterface $input, OutputInterface $output): void { + // If obsolete --drall-* options are present, then abort. + if (method_exists($input, 'getRawTokens')) { + foreach ($input->getRawTokens() as $token) { + if (str_starts_with($token, '--drall-')) { + $output->writeln(<<preExecute($input, $output); diff --git a/test/Integration/DrallTest.php b/test/Integration/DrallTest.php index 69489b1..2f55e63 100644 --- a/test/Integration/DrallTest.php +++ b/test/Integration/DrallTest.php @@ -12,7 +12,10 @@ */ class DrallTest extends TestCase { - public function testVersion() { + /** + * @testdox Returns correct version string. + */ + public function testVersion(): void { $process = Process::fromShellCommandline('drall --version', static::PATH_DRUPAL); $process->run(); $version = InstalledVersions::getPrettyVersion('jigarius/drall'); @@ -20,9 +23,9 @@ public function testVersion() { } /** - * Run drall with a command it doesn't recognize. + * @testdox Suggests "drush" for unrecognized commands. */ - public function testUnrecognizedCommand() { + public function testUnrecognizedCommand(): void { $process = Process::fromShellCommandline('drall st', static::PATH_DRUPAL); $process->run(); $this->assertOutputEquals(<<getErrorOutput()); } + /** + * @testdox Shows error when --drall-* options are detected. + */ + public function testShowErrorForObsoleteOptions(): void { + $process = Process::fromShellCommandline('./vendor/bin/drall exec --drall-foo drush st', static::PATH_DRUPAL); + $process->run(); + $this->assertOutputEquals(<<getOutput()); + $this->assertEquals(1, $process->getExitCode()); + } + }