Integrate Symfony Console component to natively support command line operations

The issue summary should really describe at which point drush starts and when the console ends.

@mile23
Once we reach an agreement that we ant to have the symfony console in core, we should open on issue to pull in the code.

+1,000.

A key aspect here is a pluggable architecture.

First step towards that is to move e.g. the cache command into Drupal\Core\Cache\Console\ClearCache.

In other words, in abstract: <namespace>\Console\<Command>.php

From there on, things will become much more clear.

@Mile23, @sun & @dawehner We have a project named console => https://drupal.org/project/console

The purpose of this project is to leverage the Symfony Console Component to provide a CLI tool to automate the creation of drupal 8 modules by generating the directory structure for a module, routing, controllers, forms, services, plugins and required configuration files.

We have the Symfony Console component fully functional

This will be the installation instructions for the module

$ cd path/to/drupal8.dev
$ COMPOSER_BIN_DIR=bin php composer.phar require --dev drupal/console:dev-master
$ ./bin/console --help

After installation you can access the console at

$ bin/console

Related to the scaffolding commands we have:

Module generator
$ bin/console generate:module

Controller generator
$ bin/console generate:controller

Form generator
$ bin/console generate:form

Plugin Block generator
$ bin/console generate:plugin:block

Also makes the automatic registration of commands for each installed module.

Your commands must inherit from the class
Symfony\Component\Console\Command\Command

True @dmouse I forgot the automatic registration of command part.

Also after a talk with @Crell at a Camp he recommended to decouple the console component and the scaffolding tool and it sound kinda interesting this could be an opportunity to do what he mentioned

re: #7: @Mile23:

I cannot agree with the structure you're suggesting. Every component is supposed to be an independent, decoupled component. Components MAY ship with Console commands. If they do, then the code is provided, owned, and maintained by the component maintainers.

The only reason for why tests are split into an own namespace is that all test code should not be available and loadable at regular runtime.

Every component and every module can have Console commands. Just like they can provide e.g. Entity type plugins. Therefore, the proper home for Console commands is in a relative ./Console subnamespace/subdirectory of each component/module.

There is no need to re-invent this pattern; simply have a look at how Symfony bundles and many other PHP components (that are only using Symfony Console) are doing it. What you'll see is a ./Console subnamespace/subdirectory.

That structure allows for clean discovery of available Console commands. The defined autoload namespaces are driving the discovery. So when e.g. Guzzle ships with Console commands, then those will be automatically discovered and offered, too.

If the Console component doesn't ship with a namespace-based discovery already, then we already have plenty of our own. Just use 'em.

This is how it's supposed to work:

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Htt...

The only addition/customization to that is that our components in Drupal\Core\* do not follow the concept of Bundles (only Drupal\Core itself is an equivalent), but as mentioned before, our plugin discovery actively supports to treat every core component as a separate unit, and we should continue to do that.

Plugin discovery is an important keyword: Console commands are pretty much the textbook example of plugins.

@Mile23 in the console project this is the discovery and registration commands

https://github.com/hechoendrupal/DrupalAppConsole/blob/master/lib/Drupal...

1. +++ b/composer.json
   @@ -23,12 +23,16 @@
   +    "symfony/console": "2.4.*",
   

   this dependency shouldn't be ~2.4 instead of 2.4.*?

2. +++ b/core/lib/Drupal/Console/Command/CommandBootstrapBase.php
   @@ -0,0 +1,73 @@
   +    require_once __DIR__ . '/../../../../includes/bootstrap.inc';
   

   i thought the drupal api already autoloaded all that needs to be autoloaded?

3. +++ b/core/lib/Drupal/Console/Command/CommandBootstrapBase.php
   @@ -0,0 +1,73 @@
   +      $container = $kernel->getContainer();
   ...
   +      $container->get('request_stack')->push($request);
   

   have you checked how app/console container:debug does it? i think if the container is dumped there may be no need for this

4. +++ b/core/lib/Drupal/Console/DrupalApplication.php
   @@ -0,0 +1,72 @@
   +  private function discoverCommands() {
   

   can we totally avoid this type of stuff and just $this->add(new CommandHere) ?

   or maybe instead of having this let's tag the service commands definitions and let the compiler pass gather the right commands needed. Having this instead is really ugly and prone to all sort of messiness.

5. +++ b/core/lib/Drupal/Core/Discovery/DrupalFinder.php
   @@ -0,0 +1,94 @@
   +      $drupal_root = realpath(dirname(dirname(dirname(dirname(dirname(dirname(__FILE__)))))));
   

   please do not repeat symfony's way of doing this if we can register the command as services

Re-roll.

1. I would like to see us include non-Drupal commands as well as Drupal-specific commands, which the current code does not support yet. There does not seem to be a generic way to expose Symfony Console commands. Symfony's FrameworkBundle uses a common discovery method, but that relies on bundles, which are a concept that does not exist within Drupal.
2. Do we have good use cases for using a console-specific container?
3. Why do we register the console application as a service in the console-specific container instead of building it in the console file? Testability?
4. Is there anything else we need to do in order to provide a good basis for this to replace Drush?

Hi, can we talk on IRC, just to hash a few things out?

This patch includes the vendor packages. I did not make any other changes yet.

There is a binary file in here, so use --binary when creating patches.

I cleaned up the directory structure and code comments. I also added a cache clear command and tests.

I'd like to add at least one command that belongs to a module so we know the services finder does not only *.console.services.yml for core, but for modules as well. What about the user cancellation command, which belongs to Drush, but is fairly simple? The only module-specific command in core is run-tests.sh, which is too big for this issue to port to a command.

Should fix the failing PHPUnit test

missed the --binary

But besides that, it doesn't work.

After fixing the include path for bootstrap.inc (needed another ..) I get

Getting 'PHP Fatal error: Class 'Drupal\block\EventSubscriber\CurrentUserContext' not found'

Traced that back to #2278541: Refactor block visibility to use condition plugins.

But essentially any module-based event-listener would cause this to blow up because the call to \Drupal::config() in drupal_bootstrap_pagecache() triggers a build before modules are registered with the autoloader.

The issue was #2228215: Remove module check in DrupalKernel, we need to prevent loading.

Core only changes

+++ b/core/lib/Drupal/Core/Console/ServicesFinder.php
@@ -0,0 +1,24 @@
\ No newline at end of file

Only issue I can find :D. Great work everyone.

I have two main concerns:

1. Is it guaranteed that drush is going to use this? We don't want two approaches, one living in core (the current scripts don't count as 'two approaches', I'd happily commit an issue that is just git rm for those).

2. Is there any evidence that the console component has been used for tasks such as migrating tens/hundreds of millions of items over several hours. We know drush can do this, and it took some time to get that right in terms of memory management, starting new processes etc.

This feels like 8.1.x material at this point, I think.

@Mile23, you should take a look at this issue in the Drush issue queue Leverage symfony/console package, if you haven't already seen it. I do like the idea of a CLI that works with core without users having to do anything else, but as others have noted, Drush might be the better place for this.

updating the issue summary with the 8.1.x rationale

Looking forward to this

Just as a reminder, for when this problem is changed to active status again, we have been working on this contributed project http://drupalconsole.com/ there is already code that could be reused on core or contributed space to facilitate the use of Console component in order to enable Drupal users to create commands easily.

What's the status of this issue after #2493807: Add symfony/console component to core? Is this a task now? Is this still pending till 8.1.x?

Yes, because it's still a feature request. Console was committed only in support of closing the critical issue at #2447573: [meta] Make sure 8.x - 8.x hook_update_N() testing is possible.

1. #2447573: [meta] Make sure 8.x - 8.x hook_update_N() testing is possible is a critical issue.
2. It requires the current shell script to be converted to a console command for reasons related to QA.
3. To do this, Symfony's Console component was added to Drupal core in #2493807: Add symfony/console component to core.
4. Any Console command requires a console application. Such applications can handle multiple commands, and are not tied to any specific one of them.
5. As such, it makes sense to add the application with command discovery in a separate issue, such as the one you are currently reading, and then use #2447573: [meta] Make sure 8.x - 8.x hook_update_N() testing is possible to add specific command for database dumps.

Following this reasoning, this issue is actually a blocker of a critical issue and should therefore be made active and reset to 8.0.x again.
Building the application does not mean all shell scripts should or can be converted to console commands for 8.0.x. It merely means the basis is in place to do this anytime we think is appropriate, such as for 8.1.x or later minor versions.

No, that critical will just use a simple application for that one command in core/scripts/some-app.sh, which is three lines of code.

My favorite example is still:

https://github.com/msonnabaum/XHProfCLI/blob/master/src/XHProfCLI/App.php

Clear, easy and concise.

Any Console command requires a console application. Such applications can handle multiple commands, and are not tied to any specific one of them.

While this is the default behavior, it is possible to use the console component to http://symfony.com/doc/current/components/console/single_command_tool.html">support single commands. In the interest of moving along the upgrade path, I'd prefer to save the generalized application with multiple commands for this issue/8.1.

#61 I totally agree with @Xano, there is a need for the Application class, we have been discussing this on the IRC earlier today.

#62 I like the idea of @Fabianx, we can create a simple Application Class to fix this issue.

#63 @jhedstrom The example you provided is a single command application but it also requires two classes, the Application class MyApplication and the Commands class MyCommand

@jhedstrom: Thanks for the example. I see your point and agree. I would like to ask we focus on building the small application in such a way that it will only support the single command, but is not specific to it, meaning that in 8.1.0 we can extend it to support multiple commands without having to break BC.

Still no clear for me why we keep talking about supporting a single command when we are trying to convert multiple scripts to console commands https://www.drupal.org/node/2289405

If we code the Application class (a small and simple one), adding more commands does not look like a big problem.

 $this->addCommands([
      new PasswordHashCommand(),
      new RebuildTokenCalculatorCommand()
 ]);

No, there will be one app per command for now and a shell script to wrap it.

Conversions won't happen until 8.1 - per this being a feature request.

You can play all you want for 8.1, but lets get D8 shipped first, please.

No, there will be one app per command for now and a shell script to wrap it.

If so, could you please explain the need to enforce one app per command?

In order to limit the disruption during beta.

How is this a need right now? Core does not ship with any Symfony console commands right now, so adding any command will most likely not be disruptive, but additive. On top of that I have seen no reason not to ship the first command with a generic console executable/application that can discover other commands in the future, so we'll at least have a single executable. Commands themselves would then be responsible for booting core, or not, for instance.

1. +++ b/core/composer.json
   @@ -9,6 +9,7 @@
   +    "symfony/finder": "2.7.*",
   

   Do we need to add symfony/finder here? I know the Behat project removed use of this due to memory issues, and now uses native php instead.

2. +++ b/core/core.console.services.yml
   @@ -0,0 +1,24 @@
   +services:     ¶
   

   White-space.

If this were in, would it allow us to start refactoring the various scripts in code/scripts to use console? The one script currently using console is dump-database-d8-mysql.php, but it relies on a single-command application class (DbDumpApplication). Ideally we'd have a more generic single-command class that these scripts could use.

We would indeed be able to do that. With generic command discovery (plugins?), every module (core & contrib) can provide commands.

@Mile23 we implement code on the discovery part to decide which commands to load based on drupal is booted or installed, also this implementation allows for custom, and contrib modules to provide commands when installed, we are about to merge a PR to allow themes to provide commands as well and we are currently working on support load commands based on arbitrary location like ~/.console or d8.dev/console

Why do we need discovery mechanism at all? Drupal core itself needs just a few console commands which can be registered statically, as usual. For other consumers (custom and contributed modules) there is Drupal Console. Furthermore Drush is also going to provide discovery mechanism using annotated command.

I propose we instantiate all core console commands explicitly in a single Symfony Console application and run them like this: bin/drupal run-tests

Suppose discovery is needed cos commands should be able to use DI

I am curious to know if we have some inventory of core/scripts or respective documentaion. How many of them need to be ported to Sympfony Console commands?

./core/console drupal:cron

I wonder if it is a good idea to create ./bin or ./core/bin directory for command line scripts?

we already have "core/scripts" so why not use this way?

On other hand the core's commands could use composer https://getcomposer.org/doc/articles/scripts.md#writing-custom-commands

+++ b/core/core.console.services.yml
@@ -0,0 +1,24 @@
+services:     ¶

+++ b/core/tests/Drupal/Tests/Core/Console/ApplicationTest.php
@@ -0,0 +1,23 @@
+ * ¶

+++ b/core/tests/Drupal/Tests/Core/Console/Command/RunCronTest.php
@@ -0,0 +1,85 @@
+ * ¶
...
+ * ¶

trailing whitespaces

This patch needs a reroll, it doesn't apply anymore. So I couldn't test this out, I did find some nits to pick though.

1. +++ b/core/console
   @@ -0,0 +1,31 @@
   + * @file
   + * Contains the Drupal console.
   

   I'm not sure about calling this drupal console, this might be really confusing.

   I think namespacing it as console is not a problem, because then we can call it "core console".

   So we can probably use Contains the core console script. here?

2. +++ b/core/core.console.services.yml
   @@ -0,0 +1,24 @@
   +services:     ¶
   

   Trailing space should be removed.

3. +++ b/core/lib/Drupal/Core/Console/Bootstrap.php
   @@ -0,0 +1,57 @@
   +  protected $classLoader;
   

   We should give this variable a docblock.

4. +++ b/core/lib/Drupal/Core/Console/Bootstrap.php
   @@ -0,0 +1,57 @@
   +  public function __construct($class_loader) {
   

   Can we typehint this? Also needs docblock.

5. +++ b/core/lib/Drupal/Core/Console/Bootstrap.php
   @@ -0,0 +1,57 @@
   +    } catch (\Exception $e) {
   

   Should be on the next line.

6. +++ b/core/lib/Drupal/Core/Console/Bootstrap.php
   @@ -0,0 +1,57 @@
   +        'Insufficient Drupal to proceed.',
   +        'This command requires a bootable Drupal installation.',
   

   I love this, insufficient Drupal. I'm not sure if it's clear enough for everyone though. Should we say that we need to have an installed Drupal?

7. +++ b/core/lib/Drupal/Core/Console/BootstrapInterface.php
   @@ -0,0 +1,38 @@
   +/**
   + * @file
   + * Contains \Drupal\Core\Console\BootstrapInterface.
   + */
   

   This should be removed.

8. +++ b/core/lib/Drupal/Core/Console/Command/ClearCache.php
   @@ -0,0 +1,37 @@
   +      // @todo: This doesn't actually clear all caches.
   +      $cache = $kernel->getContainer()->get('cache.default');
   +      $cache->deleteAll();
   

   drupal_rebuild(); does a rebuild of all caches, let's use that instead.

9. +++ b/core/lib/Drupal/Core/Console/Command/CommandBootstrapBase.php
   @@ -0,0 +1,49 @@
   + * It's very important that we don't bootstrap Drupal before the subclass asks
   + * for it.
   

   Sure, I believe ya'll, but instead of saying it's important, can we say why it's important?

10. +++ b/core/lib/Drupal/Core/Console/Command/CommandBootstrapBase.php
    @@ -0,0 +1,49 @@
    +    $this
    +      ->addOption(
    +        'environment', 'e', InputOption::VALUE_REQUIRED, 'Kernel environment.', 'prod'
    +    );
    

    This looks really weird, I don't think we use this style very often.

    Let's put this on one line instead?

11. +++ b/core/lib/Drupal/Core/Console/ServicesFinder.php
    @@ -0,0 +1,24 @@
    +class ServicesFinder extends DrupalFinder {
    

    Needs a docblock.

12. +++ b/core/lib/Drupal/Core/Discovery/DrupalFinder.php
    @@ -0,0 +1,95 @@
    +/**
    + * @file
    + * Contains \Drupal\Core\Discovery\DrupalFinder.
    + */
    

    Should be removed.

#96, should we open up a new issue in the ideas queue? In any case, I really think that having a couple of commands available trough core would be an awesome thing. All I really need on production machines is 4 commands, so using that instead of the entire drush or drupalconsole packages would be nice.

It also makes it a lot easier for people just getting started. I am +1

+++ b/core/lib/Drupal/Core/Console/Command/ClearCache.php
@@ -0,0 +1,45 @@
+      $output->write('<error>Unable to clear caches.</error>');

We can use stderr on such occasions and return non-zero exit code.

$output->getErrorOutput()->writeln('<error>Unable to clear caches.</error>');
return 1;

This worth noting that we've already implemented a few Symfony console commands and applications. Should the console.app include them as well?
https://github.com/drupal/drupal/tree/8.5.0-rc1/core/lib/Drupal/Core/Com...

Over in #2911319: Provide a single command to install & run Drupal we're having a debate about what to call that script. I see this issue has gone for core/scripts/console. I think it'd be great if we could agree that core/scripts/drupal is the way to go. This is inline with other CMS's for example CraftCMS (which provides a console command ootb) and makes sense as you are running commands to interact with the Drupal application. Input over on #2911319: Provide a single command to install & run Drupal appreciated.

Note from the "CLI in Core BoF"

1. Commands in Core is a good idea. Some folks still hold reservations.
2. It is tbd how and when contrib modules should implement their commands as core extensions instead of as a Drush or Drupal Console extension. For the first release we should document this facility as "@internal" for at least one minor release.
3. Converting the existing core scripts would be a good place to start. It's a net benefit to be able to be able to get a list of all of the available cli commands available in core. Existing scripts could remain with their original name, but they would just call through to the new commands.
4. We need to be careful about how we promote it, so that folks realize that this does not replace (does not have as much functionality as) existing tools. Adding a third way to define module commands will be confusing for users in the short term.
5. The command name should not be called "drupal" because a lot of existing sites use Drupal console, and the Core "drupal" would conflict with the Drupal Console "drupal", both of which would install to vendor/bin.
6. It's unclear who would actually implement e.g. the port of existing Drupal Console / Drush commands, and whether this would gain traction.

EDITED only to fix numbering.

I really disagree with

1. The command name should not be called "drupal" because a lot of existing sites use Drupal console, and the Core "drupal" would conflict with the Drupal Console "drupal", both of which would install to vendor/bin.

. It makes Drupal harder to get out-of-the-box.

To quote myself from #2911319-169: Provide a single command to install & run Drupal

@Mile23 well the aim of #2242947: Integrate Symfony Console component to natively support command line operations is to build a generic Drupal CLI provided by core so in my mind it is that issues job to sort out the clash with drupal/console and ponder the composer bin questions. All we should do here is worry about what we are telling people to do here and unfortunately drupal-installer quick-start does not make sense. Also yes it is not nice but Drupal is this projects name and console calling their command drupal is actually the clashing thing - there already was core/scripts/drupal.sh and has been for years. There's no way around it that the obvious name for a drupal cli tool provided by core is drupal - it's what anyone would expect.

@alexpott The issue here is that if Drupal core defined a command called "drupal", then any existing site that includes Drupal Console already (e.g. any site build with drupal-composer/drupal-project) will already have a file `vendor/bin/drupal`. When one of these sites use `composer update` to pull in the new version of Drupal core that also contains a `vendor/bin/drupal`, you will not be able to predict which `drupal` Composer will put in `vendor/bin`.

@greg.1.anderson I understand that. But how about we allow console to add commands to the drupal command provided by core? And when we do that console updates to that. The still have drupal, core has drupal, drush could have drupal too.

@alexpott I can't really mediate the use of the "drupal" namespace between core and console, but your idea is certainly technically possible. If you or any gatekeeper have any requirements to get this patch committed (e.g. a way for non-modules to add commands, maybe a way to do so and still participate in dependency injection, for example), then let me know, and I'll try to move this patch forward.

I might recommend, though, that the first version of a core cli released, at a minimum not use "drupal" as the command name. If this was done before Console had a chance to adjust, then everyone's CI scripts et. al. that depended on console would break. There was pretty strong consensus in the BoF that an abrupt b/c break of this sort would be undesirable.

@greg.1.anderson there is another option. We continue to use core/scripts for core scripts and build the drupal command there. And move to vendor/bin once command integration is possible and console has had time to adjust and integrate. Since Drupal is never installed in vendor at the moment and it's current scripts are not moved to vendor/bin to me this feels a good solution that allows us to move in small steps.

The BoF spent little time talking about the name 'drupal' versus other names. I would not characterize the room's tone as strong consensus that core use any name other than 'drupal'.

Yes, @moshe is correct; I mis-characterized item 5 in the summary in #107. It was not a goal of the BoF to decide what the script's name should be. There was a discussion about the problems that would arise if both core and Drupal Console installed a "drupal" script to /vendor/bin.

Talking about the name drupal versus other names was not the goal for sure @moshe weitzman since we covered several other points as mentioned by @greg.1.anderson on #107 but we all agree is not a good idea to reuse the same binary name drupal that is already used and had been used for a long time on another project drupal/console.

Making a change like this for drupal/console project is not possible. A change like this will break compatibility with current mayor release 1.x and people has been using the tool for almost 4 years now.

@alexpott related to:

Your comment on #108 How using a different name cause this:

"It makes Drupal harder to get out-of-the-box."

We can use bin/console as binary name as Symfony defined by default.

Your comment on #110

But how about we allow console to add commands to the drupal command provided by core? And when we do that console updates to that. The still have drupal, core has drupal, drush could have drupal too.

Making the “drupal” core binary able to load drupal/console commands is out of the scope for this issue as first attempt to add a CLI to Drupal Core.

If the script is in core/scripts and we don't put it into vendor/bin then we are fine too. Drupal core is not moving to vendor anytime soon and as for the name drupal as a shell command core/scripts/drupal.sh has been around for 11 years.

@alexpott The name core/scripts/drupal.sh does not cause any conflict. The issue is in the case core will decide to use the vendor/bin/drupal for the binary file name.

I don't think core should move anything into the vendor namespace until (a) we support that better for all of core code (b) the CLI runner can harness both drush and console commands with a bridge or something. At the moment core's scripts go in core/scripts. There's no need for us to change that in this issue.

Reroll #94 patch for 8.7.x

The console implementation doesn't work fine. Currently the command clear cache fail int the test.
I try to improve this but I can't use the console commands that was created.

I try to run this:

$ php ./core/console drupal:cron

(not work)

$ core/console

(not work)

How to run the console?

So this issue probably either needs an issue summary update or needs to be considered 'closed' and some new issues opened.

We have a symfony console CLI in core now. It lives at /core/scripts/drupal . and currently does not conflict with /vendor/bin/drupal from symfony console. If we need to solve that, we should do it in another issue.

We add two different types of console commands: Those which require a booted Drupal, and those which don't

The current CLI app supports "those which don't" - there are patches on this issue which were attempting to support 'those that do'. I believe we should open a new issue to add that bootstraping command separately.

Once we have a way to do bootstrapping commands with core natively, we can clean up the junk store of scripts we currently have: #2289405: [META] Port all shell scripts to console commands

But, IMO, we've solved the issue title "Integrate Symfony Console component to natively support command line operations" -> that is done.

Just a straight reroll for now, need to dig deeper about why to build container each time

Though the service finder looks for services inside /modules directory, it is not possible to create a console command inside a module because module namespaces are not registered in class loader.

Here is an alternative approach to register commands. The command discovery is based on class namespaces and it is similar to Symfony 3 implementation which has been removed in Symfony 4.

That patch has no tests yet just PoC.

Ignore.

Removed unused service definition.

I haven't figured out a good way to discover commands form vendor directory.

I like the discovery technique in #141. DrupalKernel::compileContainer() is already doing us the favor of collecting all of the module namespaces, plus the namespaces in Core and Components, so this seems like a natural facility for us to leverage. It is helpful that this mechanism works even without bootstrapping Drupal, so that commands that don't need a bootstrap can be executed quickly.

I think it is also important to allow commands to be constructed via the service container. Both Drush and Drupal Console currently do this; it is very useful for commands that already require a Drupal bootstrap to be able to utilize Dependency Injection for their commands.

It's also great that it is possible in #141 to make DrupalCommandInterface commands that do not require a bootstrap. It is possible to have the best of both mechanisms if the command dispatcher delays bootstrapping Drupal until necessary. Drush does this by extending the Symfony Console Application to bootstrap further in Application::find(), so that more commands can be found if needed. Of course, the downside is that "command not found" and "help" both need to bootstrap Drupal.

Some other comments about the issue summary:

This CLI integration should be viewed as a separate application which ships with Drupal. This might include going so far as to turn it into a subtree split similar to Drupal components.

Providing the CLI integration via a subtree split is not a good thing to do. This would create a separate global CLI application that would ostensibly be able to target multiple different Drupal sites. Both Drush and Drupal Console have shown that this is a Hard Problem. It is much better to consider the CLI integration to be a separate front controller on the SAME Drupal application / site. Bundling the CLI front controller with Drupal is the primary benefit to having the CLI in core; we don't really need Yet Another Global CLI to bolt onto the side of Drupal. We already have enough of those, and they work well.

Figure out the name. drupal/console project is already using the name 'drupal' as its bin file. Arm wrestle over it?

As pointed out in #131, core/scripts/drupal is the Drupal core CLI, and vendor/bin/drupal is Drupal Console (if it has been added to the site). There is no need for core/scripts/drupal to go in the user's global $PATH. Users might wish to put relative paths core/scripts and vendor/bin in their $PATH, in which case they could use drupal to run either Drupal Console or the core Drupal CLI per their preference. Most users will probably directly address the scripts (e.g. `core/scripts/drupal commandname`) or use a launcher. I therefore do not think this is a technical issue that needs to be solved in this issue.

I'd be happy to update the issue summary if there is general agreement on those points (or someone else can do it).

Oh yeah I forgot to address #142. I don't think we really need to be able to run commands out of vendor at this point. It should be sufficient to provide a way to add commands to modules. If we did want Yet Another Way to define commands in Drupal, though, Robo provides an extension mechanism based on the psr4 autoloader and well-known namespaces that could be referenced for inspiration.

I think that #141 is the better option for Drupal core, though.

Allowing module commands to be able to participate in dependency injection is an important feature. If we can somehow do that with vendor-discovered command classes, then I'm all in favor of that route.

Dependencies can be injected via create factory method. Just like in plugins, controllers and forms.

I would like to have command discovery in vendor though it won't be a trivial task.
Firstly it requires a reliable way to figure out the location of vendor directory. The patch in #136 assumes that vendor directory is inside Drupal root, but it's kind of outdated because nowadays Composer based Drupal installations are ubiquitous.
Secondly, the discovery command with refection is not applicable to vendor packages because their namespaces are unknown. It's tricky in PHP to figure out class by knowing the filename. We probably can introduce some predefined namespace as was mentioned above. Another approach could be declaring command classes explicitly in package composer.json file under extra section.

Here is a relevant issue in Drush.
https://github.com/drush-ops/drush/issues/3050

Also each command should indicate weather or not it needs fully bootstrapped Drupal. I think we can add DrupalAwareInterface for this purpose.

If we have the class loader, we can ask it for all the namespaces

I could not find an appropriate method for this.
https://github.com/composer/composer/blob/master/src/Composer/Autoload/C...

Interesting, if define a command as service like this, class resolver can instantiate it from container with proper dependency injection.

services:
  Drupal\foo\Command\FooCommand:
    arguments: ['@entity_type.manager']

[duplicate]

I could not find an appropriate method for this.

The RelativeNamespaceDiscovery::getClasses() from Robo works perfectly though. However, it brings back the dependency on symfony/filesystem component.

Yeah, okay, I could be +1 on using #152 with #147 -- forget about services and just pass the container into a static factory (create) method and let command classes pull whatever dependencies they need.

I don't think we need a marker interfaces for bootstrapping; I think it would be better to just have a bootstrapping trait (or even class) that commands could use to bootstrap if they need to. Previous patches on this issue had bootstrapping in a base class, but I think it would be better to keep the bootstrap utility out of the inheritance tree. Minor point, just code style.

I think it would be better to just have a bootstrapping trait (or even class) that commands could use to bootstrap if they need to.

I suppose about 90% of custom and contrib commands will need it. Forcing each of them to boostrap Drupal manually will bring a good amount of duplicated code. And besides, in order to register commands from installed modules we have to bootstrap Drupal anyway.

Another point is that instantiation of commands that have dependency injection via constructor is not safe when Drupal is not fully bootstraped.

Drush only instantiates commands from enabled modules, for safety. I didn't try to instantiate commands without a bootstrap, but I agree with #155 that it would be safer to not do this. It might therefore be better to stick with established conventions and always bootstrap for module commands. Then, most commands won't need to worry about bootstrapping.

The attached patch takes a different approach. Commands needs to be registered explicitly in composer.json files similar to Composer scripts.

"extra": {
    "commands": [
        "Drupal\\foo\\Command\\BarCommand"
    ]
}

Here is a brief summary of the patch:

1. The command discovery works for core, enabled modules and themes, vendor packages.
2. Commands that needs a fully bootstrapped Drupal installation must implement empty DrupalAwareInterface.
3. For "Drupal aware" commands dependencies can be injected via create factory method. We may consider creating a separate class resolver to support DI for all commands.
4. For modules it is possible to define commands as a services (see note #151).
5. The implementation is less magic because it does not rely on reflection, well-known name spaces and file naming conventions.
6. The patch does not bring new dependencies to core.
7. It's possible to register global (Side Wide) commands that do not belong to a module in main composer.json file.

It hasn't been discussed yet. Do CLI commands need to be translatable? I think Drush 9 does not support it. On the other side Drupal Console team had worked hard to make commands multilingual.

Re #158 Drupal's ability to work in multiple languages is one of its strongest features. I would say that we probably definitely want to be able to translate commands.

It would also require too much setup for a given command to work out of the box.

Why? It is as simple as copying class name to composer.json file. Same needs to be done with service based commands. A developer has to configure new command in console.services.yml file.

I am not fond of #157. Searching for composer.json files and aggregating information from them reminds me of the old Composer Manager. The right way to get info from the `extra` section is via the Composer APIs, but those are only available to plugins, and it wouldn't make sense for Drupal core commands to be Composer plugins.

I like the direction that #167 is going. I have a couple minor comments, but perhaps the items below are only the way they are because this is a PoC.

1. +++ b/core/lib/Drupal/Core/DrupalKernel.php
   @@ -617,6 +618,10 @@ public function discoverServiceProviders() {
   +    if ($this->environment === 'console') {
   

   It would be better if the ConsoleServiceProvider were somehow injected by the Console front controller rather than having a conditional here.

2. +++ b/core/lib/Drupal/DrupalConsole/CommandServiceDefinition.php
   @@ -0,0 +1,26 @@
   +  public function getDefinitions() {
   

   This isn't really a magic name; this is an explicit declaration of a command definition. I'm not sure how a module would extend this. I think it would be possible to replace this function with something that searched for well-known (magic) names though.

The right way to get info from the `extra` section is via the Composer APIs

Correct, but it's not forbidden to read that information directly. Drush does it and Drupal core possibly will do it at some point.
#1398772: Replace .info.yml with composer.json for extensions

I do not dispute that it is possible to use composer.json for such purposes; there are use cases where it is reasonable to put non-Composer metadata in composer.json. However, it's not the best idea. I am skeptical about #1398772: Replace .info.yml with composer.json for extensions, and we probably would have put the Drush metadata in the info.yml file instead of the composer.json file but for the fact that composer.json has extra and info.yml files do not.

The larger issue with #157 is not that it reads info from composer.json as much as that it has to search the filesystem for composer.json files and then aggregate information from them. Discovery should happen using information already available to use. There's an example above that does it, but I'll have to look at it later.

As another example, Robo's plugin mechanism uses $this->classLoader->getPrefixesPsr4() as $baseNamespace => $directories to find the set of namespaces to search, and then uses a filesystem search over just a small subset of the filesystem. This is much better then searching the entire modules directory. It could be improved even further by making a single well-known class in each namespace that declared and returned the list of classes with commands. I wish it had been implemented this way (maybe for Robo 2.x?). I similarly regret the command discovery class of the annotated command library, and will probably replace that with something better in the future.

We can't inject the service provider because the kernel doesn't give us any opportunity to.

But we are core. Can we add a setter to the Kernel?

The magic name is the name of the class we discover.

How would a module add a command with this mechanism? It looks like all of the command names (just one ATM) are listed in code. This is fine for core commands, but we need discovery for modules. Maybe I'm just not seeing it.

Why should the console main application be registered as a service ? Symfony does not do this, and just:

$kernel = new Kernel($env, $debug);
$application = new Application($kernel);
$application->run($input);

Of course, it needs to be adapted to Drupal, but it seems much saner.

Why wouldn't Drupal have two different binaries, one "console" that work on a working site, and another such as "install" or "maintainance" that does everything that will need to run over a non bootstrapped site, spawning a generic application instance without the kernel (you can use the console component without any kernel). Then you just separate both types of commands, in the maintainance script you just add them to the application manually calling successively add():

use Drupal\...bla\Unbootstrapped1Command;
use Drupal\...bla\Unbootstrapped2Command;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Input\ArgvInput;

require_once dirname(__DIR__).'/vendor/autoload.php';

$application = new Application("Drupal core - maintainance");
$application->add(new Unbootstrapped1Command());
$application->add(new Unbootstrapped2Command());
$application->run(new ArgvInput());

And in the other script, you just bootstrap the DrupalKernel, inject it into a new Application instance, and run it.

This way it would make your life much simpler, ask people to tag using console.command their commands and register it into a Symfony\Component\Console\CommandLoader\ContainerCommandLoader instance, so they end up into the container and your application will discover them much transparently and you're good to go.

The larger issue with #157 is not that it reads info from composer.json as much as that it has to search the filesystem for composer.json files and then aggregate information from them.

Both glob and file_exists are quite fast in PHP and #157 does not use recursive file search. I've just made a package from it to ease testing and benchmarking. On brand new Drupal 8 installation getComposerFiles() takes only 12ms. On a real site with lots of modules and vendor packages the time rises to 15ms which is perfectly acceptable for CLI application.

Discovery should happen using information already available to use.

The problem here is that we don't have enough information. Class loader collects prefixes not namespaces. Robo extension discovery relies on symfony/filesystem to identify namespaces. And without it even class_exists() may force class loader to call file_exist().

Why wouldn't Drupal have two different binaries, one "console" that work on a working site, and another such as "install" or "maintainance"

Technically we can make two different applications that share the same binary.

Re: #170#171 The general consensus on #1398772: Replace .info.yml with composer.json for extensions is that we are not planning on it being an either/or scenario. The plan is to only deprecate the core key, and the dependencies/test_dependencies keys, and leave .info.yml files, the bulk of that work is actually being done in #3005229: Provide optional support for using composer.json for dependency metadata.

This is because we are not interested in overloading composer.json to be a metadata storage facility, and instead only want to rely on it for dependency resolution purposes. .yml files are vastly superior to .json files for configuration.