We're going to be moving the drush_extras commands back into Drush, and will be loading version specific code through conditional includes.

This will allow us to port install and update commands from provision into drush, so it's quite important.

The api will be a port of http://drupalbin.com/8564 , made to look and work a bit more like http://api.drupal.org/api/function/module_load_include

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Owen Barton’s picture

Status: Active » Needs review
FileSize
3.21 KB

Here is a first cut at this - with essentially a copy-pasted version of provision_platform_include(). The one change I did is to allow commands to include a "path" element, that is functionally similar to the "file" element in hook_menu, in that it will automatically include any files matching the drush_include rules for that command in that path. This is just a convenience of course, but I find it useful to group the path information for commands in the same place, rather than scattered all through the commandfile.

I left in the platform stuff, because it seems like it would be beneficial in some situations (e.g. if you need to handle complex installs of contrib module dependencies, like CiviCRM, that might require their own version specific code).

Owen Barton’s picture

FileSize
7.21 KB

Adrian pointed out that if we did the hook_drush_command invocation in drush_get_commands() we could both add the commandfile name and path to the command array and avoid 4 separate loops over all the commands (which is redone for each bootstrap increment).This allows fully automagical inclusion of .inc,
__.inc and
_.inc files.

We will probably want to tweak and improve the drush_include function some as we go - for example I would prefer that it included all files that match, rather than just the most specific/best fit one - because that would allow the central command logic to go in the .inc file and the version specific functions only to live in the
...inc files.

moshe weitzman’s picture

code looks good. please add doxygen to drush_include(). especially that mysterious $platform param :).

in drush_get_commands(), we do $result = call_user_func_array($function, $args);. Where does $args come from? Perhaps should be 'callback arguments'.

feel free to commit this after those 2 issues are addressed.

dugh’s picture

subscribe

Owen Barton’s picture

Assigned: adrian » Owen Barton
FileSize
19.63 KB

Here is an improved patch that adds a drush_include() as a lower level API that can be used for ad-hoc includes where you need the option of version specific files.

It then implements a higher level API on top of drush_include() that introduces the concept of "engines". These are essentially pluggable subsystems, of the kind we have in core (such as image, caching password hashing etc) with an optional hook_drush_engine_$type that lets you (a) add useful end-user information and metadata, such as descriptions and option lists and (2) allow other commandfiles to easily add their own handlers. If you have no need for friendly user selection or extensibility you can also call drush_include_engine() with a specific path where you keep them.

The idea is that this system should be usable in many places where you need 2 or more interchangable blocks of code that perform the same high level function. For example, "wget" and "cvs" are both "handler" type engines; "apache" and "lighttpd" are both "webserver" type engines. And so on.

I have implemented this API for the single wget handler we have in core. Note that because dl is still implemented by the core commandfile (and not by pm) I am providing a specific path to the handler - once the dl command (and the hook) move over to the pm commandfile I won't need to specify a path, because it defaults to the calling commandfile path (i.e. commandfile/enginetype/engine[_version].inc). As you can see, the code is much nicer than the previous hardcoded nastyness.

If you want to test drush_include_engine() in "hookless" mode, you can comment out core_drush_engine_handler() and replace:
drush_include_engine('handler', 'wget');
with:
drush_include_engine('handler', 'wget', NULL, DRUSH_BASE_PATH . '/commands/pm/handler');

adrian’s picture

Status: Needs review » Fixed

I tested this, and it works smashingly well. It has complete doxygen, and i converted provision to this new format.

committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.