| Project: | Drush |
| Version: | All-versions-4.x-dev |
| Component: | PM (dl, en, up ...) |
| Category: | feature request |
| Priority: | normal |
| Assigned: | msonnabaum |
| Status: | closed (fixed) |
Issue Summary
Similarly to how Drush will automatically download dependencies when you enable a module with missing dependencies (e.g. "drush en views" with give the option to download ctools if it wasn't already present), it would be really useful if Drush could also download the necessary libraries for modules. To do this, there would need to be a new hook, e.g. hook_library_info(), that would be used to define the libraries a given module needed. An example usage would be e.g.:
<?php
/**
* Implements hook_library_info().
*/
function mymodule_library_info() {
$items['jquery.cycle'] = array(
'source' => 'http://malsup.github.com/jquery.cycle.all.js',
);
}
?>This would tell Drush that it expects to use the 'jquery.cycle' library, which should be placed in sites/all/libraries/jquery.cycle. Because this specific example is just a single file it would be just downloaded on its own, but it could just as easily be a zip or tar.gz which would then be extracted and renamed to match the library definition.
The cool part would be when going to enable the module, Drush would check to see if the library existed and if not present the option to download it.
Taking it a step further, there should also then be a Drush command to automatically download all currently defined libraries and put them in the correct location.
(While this could be driven by the Libraries API module, I don't believe the module should be required in order for this to work.)
Comments
#1
A while back I was thinking that pm-download should add all of the drush commandfiles that it finds to the commandfile list immediately after it downloads them -- similar to how pm-enable adds in the commadfiles for the modules that were just enabled.
If we did this, modules could provide a modulename.drush.inc that implemented the post hook for pm-download, and it would fire when the module was downloaded. You'd have to check the arg list (same as w/ the post-enable hook), and could then download your dependencies with code.
The suggestion above is perhaps simpler for the client, but would not necessarily support 100% of the post-library enable needs.
#2
Cool. I was thinking of invading the Libraries API namespace to make it generic so other modules could possibly benefit too.
So, what other functionality would be needed in a hook like this?
#3
The advantage of #1 is that it would be very little code, and you could do whatever you like in your post-download hook. Therefore, I don't think any other functionality would be necessary in your download hook. With a recipe or two, it wouldn't be too much harder this way than #0 would be.
#4
For an example of what I'm talking about, see the post-enable hook in the devel module. This technique works well for modules that do not need their libraries during installation. A post-download hook would cover the later requirement.
#5
Some libraries like colorbox or fullcalendar ship with the files in a subdirectory, alongside demo code which is unadvisable to leave around. In addition, fullcalendar tries to be helpful and include a full copy of jquery and jqueryui.
Can we try to address unzipping and then only keeping the contents of a given subfolder?
#6
Perhaps adding an element that defines the list of files & directories to keep? Alternatively, an element that defines a list of files & directories to remove? Maybe also an option to rename/move the given directory, e.g. so you don't end up with "sites/all/modules/tinymce/tinymce/jscripts/tiny_mce/tiny_mce.js" (i.e. it should be "sites/all/modules/tinymce/jscripts/tiny_mce/tiny_mce.js").
#7
Here is a patch that allows #0, #5 to be developed. With this patch, #1 is essentially done; just add recipes. (see devel module per #4). You may continue to enhance as you suggest above by calling your hook after the line added by this patch. Without this line, your patch will be called for all drush commandfiles _except_ the ones that were just downloaded. With this patch, you will need to filter out the returned results and only process info from the projects just downloaded.
If you ask me, I think that #0 and #5 are getting a bit complicated. With recipes, I think that post-download hooks are sufficient, and more flexible in that it is not necessary to anticipate what operations are necessary, implement a bunch of different unpack algorithms, handle different cleanup needed after repatching, etc.
If you really do want to go down this path, though, then I think that you should get your auto-download info from each module's .info file, not from a commandfile hook.
#8
@greg.1.anderson: Thanks, I'll give it a spin.
#9
Here is another patch I think would be good for Drush-5. Could we review and commit, or close it?
#10
Per irc discussion with moshe, pm-download is not a good time to download dependencies. Let's consider a pre-enable hook instead.
#11
Here is a patch that adds modules to the search path during the pm-enable validate phase, so that libraries & c. can be downloaded in a pre-hook if desired.
One other difference between this and current implementation is that with the current code, the post-enable hook of disabled modules that could not be enabled will not be called. With this patch, both the pre-hook and the post-hook of any module listed on the commandline will be called, regardless of whether the module could be successfully enabled or not. Clearly this is required for the pre-hook; I'm unsure whether it is a real problem for the post-hook. Drush currently has no facility for removing commandfiles from the commandfile list, but a function to do this could be added if it was deemed important enough. n.b. the module itself could check its own enabled status in the callback hook if necessary.
#12
All tests pass.
#13
Actually, the patch in #11 is perfect. The pm-post-enable stage is actually not called if there are any extensions that cannot be enabled, so there is no change of behavior from Drush-4. Also, the code to download libraries can be simplified now. Using devel as an example:
function drush_devel_pre_pm_enable() {
$modules = drush_get_context('PM_ENABLE_MODULES');
if (in_array('devel', $modules) && !drush_get_option('skip')) {
drush_devel_download();
}
}
This works fine. The same code also works equally well as a post-enable hook (as it currently is). If everyone agrees with #11, I would suggest that we commit this to Drush-5 and then backport to Drush-4 so that it can go in 4.6. This will help adoption of the auto-download of dependencies as people move from Drush-4 to Drush-5. I'll roll the 4.x patch if this is agreeable.
#14
Code looks good. Docygen needs work The validate callback has the comments for the command callback. Also, "// Add all modules that were enabled" isn't quite right anymore. I think we mean "Add all modules that passed validation."
#15
Fixed comments, updated drush.api.php, and committed. Will work on a Drush-4 version next.
#16
Here is a Drush-4.x version of the same patch. All tests pass.
#17
In the interest of facilitating a quick release of Drush-4.6 (should it be desired), committed 5a0abe8. This works well and is compatible, and I think 4-to-5 parity on pre-enable hooks is valuable. Back it out if you have any concerns, though.
#18
Automatically closed -- issue fixed for 2 weeks with no activity.