Use Symfony Console application and annotation commands in Drush

[greg-1-anderson]

CONTINUED IN #2175.

Setting the DRUSH_SYMFONY environment variable makes the Symfony Console Application the main command runner. Otherwise, the original Drush command dispatcher is used first, and the Symfony Console Application is used as a backup.

There's a lot of work needed here before Drush will be fully functional under the Symfony Console Application. One interesting thing about Symfony Console is that option processing is not fully done until $application->run() is called. This means that work like site selection must be done via event listeners.

The list below enumerates what needs to be done, cross-referenced to other issues, PRs or commits that are relevant. Items that have existing code are checked off, but nothing should be considered to be "done" until reviewed, discussed and merged.

• Command dispatching
  • @hook annotations to allow annotated commands to hook other commands Command hooks consolidation/annotated-command#6
  • Command discovery Add CommandFileDiscovery class. consolidation/annotated-command#10
    • Decide how to find built-in command classnames 9410e40
    • Decide how to find and load module cli commands Standard naming conventions and search locations for command files consolidation/annotated-command#12 9410e40
    • In Drupal 8, disable filesystem scanning and use the service container to identify commandline tools for modules. 352cec0
  • Decide how to test requirements, and filter out commands where they are not met
  • Redispatch to remote servers
• Bootstrapping
  • If command classes implement BootstrapAwareInterface, give them a reference to Boot via setter injection Add bootstrap aware interface consolidation/bootstrap#1
  • Command classes that do not implement this interface are always fully bootstrapped
  • Bootstrap object should be able to provide specific implementations of certain services (e.g. SQL access).
• Option processing
  • Allow default options to be specfied for global commands
  • Handle --global-option=0 options
  • Handle --no-global-option synonyms
• Configuration file handling
• Alias file handling
• Backend invoke
  • Pass annotated command results to backend invoke 4d110d2
• SQL classes
• Output formatting
  • Integrate Robo logging with Drush, so that interpolation etc. works the same way e5c357b
  • Always print LogLevel::NOTICE, and move existing NOTICE logs to INFO Drush log levels #2115
  • Formatter plugin interface Formatters consolidation/annotated-command#7 / consolidation-org/output-formatters
  • Provide existing basic formats
    • string
    • yaml
    • json Add json formatter. consolidation/output-formatters#2
    • print-r Add print-r formatter. consolidation/output-formatters#3
    • var_export Var export consolidation/output-formatters#4
    • list Add list formatter. consolidation/output-formatters#5
    • table Add TableFormatter. consolidation/output-formatters#1
    • csv Add csv formatter. consolidation/output-formatters#6
    • php 40f238c
  • Drop old formats that can now be handled by one of the standard formatters
    • key-value now handled via --table
    • key-value-list now handled via --sections format Add SectionsFormatter to implement --sections data format. consolidation/output-formatters#17
    • csv-list now handled via the --csv format
    • nested-csv now handled via --csv with a cell renderer
    • csv-or-string only used by nested-csv, and therefore no longer needed
  • Ensure old Drush outputformat bugs are fixed by consolidation/output-formatters (n.b. commands must be rewritten as annotation commands to benefit)
    • pmi --format=csv skips missing fields #502: csv tables should not skip missing or empty fields
    • Handle --fields better in output formatters that do not explicitly support fields. #333: handle fields better for output formats that do not explicitly support fields
    • No field labels #130: omit field names
  • Decide what to do with unusual formats
    • message (Only used by search-status command. Drop & replace with a key-value --list?)
    • variables (Unused?)
    • config (*Default format for site-alias command, pipe format for variable-get command. Drop and replace with yaml?)
    • labeled-export (Unused?)
    • html (Used by help)
  • Configure Symfony Table styling to a format we like
  • Provide a mechanism to determine which formatters are compatible with a given command
• Existing command compatibility layer
  • Do legacy dispatching first; dispatch with Symfony Application if no legacy command found 0057d55
  • Convert an example command, core-init, to InitCommand
  • Convert core-eval to EvalCommand, so that we can try basic formatters.
  • Create a wrapper Symfony Command to call old commands and hooks from the Symfony Application dispatcher
    • Determine mapping from Drush procedural hooks to new hooks Determine mapping from Drush procedural hooks to new HookManager #2116
    • Decide if we should continue to support rollback hooks, or if we can rely on Robo Collections for this.
  • Ensure that Drush notify still works with Symfony commands
• Help changes Help command for annotation classes #2106
  • The list command shows command aliases as duplicate entries; duplicates should be hidden
  • All aliases and all usages appear in the "Usage" section; should be split up
  • All global options are shown all the time
  • @topic is needed, so help text can show related topics
  • Commands should provide their return type, and the formatter manager should provide the valid formats that work with a give return type. output-formatters 43aeb9a + annotated-command 2d7d52a

[weitzman]

Nifty. It I like annotation classes as commands.

Great checklist - lots of work to do! Maybe we should split out the major ones into their own issues and link from there. Lets use the drush9 milestone to collect such issues.

This notice is noisy: Notice: Undefined index: default-value in /Users/moshe.weitzman/c/h/drush/includes/preflight.inc on line 266

[jhedstrom]

Awesome to see this taking shape!

[Chi-teck]

Decide how to find and load module cli commands

You might find this useful:

Drupal Console discovers commands though name conventions.
https://github.com/hechoendrupal/DrupalConsole/blob/master/src/Helper/CommandDiscoveryHelper.php

DCG discovers commands using PHP reflection.
https://github.com/Chi-teck/drupal-code-generator/blob/master/src/GeneratorDiscovery.php

There is core conversation about Symfony Console integration with some interesting points about command discovery.
https://www.drupal.org/node/2242947

[weitzman]

Thanks a lot for those links. Will read up on command discovery.

I just played with DCG and reviewed the code. Terrific work!

[greg-1-anderson]

Thanks for the references; I was unaware of DCG.

I am ambivalent about the DCG discovery technique vs the DrupalConsole discovery technique. Being able to use instanceof is clearly a good thing; however, DCG at its core is still using filesystem iteration to discover classes, and furthemore it instantiates classes it does not need in order to use instanceof.

In the balance, I think I prefer the model that commandfiles must conform to some naming convention in their filenames, e.g. *Command.php, as DrupalConsole does.

[Chi-teck]

*Command.php

There should be a way to distinguish Drush commands from Drupal Console commands and other types of commands. Perhaps *DrushCommand.php or Drush/*Command.php

[greg-1-anderson]

Actually, it is my goal to make it possible to write a Drupal 8 module cli tool that is loadable by both DrupalConsole and Drush. So, we'll instantiate any *Command.php, but will ignore anything that is an instanceof Symfony Console. Drush will only support annotation commands (and legacy procedural Drush commands, of course); DrupalConsole will use instanceof to distinguish between annotation commands and native Drupal Console commands. I'm working on the Drupal Console PR right now; I will cross-reference here when it is ready.

UPDATE: See hechoendrupal/drupal-console#2081 below.

[greg-1-anderson]

Converted one command as a proof-of-concept, and got the tests to pass. Still a lot of work remaining, but a good milestone.

[jhedstrom]

I see the @option annotations below on the individual methods, but should there be an annotation on the class too for command discovery? Or are those intended to be discovered by other means?

[greg-1-anderson]

Commandfile discovery hasn't been designed yet, exactly. The basic mechanism is similar to what Drush does today: find all commandfiles, and then register all of the commands they define. Note that annotations on the class, or using instanceof are both possibilities for secondary techniques for command discovery / selection; however, there needs to be an additional initial filter function as well, as we would not want to load every class file available just to determine which are commandfiles.

DrupalConsole searches for files called *Command.php (in certain paths), and registers all of those. We should probably do something similar. Suggestions / PRs welcome.

[greg-1-anderson]

Hooks and formatters are now working; I think the next step is to actually load a commandfile provided by a module, and see if we can make that work.

n.b. Module commandfiles will invariantly given a full bootstrap. We'll support allowing global and site commands to set their bootstrap level later.

[greg-1-anderson]

I decided that Chi-teck was right, above; annotation commandfiles should use different filename patterns than Symfony commands. Discussion at consolidation/annotated-command#12

[greg-1-anderson]

I renamed consolidation/annotation-command to consolidation/annotated-command. GitHub will automatically redirect the old links above to their new location in the new project name.

[weitzman]

Why does an annotation command specify @default-format and $options['format']? Also, I have not seen this use of use of an associative array as default value. Is that new in PHP?

[greg-1-anderson]

I put in both @default-format and 'format' => 'var_export' because I was experimenting; only one is needed on any given command. If you are using a Symfony Console command (whose parameters are always InputInterface, OutputInterface), then you must use @default-format.

The associative array as a default value, as far as I know, has always worked in php, but I have only verified this as far back as php 5.4. Its use in this context -- to define the default values for your options -- originated in Robo.

[greg-1-anderson]

Going to investigate service tags, similar to what was done in an earlier Drush PR. See: tags example and the compiler pass.

Actually, it looks like it won't be necessary to use a compiler pass here. (UPDATE: It was necessary to use a compiler pass here.)

[weitzman]

Is this .gitignore really needed?

[greg-1-anderson]

Rather than make two copies of the same 'WootCommands', my scripts copy or symlink the Drupal 8 version into the Drupal 7 test module directory. I don't want this test artifact to be accidentally committed. It might also be a good idea to just erase the duplicate after the test runs.

[greg-1-anderson]

I merged in #2169, so this PR now uses tagged services to find commands in Drupal 8 modules. In order for this to work, we need to create a subclass of DrupalKernel. A core patch that adds the functionality we need is here: https://www.drupal.org/node/2718933

[greg-1-anderson]

I squashed and rebased this with the master branch.

[greg-1-anderson]

Continued in #2843.