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.