Deprecate global configuration commands

[claudiu-cristea]

No description provided.

[weitzman]

I would give a full URL to the docs. A relative path is not giving the user much info, as our code is buried in vendor.

[claudiu-cristea]

The last commit was suggested by the Personal note from this blog post https://mglaman.dev/blog/auto-discovery-global-commands-drush

I find the file naming requirement a bit redundant since we already have a specific namespace, and the class is inspected to ensure it isn't abstract, an interface, and is a subclass of Drush\Commands\DrushCommands.

[weitzman]

Should this be just *Commands now?

[weitzman]

• The method is service(), not services() (not plural)
• Since 'bootstrap' is a link from one .md file to another .md file, you should use a relative path instead of an url.

[greg-1-anderson]

Do we need to remove this completely? Pantheon still relies on global commandfiles to run the site audit tool on a bootstrapped Drupal site. This is safe for commandfiles that do not have dependencies of their own.

Can we merely deprecated supporting dependencies in global commandfiles? We can't stop folks from loading their own autoload.php file, but we can tell them not to do it.

[weitzman]

I'd like to deprecate those commands. I think that sends the right signal that they are going away, and we avoid the autoloader foot gun. As for when we actually remove support, thats a flexible thing. It doesnt have to be Drush 12 (which has no release timeline yet anyway).

[claudiu-cristea]

The discovery we try to deprecate here is the one where command classes are listed in this way, in drush.yml:

drush:
  commands:
    Drush\Commands\ExampleCommands: /path/to/drush/Commands/ExampleCommands.php

This I found by reading the code because the documentation seems wrong. See https://github.com/drush-ops/drush/blob/11.x/docs/commands.md#commands-discovered-by-configuration

These commands don't bootstrap Drupal either. Drush is only doing the autoload job by knowing their namspace and file path from drush.commands.

There is a 3rd kind of commands: Site-Wide Drush Commands which I don't understand exactly if they are bootstraping Drupal. Note that we don't touch this kind of discovery in this PR.

@greg-1-anderson So, what kind of discovery is Pantheon using: is it Site-Wide Commands (which in a Drupal project are typically under ./drush/Commands) or Configuration Commands (specified in drush.commands)?

EDIT: Because on a long run I think we should also deprecate the Site-Wide Commands and stick with auto-discovery
EDIT2: @greg-1-anderson could you explain what makes Pantheon commands non-convertible to auto-discoverable? I don't have the whole picture

[claudiu-cristea]

In fact I see no reason why we wouldn't deprecate also Site-Wide Commands discovery. Right now we have 3 ways of adding non-module commands and this looks very confusing. PSR4 auto-discovery is a better way. I see no reason why a project could not provide auto-loading. But maybe I'm missing something...

[greg-1-anderson]

The Pantheon platform has a number of features that interact with the Drupal site being managed. In order to run code on the Database, Pantheon makes an ssh request through a custom sshd that provides authentication and then allows an ordinary shell command to be executed, just like standard ssh. A typical example of an ssh command that Pantheon might run is drush cr, but there are also some custom Drush commands for doing different things, e.g. the site-audit command, which returns a report with a number of interesting tidbits about the site that Pantheon displays on the site dashboard.

The way these custom commands work is that the shell user that Pantheon uses w/ ssh has a $HOME directory, and in that home directory there is a Drush configuration file. This configuration command uses drush.commands as you showed above. These commands may bootstrap Drupal and make API calls. These commands do not have any Composer dependencies, and therefore do not load a second autoloader.

If Drush at some point removed this capability, then Pantheon could re-implement this functionality with drush php:script instead of Drush commands. This facility is less convenient than a command, and not necessarily any safer; there's nothing stopping someone from including an autoload.php file from a script. I would prefer to retain the ability to package global commands, even if it's perpetually deprecated.

As I mentioned before, Pantheon would prefer not to add code to the Drupal site itself. Doing this is unreliable, e.g. the customer might decide to remove or disable the code, unaware of the consequences. Perhaps when we deprecate and update the documentation, we could stipulate that global command discovery is only intended for use by service providers.

[greg-1-anderson]

I have no objection to removing site-wide command discovery in Drush 12 in favor of autodiscovery.

[weitzman]

Deprecating site-wide discovery sounds good to me, if we are confident in the performance if RelativeNamespaceDiscovery. It traverses a directory for every namespace, so I pause to ask. On Mass.gov thats 174 namespaces.

[greg-1-anderson]

RelativeNamespaceDiscovery just traverses the list of namespaces, and does just one is_dir for each.

[weitzman]

Not sure that getting rid of Site-Wide Commands discovery is worth the hassle for site owners because site aliases currently discover in these locations also and they cant migrate to PSR4 auto-discovery since they are yml files. For example both aliases and commands are found at https://github.com/massgov/openmass/tree/develop/drush.

[greg-1-anderson]

Site-wide commands share their autoloader with Drupal, so leaving them is safe. We could maybe reduce the directory iteration depth in Drush 12 if it's too deep, and gets off in the weeds in some sites. Didn't check what it's at now.