Stop by bootstrap to DRUPAL_ROOT when searching for commandfiles

[jonhattan]

Drush is unable to detect drupal-version-specific commandfiles without a full bootstrap.

This is not neccesary for such commandfiles that only want to operate on DRUPAL_ROOT or DRUPAL_SITE phases. Indeed it is very bad if the site is not installed yet.

My case: I'm willing to provide settingsphp.d8.drush.inc and settingsphp.d7.drush.inc for https://www.drupal.org/project/settingsphp. Drush fails with "command not found" because I'm running on a site not installed yet so it can't bootstrap to DRUPAL_FULL and the commandfiles are not reachable from the Drush preflight phase.

[weitzman]

I'm a bit surprised that this is needed. During preflight, we honor any --root thats provided so we should be able to find this commnadfile then (i.e. DRUSH_BOOTSTRAP_DRUSH). Are you providing --root or are you relying on Drush to discover the root? Maybe this is needed in the discovery case.

[jonhattan]

I'm running from the drupal root. Also tested with an alias and --root with no success:

drush8 @d8 settingsphp-generate --db-url="mysql://root:root@localhost/d8" 
drush8 --root=/var/www/d8/docroot settingsphp-generate --db-url="mysql://root:root@localhost/d8"

As I understand it, in DRUSH_BOOTSTRAP_DRUSH, the drupal version is unknown, and COMMANDFILE.d{DRUPALVERSION}.drush.inc is unreachable.

[jonhattan]

The commandfile is excluded here because drush doesn't know the drupal version at preflight:
https://github.com/drush-ops/drush/blob/master/lib/Drush/Command/Commandfiles.php#L52

[weitzman]

Thanks for figuring that out. I think the PR makes sense. I'd like to hear @greg-1-anderson opinion. I'm not sure about the full repercussions for adding another phase to this array.

[greg-1-anderson]

Okay, here is what is going on. The main reason that Drush skips past bootstrap levels like this is so that we can decide whether to look for commandfiles the slow way -- only in enabled modules -- or the fast way -- in all modules that are available, enabled or not.

So, under normal circumstances, we find a global Drush command, and then we bootstrap to whatever level it says we should bootstrap to, and then we look for additional commandfiles, so that they can hook our command if they want. We do the fast or slow version based on how far the command wants to bootstrap. This stops us from considering commandfiles in disable modules.

I haven't tried this patch yet, but it seems to me that what is going on is that, with this change, we fail to find a global commandfile, then we bootstrap to DRUSH_BOOTSTRAP_DRUPAL_ROOT (new with this patch), so we now know the version of Drupal we are running. I would expect that we still would not find your commandfile, because DRUSH_BOOTSTRAP_DRUPAL_ROOT does not search for commandfiles in modules. At this point, I would expect that Drush would try to bootstrap to DRUSH_BOOTSTRAP_DRUPAL_FULL. I'd think that at this point bootstrap would fail, and you'd be done with an error, before your commandfile is found. Does it instead bootstrap only as far as it can for the site, and then grab your commandfiles? If that is the case, then does it also allow you to run commandfiles in disable modules?

I think in some instances, we should allow commandfiles in disabled modules to run. "generate" type commandfiles, for example, are often fair game, especially for themes ("make me a new subtheme of this contrib base theme"). Themes currently work differently than modules. Devel's fn-view and fn-hook are other examples that could be available even when devel is not enabled. I think that before we commit this patch, we should decide what we want the behavior of commandfiles in disabled modules to be. I think that bootstrapping was changed to work this way because there were some examples (which I have forgotten) that caught fire, if a drush command tried to call APIs of the disabled module it lived with. Perhaps this was deemed to be common. Once we know what sort of seatbelts we want to apply, we should insure things still work the way we want them to with this patch.

[weitzman]

Drupal is very clear that disabled modules never even get their code loaded. I think our historical consistency with that is a good thing, and we should not load any commandfiles from such modules. Folks can include the commandfiles manually with --include

[jonhattan]

To clarify, settingsphp is not a module. It is a standalone commandfile, located at /usr/share/drush/commands.

I'll test what happens with disabled modules commandfiles and report back.

[greg-1-anderson]

Oh, okay, I was confused about the location you were putting your commandfile. If your commandfile is in a global location, then this patch is fine -- it seems to me that there should be no affect on how Drush searches for commandfiles in modules with this change. Thank you for testing and confirming this, though.

[jonhattan]

Ok, ctools module disabled. With and without this PR, it doesn't find it:

jonhattan@nuro:/var/www/d7/docroot$ drush ctools-export
Command ctools-export needs the following module(s) enabled to run: ctools.                                                                                       [error]
The drush command 'ctools-export' could not be executed.                                                                                                          [error]

[greg-1-anderson]

Cool. This is definitely an improvement, then.

[greg-1-anderson]

Wouldn't hurt to backport this to Drush 7, but not necessary either, I think.