I've modified the drush function that handles the downloading of defined libraries so that they are actually downloaded. It is based on the drush functionality for downloading projects. The patch I've included contains all code to get it working. But aware that it only provides the base for getting the download functionality in the module. If there is anything I can do to help, let me know!


MKorostoff’s picture

Issue summary:View changes
Status:Active» Needs review
MKorostoff’s picture

new8.18 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]
new651 bytes

Great patch! I had to make one small bug fix to get it working.

When I applied the patch, and ran it, I got this error:

$ drush libraries-download erta -v
Initialized Drupal 7.31 root directory at /Users/matt/Sites/vmt.local/docroot    [notice]
Initialized Drupal site default at sites/default    [notice]
Executing: mysql --defaults-extra-file=/private/tmp/drush_Pg4LDA --database=vmt --host= --port=3306 --silent  < /private/tmp/drush_XQU1mx
Using destination directory /Users/matt/Sites/vmt.local/docroot/sites/all/libraries  [notice]
Downloading library erta to /tmp/drush_tmp_1409240715_53ff4e8b9ac0f ...        [notice]
Executing: wget --version
Executing: wget -q --timeout=30 -O /private/tmp/download_fileTPnfOz https://github.com/samsono/Easy-Responsive-Tabs-to-Accordion/archive/master.zip
Downloading erta was successful.          [notice]
Mime type for /tmp/drush_tmp_1409240715_53ff4e8b9ac0f/master.zip is application/zip        [notice]
Executing: unzip /tmp/drush_tmp_1409240715_53ff4e8b9ac0f/master.zip -d /tmp/drush_tmp_1409240715_53ff4e8b9ac0f
Undefined offset: 0 libraries.drush.inc:127        [notice] <--------- **here's the bug**
Drush command terminated abnormally due to an unrecoverable error.        [error]
Error: Maximum function nesting level of '100' reached, aborting! in /usr/local/Cellar/drush/HEAD/libexec/includes/environment.inc, line 571

Basically, the problem was that drush_tarball_extract() returned an array whose first key was 1, not 0 (weird right?). I just threw in a $file_list = array_values($file_list); and it worked!

Thanks again :-)

BR0kEN’s picture

Assigned:Unassigned» BR0kEN
new10.52 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]

Hi guys.

At first I've created a sandbox in which implemented the Drush command for downloading non-existent libraries. I did not publish a project and used the command by downloading it via Composer. Now, when I've upgrade it, think it's ready for the Libraries module.

Abilities and use cases:

  1. You can download all non-existent libraries by execution of one command. Use drush ld to do that.
  2. You able to download (or move them from sites/all/libraries if they are exist there) libraries to profiles/<PROFILE>/libraries. Use drush ld --profile to do that.
  3. You able to download libraries, specified in command line. Use drush ld twitteroauth parsedown to do that.
  4. You can see the help for this command by executing the drush help ld.
  5. You can combine command arguments and parameters as you want.
  6. Patch can be applied to 7.x-2.x and 7.x-3.x branches.

I hope for attention for this patch.

BR0kEN’s picture

new10.58 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]
new1023 bytes

This patch differs from previous only by using human-readable names for libraries instead of machine names in dialog for re-downloading.

m1r1k’s picture

Status:Needs review» Needs work

Look at drush pm-download command:
- use --destination instead of profile name
- add optional --package-handler=wget|curl
- add --use-site-dir

tstoeckler’s picture

  1. +++ b/libraries.drush.inc
    @@ -9,34 +9,29 @@
    -  $items['libraries-list'] = array(
    +  $commands['libraries-list'] = array(

    Fine by me.

  2. +++ b/libraries.drush.inc
    @@ -9,34 +9,29 @@
    +    'aliases' => array('ll'),

    Mhh will have to sleep over this, but don't see why not ATM.

  3. +++ b/libraries.drush.inc
    @@ -9,34 +9,29 @@
    -function libraries_drush_help($section) {

    Not sure why you removed this? Mmhh I guess because it just duplicates the command description at the moment?

  4. +++ b/libraries.drush.inc
    @@ -9,34 +9,29 @@
    +    'options' => array(
    +      'profile' => dt('Specify this empty option if you want to download libraries into profile installation folder.'),
    +    ),

    We should look at drush dl and maybe mirror that behavior of where stuff is placed?

  5. +++ b/libraries.drush.inc
    @@ -44,7 +39,7 @@ See libraries-list for a list of registered libraries.');
    -function libraries_drush_cache_clear(&$types) {
    +function libraries_drush_cache_clear(array &$types) {


  6. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    -function libraries_drush_list() {
    +function drush_libraries_list() {

    The rename is fine with me.

    Not sure why you included a bunch of changes to the function though?

  7. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +  // Remove the command name from arguments.
    +  $arguments = array_slice(drush_get_arguments(), 1);

    Mhh, does Drush not provide a cooler way to do this? I would have thought some func_get_args() fu?

  8. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +  $download_all = empty($arguments);

    I think that should be a separate --all flag or something, seems counter-intuitive otherwise I would think? Also inconsistent with Drush dl.

  9. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +    $path = drush_get_option('profile') ? drupal_get_path('profile', drupal_get_profile()) : 'sites/all';

    As mentioned, we should look at how drush dl does this, but it would be cool, to make this even more configurable. (e.g. sites/example.com, etc.)

  10. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +    if (!empty($downloaded) && drush_confirm(dt('The next libraries were already downloaded: "!libs". Do you want re-download them? Old data will be erased!', array('!libs' => implode('", "', $downloaded))))) {

    Again, this is something I would copy drush dl for. I don't think drush dl views does anything, if I already have Views module downloaded, does it?

  11. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +      // Set affirmative context to "TRUE", because a user gave his consent
    +      // for re-downloading.
    +      drush_set_context('DRUSH_AFFIRMATIVE', TRUE);

    Never seen that before. Cool, learned something new today! :-)

  12. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +        if (!empty($library['download url']) && _drush_download_file($library['download url'], $archive)) {

    So this is the heart of the patch and I think it's also the most problematic part. We cannot just assume that $library['download url'] is the *actual* downloadable file. So far we have always assumed that to be a URL where the library can be downloaded. It was included in the API for a future UI that helps people download the library themselves. We cannot easily make it the filename unconditionally, because not all libraries have a static download link because they often include the library version. I.e. if the URL is http://code.jquery.com/jquery-2.1.4.min.js, how would you specify that in hook_libraries_info()? You don't want to update your module everytime a new jQuery comes out!?

    That is also the reason why this has not been implemented so far, it's a non-trivial problem.

  13. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +          // Extract data into "*/libraries/LIBRARY-tmp" directory and get
    +          drush_log(dt('To download a library, the "download url" parameter shall point to an archive file.'), 'error');
    +        }
    +      }
    +    }
    +    else {

    I think that's problematic, we should not be messing with people's file system in that way. Instead we should use the system /tmp dir.

  14. +++ b/libraries.drush.inc
    @@ -59,112 +54,161 @@ function libraries_drush_invalidate_cache() {
    +          $listing = drush_tarball_extract($archive, "$path/$name-tmp", TRUE);

    I don't know how failsafe drush_tarball_extract() is, but how can you be sure that what you're donwloading is a tarball? As mentioned in the previous comment, it might just as well be a single JS file or something.

tstoeckler’s picture

Will commit some of the more general fixes, to get that out of the way for the next patch reviews.

BR0kEN’s picture

Status:Needs work» Needs review
new11.25 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]
new7.21 KB

1. Ok.
2. I guess you are right with this. Changed for libl and liblist.
3. Yes. drush help <COMMAND> is informative, as for me.
4. Added support of Drush --uri (-l) parameter and didn't touch the --profile parameter. I think that we don't need to copy the workflow of --destination parameter from pm-download because typing all time something like --destination=sites/default/libraries is not useful.
5. Ok.
6. Just refactoring.
7. Unfortunately I don't know a better way.
8. Added --all parameter and error message if command run without names of libraries.
9. See item #4.
10. drush dl asking about re-downloading. Also, using this patch, you can move libraries between directories (from "sites/all" to "sites/example.com", for example).
11. Ok.
12. Added possibility to download single files (not archives). What about unique URLs? I guess this is not a problem of patch. Every code can be found on GitHub or somewhere else.
13. System /tmp directory could not be used properly every time.
14. See item #12.

Also, added support of --root (-r) Drush parameter.

I guess we should start from this one, at least. Because otherwise we will be at the start point all time.

P.S. Thanks for review.

BR0kEN’s picture

new11.4 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]
new5.62 KB
  1. Added fixes to documentation.
  2. Prevent non-existent "libraries" directory.
  3. Added "Module" column to table with list of libraries (libraries-list command).
BR0kEN’s picture

new19.78 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/libraries/tests/libraries.drush.test.
[ View ]
new8.63 KB

Was added manager for downloading/removing libraries on enabling/disabling the module. To use these opportunities, use the following keys in definition of library.

  • 'auto remove' => TRUE
  • 'auto download' => TRUE

Also, SimpleTest test suite was added for testing drush libget command.

Status:Needs review» Needs work

The last submitted patch, 10: libraries-automatic-drush-download-1884246-10.patch, failed testing.

BR0kEN’s picture

Status:Needs work» Needs review
new19.79 KB
FAILED: [[SimpleTest]]: [MySQL] 150 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new436 bytes

Status:Needs review» Needs work

The last submitted patch, 12: libraries-automatic-drush-download-1884246-12.patch, failed testing.

BR0kEN’s picture

new19.92 KB
PASSED: [[SimpleTest]]: [MySQL] 149 pass(es).
[ View ]
new5.16 KB
BR0kEN’s picture

Status:Needs work» Needs review
BR0kEN’s picture

I'll try to explain what's going on.

- created the command that helps to download libraries defined by "hook_libraries_info()";
- URL for downloading should point to single file (js/css/gz/zip/tar);
- improved the command for list libraries: refactored code, added column with name of module that defined that library;
- added SimpleTest test suite for testing libraries downloading by Drush;
- added test module that defines library with a correct URL for downloading;
- added an object that manages downloading/removing the libraries;
- added support of "auto download" and "auto remove" properties to definition of library;
- added "hook_modules_enabled()" and "hook_modules_disabled()" which download/remove defined libraries depending on properties described above;

Some notes:

- Test cannot be run by Drupal.org or on your local site from UI. It requires Drush availability in environment and checks this by looking for existance of some part of Drush API that is available when tests run by Drush (drush test-run LibrariesDrushTestCase -y).
- This commands are not designed to work without Drupal.
- Changed work of "-l" or "--uri" Drush option: it takes "all", "default" or any subsite name as an argument (if missed, "all" will be used).
- "--profile" option will ignore "-l" and download libraries to current Drupal profile.
- "--all" option download all libraries which can be downloaded.