Drush download of defined libraries

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=127.0.0.1 --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 :-)

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.

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

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

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) {
   

   Sure

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.

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

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.

1. Added fixes to documentation.
2. Prevent non-existent "libraries" directory.
3. Added "Module" column to table with list of libraries (libraries-list command).

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.

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.

I'm not sure what needs to happen for this patch to be committed, but I can at least say that I applied the patch from #14 and was able to download all of my libraries just fine! I'm also more than happy to try my hand at fixing any outstanding issues if this patch isn't enough to get it committed to the module.

Okay, update to my last comment: I did have trouble downloading a few libraries because their 'download url' was not actually a direct URL to a file that could be downloaded. I don't know if hook_libraries_info() has just been implemented incorrectly, or if it's the intended behavior of modules implementing this hook to define the download URL as a page with a link to the file or archive for the library rather than a direct url to the file that needs to be downloaded. If there's no direct reference to the file to be downloaded in hook_libraries_info(), I don't know how we could possibly automate downloading a library, since it requires human knowledge to click on a "Download" button or choose a release version on the linked page.

I think the best solution is to update hook_libraries_info() to let module developers know to include a full URL to the file to be downloaded or provide their own callback for downloading the library, which we could then hook into.

This looks great thanks @BR0kEN. There is some minor things, the only thing I think we need to change is the zip location to a file on your github account @BR0kEN

1. +++ b/libraries.drush.inc
   @@ -1,5 +1,4 @@
    <?php
   -
    /**
   

   nit: this change is not needed and all of core has that space.

2. +++ b/tests/modules/libraries_test_download/libraries_test_download.module
   @@ -0,0 +1,32 @@
   +    'vendor url' => 'http://parsedown.org/',
   +    'download url' => 'https://github.com/BR0kEN-/parsedown/archive/master.zip',
   

   This looks sketchy @BR0kEN. Could you point it to a common d.o resource?

Actually this works well, I still think we need to change the test file download but leave that up to committers.

There's nothing really automatic about this, just title change and it's a task not really a feature request because the code replaces an @todo/commented out feature in the existing code.

+++ b/libraries.drush.inc
@@ -59,112 +57,201 @@ function libraries_drush_invalidate_cache() {
-      $version = (($library['installed'] && !empty($library['version'])) ? $library['version'] : '-');
...
+        $library['installed'] || empty($library['version']) ? '-' : $library['version'],

These two lines changed and now if it is installed it will show a - even if it does have a version. Just need to change back to && !empty()

May be a good opportunity to tackle feedback in #19. LMK if you don't have time and I'll roll that in.

Committed some of the unrelated changes, to make this patch a bit smaller. Proper review coming up.

So this took me a bit longer than expected, but I did actually spend a significant amount of time looking into how drush pm-download works and trying to adapt that for our use-case here. I did come up with something reasonable to my mind, that mostly follows the same logic. This supports downloading archives and single files. I did start from scratch as the previous suffered from sever scope screep from my point of view.

The whole LibrariesManager etc. is something for another issue along the lines of "Allow modules to have dependencies on libraries" (although I'm not sure I'm fond of adding a class for that, though). I thought there was an issue for that already but can't find it ATM.

This allows a library to specify a download file url (still needs to be documented) and then drush libraries-download foo will download the foo library if it did that. I kept out a bunch of more advanced features but left @todo's all around. I will also try to get some improvements into Drush upstream to make our lives a little bit easier here.

If someone gives this a whirl and sees whether it works for them, I would be fine committing this (with added docs) and then refining further with that as a starting point.

Ehhh, rhetoric: why everything should be so difficult every time?

@BR0kEN: I realize that the course this issue has taken must be incredibly demotivating for you. You have spent a lot of time on it and yet this has seen little progress largely because of my lack of involvement. I sincerely apologize for the not being more active here, but I'm always trying to balance Libraries API with a bunch of other commitments. We are now actually a team of 3 maintainers so maybe things will improve in the future.

That said, I did not find your patch in a state where I felt I could commit it. And you did not incorporate a lot of my feedback into your patch. So instead of arguing back and forth I now proposed something along the lines I was thinking. It surely is not perfect and I look forward to your constructive criticism so maybe we can get something we're both happy with. But I honestly did not know what to do otherwise because I did not feel like I could commit your patch as is. Hope to still see you around and would really appreciate your input on this, although as I noted I can totally understand if you're not motivated to work on this anymore. Again, sincere apologies. I will try to be more responsive in the future but looking at the Libraries API issue queue right now I already know that I will break that promise :-/ it's just what it is...

Added some docs to the patch and fixed a bug that I forgot to commit in my local branch so was not part of the last patch. So now with the added docs this is now something that I feel is "ready"(-ish), but would really like some reviews before going any further.

@joelpittet reported that https://www.drupal.org/project/commerce_yotpo and https://www.drupal.org/project/mailchimp are not working with this. Will try it out myself...

Duh, I included a really dumb error in the latest patch where we would detect an error properly but then continue to try to download the file anyway....

Updated patch.

Adapting/adding the following to Mailchimp's hook_libraries_info() actually allows me to download mailchimp with drush dl mailchimp:

    'download url' => 'https://github.com/thinkshout/mailchimp-api-php/releases',
    'download file url' => 'https://api.github.com/repos/thinkshout/mailchimp-api-php/tarball/v1.0.1',
    'version arguments' => array(
      'file' => 'composer.json',
      // Version 1.0
      'pattern' => '/\"version": \"((\d+)\.(\d+)\.(\d+))\",/',
    ),

Great thank you, that works. I'll add that to any module I co-maintain and a patch to mailchimp.

from dawehner ... works for me. The code seems logical.

Alright, thanks everyone!!! Committed to 7.x-2.x