Allow commands to call other commands

[grasmash]

I would like to be able to calls other commands from my annotated command using an annotation. E.g.,

<?php

  /**
   * @command tests:all
   *
   * @calls tests:behat
   * @calls tests:phpunit
   */
  public function testsAll() {
  }

  /**
   * @command tests:behat
   * @validateme
   */
  public function testsBehat() {
    ...
  }

In this example, using @calls tests:behat would cause the tests:behat command to be executed prior the the body of testsAll().

This is better than simply calling $this->testsBehat() from $this->testsAll() because it will invoke the hooks for $this->testsBehat().

[greg-1-anderson]

Yes, we definitely this functionality. I'm ambivalent about adding an annotation for this feature, though. I think it might be better to simply have folks use php to call the commands they want to invoke. Something like:

/**
   * @command tests:all
   *
   * @calls tests:behat
   * @calls tests:phpunit
   */
  public function testsAll() {
    $result = $invoker->invoke('tests:behat');
    return $result;
  }

This would give the caller a chance to do something with the result, if needed.

The advantage of the annotation is that the tests:all command does not need to obtain a reference to the invoker. This is not easily solved in terms of the annotated command library, so this would be an additional burden on the caller. An annotation could be processed right inside the CommandProcessor, though, which would be easier. @calls is probably not difficult to write; the vestigial functions that contain nothing but annotations just seem a bit odd. We already have that sometimes, though, e.g. if someone wants to use annotations simply to factor out a common set of options shared between multiple commands.

Undecided about the best choice here; I'm okay with moving forward with @calls if you think that's best for the current needs.

[grasmash]

I think I'd prefer to go with the @calls tag.

Let's assume that a command needs to call multiple other commands, and fail if any one of those other commands fail. To do this with PHP you'd need:

<?php

  /**
   * @command tests:all
   */
  public function testsAll() {
    $result = $invoker->invoke('tests:behat');
    if (!$result) {
      return 1;
    }
    $result = $invoker->invoke('tests:phpunit');
    if (!$result) {
      return 1;
    }
    $result = $invoker->invoke('tests:custom');
    if (!$result) {
      return 1;
    }
  }

This quickly becomes cumbersome.

Ideally the responsibility of handling failure and returning an exit code would be moved out of the calling command itself. Using an annotation accomplishes this.

  /**
   * @command tests:all
   *
   * @calls tests:behat
   * @calls tests:phpunit
   * @calls tests:custom
   */
  public function testsAll() {
  }

[greg-1-anderson]

OK, how about something like this:

  /**
   * @command tests:all
   */
  public function testsAll() {
    $builder = $this->collectionBuilder();
    return $builder
        ->taskCall('tests:behat')
        ->taskCall('tests:phpunit');
  }

Collections will already abort if one step fails. We'd just need a task implementation that had a reference to the Symfony Application -- easy enough to build that.

[greg-1-anderson]

That ^^ would have to move to the Robo PHP project, of course.

[weitzman]

Drush does a lot of this. For our needs, I think using straight PHP per #64 (comment) is our best fit.

[JacobSanford]

This would be wonderful to have implemented - in any form. As mentioned by @grasmash , hooks do not (understandably) fire with $this->commandMethod().

However, annotations completely aside - I don't think this is possible at all, correct?

To leverage the hook system, in rare cases where we want to run a command from within a command we have (unfortunately) resorted to standardizing on using exec() to run a second robo from within robo to allow the command hooks to fire.

Many Thanks!

[greg-1-anderson]

With a reference to the $application object, it is possible to call $application->run($args) to run another command within the same PHP process. The awkward part is that you need to instantiate an ArrayInput or a StringInput to do so.

[Sweetchuck]

I ran into a use case where it would be very handy if I were able to call other commands, so I found this issue.

I like the idea of the command caller task as suggested by Greg Anderson a few comments above.

For me the @calls annotation is a little bit weird, because in that case I define in the annotation what the command will do. I think the annotation is only for providing meta info and do the action in the method with PHP code.
And what would happen if I were use the @calls annotation and I write some code in the method body?
Which one runs first?

---

I think one of the problem is that the \Consolidation\AnnotatedCommand\AnnotatedCommand creates an instance from the FooCommands class, and it uses the same instance for all the @command which are defined in that class.
In the background the Applicationcreates as many AnnotatedCommand instance as many @command we have and stores it in the \Symfony\Component\Console\Application::$commands, but
all the \Consolidation\AnnotatedCommand\AnnotatedCommand::$commandCallback property refers to the same FooCommands instance.
Which means that they use the same $this->input() object. (And of course every other object property is the same)
So the sub-called command can't use the $this->input() because it contains the arguments and options of the main command. (Which come from the CLI)

I think the real command class instance should be created in the \Consolidation\AnnotatedCommand\AnnotatedCommand::execute() based on the passed $input and $output.

[Sweetchuck]

<?php

use Robo\Common\ConfigAwareTrait;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Yaml\Yaml;

class RoboFile extends \Robo\Tasks implements \Robo\Contract\ConfigAwareInterface
{
    use ConfigAwareTrait;

    /**
     * @var int
     */
    protected static $instanceCounter = 0;

    /**
     * @var int
     */
    protected $instanceIndex = 0;

    public function __construct()
    {
        static::$instanceCounter++;
        $this->instanceIndex = static::$instanceCounter;
    }

    /**
     * @command main:app-get-run
     */
    public function mainAppGetRun(
        ?string $arg01 = null,
        array $options = [
            'option-01' => '',
        ]
    ) {
        $this->action(
            '-= Main App Get Run in action =-',
            $arg01,
            $options['option-01']
        );

        /** @var \Robo\Application $app */
        $app = $this
            ->getContainer()
            ->get('application');

        $sub01Command = $app->get('sub:01');
        $sub01InputDefinition = $sub01Command->getDefinition();
        $sub01InputParameters = [
            'arg01' => 'subArg01Value',
            '--option-01' => 'subOption01Value',
        ];

        $sub01Input = new ArrayInput($sub01InputParameters, $sub01InputDefinition);

        $sub01Command->run($sub01Input, $this->output());
    }

    /**
     * @command main:app-run
     */
    public function mainAppRun(
        ?string $arg01 = null,
        array $options = [
            'option-01' => '',
        ]
    ) {
        $this->action(
            '-= Main App Run in action =-',
            $arg01,
            $options['option-01']
        );

        /** @var \Robo\Application $app */
        $app = $this
            ->getContainer()
            ->get('application');

        $sub01Command = $app->get('sub:01');
        $sub01InputParameters = [
            'command' => $sub01Command->getName(),
            'arg01' => 'subArg01Value',
            '--option-01' => 'subOption01Value',
            '--define' => 'environment=sub01Overridden',
        ];

        $sub01Input = new ArrayInput($sub01InputParameters);

        $app->run($sub01Input, $this->output());
    }

    /**
     * @command sub:01
     */
    public function sub01(
        ?string $arg01 = null,
        array $options = [
            'option-01' => null,
        ]
    ) {
        $this->action(
            '-= Sub 01 in action =-',
            $arg01,
            $options['option-01']
        );
    }

    protected function action(string $header, ?string $arg01 = null, ?string $option01 = null)
    {
        $input = $this->input();
        $output = $this->output();

        $output->writeln($header);
        $output->writeln("\$arg01 = $arg01");
        $output->writeln("\$option01 = $option01");
        $output->writeln('---');
        $output->write(Yaml::dump($input->getArguments(), 42));
        $output->writeln('---');
        $output->write(Yaml::dump($input->getOptions(), 42));
        $output->writeln('...');
        $output->writeln('');
    }

}

Run: robo main:app-get-run --option-01=mainOption01Value mainArg01Value
The output is something like this:

-= Main App Get Run in action =-
$arg01 = mainArg01Value
$option01 = mainOption01Value
---
command: 'main:app-get-run'
arg01: mainArg01Value
---
option-01: mainOption01Value
help: false
quiet: false
verbose: false
version: false
ansi: false
no-ansi: false
no-interaction: false
simulate: false
progress-delay: 2
define: {  }
...

-= Sub 01 in action =-
$arg01 = subArg01Value
$option01 = subOption01Value
---
command: 'main:app-get-run'
arg01: mainArg01Value
---
option-01: mainOption01Value
help: false
quiet: false
verbose: false
version: false
ansi: false
no-ansi: false
no-interaction: false
simulate: false
progress-delay: 2
define: {  }
...

In this case you can use the passed arguments of the ::sub01 method, but it is different then the $this->input().

---

In my use case I want modify the behaviour of a command in certain circumstances

<?php
/**
 * @hook option my:xyz
 */
public function myXyzHookOption(Command $command) {
  if ($this->getConfig()->get('foo') === 'bar') {
    $command->setDescription('Something else.');
    $this->hookOptionAddArguments(
      $command,
      [
        'arg01' => [
          'name' => 'arg01',
          'mode' => InputArgument::REQUIRED,
          'description' => 'Useful description',
          'default' => NULL,
        ],
      ]
    );
  }
}

/**
 * @command my:xyz
 */
public function myXyz() {
  if ($this->getConfig()->get('foo') === 'bar') {
    // ...
  } else {
    // ...
  }
}

In this case I can't explicitly define the parameters of the PHP method, so I have to use the $this->input() in order to retrieve the passed arguments and options.

[greg-1-anderson]

See also consolidation/robo#675

I think Robo is likely the better layer to provide this sort of functionality. The above PR has been approved, but I am still considering improvements (e.g. an invoker) to make it more convenient.

[Sweetchuck]

I think you can't solve this in Robo (or any kind of top level Application layer) while \Consolidation\AnnotatedCommand\AnnotatedCommand::$commandCallback refers to the same instance.
Every command should have a dedicated object instance.

[DieterHolvoet]

We have been accomplishing this for some time using the following trait: https://github.com/wieni/wmscaffold/blob/release/v1/src/Commands/RunCommandTrait.php.

Usage example:

<?php

namespace Drupal\some_module\Commands;

use Consolidation\SiteAlias\SiteAliasManagerAwareInterface;
use Drush\Commands\DrushCommands;
use Symfony\Component\Process\Exception\ProcessFailedException;

class FreshCommands extends DrushCommands implements SiteAliasManagerAwareInterface
{
    use RunCommandTrait;

    /**
     * Bring your site up to date with the latest file changes
     *
     * @command fresh
     * @aliases fr
     * @bootstrap none
     */
    public function fresh()
    {
        try {
            $this->drush('cache-rebuild');
            $this->drush('updatedb', ['no-post-updates' => true]);
            $this->drush('config-import');
            $this->drush('locale:check');
            $this->drush('locale:update');
            $this->drush('updatedb');
            $this->drush('cache-rebuild');
        } catch (ProcessFailedException $e) {
            return $e->getProcess()->getExitCode();
        }
    }

    /**
     * Bring your site up to date with the latest file changes
     * and export any new configuration from module update hooks.
     *
     * @command post-update
     * @aliases pu
     * @bootstrap none
     */
    public function postUpdate()
    {
        try {
            $this->drush('cache-rebuild');
            $this->drush('locale:check');
            $this->drush('locale:update');
            $this->drush('updatedb');
            $this->drush('config-export');
            $this->drush('cache-rebuild');
        } catch (ProcessFailedException $e) {
            return $e->getProcess()->getExitCode();
        }
    }
}