Posted by joachim on August 7, 2009 at 4:04pm
| Project: | Libraries API |
| Version: | 7.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
It would be awesome if modules that use this API could also have a way of telling Drush the URL of the library to download and where to put it.
So you could do:
$ drush dllib jquery_ui
// download the library files needed by jquery_ui module
Comments
#1
FYI, this patch in the
drush_makequeue allows for libraries to be defined in your makefile and retrieved via the download method of choice.http://drupal.org/node/583706#comment-2067524
#2
From what I understand drush_make works to build new sites and not add stuff to existing sites. We need something to add stuff to existing sites as well.
#3
I don't think you need drush make.
Use the same sort of stuff as the drush pm module -- that downloads files from a server, unpacks them, puts them in the right folders.
#4
A couple questions....
Is the libraries api intended to hold the definition of where certain libraries files are to be download from? This would include an index of a lot of libraries? In D7 it will be easier to deal with because of hook_library().
Or, is the intent for the definition to live in the jquery_ui module and the libraries api just has the ability to download it for the jquery_ui module?
Or, both to some degree?
#5
I would suggest it is up to the modules to say where they need things downloading from. More sense from an architecture point of view, and also means the module maintainer can specify the version that matches their code.
I can't find docs on how the API works so I can't make more concrete suggestions...
#6
@joachim - http://api.drupal.org/api/function/hook_library/7
We would need to add a key for the files location which is fine. At the same time, you shouldn't have 2 versions of the same library on a site in most cases. It will just create namespace conflicts and other issues.
#7
@mfer: How. many. times. do. I. need. to. repeat. that. hook_library. is. NOT. the. same. but. a. totally. different. thing?
I also recommended to read up most of the details in the Libraries API issue linked on the project page, which should answer 90% of all questions. Or just look into the queue: #618370: Reasoning Behind Libraries API?
This is one of the remaining major issues that needs to be discussed (elsewhere). We face a N:N relationship here, where neither of both parties can reliably provide standardized library information. So the logical conclusion would seem to additionally allow a N:1:N relationship, which would mean that Libraries API would define a couple of libraries, but installation profiles as well as other modules would still be able to provide their own definitions. However, as soon as there is a 1:N relationship in terms of the library definition, we'll most likely have a problem. This particular question requires to understand the underlying, multi-dimensional challenge, which Libraries API is trying to solve.
#8
http://drupal.org/cvs?commit=409402
Linking your work in progress here for potential discussion.
#9
Thanks! Seems I forgot to mention it here -- if it's not obvious already - that code was blatantly copied from Colorbox module; only minor adjustments, not working at all yet. ;)
#10
I worked a bit on the listing part.
Attached is a patch and a screenshot of how it looks with the libraries_test.module enabled.
#11
#12
The last submitted patch, libraries_drush_list.patch, failed testing.
#13
Now showing variants as well.
Patch also removes a variant from libraries_test.module, because that was clobbering the otherwise very nice table :)
Screenshot again attached (sorry, not on my TFT currently, that's why it's pretty small).
#14
Let's see if the bot likes this one.
#15
The last submitted patch, 542940-libraries_drush-13.patch, failed testing.
#16
mmm, nice! :)
1) Name and title next to each other will be too wide for many shells. Can we either drop title or think of a way to output name below title?
2) The Version column should come after the Status column.
3) I don't understand the difference between "-" and "undetected" in version.
4) Can we shortcut "installed" with "OK" in the status column? We should make sure that there is a immediately visible difference between good and bad rows.
#17
Here we go.
Dropped title.
Made 'version' always show '-' in case of an error and 'status' show the error.
switched status and version.
'Installed' => 'OK'.
#18
Next round.
Status is now consistent:
'Not found'/'Not detected'/'Not supported'
#19
+++ libraries.drush.inc 10 Oct 2010 02:30:18 -0000@@ -41,6 +41,65 @@ See libraries-list for a list of registe
+ switch ($library['error']) {
+ case t('%library could not be found.', array('%library' => $library['title'])):
UGH. :-|
Looks like we need to change 'error' into a constant (i.e., LIBRARIES_NOT_FOUND, etc) and move the error message into a separate 'error message' key.
+++ libraries.drush.inc 10 Oct 2010 02:30:18 -0000@@ -41,6 +41,65 @@ See libraries-list for a list of registe
+ break;
+ case t('The version of %library could not be detected.', array('%library' => $library['title'])):
There should be a blank line between break + case lines, unless a case purposively falls through into the next case.
Powered by Dreditor.
#20
The last submitted patch, 542940-libraries_drush-18.patch, failed testing.
#21
Changed libraries_detect to not only the store the error messages (now $library['error message']) but also a short error "code" ($library['error']), which we directly display in the table.
Screenshot attached. (No visual changes, between #18 and this one).
#22
Now with less leftovers.
#23
Yay, good job! :)
#24
The last submitted patch, 542940-libraries_drush-21.patch, failed testing.
#25
Committed #22 to CVS HEAD. Thanks for the great review session.
http://drupal.org/cvs?commit=434128
Marking 'needs work' for the harder part of this issue: libraries-download
#26
The 'error' -> 'error message' change broke tests. Committed the trivial attached patch to fix tests. The test bot doesn't like me nowadays, but I verified locally that there are exactly 4 fails before applying this patch, and applying fixes them.
http://drupal.org/cvs?commit=434230
#27
To actually allow downloading of libraries, we need to think about how Libraries API should access the files. Currently the $library['download url'] is for humans to visit and click on the right link, depending on the most recent version. So we need some automatism to do that.
Idea:
- Specify 'download url' like the following:
http://jquery.com/download/jquery.[VERSION].jsor similar.
- Either declare the latest version statically (bad), or introduce a 'latest version callback' that determines the latest version (hard)
- When downloading, do:
<?php$download_url = $library['download_url'];
str_replace('[VERSION]', $version, $download_url);
?>
#28
The latest known and supported version is the "largest" array key value in $library['versions'], no? version_compare() is able to figure that out.
EDIT: Also note that not all libraries use a simple pattern like "foo.[version].bar"... the URLs can be entirely different.
Additionally, we would have to set a (md5) hash to make sure that the downloaded library actually is the expected file. (Hackers everywhere, sadly.)
#29
1. Not all libraries have versions, but more importantly, because version_compare is so smart, the latest version might be 3.4.2, while versions only lists 3.
2. Right, so instead of having a 'latest version callback' we should have a 'url to download the latest version callback'?
3. Right. Doesn't make this any easier.
#30
Is this an active plan? Drush support for the Libraries API in version 6 would be great.
#31
+1 and subscribing and all that...
#32
Depends on Drush Make.
<?php$libraries['example'] = array(
// Only used in administrative UI of Libraries API.
'name' => 'Example library',
'vendor url' => 'http://example.com',
// Optional: A website link to the Library's download page.
'download url' => 'http://example.com/download',
// Optional: Information for automated downloading of the Library using
// Drush Make. Visit Drush Make's documentation to read more regarding
// the "download" property.
'download' => array(
// The type of download (file, git, svn, or cvs).
'type' => 'git',
// The URL or repository to download the Library.
'url' => 'git://github.com/jane_doe/mytheme.git',
// The revision information to check out.
'revision' => '7.x-1.x',
),
?>
#33
That indeed looks very good. I'm not too intimate with Drush Make, but I know that it supports downloading of archives as well, so it would be cool to give an example that does that.
+++ b/libraries.api.php@@ -177,7 +177,19 @@ function hook_libraries_info() {
+ // The revision information to check out.
+ 'revision' => '7.x-1.x',
As briefly discussed above, this seems to the crux of the issue: How do we find out which version to donwload? What this would mean is that everytime a new version of a library comes out, we would have to change the library information. Maybe as a first step, which would probably already help people a lot, we could pass the current version as an argument to the drush command. Then modules using Libraries API could document on their project page for instance: "Run
drush libraries-download tinymce 4.3.1for automatic installation of the library." Of course we would ideally like to find a way to automate all of that, but that would already be a huge step forward I think. Thoughts?#34
Here we go...
One question:
drush libraries-makesince it uses Drush Make?drush libraries-make tinymce#35
- The empty $name to download all needs to be an explicit --all option. Wysiwyg and other modules are going to register many libraries ;) That said, not sure whether this makes sense at all; dropping it entirely might make more sense.
- As soon as this works with (tar) archives, I'd be happy to commit alpha-quality code.
- We need a way to declare whether the downloaded library archive extracts itself into a folder already or not. The folder possibly has to be renamed. Quite potentially, it's going to be easier to define whether extraction should skip a folder level (à la patch -p1) and declare the folder name to extract into (which we already know == library name).
- Revision might make sense for version controlled, checked out repositories, but perhaps not for releases. We might need an additional release identifier option.
- Speaking of, can we support something like drush pm à la tinymce-3.3, i.e., [libraryname]-[version]?
- The download URL often contains the version number. However, we could as well limit this feature to libraries providing a "latest stable" download URL only.
#36
I like your --all idea, but I think we should just drop the support for that and push it to a separate issue.
#622224: add support for .tar files ;-)
Drush Make handles that and renames the extracted folder to the library name. Probably seems like something we should push over to the Drush Make issue queue?
It definitely depends on which type of download you're getting. You don't need the revision option when you're downloading a "file" type. Maybe this just means that the module developer needs to be more cautious with the version callback and variants? It should be up to the maintainer as to how that's handled.
It depends on how the library is downloaded. If TinyMCE is handled by just a file download, then
drush libraries-download tinymce --tag="3.3"does not make sense. If it's handled via GIT or SVN though, then that's a valid call. Again, completely depends on the type of download.Maybe extend support for the "versions" array to allow the "download" options?.... Then we could have:
drush libraries-download example --version="3.0"and it uses the download options from that version if it's defined?#37
I don't really get why .tar archive support is a Drush Make issue and not a Drush Core issue, but who am I...
Version callbacks and stuff are only invoked after the fact. This code here runs before the fact.
Libraries API's design idea is that integration code within Drupal mainly or ideally cares for major versions of a library only. As such, a download facility would ideally download the latest version of a library, if provided by the library vendor. After downloading, the Drupal/Libraries integration code determines the library's version and primarily differs per major version. In an ideal world, integration code is compatible with libFooBar version 3.00 to 3.99, and it only needs to change if and when a 4.x comes out.
Long story short, we don't know which exact version to download upfront, because anything < 4.x should work. If the vendor provides a static latest stable release download URL, then we're good to go.
However, we definitely don't want to hard-code any library version or revision to download. So let's simply leave that out entirely.
#38
There are talks to fix that: #1310130: Put drush make in drush core ..... Hmm, #1267228: Drush Make should use Drush core's native download abilities concurrently.
#39
Did some work on this....
drush libraries-make" as it pretty much is Drush Make support for Librariesdrush libraries-make --alloptionWould be nice to have these patches in Drush:
#40
$libraries['tinymce']['make']['url'] = 'asdffsda.tar.gz';drush libraries#41
This issue is kind of required for this to go in before Libraries API can have it: #1369922: Fix up the Return values of Make Download
Otherwise, Libraries API always returns a failure when downloading libraries, even though it succeeds.
#42
#1369922: Fix up the Return values of Make Download and #1354778: Drush prints out HTML into terminal are now fixed.
#43
First off, thanks again for starting this, I really think this will be a game-changer for our users!
I finally got around to fiddling with this for a bit, and in trying to grasp it and make it work for a few example libraries, I changed this and that, and in the end I deviated enough from your patch, to roll a new version (after cleaning up after me, of course).
The patch might look pretty finished, or "full-fledged" (docs, etc...), but it's really just my proposal of how I would do it, what I think are the most usual use-cases, etc.
So if it is the case, "Your patch sucks, #39 is better!" is very much a possible response to this. :)
That said...
...this patch makes it so that you can/have to specify the version you want to download when you are downloading it, i.e.:
drush libraries-make jquery 1.7.1I think that is more in-line with our goal to not have to adapt our release-cycle(s) to that of any libraries.
It enables that by allowing the use of a "!version" placeholder in the download options, for example:
$library['download']['url'] = 'http://code.jquery.com/jquery-!version.js';(Yes, there's also a !variant placeholder.)
When downloading from a Git repository, you can specify
$library['download']['tag'] = '!version';And then the "1.7.1" tag will be checked out upon download.
NOTE: I used this as my primary resource for Drush Make (and also documented it in-code). It only lists the 'tag' parameter (and 'branch', etc.) for git, not for bzr or svn. So for now I only documented the whole thing for 'git'.
With this patch I was successfully able to download:
For convenience, I've attached the .libraries.info files I used.
NOTE2: jQuery does
http://code.jquery.com/jquery-1.7.1.min.js vs.
http://code.jquery.com/jquery-1.7.1.js
so you can't really specify the !variant placeholder as sometimes it's there and sometimes it's not. For that reason I've allowed the same kind of version- and variant-overloading we do elsewhere for this.
Because I was lazy, I then also utilized that for the different download methods for jQuery UI.
See the info files, for what I mean in detail.
The commands are:
# static filedrush libraries-make jquery 1.7.1
# extra credit
drush libraries-make jquery 1.7.1 --variant="min"
# git
drush libraries-make jquery-ui 1.8.16
# zip
drush libraries-make jquery-ui 1.8.16 --variant="zip"
# tar
drush libraries-make jquery-ui 1.8.16 --variant="tar"
Have fun! :)
NOTE3: I changed how the drush command detects success/failure, because you're version wasn't working with my version of Drush (4.5), but I think you a lot more into Drush than I am, so I would defer that to you. The way it is in this patch is only because that worked for me.
#44
+++ b/libraries.api.phpundefined@@ -329,6 +385,11 @@ function hook_libraries_info() {
+ 'download' => array(
+ 'type' => 'git',
+ 'url' => 'http://github.com/tinymce/tinymce.git',
+ 'tag' => '!version',
Could modules provide a default version number?
+++ b/libraries.drush.incundefined
@@ -88,64 +102,125 @@ function libraries_drush_list() {
+ $destination = drush_get_option('destination', 'sites/all/libraries');
+ $destination = DRUPAL_ROOT . '/' . $destination . '/' . $name;
+
+ // Create the directory.
+ drush_mkdir($destination);
- // Set working directory back to the previous working directory.
Thanks so much for testing this out and pushing it forward! Although I won't get a chance to try it until tomorrow, I thought I'd point at #1143936: Downloading a single file breaks if the folder doesn't exist regarding jQuery and failing on single file downloads.
+++ b/libraries.drush.incundefined@@ -88,64 +102,125 @@ function libraries_drush_list() {
+ * @see libraries_invoke()
+ */
+function libraries_prepare_make_download(&$library, $version = NULL, $variant = NULL) {
+ if (!empty($library['download'])) {
+ // We support the !version and !variant placeholders for all of these values.
+ foreach (array('url', 'filename', 'tag') as $key) {
+ if (isset($library['download'][$key])) {
+ $value = $library['download'][$key];
+ // If one of the placeholders was used in the value, but was not specified,
+ // set an error.
+ if (strpos($value, '!version') && empty($library['version_to_download'])) {
Unfortunately this hook isn't called if installing a distribution from the .make file via libraries[]. But I suppose a .make file would actually provide the straight up version number and not the "!version" ;-).
+++ b/libraries.drush.incundefined@@ -88,64 +102,125 @@ function libraries_drush_list() {
+
+ // Create the directory.
Thanks so much for testing this out and pushing it forward! Although I won't get a chance to try it until tomorrow, I thought I'd point at #1143936: Downloading a single file breaks if the folder doesn't exist regarding jQuery and failing on single file downloads.
#45
1.
Hmm... what is the use-case for that?
What I'm trying to avoid is anything that has to be updated in our code as soon as library X decides to roll a new release.
Thinking about it, though, that would allow
drush libraries-make jqueryinstead of
drush libraries-make jquery 1.7.1though, which is very appealing, obviously.
Hmm.... :) Not sure.
2.
I wasn't aware of #1143936: Downloading a single file breaks if the folder doesn't exist, thanks.
That's exactly what I added that mkdir() for, and it felt pretty hacky.
So I guess we can put this postponed on that. It's pretty minor, though so I guess we can still flesh this out in the meantime.
3.
Hmm... make files will simply call
drush makeand notdrush libraries-makeso you have to specify the whole information yourself anyway.In the long run, of course, it would be nice, if we could have some sort of automation, instead of having to duplicate that information, but I can't even start to wrap my hand around that right now :)
#46
There should be no version numbers anywhere, neither in the library base definition nor on the drush command line.
Libraries API is always passive. Users smack an arbitrary library on it and it is supposed to figure out which library that is and which version. API consumers then react to that knowledge.
The only place where versions of a library are contained is in the extended library definition. However, those versions act as "minimum version and above", or in other words, the "last known version[s] that we know how to deal with."
This means that the library definition and most often also integration code can stay the same until there is an API change in the upstream library. Only if that happens, the library definition needs to be updated.
Users should always download the latest version of a library.
For this reason, there should never be any hard-coded version numbers anywhere in Libraries API and library definitions.
--
This is key to understand. Would be beneficial to document this in the handbook, or mayhaps even in the code docs.
Thinking through this, I really think that we should only support downloading of libraries, which provide a stable download URL. Or in precise other words:
Unsupported: http://example.com/lib/release/example-1.23.tgz
Supported: http://example.com/lib/download/example-latest-stable.tgz
If a library does not support this, then downloading is not supported. People need to ask the vendor to provide it. Possible in multiple ways, most simple being a symlink to an existing, version-specific file like the former.
#47
I totally agree, which is why I wrote the patch in that way :)
...they just have to quickly look up, which one that is. Nothing is hardcoded.
In short: Your reply seemed to be critical of my approach, I just didn't really get in which way :)
It'd be awesome if you could clarify how the user-interaction would work in your perfect world.
I thought so too, at first, until I was unable to find such a URL for jQuery even.
So maybe, that's unrealistic. Not sure. (I also didn't look for too long, so maybe it's me...)
#48
I definitely like the direction of this. It would just be nice to say "install this library", and then it just gets it, without having to worry about the version number.
As much as I would love for this to happen, most projects don't actually support this. Symfony has a pretty complex build system, and it might actually be a good test-bed to try this stuff out. I stuck the download options into the Symfony module, as well as the option to build from the sources via
drush symfony-download. But, it doesn't support downloading the latest stable build.Maybe this is where Packagist and Composer come in to handle that stuff? Either way, I'd say the latest patch is the way to go.