Allow users to force Drush to recognize validity of Drupal root and site

[bangpound]

This is a long shot, but I figured I'd ask!

Would drush accept a pull request that creates a global option that forces drush_valid_drupal_root() to return true and also forces the return value of drush_drupal_version() to be a user specified value? (This would have to be an at-your-own-risk kind of option of course.)

I'm happy to do this work myself if Drush maintainers would consider committing it when it's complete. If so, do you have any advice about it? Should I try addressing the problem some other way?

Background

I am running Drupal 7 inside the Symfony framework. I've replaced Drupal's bootstrap and front controller, but more or less everything can work the same with Drush except that Drush searches for index.php to determine the validity of a Drupal root directory.

Symfony uses app.php, and if I symlink or copy app.php to index.php, there are unwelcome side effects. (In particular, the PHP built-in web server privileges index.php even when it is told to use a router script.)

For now, I've hacked Drush and replaced index.php with app.php in various places where it's needed. Since my use is very much an edge case, I don't expect Drush to change much to accommodate my use.

[greg-1-anderson]

Backdrop wanted to do something similar (c.f. backdrop/backdrop-issues#47).

I think a global option is a bad idea. I think a hook that some d7symfony.drush.inc or backdrop.drush.inc could implement to provide its own "valid root" detection would be a good idea.

It would also be nice if the "flavor" of Drupal that was detected could also in some way affect the way commandfile searching was done, so that you could make global Drush commandfiles that were specific to only certain Drupal variants. See also #88.

[bangpound]

Please don't rush to reply to this. I have to get up and will have to return to it later. These thoughts are still in formation.

I read the issue you mentioned, and I think it's quite a long shot, though if someone were writing Drush from scratch today, Symfony Console would probably be the kit to use.

I haven't tried Backdrop at all, but my strategy for Symfony Drupal is simply to port Drupal 7 onto Symfony's kernel and foundation objects. This requires minimal changes, and I've tried to make as few changes as possible so that I'm not creating something that is totally unsustainable. (My intent is to continue merging Drupal 7.x and remain API compatible wherever possible.)

I'm only marginally acquainted with Drush's guts, so let me try to expand your idea.

Rather than a global option, there should be a Drush extension file that is specific to each Drupal fork. I guess this would be placed in one of the six paths (identified at the top of example.drushrc.php) where Drush searches for extensions. Its presence would mean it is automatically included, and it would be a file that only implements Drush hooks (similar to a the policy.drush.inc file that is also in examples).

For Symfony Drupal, it would be stored in sites/all/drush. (If another fork exists that doesn't have a distro-specific directory where Drush already looks, it would have to be specified with the --config option? This doesn't apply to Symfony Drupal, though.)

This fork-specific file would implement hooks that don't exist yet. These hooks would be invoked at almost every phase of bootstrap (or would replace bootstrap entirely?) to supply alternative or additional validation routines for identifying the drupal_root, site, conf_path, and database settings.

Am I getting it right so far?

I've already hacked Drupal's bootstrap so that I can override it. I wonder if it would make more sense for forks to provide their own bootstrap.inc and environment.inc?

[greg-1-anderson]

Don't worry about supporting backdrop, or rewriting Drush to use the Symfony Console; those are side-notes that I include for reference, so folks who work on changing the way bootstrapping works will be aware of other issues where similar things might be happening at some point.

I don't really like the idea of forks providing entire bootstrap.inc / envrionment.inc files. It would be better to make a new bootstrap class that knows how to bootstrap a certain flavor of Drupal.

For your purposes, all you would need to do is create the bootstrap class, make some way for forks to provide their own derived class during the bootstrap process, make a method that returns 'index.php' in the base class, and override that method to return 'app.php' in your derived class. (To be clear: DRUSH_BOOTSTRAP_DRUSH and all of the site-selection code stays hardcoded in Drush; the bootstrap class is just to bootstrap Drupal, or some variant thereof.) Then, obvously, Drush would need to call your method everywhere that you replaced index.php in your prototype. Extending Drush to use this class for more things (e.g. to make it possible to bootstrap Backdrop) could be left as an exercise for folks who needed such things.

[weitzman]

Right. I have long wanted to use factor out the Drupal from Drush. The bootstrap class would be a big step forward.

[bangpound]

I think I can do this! I'll give it a shot soon.

[greg-1-anderson]

The only wrinkle is that the methods you are modifying are needed to select the bootstrap class. :) But I think you can figure that out. Maybe every hook is allowed to create its bootstrap object, and the ones that do not validate (hopefully all but one!) are thrown out.

Thanks for taking this on.

[jenlampton]

@greg-1-anderson I just wanted to check and see if this was still the direction you wanted to go with this... since it's been a year since the last comment :) I may be able to drum up some people who are working on Backdrop (but really miss Drush) to take a stab at this, as long as you guys still approve of the direction.

[greg-1-anderson]

There are a couple of different directions that we could potentially go here.

Please see the discussion in #624 (referenced above); I claimed late in that thread that the PR merged there implements this issue. That declaration was a bit optimistic, though; while the scaffolding is now in place, but there still isn't any way to plug in some code to allow Drush to recognize a Backdrop bootstrap class.

Here is the code that runs first thing in drush_main() in drush.php:

  $bootstrap_class = drush_get_option('bootstrap_class', 'Drush\Boot\DrupalBoot');
  $bootstrap = new $bootstrap_class;
  drush_set_context('DRUSH_BOOTSTRAP_OBJECT', $bootstrap);
  $bootstrap->preflight();

There's the global option that allows you to specify on the command line that you'd like to use some bootstrap class other than DrupalBoot. This is done for the benefit of 'early' code that might want to use the bootstrap object. So, with this code, you can today support Backdrop, but you'd have to do so by adding a --bootstrap_class option to the command line (perhaps via a convenient bash alias).

I think what we actually need, though, is to postpone finding the bootstrap class until after (or during) the DRUSH_BOOTSTRAP_DRUSH phase. Until we have bootstrapped Drush, we can't really provide plugin classes that might be consulted to find a better bootstrap class. I don't have a plan on what to do with the 'early' code in this scenario, though.

Another possibility would be to depend on #572. If we did this, and were therefore using Composer to load our Drush extensions, then we could use php reflection to find the available bootstrap classes. The only trick here is that we would need to do this after we included the Composer autoloader.

Not sure what Backdrop's opinion on Composer is. Hopefully, Composer will be used in Backdrop as well, as this is the direction we'd ultimately like to go in Drush as well.

[Kazanir]

I confess to some confusion with this issue. I would definitely like to see Drush support Backdrop somehow, but I have read through the code and I do not see how the Boot interface and DrupalBoot class could help us accomplish this.

Here is my chain of thought from exploring the code:

• The offending code here seems to be in the function drush_valid_drupal_root() which is defined in includes/environment.inc. The other offender is drush_drupal_version() which is similarly in includes/drupal.inc.
• Both of these files are included from drush_preflight_prepare() in includes/preflight.inc.
• The bootstrap code is run first thing in drush_main(), but drush_main() is not actually the first thing that runs in drush.php.
• Rather, includes/preflight.inc is included, and drush_preflight_prepare() is run before drush_main() is executed. (Hence the name "preflight", presumably.)
• This means, as far as I can tell, that there is no way to override the includes that happen during preflight and no way to override the drush_valid_drupal_root() function.
• This function is hard-coded into several other places in includes/environment.inc as well as includes/sitealias.inc, so even if the DrupalBoot class were replaced with one which doesn't invoke these offenders, this problem would still crop up elsewhere in the code.
• Finally, I observe that overriding the DrupalBoot class is going to be kind of a pain. It has two main methods, one which includes 950 lines of code in other include files and one which is a 70-line method that primarily calls the functions from those 2 files. It seems like overriding a small piece of the Boot class functionality is going to end up requiring a lot of needless copy/paste.

Please let me know if I'm totally barking up the wrong tree here or have missed something obvious. I'm happy to provide some firepower to get this done but it is hard to know what the "right" path to take is.

[greg-1-anderson]

Thanks for looking at this Kazanir. First off, you are correct in your analysis. A lot of the early-execution code in Drush has evolved over a period of many releases, and could stand to be improved.

Some comments.

• drush_valid_drupal_root() should probably become a method of the bootstrap class. This will require some refactoring.
• Anything included from preflight.inc should be Drupal-agnostic. Drupal-specific code should move to the bootstrap class.
• The primary purpose of the preflight code is to find out if something other than running a local Drush command is happening. For example, drush @remote ssh will execute without ever running drush_main(). This is perhaps odd, but in the short term, would not need to be fixed to support Backdrop.
• I haven't surveyed the usage of drush_valid_drupal_root() in sitealais.inc, but often this is done to process long-form site aliases. This problem will have to be resolved one way or another, but I can probably help out here if we get to the point where a basic bootstrap is working.
• If you need to change something buried deep inside some overly-large method of DrupalBoot, then by all means, factor out a separate method that returns just the information you need to modify, and insure that your new method is called whenever needed. Once DrupalBoot is fixed up, you should be able to override it and change just a few specific methods.

Looks like there's still quite a bit of work needed here--sorry to say that I don't see any silver bullet, it's just going to be a simple ("simple") matter of refactoring and fixing the shortcomings.

[Kazanir]

Your post is a big relief actually -- I'm more scared of doing something completely wrong than of a bunch of refactoring. I'll try to knock out the outlines of a PR "soon" based on your comments, probably this weekend.

[jenlampton]

To summarize, what is needed for Drush is as follows:

• Refactor `drush_valid_drupal_root' as part of bootstrap class
• Move Drupal-specific functions from preflight into bootstrap (if needed / carefully)
• Refactor DrupalBoot to be less of a pain when overridden
• Create a way to plug in some code to recognize a different bootstrap class ** doozy?

And what is needed for Backdrop:

• Specify a different bootstrap class
• Provide the bootstrap class specified