Drush throws class not found error because of DrupalConsole integration in Metatag?

[damienmckenna]

With the Console integration in Metatag, any time Drush is used it fails with the following error:

PHP Fatal error: Class 'Drupal\Console\Command\GeneratorCommand' not found in /var/www/drupal8/modules/metatag/src/Command/GenerateGroupCommand.php on line 27
Drush command terminated abnormally due to an unrecoverable error.
Error: Class 'Drupal\Console\Command\GeneratorCommand' not found in
/var/www/drupal8/modules/metatag/src/Command/GenerateGroupCommand.php,line 27

OTOH, Console commands work, and there aren't any problems with the site itself due to the integration.

Could this be a problem in Drush 8?

[damienmckenna]

I've opened an issue for Metatag about it too: https://www.drupal.org/node/2786795

[jmolivas]

I will give a try, wonder why is drush trying to load DrupalConsole classes

[VladimirAus]

Same happend to me after updating metatag module from 8.x-1.0-beta9+33-dev to 8.x-1.0-beta10.

[greg-1-anderson]

@jmolivas: you might happen to recall that at DrupalCon New Orleans, we agreed that using services was a good way for Drupal 8 modules to load CLI commands, and we also agreed that the tag 'console.command' should be used to identify a CLI command that was based only on Symfony Console and Drupal 8 APIs.

Since that time, for various reasons, no one has really stepped up to ensure that there can be unified console commands that work with both Drush and Drupal Console. This is unfortunate, but I don't attribute any blame to this particular state of affairs.

The thing that is broken, though, is that DrupalConsole implemented code to search for 'console.command' service tags, and used them to load DrupalConsole-specific classes. Of course this causes Drush to blow up; you got this identifier from me at dcnola. You just forgot.

So, what's the best solution? If we were ever ever ever going to try to allow module developers to use generic Symfony commands in a Drush / DrupalConsole agnostic sort of way, then DrupalConsole would rename their service tag to 'drupalconsole.command', and ensure that any Command class constructed from 'console.command' was never passed any DrupalConsole-specific objects. Of course, DrupalConsole documentation would also have to instruct module developers to follow these conventions, and there would be some work involved for anyone who followed the existing pattern to adjust.

The irony of this bug is that the non-conforming tool makes the other one look broken. If Drush passed Drush-specific objects to 'console.command' commands, then that would break DrupalConsole if any module developer used that facility. In light of that, and the Sisyphean task of keeping disparate projects compatible (or even getting to the top of the hill the first time), maybe Drush should just get out of DrupalConsole's way, and stop trying to load 'console.command' commands.

[jmolivas]

@greg-1-anderson New way of registering commands on DrupalConsole for RC-1 will be:

• Service definition
  https://github.com/hechoendrupal/DrupalConsole/blob/master/config/services/cron.yml#L2-L6

  console.cron_debug:
    class: Drupal\Console\Command\Cron\DebugCommand
    arguments: ['@module_handler']
    tags:
      - { name: console.command }

• Command class
  https://github.com/hechoendrupal/DrupalConsole/blob/master/src/Command/Cron/DebugCommand.php#L27-L30

<?php
/**
 * @file
 * Contains \Drupal\Console\Command\Cron\DebugCommand.
 */
namespace Drupal\Console\Command\Cron;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Command\Command;
use Drupal\Console\Command\Shared\CommandTrait;
use Drupal\Console\Style\DrupalStyle;
class DebugCommand extends Command
{
    use CommandTrait;
    /** @var \Drupal\Core\Extension\ModuleHandlerInterface  */
    protected $moduleHandler;
    /**
     * DebugCommand constructor.
     * @param $moduleHandler
     */
    public function __construct($moduleHandler) {
        $this->moduleHandler = $moduleHandler;
        parent::__construct();
    }
    protected function configure()
    {
        $this
            ->setName('cron:debug')
            ->setDescription($this->trans('commands.cron.debug.description'));
    }
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $io = new DrupalStyle($input, $output);
        $io->section(
            $this->trans('commands.cron.debug.messages.module-list')
        );
        $io->table(
            [
                $this->trans('commands.cron.debug.messages.module')
            ],
            $this->moduleHandler->getImplementations('cron'),
            'compact'
        );
    }
}

This changes should avoid any errors.

Unfortunately I have not had a chance to review the code on metatag module since this error happened. I can probably take a look to metatag issue during the weekend.

[greg-1-anderson]

@jmolivas: I guess you're telling me to get out of the way, then? Misunderstood the upshot of the code 1^

[jmolivas]

@greg-1-anderson What?, What I told you is that this will be fixed on next release and I will be able to take a look at metatag issue in a few days, to see if I can help fixing it.

[greg-1-anderson]

Does 2^ mean that you are moving all of the DrupalConsole-specific classes out of constructors consistently?

Translation is moving to a trait? That is cool.

Is the upshot that 'console.command' is safe to use universally?

[greg-1-anderson]

Thanks for fixing this.

[greg-1-anderson]

I'm not sure that traits fully solve the problem, though. How does Drush know that the command is DrupalConsole-specific, if it is?

[jmolivas]

@greg-1-anderson which version of drush is having the issue 8 or 9 so I can try.

[greg-1-anderson]

@jmolivas Both Drush 8 and Drush 9 will attempt to load services tagged 'console.command'.

It looks like the RC1 changes come close to the point where any errors would be deferred to the time when the command was used. That would only be fully the case if (a) translation was not used in configure, or (b) the translation traits were moved to a separate library, and included by Drush.

Anyway, if we can catch exceptions, then we'll catch exceptions, if that works. It would be even better if we could tell definitively if there were unresolved dependencies in a command service.

[greg-1-anderson]

In #2593, users are advised to use:

   tags:
      - { name: console.command }

in their services file. However, they are also advised to use DrupalConsole-specific traits.

Choose one of three possible resolutions:

1. Update this documentation to advise the use of drupalconsole.command whenever DrupalConsole-specific traits are used
2. All of the DrupalConsole-specific traits need to move to Consolidation-org, so that Drush can use them (make them not DrupalConsole-specific)
3. Drush needs to stop loading console.command services (maybe rename to drush.comman) if "console.command" is going to mean "Drupal Console" commands rather than "generic Symfony Console commands".

[damienmckenna]

With DrupalCon next week, can we please solve our differences and get this resolved?

What we want is:

• A documented procedure for building Drupal Console commands in Drupal modules.
• Assurance that using these procedures won't blow up other things, e.g. Drush.
• Documentation on what changes need to be made to modules that have the integration already added.

[greg-1-anderson]

I regret that I won't be at DrupalCon; it would be good to work this out at a sprint. I am ready to pitch in to help make this work as soon as the DC maintainers pick a strategy they're happy with. Some options (non-exhaustive) are listed above.

[greg-1-anderson]

This blog implies we need to move forward with #3 above. I would prefer another solution, but it would be good to have clear direction on which way we're going here.

[jmolivas]

@greg-1-anderson as mentioned before we are need to test DrupalConsole + Metatag + Drush and this will happen on Dublin.

We are also going to BADCamp, we can solve any remaining issue there if any.

[greg-1-anderson]

Wish I was going to Dublin. BADCamp is kind of a long time to leave this issue open. I'll try to be online whatever odd hour (odd in PDT) you decide to work on this issue, if you let me know when you'll be working on it.

[damienmckenna]

I'm going to be working on Metatag next week, so I'm hoping that the issue won't exist in two weeks :)

[jmolivas]

We decide to rename the service tag. The changes on DrupalConsole to avoid this are:

• Rename service tag console.command => drupal.command
• Rename service tag console.generator => drupal.generator

Related PRs:

• Update service tag #2729
• Update service tag drupal-console-core#53
• Update service tag drupal-console-launcher#28

[jmolivas]

^^ @greg-1-anderson @damienmckenna @VladimirAus ^^

[greg-1-anderson]

Great, thanks so much for that.

Looks like console.command is no longer supported at all. I think it might be best for Drush to rename Drush commands to drush.command as well, and reserve console.command for some future time, if ever, when generic commands could be written to run in both places, so that we don't have this problem in the opposite direction.

[greg-1-anderson]

FYI, drush master and drush 8.x branches now ignore console.command, and only load services tagged drush.command.

[jmolivas]

Fixed