Postponed on 8.1.x being open for development.

Problem/Motivation

Drupal 8's new OOP niftiness just sits there waiting to be driven around from the console.

A simple composer requirement plus a few classes later, and it can be.

Here's an easy introduction to Symfony's Console component: http://symfony.com/doc/current/components/console/introduction.html

Drush is very useful for complex tasks such as drush make, and remote usage with site aliases, as well as working with Drupal versions other than 8. These type of tasks would be outside the scope of the console.

However, the addition of console commands to core, with stringent code review and testing requirements, would be a framework useable by Drush, allowing Drush to slim down in the Drupal 8 use case.

Beta phase evaluation

evaluating for https://www.drupal.org/contribute/core/beta-changes

the first thing that needs to be decided is, is this a feature request.
which leads me to https://www.drupal.org/core/issue-category

Reading that it does seem this is a feature, so moving this to 8.1.x and postponing it. (also see @webchick comment in #44)

Proposed resolution

Add the dependencies, create a core/console script, and off we go.

Remaining tasks

  • Proof of concept... See below.
  • Determine scope for console-oriented tasks.
  • Determine testing regimen. Symfony gives us ConsoleTest.
  • Contemplate contrib extensions.
  • Add your consideration here...

User interface changes

Users will be able to type things like core/console module:install my_module. That seems desirable, doesn't it?

API changes

CommentFileSizeAuthor
#74 2242947_74_core_only_do_not_test.patch24.02 KBMile23
#74 2242947_74.patch1.05 MBMile23
#71 2242947_71_core_only_do_not_test.patch60.73 KBMile23
#50 core_only.txt23.1 KBMile23
#50 2242947_50.patch984.49 KBMile23
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View
#46 2242947_46.patch2.36 MBMile23
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2242947_46.patch. Unable to apply patch. See the log in the details link for more information. View
#46 2242947_46_core_only_do_not_test.patch69.56 KBMile23
#46 interdiff_37_core_only.txt7.38 KBMile23
#38 symfony-console-2242947.core-only.do-not-test.patch31.62 KBlarowlan
#37 symfony-console-2242947.6cb8b52.patch876.74 KBlarowlan
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch symfony-console-2242947.6cb8b52.patch. Unable to apply patch. See the log in the details link for more information. View
#37 interdiff.txt877 byteslarowlan
#36 symfony-console-2242947.hmm_.patch876.72 KBlarowlan
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,602 pass(es). View
#34 console-token-2289447.5add5f2.patch871.41 KBlarowlan
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch console-token-2289447.5add5f2.patch. Unable to apply patch. See the log in the details link for more information. View
#34 interdiff.txt2.17 KBlarowlan
#30 drupal_2242947_30.patch876.75 KBXano
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,619 pass(es), 1 fail(s), and 0 exception(s). View
#30 interdiff.txt3.31 KBXano
#28 drupal_2242947_28.patch875.65 KBXano
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: failed during invocation of run-tests.sh. View
#28 interdiff.txt34.09 KBXano
#27 drupal_2242947_27.patch867 KBXano
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,540 pass(es). View
#25 drupal_2242947_25.patch858.99 KBXano
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal_2242947_25.patch. Unable to apply patch. See the log in the details link for more information. View
#22 drupal_2242947_22.patch17.17 KBXano
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,552 pass(es). View
#21 2242947_21_do-not-test.patch14.47 KBMile23
#20 2242947_20_do-not-test.patch424 bytesMile23
#17 2242947_17_do_not_test.patch13.99 KBMile23
#5 2242947_5_do_not_test.patch8.72 KBMile23
#2 2242947_2_do_not_test.patch11.44 KBMile23
#1 2242947_do_not_test.patch9.39 KBMile23
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Mile23’s picture

Status: Active » Needs review
FileSize
9.39 KB

Here's a proof of concept.

1. Apply the patch.

2. Run composer update

Then you'll be able to do this:

core/console

This will show you a help screen.

I've added three commands: cache:clear, module:install, and module:uninstall.

You can say core/console help module:install and such to get help with individual commands.

All three work, though uninstall managed to bork my Drupal when I told it to uninstall node. Obviously this is proof of concept, not production code.

Mile23’s picture

FileSize
11.44 KB

Teensy bit of refactoring, added user:login, which returns a one-time login URL for the site.

Because it makes no sense whatsoever to be able to type core/console user:login 1 | pbcopy, does it? :-)

dawehner’s picture

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.

Mile23’s picture

Issue summary: View changes

@dawehner: It wouldn't be the symfony console in core. It would be the Drupal console. :-)

Let's add console-based objects to core, which Drush can then use.

Mile23’s picture

FileSize
8.72 KB

How about replacing core/scripts/drupal.sh?

core/console drupal:request /path will give you the HTML for that path.

It has an option to return only the response code, like this:

$ core/console drupal:request /nonexistent -s
404
sun’s picture

Title: Add symfony/console, allow users to drive Drupal natively from the command line » Add Symfony Console component to natively support command line operations
Component: other » base system

+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’s picture

@sun: Exactly the conversation I was hoping for. :-)

I think the big deal here is that any command should be very small in terms of lines of code. Let's say 20 for the execute() method. I think setting an artificial (but reasonable) limit on the size of any command implementation forces people to think about how to refactor and improve the code they're driving, rather than add to the console complexity.

For instance: We'd have one Command base class that boots Drupal for you (see above), and if you discover that you need to change the way it does that, or you need to boot Drupal in a different way, then really the solution to that problem should move 'upstream' to Core or Component namespaces to better deal with your use-case.

This is one reason I think we should have a Drupal\Console namespace, with Core\ and Component\ mirrored underneath it, similar to the way it works for PHPUnit's tests/ directory.

jmolivas’s picture

@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

dmouse’s picture

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

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

jmolivas’s picture

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

Mile23’s picture

Hey cool. Looks a little like this: https://drupal.org/sandbox/mile23/2230027

:-)

The autodiscovery part is great. I agree about decoupling. If you check the patches above you see you can have a light touch.

sun’s picture

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.

sun’s picture

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’s picture

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

Of course we want to autodiscover console commands. See this from the patch in #5:

+++ b/core/console
@@ -0,0 +1,24 @@
+// @todo: Replace this with autodiscovery.

However: They're not plugins. They don't even need a booted Drupal. There's nothing for them to plug into. It's like saying PHPUnit tests are plugins. Even under Symfony, commands aren't plugins.

Note that in Symfony2, they dropped the term 'plugin,' and rightly so. It's a confusing term. Guess what replaced it?

Hmmmm.... You've linked to code that uses Symfony\Component\Finder\Finder. Sure would be handy to have that, too, for autodiscovery. :-)

dmouse’s picture

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

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

Mile23’s picture

Thanks.

Currently traveling, so this will go slowly.

Mile23’s picture

FileSize
13.99 KB

Very work-in-progress-y.

Adds some utility classes for finding stuff in core, which might be a bit out of scope but we roll with it. The challenge is to find commands before bootstrapping Drupal.

Also gives a basic framework of stuff to debate. :-)

To install, you need to run composer update. Then you can do core/console to see the help screen, and core/console drupal:cron to see the autodiscovered cron command in action.

cordoval’s picture

  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

Mile23’s picture

Thanks, @cordoval.

Point 1: I tried various combinations of ~ and .* and others too, and this was the best result. See also the 'prefer-stable: true' line.

Point 2: The console application (\Drupal\Console\DrupalApplication) only has the composer-generated loader at that point. This is intentional... Only bootstrap if the command we're running needs it.

Point 3: These are based on the current front controller code, ripped from drupal_handle_request(). Once the kernel is refactored, this will all change. That's one reason it's abstracted the way it is. Will poke through container:debug, but really what to do next depends on #2016629: Refactor bootstrap to better utilize the kernel

Point 4: See discussion above about plugins and autodiscovery. :-) In my mind, we want to include commands from the standard contrib places, as well as from /vendor. (Allowing composer require somevendor/someconsolecommand is very appealing to me.)

Point 5: I want it to be true that if you just type core/console you don't bootstrap a Drupal, and having a container or a kernel is not a requirement of any command you might write. It'd be great to leverage what's already there, but I'm not sure that Drupal's container can be loaded or updated without the database or without bootstrapping a bunch of stuff at some level. That is: I really don't know. :-) Any pointers would be helpful.

Mile23’s picture

FileSize
424 bytes

Here's some development on the console. Important changes include:

The console app and all commands are services, discovered from yaml files.

The yaml files are parallel to the standard Drupal ones. There is a core.console.services.yml file, and the discovery process finds files named *.console.services.yml.

This follows the pattern of the rest of Drupal, but is in parallel because the core services have lots of dependencies that they really shouldn't. :-)

Mile23’s picture

FileSize
14.47 KB

Woops... Wrong diff. :-)

Xano’s picture

FileSize
17.17 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,552 pass(es). View

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?
Mile23’s picture

1) We don't want non-Drupal commands, because this is the Drupal console. :-)

It's true that there isn't a generic way to do discovery. This is one of the central problems of this exercise... It would be great to make commands into services, so they could be discovered by and cached by the container, but the container has too many dependencies (and is a confusing mess to try to build outside the kernel). I also tried using symfony/finder to do a file search and then determine whether the file is a subclass of the command base class. Fun times.

2) The use-case for the console-specific container is that I wanted to figure out how to make commands into services. The problem of dependencies led me to have a parallel system of service definitions. The idea was to eventually fold those in with core as was possible.

I'm currently working on having some commands be hard-wired in to the app, just adding them in the constructor of the app. This is for commands that need to show up very early in the boot process, like drupal:install.

In an ideal world, there would be a policy document on what dependencies a service can have. There's also this: #1973618: DIC: Lazy instantiation of service dependencies (ProxyManager for "proxy services")

3) The console app is a service because this allows us to add a compile phase to the container. It finds tagged commands and adds them to the app automagically. Also, like you say, testability. Symfony/console has its own testing infrastructure.

4) No desire to replace drush. :-) I simply want to replace /core/scripts/ with testable code. Drush can do complex things that the Drupal console shouldn't. Over time, it might be that people make drush-equivalent stuff under this system, but that's out of scope here.

Xano’s picture

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

Xano’s picture

FileSize
858.99 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal_2242947_25.patch. Unable to apply patch. See the log in the details link for more information. View

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

Status: Needs review » Needs work

The last submitted patch, 25: drupal_2242947_25.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
867 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,540 pass(es). View

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

Xano’s picture

FileSize
34.09 KB
875.65 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Failed to run tests: failed during invocation of run-tests.sh. View

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.

Status: Needs review » Needs work

The last submitted patch, 28: drupal_2242947_28.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
3.31 KB
876.75 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,619 pass(es), 1 fail(s), and 0 exception(s). View
Xano’s picture

Status: Needs review » Needs work

The last submitted patch, 30: drupal_2242947_30.patch, failed testing.

Mile23’s picture

We don't need to add a module to test file discovery. In subsequent versions I added vfsStream to test that. https://github.com/mikey179/vfsStream/wiki

The earliest patches in this issue have a few different commands that could be re-added. I took them out to concentrate on the framework rather than the application.

Will do more review tonight.

Thanks. :-)

larowlan’s picture

Status: Needs work » Needs review
FileSize
2.17 KB
871.41 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch console-token-2289447.5add5f2.patch. Unable to apply patch. See the log in the details link for more information. View

Should fix the failing PHPUnit test

Status: Needs review » Needs work

The last submitted patch, 34: console-token-2289447.5add5f2.patch, failed testing.

larowlan’s picture

Status: Needs work » Needs review
FileSize
876.72 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,602 pass(es). View

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.

larowlan’s picture

FileSize
877 bytes
876.74 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch symfony-console-2242947.6cb8b52.patch. Unable to apply patch. See the log in the details link for more information. View

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

larowlan’s picture

Core only changes

jibran’s picture

+++ 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.

catch’s picture

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.

Mile23’s picture

@catch:

1) It'd be great to get the opinion of one of the Drush leads. They should walk themselves through the 'your first console app' documentation here: http://symfony.com/doc/current/components/console/introduction.html

That said, however: Drush can do big complicated stuff like site aliases for three major versions of Drupal. The Drupal console is for smaller, easily implemented stuff like clearing the cache. Also it will be tested in core's continuous integration, so you'll always be able to clear your cache when you install Drupal.

2) Symfony/console is a framework that handles the text input and output parts of making a console application. The rest is up to us.

I'd also like to point out that the extent to which this is easy is a consequence of good OOP in Drupal. The better the refactoring, the easier it is to make a console command for it, and also the more maintainable that command is.

For instance, this from @larowlan above:

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.

Oops. Dependency problems in the config system... Looks like something to fix in core. :-)

Status: Needs review » Needs work

The last submitted patch, 37: symfony-console-2242947.6cb8b52.patch, failed testing.

webchick’s picture

Version: 8.0.x-dev » 8.1.x-dev

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

kostajh’s picture

@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.

Mile23’s picture

Status: Needs work » Needs review
FileSize
7.38 KB
69.56 KB
2.36 MB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2242947_46.patch. Unable to apply patch. See the log in the details link for more information. View

Again: This is not an issue that says 'replace Drush with a native console.'

It is an issue to add functionality to Drupal that everyone wants, which is why Drush exists. If memory serves, Drush came about because of the same sort of friction that's facing this issue. Only, ironically, now it's because there's a Drush that there's friction against core console here.

Some of the problems solved by adding a console component are:

1) Functionality that is required for core is maintained along with core. For instance, a command-line installer will be maintained and will work with core always, because we have CI and it's not a separate project.

2) The Drupal console will be able to find commands within vendor/. You can add a command, put it on packagist, and everyone can get it.

3) I won't have to wonder whether the current Drush will save me time or waste it for simple tasks. This is due to #1 above.

4) You really should take a look at https://www.drupal.org/project/console which generates Drupal 8 modules for you. Just like Symfony does for all the Symfony people. Very convenient. Would be super useful to have in core, or as a contrib project which you add through composer.

That's the sort of thing we'd enable here. Code's ready, FTMP, because it's not really that complex. Drush has to work with a wide array of Drupal versions, whereas this project only cares about 8.

The decision is whether to support CLI for common tasks such as installation. I'd write the installer, but I'm not sure why I'd do it if no one cares.

Adding in 8.1.x is fine.

Here's a re-roll.

Status: Needs review » Needs work

The last submitted patch, 46: 2242947_46.patch, failed testing.

mgifford’s picture

Issue tags: +Needs reroll
mgifford’s picture

Mile23’s picture

Version: 8.1.x-dev » 8.0.x-dev
Status: Needs work » Needs review
FileSize
984.49 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View
23.1 KB

Reroll against 8.0.x, so I'm setting the version. Will set it back.

YesCT’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes
Status: Needs review » Postponed
Issue tags: -Needs reroll

updating the issue summary with the 8.1.x rationale

Mile23’s picture

Yah, I set it to 8.0.x for the testbot, because 8.1.x branch is way behind.

Status: Postponed » Needs work

The last submitted patch, 50: 2242947_50.patch, failed testing.

YesCT’s picture

Status: Needs work » Postponed
jmolivas’s picture

Looking forward to this

Mile23’s picture

Title: Add Symfony Console component to natively support command line operations » Integrate Symfony Console component to natively support command line operations
Mile23’s picture

Adding this as a related issue: #2493807: Add symfony/console component to core

That issue only adds the component to vendor/ and doesn't do any integration.

jmolivas’s picture

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.

jibran’s picture

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?

webchick’s picture

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.

Xano’s picture

  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.

Fabianx’s picture

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.

jhedstrom’s picture

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.

jmolivas’s picture

#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

Xano’s picture

@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.

jmolivas’s picture

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()
 ]);
Fabianx’s picture

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.

Mile23’s picture

*This* issue represents an integration which does the following things, among others:

  • Performs discovery of commands in Drupal-contrib and composer-based packages.
  • Allows the console to use the container, and thus the kernel.
  • The console builds its own service container to accomplish discovery, but this implementation detail would probably change.

These are features which would be nice to have, but aren't vital at this late stage in the 8.0 release cycle.

Scripts for 8.0.x should use the one-file-per-script model that console allows through fluent interfaces and closures, so that we get:

  • The benefit of the framework.
  • The ability to refactor these scripts to an integration more easily should the need arise.
  • Console's testing framework.

Also, #2289405: [META] Port all shell scripts to console commands is postponed on this one. It would be great to refactor all of them for grins, but not during beta.

Xano’s picture

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?

Mile23’s picture

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

In order to limit the disruption during beta.

Mile23’s picture

Just did a re-roll to see how things have changed.

The commands work (as PoC), the tests fail, and you'll need to do composer install to get symfony/finder.

This is still postponed for 8.1.x, but the patch applies against 8.0.x.

Xano’s picture

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.

Mile23’s picture

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.

Agreed.

However, if this patch is postponed until 8.1.x, then it's not happening, and we should limit the footprint of any console-based scripts so we don't end up with a bunch of DrupalWTF later.

So change the status here and make the case if you'd like.

Also: It sure would be nice if it were easier to boot core in one or two lines of code, wouldn't it? :-)

Mile23’s picture

Status: Postponed » Needs review
FileSize
1.05 MB
24.02 KB

Since the 8.1.x branch has been merged, here's a reroll/fix patch against it.

This patch adds the symfony/finder component to core, so there is a big patch and a core-only patch.

jhedstrom’s picture

  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.

Xano’s picture

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

Mile23’s picture

If this were in, would it allow us to start refactoring the various scripts in code/scripts to use console?

Convert existing core/scripts/ to Console you say? :-) #2624926: Refactor run-tests.sh for Console component.

Basically what this integration gives us is a discovery mechanism and a unified UX at the command line. It'd be nice to have a unified single console app with lots of commands, but existing scripts could just be converted as desired.

Also, we could just say composer require drupal/console because a lot of the problems have been solved there. :-)

As for symfony/finder, it's very convenient but yah, no real attachment to it. The problem here is how to do discovery before booting the kernel (since some commands won't need the kernel). We also want to search in vendor/, or come up with some elegant discovery mechanism that includes vendor/, so that stand-alone commands can just be required using composer.

jmolivas’s picture

@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

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Chi’s picture

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

Mile23’s picture

We already require symfony/console for core, so that we can make commands like that using the console framework. The main problem is that you then have a lot of repeat effort for things like output styling, for instance.

I already refactored run-tests #2624926: Refactor run-tests.sh for Console component. but no one seems to care. :-)

andypost’s picture

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

Chi’s picture

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?

Mile23’s picture

'Need' is a strong word. :-)

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.