Problem
* It appears that version arguments declared within variants are not taken into account when performing version detection within libraries_detect().
* The documentation indicates that top-level properties (such as 'version arguments' would be overridden.
Details
An example of the the hook_libraries_info definition is:
$libraries['backbone'] = array(
'name' => 'Backbone',
'vendor url' => 'http://documentcloud.github.io/backbone/',
'download url' => 'http://backbonejs.org/backbone.js',
'files' => array(
'js' => array(
'backbone.js',
),
),
'version arguments' => array(
'file' => 'backbone.js',
'pattern' => '/Backbone.js\s*([\d\.]+)/',
),
'versions' => array(
'1.0.0' => array(
'variants' => array(
'source' => array(
'files' => array(
'js' => array(
'backbone.js',
),
),
),
'minified' => array(
'files' => array(
'js' => array(
'backbone-min.js',
),
),
'version arguments' => array(
'file' => 'backbone-min.js',
'pattern' => '/VERSION="([\d\.]+)"/',
),
),
),
),
),
'dependencies' => array('underscore (>=1.4.4)'),
);
Comments
Comment #1
eshta commentedComment #2
eshta commentedComment #3
tstoecklerYeah, so the current API simply does not allow for this currently.
What you could do is provide a custom version callback that checks which file is available and then performs the check on the correct file.
We do eventually want to support this, but it will almost certainly be an API change, so moving to 7.x-3.x ad re-titling a bit.
Comment #4
coredumperror commented+1 for this issue. I just flailed around for a few hours trying to make this work for a library who's main filename changed from one version to the next, so I needed a versioned "version arguments" array. Debugging into the code showed that the version detection is only done before the config overrides are applied from the "versions" setting. This obviously makes sense: how else could it know which of the "versions" to use? But it would be really nice if the version detection could at least take a glance at the "versions" setting if it fails to detect the library using the default settings, and loop through the versions until it finds one that it can detect.
I could write up a patch that would make it do this, if it would make it into a 2.x release for Drupal 7 (I have no interest in Drupal 8, as my code shop won't be migrating our D7 sites to D8).
Comment #5
tstoecklerI thought about this and I think it makes sense. I think we can simply add another version callback that implements the behavior described in #4 and then we could even do this in 7.x-2.x so moving back for that.
Comment #6
tstoecklerSomething like this I think should be enough.
Needs docs and tests, but maybe someone can give this a test run.
Comment #7
coredumperror commentedI'm afraid I'm not sure what to do with the two new functions you've added in this patch. Where would one call
libraries_get_version_from_versions()in order to make use of it for the purpose discussed in this issue?Also, as a note for anyone who might also want to test this patch, be sure to update to the latest dev release before you apply it. That'll make the patch actually work, and it will also add the wonderful new "admin/reports/libraries" page, which is very helpful.
Comment #8
tstoecklerYeah, sorry for not providing any docs. Those functions can be used as version callbacks.
Something like this:
Something like that.
}
Comment #9
coredumperror commentedTo make this work, I had to throw in some additional data, which should really be done by Libraries itself.
The issue here is that "library path" key I needed to include in both version arrays. That normally gets constructed by the Libraries code automatically, and its assumed to be there by
libraries_get_version().So, to handle that problem and also allow for more DRYness, I'd like to propose this alternate patch from the one in #6. It adds
$version_properties['library path'] = $library['library path'];to the loops in the new version callback functions, and also defaults$version_properties['version callback']to 'libraries_get_version' if it isn't already set. This cuts 5 lines from myhook_libraries_info()implementation.Comment #11
coredumperror commentedWhoops, I'm an idiot. Should have known not to just edit your patch file. This one's created by git directly.
Comment #12
coredumperror commentedShoot, found a problem. Using the new version detection code, if you have an older version of the library installed, the code will collect that older library's version number. But then it'll overwrite that with the detected version number of any newer library version, including ones that aren't installed. Thus,
libraries_get_version_from_versions()ends up failing to detect the version number unless you have the newest version of the library installed.Here's another updated patch that solves this problem.
libraries_get_version_from_versions()and the variants one will no longer overwrite a detected version with a higher, undetected version.With this patch in place, I can use these two functions:
...to make Date iCal capable of detecting the version number of iCalcreator regardless of either iCalcreator's version or Library API's version.
Comment #13
hansfn commentedJust adding a related/similar issue about versions. That issue contains a version callback function too.
Comment #14
rohit.rawat619 commentedComment #15
rohit.rawat619 commented