As part of the refactoring I'm doing to drush to make the drush invoke api cleaner (http://drupal.org/node/349923), I am working on a patch that replaces the use of globals, and provides a central storage mechanism for Drush.
It also gives us the ability to do some introspection on how things were set, and where.
Normal usage is not changed, except for the fact that the -c / --config is now no longer a special case (ie: used INSTEAD of all the other configs, but in addition to).
It also allows us to fix things like this (from sql.drush.inc in drush_extras) :
// Hack to make sure that drush_sql_dump sees the option.
$GLOBALS['args']['options']['result-file'] = $file;
}
This would be replaced with :
drush_set_option('result-file', $file);
The context is returned as part of the json encoded output of the backend api (http://drupal.org/node/380688). This allows you to write a script which returns information about a site,
such as a command which returns how many nodes / users / terms a site has, and which modules it has enabled.
You can now write a command which iterates through all your sites and retrieves statistics of the entire set (ie: find sites with X version of module Y enabled, so you can determine which sites are affected by a security vulnerability.).
It allows a simple mechanism to save the options in a specific context back to the config file.
I do not know if this will be used by the core drush commands, but it's been pretty important to the provision system (ie: when we create a database for the site, we save it to the site drushrc.php, so we don't have to attempt to extract it from the site config, and we have a trusted source if the settings.php ever is broken.
A script that helps you to maintain patches might store a list of patches that have been previously applied to
the drupal platform, which could be useful when upgrading (as it could apply those patches in reverse, then upgrade, then attempt to re-apply the patches)
This patch seems a lot larger than it actually is, because of how well it's documented. It's about 200 lines of documentation for 100 lines of code.
As this is a non destructive patch that doesn't break any code I am aware of, I will commit it directly.
This ticket is mostly to document what change went in, and why.
Here's the main help :
/**
* @file
* The Drush context API implementation.
*
* This API acts as a storage mechanism for all options,
* arguments and configuration settings that are loaded into drush.
*
* This API also acts as an IPC mechanism between the different drush
* commands, and provides protection from accidentally overriding
* settings that are needed by other parts of the system.
*
* It also avoids the necessity to pass references through
* the command chain and allows the scripts to keep track
* of whether any settings have changed since the previous
* execution.
*
* This API defines several contexts that are used by default.
*
* Argument contexts :
* These contexts are used by Drush to store information on the command.
* They have their own access functions in the forms of @drush_set_arguments,
* @drush_get_arguments, @drush_set_command, @drush_get_command.
*
* command : The drush command being executed.
* arguments : Any additional arguments that were specified.
*
* Setting contexts :
* These contexts store options that have been passed to the drush.php
* script, either through the use of any of the config files, directly
* from the command line through --option='value' or through a JSON
* encoded string passed through the STDIN pipe.
*
* These contexts are accessible through the @drush_get_option and @drush_set_option
* functions.
*
* These contexts are evaluated in a certain order, and the highest priority value
* is returned by default from drush_get_option. This allows scripts to check whether
* an option was different before the current execution.
*
* Specified by the script itself :
* process : Generated in the current process. These are discarded at the end of the call.
* options : Passed as --option=value to the command line.
* stdin : Passed as a JSON encoded string through stdin.
*
* Specified by config files :
* custom : Loaded from the config file specified by --config or -c
* site : Loaded from the drushrc.php file in the Drupal sites directory.
* platform : Loaded from the drushrc.php file in the Drupal root directory.
* user : Loaded from the drushrc.php file in the user's home directory.
* drush : Loaded from the drushrc.php file in the same directory as drush.php.
*
* Drush commands may also choose to save settings to their matching configuration
* file through the drush_save_context() function.
*/
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | drush_context_patch.diff | 18.25 KB | adrian |
| #12 | context.inc_.txt | 13.54 KB | adrian |
| #8 | context.inc_.txt | 13.33 KB | adrian |
| #8 | drush_context_patch.diff | 17.63 KB | adrian |
| #6 | context.inc_.txt | 10.27 KB | owen barton |
Comments
Comment #1
adrian commentedHere is the code, I have it working on my own machine, both bootstrapped and non bootstrapped sites, but I have run out of time for testing on this side.
This is a patch and an additonal file for includes.
Comment #2
owen barton commentedUpdated patch to account for some bugfixes I committed - no changes to the patch (context.inc is identical, so I am not re-uploading it).
I think this looks great, and worked well in my tests - I think we need to be careful not to misuse it where we should really be passing function parameters instead (making it harder to debug and reuse), but I think that considering the amount of context we need to manage this is a major improvement what we do now.
if (drush_drupal_bootstrap($drupal_root, $command['bootstrap'])) {still references $command, but I don't think one is in scope. Perhaps have drush_parse_command() return the command as a convenience, or have drush_drupal_bootstrap retrieve the current command if one was passed in? Either way, it's nice to be able to pass in a specific bootstrap level (and we use this with drush dl).Comment #3
adrian commentedYeah, we need to be careful about that. I don't mind keeping some things returned as they are useful, I mostly
removed their return values to make sure my code was pure and wasn't relying on the return value.
Comment #4
moshe weitzman commentedCould one of Grugnog2 or Adrian commit this when you are ready?
I am marking this issue with 2.0 tag. Lets use that track issues before we make a release.
Comment #5
owen barton commentedI am going to take a shot at getting this and #387214 playing nicely together on the plane tomorrow.
Comment #6
owen barton commentedDid some work on this - an updated version is attached. I fixed what appeared to be a bug with drush_dispatch() (no $commands array was available), and also removed the "quick check" for commands in drush_bootstrap, since that is not needed any more (since the bootstrap patch is in). It isn't functional yet - it doesn't seem to find any command when you try to execute one (although the listing works correctly), but I thought I would upload my WIP in case anyone has time to polish it up.
Comment #7
adrian commentedwell. i finally got this to a point where it breaks all my provision code.
But that's a good thing. During drush 1.x i was using the global args as a workaround, but now I am forced to use the proper get/set option code.
I'm going to refactor a little bit in provision to get rid of the old bad way of doing things, and then will release a new version of the patched code.
The changes for this will also get us ready for the invoke api.
Comment #8
adrian commentedHere's the latest patch.
I'm marking this as critical, because it needs to get in to get provision / hosting (aegir) to a stable place again, it also opens the door to http://drupal.org/node/349923.
As an example of how much cleaner this api can make things ... Check out my old provision_init versus my new provision_init.
All the drush globals have been removed (other than those that drupal uses, such as db_url etc), but I have not yet removed all of the constants, because several of those need to be re-factored properly as part of http://drupal.org/node/387214. This patch makes the bootstrapping patch cleaner too, as we can now use a context for the current runlevel.
Comment #9
moshe weitzman commentedI will soon do some testing and walk throughthe code in my debugger so I can understand WTF is going on.
First, code review of context.inc
$configs['drush'] = dirname(__FILE__) . '/../drushrc.php';, perhaps use the technique in this snippit borrowed from scripts/run-tests.sh in HEAD drupal.* that do not occur through the drush_set_context and drush_set_option
* functions could have far reaching effects."
And now the patch:
Comment #10
moshe weitzman commentedWorks well AFAICT.
Comment #11
moshe weitzman commentedRunning through with a debugger now.
The problem with `drush dl help` seems to be in drush_dispatch(). We access an uninitialized variable $commands.
Comment #12
adrian commentedI will tackle the status modules issue in a further patch. It has to do with bootstrap levels, and we really need to fix the help command first.
We will tackle the site offline stuff in a future patch to get the bootstrapping process a bit smoother too.
Comment #13
adrian commentedCommitted to HEAD.