Fixed global commands not being discovered where Composer installs th…

[joachim-n]

See #3610

[greg-1-anderson]

I'm really unsure about why we have both depth('<=3') AND ^ in the path regex.

I'm ambivalent about all of the file searching here. Maybe we should call the finder twice, once at drush and web/drush with a depth of one, and once at drush/contrib and web/drush/contrib with a depth of 2 or 3. That way we match what Composer is currently doing, and we don't waste time looking for Commands directories inside drush/sites et. al.

[richardbporter]

I've got this issue as well and would love to see this (or some variation) merged in.

What is left to do here - explicitly call the finder twice and update docs?

[greg-1-anderson]

The main requirements are 1) passing tests and 2) no excessive searching. If someone has created DRUPAL_ROOT/drush/vendor or has put npm or other build results somewhere inside of drush, while this is not recommended, it should not cause Drush to search through those directories for commandfiles. We can have some deep searching in well-known locations like DRUPAL_ROOT/drush/commands, but we should be more conservative when searching at DRUPAL_ROOT/drush.

[richardbporter]

Actually, I'm just going to use the new namespaces recommended in the docs: http://docs.drush.org/en/master/commands/#global-drush-commands

I think that works better anyway since autoloading classes in global Drush commands was problematic, for me at least. In other words, there is not much value for nesting commands under src/ or whatever.

Thanks!

[joachim-n]

Actually, I'm just going to use the new namespaces recommended in the docs: http://docs.drush.org/en/master/commands/#global-drush-commands

Do those work though? The point of this issue is that commands aren't being discovered.

Nested (e.g. Commandfile is part of a Composer package)

    Filename: $PROJECT_ROOT/drush/Commands/dev_modules/ExampleCommands.php
    Namespace: Drush\Commands\dev_modules

Can you confirm if the above works?

[richardbporter]

Yes, that works for me with Drush 9.4.0. What didn't work for me was nesting further, like under a src/ directory:

Filename: $PROJECT_ROOT/drush/Commands/foo/src/FooCommands.php
Namespace: Drush\Commands\Foo

⬆️ Doesn't work.

[joachim-n]

    Filename: $PROJECT_ROOT/drush/Commands/dev_modules/ExampleCommands.php
    Namespace: Drush\Commands\dev_modules

Oh wait -- does 'dev_modules' in the filename need to match 'dev_modules' in the namespace?

If that's the case, then that won't work for projects that have hyphens in the name. I've already filed an issue for that.

[richardbporter]

No, hyphens will not work but you can get around that by specifying a custom install name for you package: https://github.com/composer/installers#custom-install-names

[greg-1-anderson]

A PR in consolidation/annotated-command to transliterate the dashes to underscore would be welcome.

[joachim-n]

Nested (e.g. Commandfile is part of a Composer package)

    Filename: $PROJECT_ROOT/drush/Commands/dev_modules/ExampleCommands.php
    Namespace: Drush\Commands\dev_modules

This is not working for me, because Annotated Command is deriving the namespace from the path in discoverCommandFilesInLocation()... see consolidation/annotated-command#155

[joachim-n]

Ok so consolidation/annotated-command#155 is by design, I am told over there.

I don't understand how this problem is meant to be fixed.

[greg-1-anderson]

I'm not sure either. I'm open to suggestions. Above I said this PR would be fine if it avoided excessive searching should anyone drop a large folder in their DRUPAL/drush directory. I'm also in favor of deprecating commandfile searching and coming up with a more clear declarative API.

@weitzman was just using some plugin mechanism that @Chi-teck extracted from one of his projects. Maybe that would be helpful here? I don't know anything about it, so I don't know.

[richardbporter]

Can you change the installer name in your package's composer.json to match the namespace?

Ex.

"extra": {
        "installer-name": "DevModules"
    }

That's what I was referring to in #3615 (comment). Sorry if I'm missing something.

[joachim-n]

Can you change the installer name in your package's composer.json to match the namespace?

Yup, I did that.

Annotated Command assumes the namespace is something different, and so the command class is not picked up.

I'm not sure either. I'm open to suggestions. Above I said this PR would be fine if it avoided excessive searching should anyone drop a large folder in their DRUPAL/drush directory. I'm also in favor of deprecating commandfile searching and coming up with a more clear declarative API.

I'm burnt out on this. Drush 9 breaks my command package completely. I don't know what direction to take, as Drush/AC/Robo seem to have different and conflicting requirements. I just want ONE way of registering a command to work so my package works -- I don't care how.

[greg-1-anderson]

I'm -1 on Drush global commands and +1 on standalone commands (see https://github.com/g1a/starter) that exec out to Drush to do introspection on Drupal sites. We're also working on factoring parts of Drush out as libraries to make it possible to do more without exec'ing Drush.

AC does not have a plugin mechanism. Robo has a recommended plugin mechanism, but Drush does not use it. Drush focused on a plugin mechanism for modules, and stopped short of providing a clean mechanism for global commands. If you want to keep on the global command bandwagon, you're going to have to push through and make the darn thing work. There's not a lot of demand for this, though, and as I said above and before, we're de-emphasizing global commands because it really complicates the dependency hell problem, especially if you want to allow your global plugins to have dependencies.

I'm sorry this has been rough on you; no one has had time to fix all of the hard problems yet.

[joachim-n]

AC does not have a plugin mechanism. Robo has a recommended plugin mechanism, but Drush does not use it.

But correct me if I'm wrong -- you're one of the principal authors of all three packages! Why don't they agree on how to do things?

If you want to keep on the global command bandwagon, you're going to have to push through and make the darn thing work.

Sorry, I am done here. I am probably the only user of the Drupal Code Builder Drush command, since now Drush comes bundled with code generator of its own, people will be using that, even if it is very limited compared to DCB. So carrying on banging my head against a wall here is just a waste of my time and sapping my morale.

[webflo]

I agree with @joachim-n, drush command discovery is a mess (sorry). I would like to see a clear vision (or example) whats supported and whats not.

I packaged my drush commands as composer package (e.g. https://github.com/webflo/config_file_import). The 2.x worked for Drush 9, but is completely broken in Drush 9.3 and 9.4. The instructions on https://docs.drush.org/en/master/commands/#global-drush-commands are not composer friendly because we are mixing custom (project specific) commands with "vendored" commands.

I tried to add an additional folder to the discovery via "drush/drush.yml", but this does not as intended.

drush:
  paths:
    include:
      - "${drush.vendor-dir}/../drush/contrib"

[richardbporter]

Here is a global command working with Drush 9.4.0 if that helps anyone.

https://github.com/richardbporter/drush-users-commands

[greg-1-anderson]

Drush users-commands requires:

"drush/Commands/{$name}": ["type:drupal-drush"]

If different global command have different Composer installer requirements, then they won't work together. I agree that global commands are messed up right now. It would be good if someone could come up with a proposal and PR that worked in a consistent way & didn't involve too much deep directory searching.

[greg-1-anderson]

OK, regarding drush/Commands/{$name}, that was changed in drupal-composer/drupal-project#394. Unfortunately, there's a problem with this scheme that @zaporylie pointed out in #3566 (comment). Unfortunately, no one took notice. This is the same issue that @webflo mentions above (#3615 (comment)).

It seems that per #3566 (comment) (and per @richardbporter in #3615 (comment)), site-wide Drush commands (those that are part of a site, but not packaged with a module) do work, but only for commands required via Composer. There is no good, documented solution for global Drush commands included directly in the repository of a site (site-specific site-wide commands).

I think there is nothing for it but to support the paths drush/contrib and drush/custom, as we did in Drush 9.2.x and earlier, and as this PR is attempting to do.

I am continuing this work in #3687