You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by riknoll <gi...@git.apache.org> on 2016/01/23 01:55:05 UTC
[GitHub] cordova-lib pull request: New plugin version selection implementat...
GitHub user riknoll opened a pull request:
https://github.com/apache/cordova-lib/pull/363
New plugin version selection implementation
@stevengill @dblotsky please review. This is an implementation for the plugin version selection scheme that Dmitry and I proposed in [this discuss PR](https://github.com/cordova/cordova-discuss/pull/30). I'd appreciate some feedback! I have some docs written up for this too, I'm just holding off on that PR just yet.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-lib plugin-fetching-proposal
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/363.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #363
----
commit eaf5e77653ff438b625744a2fb078bd56d030500
Author: riknoll <ri...@gmail.com>
Date: 2016-01-15T21:35:26Z
Adding new version choosing logic for plugin fetching
commit b71e0df52db052bb3b05d9386561f32197456879
Author: riknoll <ri...@gmail.com>
Date: 2016-01-22T23:11:18Z
Fixed platform constraints in cordovaDependencies so that they start with cordova-
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54727968
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
--- End diff --
It would be useful to output a message (with log level `verbose`) if we are ignoring an entry because it either isn't a valid version or range, or is a valid range (according to semver), but something other than an "upper bound". This would be helpful particularly for plugin authors debugging their `engines` entries :smile:.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53299072
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -154,6 +135,38 @@ module.exports = function plugin(command, targets, opts) {
is_top_level: true
};
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
--- End diff --
I know this is just moved code, but while you're here could you make that sentence start with an uppercase "N" (as in "No version specified...")? :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54722038
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
--- End diff --
Minor nit: superfluous `else` - remove it, and decrease the indentation level of the rest of the function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53779104
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -185,6 +187,22 @@ function listPlatforms(project_dir) {
});
}
+function getInstalledPlatformsWithVersions(project_dir) {
+ var result = {};
+ var platforms_on_fs = listPlatforms(project_dir);
+
+ return Q.all(platforms_on_fs.map(function(p) {
+ return superspawn.maybeSpawn(path.join(project_dir, 'platforms', p, 'cordova', 'version'), [], { chmod: true })
+ .then(function(v) {
+ result[p] = v || null;
+ }, function(v) {
+ result[p] = v || 'broken';
--- End diff --
Is this what you intended? The parameter `v` here will actually be the error. In the previous version, `v` was simply ignored (so "version" was always set to "broken").
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53358585
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
+}
+
+function satisfiesProjectRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ for (var req in reqs) {
+ var okay = true;
+
+ if(pluginMap[req]) {
+ okay = semver.satisfies(pluginMap[req], reqs[req]);
+ } else if(req === 'cordova') {
+ okay = semver.satisfies(cordovaVersion, reqs[req]);
+ } else if(req.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
--- End diff --
I originally had it that way, but decided on requiring the `cordova-` prefix to avoid ambiguity with the actual platforms. That is, Android OS version versus cordova-android version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53838895
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -185,6 +187,22 @@ function listPlatforms(project_dir) {
});
}
+function getInstalledPlatformsWithVersions(project_dir) {
+ var result = {};
+ var platforms_on_fs = listPlatforms(project_dir);
+
+ return Q.all(platforms_on_fs.map(function(p) {
+ return superspawn.maybeSpawn(path.join(project_dir, 'platforms', p, 'cordova', 'version'), [], { chmod: true })
+ .then(function(v) {
+ result[p] = v || null;
+ }, function(v) {
+ result[p] = v || 'broken';
--- End diff --
Good catch! Fixed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/363
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53305319
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
--- End diff --
Weren't we going to display a warning in this scenario?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-191085463
LGTM! Great work @riknoll! Very clean code and is easy to follow. Looking forward to switching over to this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-185850104
@TimBarham thanks for the review; I'll update the PR in a bit!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r50916728
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -440,6 +451,13 @@ function list(projectRoot, hooksRunner, opts) {
});
}
+function getInstalledPlugins(projectRoot) {
+ var pluginsDir = path.join(projectRoot, 'plugins');
+ // TODO: This should list based off of platform.json, not directories within plugins/
--- End diff --
Not my TODO. Moved from old code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-190330639
@vladimir-kotikov to also take a look.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r50916227
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -440,6 +451,13 @@ function list(projectRoot, hooksRunner, opts) {
});
}
+function getInstalledPlugins(projectRoot) {
+ var pluginsDir = path.join(projectRoot, 'plugins');
+ // TODO: This should list based off of platform.json, not directories within plugins/
--- End diff --
Is there a JIRA for this TODO?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-191504890
Okay, I added a lot of verbose logging and responded to some of the refactor stuff @TimBarham mentioned (except where noted). I plan to rebase this down to one commit and merge at end of day today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53305903
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
+}
+
+function satisfiesProjectRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ for (var req in reqs) {
+ var okay = true;
+
+ if(pluginMap[req]) {
+ okay = semver.satisfies(pluginMap[req], reqs[req]);
+ } else if(req === 'cordova') {
+ okay = semver.satisfies(cordovaVersion, reqs[req]);
+ } else if(req.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
--- End diff --
Should we also allow platform names without `cordova-`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54722281
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
--- End diff --
This comment should be moved down (just above the `events.emit` line, perhaps?)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54727324
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
--- End diff --
It would be useful to output a message if we encounter an invalid version (with log level `verbose`) - this would be helpful particularly for plugin authors debugging their engines entries :smile:.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r50916413
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +525,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!checkProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(checkProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
+}
+
+function checkProjectRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
--- End diff --
Nitpick: maybe rename to something that sounds more like a question.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54726540
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
+
+ // No constraints were satisfied
+ return null;
+}
+
+
+function getFailedRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ var failed = [];
+
+ for (var req in reqs) {
+ if(reqs.hasOwnProperty(req) && typeof req === 'string' && semver.validRange(reqs[req])) {
+ var badInstalledVersion = null;
+ var trimmedReq = req.trim();
+
+ if(pluginMap[trimmedReq] && !semver.satisfies(pluginMap[trimmedReq], reqs[req])) {
+ badInstalledVersion = pluginMap[req];
+ } else if(trimmedReq === 'cordova' && !semver.satisfies(cordovaVersion, reqs[req])) {
+ badInstalledVersion = cordovaVersion;
+ } else if(trimmedReq.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = trimmedReq.substring(8);
+ if(platformMap[platform] && !semver.satisfies(platformMap[platform], reqs[req])) {
+ badInstalledVersion = platformMap[platform];
+ }
+ }
+
+ if(badInstalledVersion) {
+ failed.push({
+ dependency: trimmedReq,
+ installed: badInstalledVersion.trim(),
+ required: reqs[req].trim()
+ });
+ }
+ }
+ }
+
+ return failed;
+}
+
+function listUnmetRequirements(failedRequirments) {
+ events.emit('warn', 'Unmet project requirements for latest version of plugin:');
--- End diff --
Will there always be enough context in our console output to be clear what plugin we're referring to here (for example where we are installing multiple plugins at once - restoring saved plugins or installing dependencies)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53297519
--- Diff: cordova-lib/src/cordova/platform.js ---
@@ -492,17 +492,13 @@ function list(hooksRunner, projectRoot, opts) {
var platforms_on_fs = cordova_util.listPlatforms(projectRoot);
return hooksRunner.fire('before_platform_ls', opts)
--- End diff --
You don't need `platforms_on_fs` anymore, since you get equivalent information from your call to `getInstalledPlatformsWithVersions()` (so you can use `platformMap` below instead of `platforms_on_fs`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54660579
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,50 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = id + '@' + ver;
+ } else {
+ // If no version is given at all and we are fetching from npm, we
+ // can attempt to use the Cordova dependencies the plugin lists in
+ // their package.json
+ var shouldUseNpmInfo = !fetchOptions.searchpath && !fetchOptions.noregistry;
+
+ if(shouldUseNpmInfo) {
+ events.emit('verbose', 'No version given in config.xml, attempting to use plugin engine info');
+ }
+
+ return (shouldUseNpmInfo ? registry.info([id]) : Q({}))
+ .then(function(pluginInfo) {
+ return getFetchVersion(projectRoot, pluginInfo, pkgJson.version);
+ }, function(error) {
--- End diff --
Sure, I can change it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-190708358
LGTM apart from a couple of nitpicks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-187701057
Other than one small remaining question, looks great! Thanks @riknoll!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-191936337
@TimBarham updated
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-174261045
Hey @riknoll,
Thanks for doing this! I'll review it after the cordova 6 release.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-186387095
Rebased to master (that was fun)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53298645
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -185,6 +187,31 @@ function listPlatforms(project_dir) {
});
}
+function getInstalledPlatformsWithVersions(project_dir) {
+ var platforms_on_fs = listPlatforms(project_dir);
+
+ return Q.all(platforms_on_fs.map(function(p) {
+ return superspawn.maybeSpawn(path.join(project_dir, 'platforms', p, 'cordova', 'version'), [], { chmod: true })
+ .then(function(v) {
+ return {
+ platform: p,
+ version: v ? v : null
+ };
+ }, function(v) {
+ return {
+ platform: p,
+ version: 'broken'
+ };
+ });
+ })).then(function(platformVersions) {
+ var result = {};
+ for(var i = 0; i < platformVersions.length; i++) {
+ result[platformVersions[i].platform] = platformVersions[i].version;
+ }
+ return result;
+ });
+}
--- End diff --
The above method would be simplified if you just accumulated the `result` object as you go... As in... initialize `result` before the `Q.all()` call, then instead of, for example, `return { platform: p, version: v ? v : null };` you would just do `result[p] = v || null;`. In the final then, you would ignore the parameter and just return `result`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53301932
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
--- End diff --
I think here you just need `return Q(null);` (returns a promise fulfilled with `null`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53536875
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
+}
+
+function satisfiesProjectRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ for (var req in reqs) {
+ var okay = true;
+
+ if(pluginMap[req]) {
+ okay = semver.satisfies(pluginMap[req], reqs[req]);
+ } else if(req === 'cordova') {
+ okay = semver.satisfies(cordovaVersion, reqs[req]);
+ } else if(req.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
--- End diff --
Sure, that's reasonable. And now that I think about it, consistent with the `engines` tag in `plugin.xml`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-190331301
I have significant changes to push in response to some feedback, so don't review yet. I'll comment when they're in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54726389
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
+
+ // No constraints were satisfied
+ return null;
+}
+
+
+function getFailedRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ var failed = [];
+
+ for (var req in reqs) {
+ if(reqs.hasOwnProperty(req) && typeof req === 'string' && semver.validRange(reqs[req])) {
+ var badInstalledVersion = null;
+ var trimmedReq = req.trim();
+
+ if(pluginMap[trimmedReq] && !semver.satisfies(pluginMap[trimmedReq], reqs[req])) {
+ badInstalledVersion = pluginMap[req];
+ } else if(trimmedReq === 'cordova' && !semver.satisfies(cordovaVersion, reqs[req])) {
+ badInstalledVersion = cordovaVersion;
+ } else if(trimmedReq.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = trimmedReq.substring(8);
+ if(platformMap[platform] && !semver.satisfies(platformMap[platform], reqs[req])) {
+ badInstalledVersion = platformMap[platform];
+ }
+ }
+
+ if(badInstalledVersion) {
+ failed.push({
+ dependency: trimmedReq,
+ installed: badInstalledVersion.trim(),
+ required: reqs[req].trim()
+ });
+ }
+ }
+ }
+
+ return failed;
+}
+
+function listUnmetRequirements(failedRequirments) {
--- End diff --
Typo: 'failedRequir**e**ments'
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54814983
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ return Q(ver);
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ return Q(id + '@' + ver);
+ } else {
+ // If no version is given at all and we are fetching from npm, we
+ // can attempt to use the Cordova dependencies the plugin lists in
+ // their package.json
+ var shouldUseNpmInfo = !fetchOptions.searchpath && !fetchOptions.noregistry;
+
+ if(shouldUseNpmInfo) {
+ events.emit('verbose', 'No version given in config.xml, attempting to use plugin engine info');
+ }
+
+ return (shouldUseNpmInfo ? registry.info([id]) : Q({}))
--- End diff --
Right! I wrote this before I was aware that jasmine could mock function calls. As you said, I want to get this in today so I'm going to leave it out for now since that would be a big change to the tests..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r50916134
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -122,25 +123,6 @@ module.exports = function plugin(command, targets, opts) {
var id = parts[0];
var version = parts[1];
- // If no version is specified, retrieve the version (or source) from config.xml
- if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
- events.emit('verbose', 'no version specified, retrieving version from config.xml');
- var ver = getVersionFromConfigFile(id, cfg);
-
- if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
- target = ver;
- } else {
- //if version exists from config.xml, use that
- if(ver) {
- target = ver ? (id + '@' + ver) : target;
- } else {
- //fetch pinned version from cordova-lib
- var pinnedVer = pkgJson.cordovaPlugins[id];
- target = pinnedVer ? (id + '@' + pinnedVer) : target;
- }
- }
- }
-
// Fetch the plugin first.
events.emit('verbose', 'Calling plugman.fetch on plugin "' + target + '"');
--- End diff --
Might `target` be undefined here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-190569199
@TimBarham @vladimir-kotikov Can you please take a look in the next couple of days? It will be good to get this committed and do a release.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53300134
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -154,6 +135,38 @@ module.exports = function plugin(command, targets, opts) {
is_top_level: true
};
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = ver ? (id + '@' + ver) : target;
+ } else {
+ // If no version is given at all, We need to decide what version to
+ // fetch based on the current project
+ return registry.info([id])
+ .then(function(pluginInfo) {
+ return getFetchVersion(projectRoot, pluginInfo, pkgJson.version);
+ })
+ .then(function(fetchVersion) {
+ // Fallback to pinned version if available
+ fetchVersion = fetchVersion ? fetchVersion : pkgJson.cordovaPlugins[id];
+ target = fetchVersion ? (id + '@' + fetchVersion) : target;
+
+ events.emit('verbose', 'Calling plugman.fetch on plugin "' + target + '"');
+ return plugman.raw.fetch(target, pluginPath, fetchOptions);
+ })
+ .then(function (directory) {
+ return pluginInfoProvider.get(directory);
+ });
--- End diff --
We don't want this code duplicating the code below. It's only a couple of lines now, but it leaves us open to problems later on if changes are made to what we do here. Instead, I'd suggest moving this whole `if` block (that is, `if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) { ... }`) to a separate method that returns a promise for a version. Then you can have a single `plugman.raw.fetch()` and `pluginInfoProvider.get()` regardless of whether the operation to determine the version is sync or async.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54663442
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,50 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
--- End diff --
Right, I just refactored this code, but I should have caught that. Changing now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54809830
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
+
+ // No constraints were satisfied
+ return null;
+}
+
+
+function getFailedRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ var failed = [];
+
+ for (var req in reqs) {
+ if(reqs.hasOwnProperty(req) && typeof req === 'string' && semver.validRange(reqs[req])) {
+ var badInstalledVersion = null;
+ var trimmedReq = req.trim();
+
+ if(pluginMap[trimmedReq] && !semver.satisfies(pluginMap[trimmedReq], reqs[req])) {
+ badInstalledVersion = pluginMap[req];
+ } else if(trimmedReq === 'cordova' && !semver.satisfies(cordovaVersion, reqs[req])) {
+ badInstalledVersion = cordovaVersion;
+ } else if(trimmedReq.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = trimmedReq.substring(8);
+ if(platformMap[platform] && !semver.satisfies(platformMap[platform], reqs[req])) {
+ badInstalledVersion = platformMap[platform];
+ }
+ }
+
+ if(badInstalledVersion) {
+ failed.push({
+ dependency: trimmedReq,
+ installed: badInstalledVersion.trim(),
+ required: reqs[req].trim()
+ });
+ }
+ }
+ }
+
+ return failed;
+}
+
+function listUnmetRequirements(failedRequirments) {
+ events.emit('warn', 'Unmet project requirements for latest version of plugin:');
--- End diff --
"I'm going to leave that as a separate issue" - agreed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-188425964
After conversation with @nikhilkh, I reworked the code a bit so that the warnings it prints are more actionable. They now list what dependencies failed for the latest version of the plugin.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54722073
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ return Q(ver);
+ } else if (ver) {
--- End diff --
Same deal - superfluous `else`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-187874856
Fixed jasmine tests and created a JIRA for this ([CB-10679](https://issues.apache.org/jira/browse/CB-10679))
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53529433
--- Diff: cordova-lib/spec-cordova/plugin_fetch.spec.js ---
@@ -0,0 +1,203 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var plugin = require('../src/cordova/plugin'),
+ helpers = require('./helpers'),
+ path = require('path'),
+ shell = require('shelljs');
+
+var testPluginVersions = [
+ '0.0.0',
+ '0.0.2',
+ '0.7.0',
+ '1.0.0',
+ '1.1.0',
+ '1.1.3',
+ '1.3.0',
+ '1.7.0',
+ '1.7.1',
+ '2.0.0-rc.1',
+ '2.0.0-rc.2',
+ '2.0.0',
+ '2.3.0'
+];
+
+var cordovaVersion = '3.4.2';
+
+var tempDir = helpers.tmpDir('plugin_fetch_spec');
+var project = path.join(tempDir, 'project');
+
+function testEngineWithProject(done, testEngine, testResult) {
+ plugin.getFetchVersion(project,
+ {
+ 'engines': { 'cordovaDependencies': testEngine },
+ 'versions': testPluginVersions
+ }, cordovaVersion)
+ .then(function(toFetch) {
+ expect(toFetch).toBe(testResult);
+ })
+ .fail(function(err) {
+ console.log(err);
+ expect(true).toBe(false);
+ }).fin(done);
+}
+
+function createTestProject() {
+ // Get the base project
+ shell.cp('-R', path.join(__dirname, 'fixtures', 'base'), tempDir);
+ shell.mv(path.join(tempDir, 'base'), project);
+
+ // Copy a platform and a plugin to our sample project
+ shell.cp('-R',
+ path.join(__dirname, 'fixtures', 'platforms', helpers.testPlatform),
+ path.join(project, 'platforms'));
+ shell.cp('-R',
+ path.join(__dirname, 'fixtures', 'plugins', 'android'),
+ path.join(project, 'plugins'));
+}
+
+function removeTestProject() {
+ shell.rm('-rf', tempDir);
+}
+
+describe('plugin fetching version selection', function(done) {
+ createTestProject();
+
+ it('should properly handle a mix of upper bounds and single versions', function() {
+ var testEngine = {
+ '0.0.0' : { 'cordova-android': '1.0.0' },
+ '0.0.2' : { 'cordova-android': '>1.0.0' },
+ '<1.0.0': { 'cordova-android': '<2.0.0' },
+ '1.0.0' : { 'cordova-android': '>2.0.0' },
+ '1.7.0' : { 'cordova-android': '>4.0.0' },
+ '<2.3.0': { 'cordova-android': '<6.0.0' },
+ '2.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.3.0');
+ });
+
+ it('should properly apply upper bound engine constraints', function(done) {
+ var testEngine = {
+ '1.0.0' : { 'cordova-android': '>2.0.0' },
+ '1.7.0' : { 'cordova-android': '>4.0.0' },
+ '<2.3.0': {
+ 'cordova-android': '<6.0.0',
+ 'ca.filmaj.AndroidPlugin': '<1.0.0'
+ },
+ '2.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should ignore upperbounds if no version constraints are given', function(done) {
+ var testEngine = {
+ '<1.0.0': { 'cordova-android': '<2.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should apply upper bounds greater than highest version', function(done) {
+ var testEngine = {
+ '0.0.0' : {},
+ '<5.0.0': { 'cordova-android': '<2.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should treat empty constraints as satisfied', function(done) {
+ var testEngine = {
+ '1.0.0' : {},
+ '1.1.0' : { 'cordova-android': '>5.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.0.0');
+ });
+
+ it('should treat an empty engine as not satisfied', function(done) {
+ var testEngine = {};
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should treat a badly formatted semver range as not satisfied', function(done) {
+ var testEngine = {
+ '1.1.3' : { 'cordova-android': 'badSemverRange' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should ignore nonexistent versions in constraints', function(done) {
+ var testEngine = {
+ '1.0.0' : { 'cordova-android': '3.1.0' },
+ '1.1.2' : { 'cordova-android': '6.0.0' },
+ '1.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.1.3');
+ });
+
+ it('should respect plugin constraints', function(done) {
+ var testEngine = {
+ '0.0.0' : { 'ca.filmaj.AndroidPlugin': '1.2.0' },
+ '1.1.3' : { 'ca.filmaj.AndroidPlugin': '<5.0.0 || >2.3.0' },
+ '2.3.0' : { 'ca.filmaj.AndroidPlugin': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '2.0.0');
+ });
+
+ it('should respect cordova constraints', function(done) {
+ var testEngine = {
+ '0.0.0' : { 'cordova': '>1.0.0' },
+ '1.1.3' : { 'cordova': '<3.0.0 || >4.0.0' },
+ '2.3.0' : { 'cordova': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.1.0');
+ });
+
+ it('should account for complex versions', function(done) {
+ var testEngine = {
+ '0.0.0' : {},
+ '2.0.0' : { 'cordova-android': '>5.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '2.0.0-rc.2');
+ });
+
+ it('should not fail if there is no engine in the npm info', function(done) {
+ plugin.getFetchVersion(project, { versions: testPluginVersions }, cordovaVersion)
+ .then(function(toFetch) {
+ expect(toFetch).toBe(null);
+ })
+ .fail(function(err) {
+ console.log(err);
+ expect(true).toBe(false);
+ }).fin(done);
+ });
+
+ it('clean up after plugin fetch spec', function() {
+ removeTestProject();
+ });
+});
--- End diff --
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-188462559
@TimBarham could you take a quick look at the updates when you get a chance?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54772365
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
--- End diff --
Sure!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54785237
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
+
+ // No constraints were satisfied
+ return null;
+}
+
+
+function getFailedRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ var failed = [];
+
+ for (var req in reqs) {
+ if(reqs.hasOwnProperty(req) && typeof req === 'string' && semver.validRange(reqs[req])) {
+ var badInstalledVersion = null;
+ var trimmedReq = req.trim();
+
+ if(pluginMap[trimmedReq] && !semver.satisfies(pluginMap[trimmedReq], reqs[req])) {
+ badInstalledVersion = pluginMap[req];
+ } else if(trimmedReq === 'cordova' && !semver.satisfies(cordovaVersion, reqs[req])) {
+ badInstalledVersion = cordovaVersion;
+ } else if(trimmedReq.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = trimmedReq.substring(8);
+ if(platformMap[platform] && !semver.satisfies(platformMap[platform], reqs[req])) {
+ badInstalledVersion = platformMap[platform];
+ }
+ }
+
+ if(badInstalledVersion) {
+ failed.push({
+ dependency: trimmedReq,
+ installed: badInstalledVersion.trim(),
+ required: reqs[req].trim()
+ });
+ }
+ }
+ }
+
+ return failed;
+}
+
+function listUnmetRequirements(failedRequirments) {
+ events.emit('warn', 'Unmet project requirements for latest version of plugin:');
--- End diff --
After looking into this, dependent plugins are not subject to any version selection code because of how cordova-lib is written. Latest will always be fetched (pinned versions won't be taken into account either). Plugins restored with `cordova prepare` are, so we should indeed print out the id in the message. The dependent plugin thing is bad, but I'm going to leave that as a separate issue since it is not a regression. For now, plugin authors should be advised to give versions for their plugin dependencies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-186466203
@TimBarham addressed your other feedback
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54722165
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ return Q(ver);
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ return Q(id + '@' + ver);
+ } else {
--- End diff --
And again :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54558183
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,50 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
--- End diff --
I'd rather reverse `if` condition here to return `target` immediately if there is no need for any processing. This is just a matter of taste of course but IMO it would help to improve code readability a bit. The same for L302 and L304
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54558329
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,50 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = id + '@' + ver;
+ } else {
+ // If no version is given at all and we are fetching from npm, we
+ // can attempt to use the Cordova dependencies the plugin lists in
+ // their package.json
+ var shouldUseNpmInfo = !fetchOptions.searchpath && !fetchOptions.noregistry;
+
+ if(shouldUseNpmInfo) {
+ events.emit('verbose', 'No version given in config.xml, attempting to use plugin engine info');
+ }
+
+ return (shouldUseNpmInfo ? registry.info([id]) : Q({}))
+ .then(function(pluginInfo) {
+ return getFetchVersion(projectRoot, pluginInfo, pkgJson.version);
+ }, function(error) {
--- End diff --
Should we catch the here if the only purpose is to rethrow?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54193600
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +535,175 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var versions = [];
+ var latest = allVersions[allVersions.length - 1];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
--- End diff --
It might be a good idea to be more robust with whitespaces here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-190482871
Alright, this should be ready for review. The changes I just pushed included a few fixes to edge cases, better handling of malformed input/whitespace, and a lot more tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54771522
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
+
+ // No constraints were satisfied
+ return null;
+}
+
+
+function getFailedRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ var failed = [];
+
+ for (var req in reqs) {
+ if(reqs.hasOwnProperty(req) && typeof req === 'string' && semver.validRange(reqs[req])) {
+ var badInstalledVersion = null;
+ var trimmedReq = req.trim();
+
+ if(pluginMap[trimmedReq] && !semver.satisfies(pluginMap[trimmedReq], reqs[req])) {
+ badInstalledVersion = pluginMap[req];
+ } else if(trimmedReq === 'cordova' && !semver.satisfies(cordovaVersion, reqs[req])) {
+ badInstalledVersion = cordovaVersion;
+ } else if(trimmedReq.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = trimmedReq.substring(8);
+ if(platformMap[platform] && !semver.satisfies(platformMap[platform], reqs[req])) {
+ badInstalledVersion = platformMap[platform];
+ }
+ }
+
+ if(badInstalledVersion) {
+ failed.push({
+ dependency: trimmedReq,
+ installed: badInstalledVersion.trim(),
+ required: reqs[req].trim()
+ });
+ }
+ }
+ }
+
+ return failed;
+}
+
+function listUnmetRequirements(failedRequirments) {
+ events.emit('warn', 'Unmet project requirements for latest version of plugin:');
--- End diff --
I hadn't considered the case of plugins being added in bulk. I might need to do a little testing on that, so I'll investigate. In any case, it is easy enough to add the id to this message so I will do so. Nice catch!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53305804
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
+ var nextHighest = versions[index - 1];
+ return allVersions[allVersions.indexOf(nextHighest) - 1];
+ } else {
+ // Return the latest plugin version
+ return allVersions[allVersions.length - 1];
+ }
+ }
+ }
+
+ // No constraints were satisfied
+ return null;
+}
+
+function satisfiesProjectRequirements(reqs, pluginMap, platformMap, cordovaVersion) {
+ for (var req in reqs) {
+ var okay = true;
+
+ if(pluginMap[req]) {
+ okay = semver.satisfies(pluginMap[req], reqs[req]);
+ } else if(req === 'cordova') {
+ okay = semver.satisfies(cordovaVersion, reqs[req]);
+ } else if(req.indexOf('cordova-') === 0) {
+ // Might be a platform constraint
+ var platform = req.substring(8);
+ if(platformMap[platform]) {
+ okay = semver.satisfies(platformMap[platform], reqs[req]);
+ }
+ }
+
+ if(!okay) {
+ return false;
--- End diff --
If we reject a version, it might be useful to output a message describing the version we rejected an why (with a log level of `verbose` of course).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53304649
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
--- End diff --
So do we only support version ranges in the form of `<x.y.z` and specific versions? I was thinking it might be handy for a plugin dev to specify other ranges. For example, `^1.0.0` to identify requirements for all 1.x releases. That might actually be very easy to handle - `semver.maxSatisfying(versions, range)` will take an array of versions (which we have) and a range, and return the highest version from the list that matches the range. If you do this to all ranges (including `<x.y.z` type ranges), could you avoid the special case `upperBounds` handling? Any range could be converted to a single version (the maximum actual version that matches the range) - could that then be handled like any other entry?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54684112
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
+ return;
+ }
+ }
+
+ // There is no req to merge it with
+ latestFailedReqs.push(failedReq);
+ });
+ }
+
+ listUnmetRequirements(latestFailedReqs);
+ events.emit('warn', 'Current project does not satisfy the engine requirements specified by any version of this plugin. Fetching latest or pinned version of plugin anyway (may be incompatible)');
--- End diff --
Glad we fetch anyways!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-191259905
This is looking great **@rikroll**! I have a few comments, but mostly pretty minor stuff.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54728499
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
+ // Handle the lower end of versions by giving them a satisfied engine
+ if(!findVersion(versions, '0.0.0')) {
+ versions.push('0.0.0');
+ engine['0.0.0'] = {};
+ }
+
+ // Add an entry after the upper bound to handle the versions above the
+ // upper bound but below the next entry. For example: 0.0.0, <1.0.0, 2.0.0
+ // needs a 1.0.0 entry that has the same engine as 0.0.0
+ if(upperBound && !findVersion(versions, upperBound) && !semver.gt(upperBound, latest)) {
+ versions.push(upperBound);
+ var below = semver.maxSatisfying(versions, upperBoundRange);
+
+ // Get the original entry without trimmed whitespace
+ below = below ? findVersion(versions, below) : null;
+ engine[upperBound] = below ? engine[below] : {};
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ if(upperBound && semver.lt(versions[i], upperBound)) {
+ // Because we sorted in desc. order, if the upper bound we found
+ // applies to this version (and thus the ones below) we can just
+ // quit
+ break;
+ }
+
+ var range = i? ('>=' + versions[i] + ' <' + versions[i-1]) : ('>=' + versions[i]);
+ var maxMatchingVersion = semver.maxSatisfying(allVersions, range);
+
+ if (maxMatchingVersion && getFailedRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion).length === 0) {
+
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ if (maxMatchingVersion !== latest) {
+ var failedReqs = getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion);
+
+ // Warn the user that we are not fetching latest
+ listUnmetRequirements(failedReqs);
+ events.emit('warn', 'Fetching highest version of plugin that this project supports: ' + maxMatchingVersion + ' (latest is ' + latest + ')');
+ }
+ return maxMatchingVersion;
+ }
+ }
+
+ // No version of the plugin is satisfied. In this case, we fall back to
+ // fetching latest or pinned versions, but also output a warning
+ var latestFailedReqs = versions.length > 0 ? getFailedRequirements(engine[versions[0]], pluginMap, platformMap, cordovaVersion) : [];
+
+ // If the upper bound is greater than latest, we need to combine its engine
+ // requirements with latest to print out in the warning
+ if(upperBound && semver.satisfies(latest, upperBoundRange)) {
+ var upperFailedReqs = getFailedRequirements(engine[upperBoundRange], pluginMap, platformMap, cordovaVersion);
+ upperFailedReqs.forEach(function(failedReq) {
+ for(var i = 0; i < latestFailedReqs.length; i++) {
+ if(latestFailedReqs[i].dependency === failedReq.dependency) {
+ // Not going to overcomplicate things and actually merge the ranges
+ latestFailedReqs[i].required = latestFailedReqs[i].required + ' AND ' + failedReq.required;
--- End diff --
`latestFailedReqs[i].required += ' AND ' + failedReq.required;`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53302711
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
--- End diff --
Could you document in the comments what we expect `cordovaDependencies` to look like? It would make this code a lot easier to understand.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53304887
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ return Q.fcall(function() {
+ return null;
+ });
+ }
+}
+
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ var upperBounds = [];
+ var versions = [];
+
+ for(var version in engine) {
+ if(version.indexOf('<') === 0 && semver.valid(version.substring(1))) {
+ // We only care about upper bounds that our project does not support
+ if(!satisfiesProjectRequirements(engine[version], pluginMap, platformMap, cordovaVersion)) {
+ upperBounds.push(version);
+ }
+ } else if(semver.valid(version) && allVersions.indexOf(version) !== -1) {
+ versions.push(version);
+ }
+ }
+
+ // Sort in descending order; we want to start at latest and work back
+ versions.sort(semver.rcompare);
+
+ for(var i = 0; i < versions.length; i++) {
+ for(var j = 0; j < upperBounds.length; j++) {
+ if(semver.satisfies(versions[i], upperBounds[j])) {
+ // Because we sorted in desc. order, if we find an upper bound
+ // that applies to this version (and the ones below) we can just
+ // quit (we already filtered out ones that our project supports)
+ return null;
+ }
+ }
+
+ // Check the version constraint
+ if(satisfiesProjectRequirements(engine[versions[i]], pluginMap, platformMap, cordovaVersion)) {
+ // Because we sorted in descending order, we can stop searching once
+ // we hit a satisfied constraint
+ var index = versions.indexOf(versions[i]);
+ if(index > 0) {
+ // Return the highest plugin version below the next highest
+ // constrainted version
--- End diff --
Typo ("constrained").
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54933484
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
The way I envisioned it, the following cases are semantically the same:
```
cordovaDependencies: {}
cordovaDependencies: {
"0.0.0": {}
}
```
However, I can see where that might be confusing. I can also see the case where we might want to add this object to the package.json template and not have it automatically apply for plugins that have it. This is not an issue for any non-core plugins so I'm fine with changing the behavior.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-179554892
@stevengill just checking in now that Cordova 6.0.0 is released. Let me know if you have any feedback.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53305710
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -154,6 +135,38 @@ module.exports = function plugin(command, targets, opts) {
is_top_level: true
};
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = ver ? (id + '@' + ver) : target;
+ } else {
+ // If no version is given at all, We need to decide what version to
+ // fetch based on the current project
+ return registry.info([id])
+ .then(function(pluginInfo) {
+ return getFetchVersion(projectRoot, pluginInfo, pkgJson.version);
+ })
+ .then(function(fetchVersion) {
+ // Fallback to pinned version if available
+ fetchVersion = fetchVersion ? fetchVersion : pkgJson.cordovaPlugins[id];
--- End diff --
`getFetchVersion()` could return null in two scenarios:
* The plugin didn't specify any constraints.
* The plugin specifies constraints and we fail to meet them.
It seems we really should be able to differentiate between these two scenarios. For example:
* Should we only fall back on pinned version if the plugin doesn't define constraints?
* If the plugin does define constraints and we fail to meet them, should we display a warning that we're picking a plugin version that might not work?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54771328
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
Actually, it should return `latest` in that case. We return `null` from this function only if every version of the plugin is constrained in some way and we fail all of them (the test cases were updated to reflect that behavior). The old behavior was weird because it didn't handle versions that are below the lowest constraint, but I think it is more intuitive to assume that if a version is not constrained then it is satisfied.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53529097
--- Diff: cordova-lib/spec-cordova/plugin_fetch.spec.js ---
@@ -0,0 +1,203 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+*/
+
+var plugin = require('../src/cordova/plugin'),
+ helpers = require('./helpers'),
+ path = require('path'),
+ shell = require('shelljs');
+
+var testPluginVersions = [
+ '0.0.0',
+ '0.0.2',
+ '0.7.0',
+ '1.0.0',
+ '1.1.0',
+ '1.1.3',
+ '1.3.0',
+ '1.7.0',
+ '1.7.1',
+ '2.0.0-rc.1',
+ '2.0.0-rc.2',
+ '2.0.0',
+ '2.3.0'
+];
+
+var cordovaVersion = '3.4.2';
+
+var tempDir = helpers.tmpDir('plugin_fetch_spec');
+var project = path.join(tempDir, 'project');
+
+function testEngineWithProject(done, testEngine, testResult) {
+ plugin.getFetchVersion(project,
+ {
+ 'engines': { 'cordovaDependencies': testEngine },
+ 'versions': testPluginVersions
+ }, cordovaVersion)
+ .then(function(toFetch) {
+ expect(toFetch).toBe(testResult);
+ })
+ .fail(function(err) {
+ console.log(err);
+ expect(true).toBe(false);
+ }).fin(done);
+}
+
+function createTestProject() {
+ // Get the base project
+ shell.cp('-R', path.join(__dirname, 'fixtures', 'base'), tempDir);
+ shell.mv(path.join(tempDir, 'base'), project);
+
+ // Copy a platform and a plugin to our sample project
+ shell.cp('-R',
+ path.join(__dirname, 'fixtures', 'platforms', helpers.testPlatform),
+ path.join(project, 'platforms'));
+ shell.cp('-R',
+ path.join(__dirname, 'fixtures', 'plugins', 'android'),
+ path.join(project, 'plugins'));
+}
+
+function removeTestProject() {
+ shell.rm('-rf', tempDir);
+}
+
+describe('plugin fetching version selection', function(done) {
+ createTestProject();
+
+ it('should properly handle a mix of upper bounds and single versions', function() {
+ var testEngine = {
+ '0.0.0' : { 'cordova-android': '1.0.0' },
+ '0.0.2' : { 'cordova-android': '>1.0.0' },
+ '<1.0.0': { 'cordova-android': '<2.0.0' },
+ '1.0.0' : { 'cordova-android': '>2.0.0' },
+ '1.7.0' : { 'cordova-android': '>4.0.0' },
+ '<2.3.0': { 'cordova-android': '<6.0.0' },
+ '2.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.3.0');
+ });
+
+ it('should properly apply upper bound engine constraints', function(done) {
+ var testEngine = {
+ '1.0.0' : { 'cordova-android': '>2.0.0' },
+ '1.7.0' : { 'cordova-android': '>4.0.0' },
+ '<2.3.0': {
+ 'cordova-android': '<6.0.0',
+ 'ca.filmaj.AndroidPlugin': '<1.0.0'
+ },
+ '2.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should ignore upperbounds if no version constraints are given', function(done) {
+ var testEngine = {
+ '<1.0.0': { 'cordova-android': '<2.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should apply upper bounds greater than highest version', function(done) {
+ var testEngine = {
+ '0.0.0' : {},
+ '<5.0.0': { 'cordova-android': '<2.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should treat empty constraints as satisfied', function(done) {
+ var testEngine = {
+ '1.0.0' : {},
+ '1.1.0' : { 'cordova-android': '>5.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.0.0');
+ });
+
+ it('should treat an empty engine as not satisfied', function(done) {
+ var testEngine = {};
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should treat a badly formatted semver range as not satisfied', function(done) {
+ var testEngine = {
+ '1.1.3' : { 'cordova-android': 'badSemverRange' }
+ };
+
+ testEngineWithProject(done, testEngine, null);
+ });
+
+ it('should ignore nonexistent versions in constraints', function(done) {
+ var testEngine = {
+ '1.0.0' : { 'cordova-android': '3.1.0' },
+ '1.1.2' : { 'cordova-android': '6.0.0' },
+ '1.3.0' : { 'cordova-android': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.1.3');
+ });
+
+ it('should respect plugin constraints', function(done) {
+ var testEngine = {
+ '0.0.0' : { 'ca.filmaj.AndroidPlugin': '1.2.0' },
+ '1.1.3' : { 'ca.filmaj.AndroidPlugin': '<5.0.0 || >2.3.0' },
+ '2.3.0' : { 'ca.filmaj.AndroidPlugin': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '2.0.0');
+ });
+
+ it('should respect cordova constraints', function(done) {
+ var testEngine = {
+ '0.0.0' : { 'cordova': '>1.0.0' },
+ '1.1.3' : { 'cordova': '<3.0.0 || >4.0.0' },
+ '2.3.0' : { 'cordova': '6.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '1.1.0');
+ });
+
+ it('should account for complex versions', function(done) {
+ var testEngine = {
+ '0.0.0' : {},
+ '2.0.0' : { 'cordova-android': '>5.0.0' }
+ };
+
+ testEngineWithProject(done, testEngine, '2.0.0-rc.2');
+ });
+
+ it('should not fail if there is no engine in the npm info', function(done) {
+ plugin.getFetchVersion(project, { versions: testPluginVersions }, cordovaVersion)
+ .then(function(toFetch) {
+ expect(toFetch).toBe(null);
+ })
+ .fail(function(err) {
+ console.log(err);
+ expect(true).toBe(false);
+ }).fin(done);
+ });
+
+ it('clean up after plugin fetch spec', function() {
+ removeTestProject();
+ });
+});
--- End diff --
This is really pretty code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54817429
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ return Q(ver);
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ return Q(id + '@' + ver);
+ } else {
+ // If no version is given at all and we are fetching from npm, we
+ // can attempt to use the Cordova dependencies the plugin lists in
+ // their package.json
+ var shouldUseNpmInfo = !fetchOptions.searchpath && !fetchOptions.noregistry;
+
+ if(shouldUseNpmInfo) {
+ events.emit('verbose', 'No version given in config.xml, attempting to use plugin engine info');
+ }
+
+ return (shouldUseNpmInfo ? registry.info([id]) : Q({}))
--- End diff --
Yep, that's fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53300300
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -154,6 +135,38 @@ module.exports = function plugin(command, targets, opts) {
is_top_level: true
};
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = ver ? (id + '@' + ver) : target;
+ } else {
+ // If no version is given at all, We need to decide what version to
+ // fetch based on the current project
+ return registry.info([id])
+ .then(function(pluginInfo) {
+ return getFetchVersion(projectRoot, pluginInfo, pkgJson.version);
+ })
+ .then(function(fetchVersion) {
+ // Fallback to pinned version if available
+ fetchVersion = fetchVersion ? fetchVersion : pkgJson.cordovaPlugins[id];
--- End diff --
Simplify to `fetchVersion = fetchVersion || pkgJson.cordovaPlugins[id];` :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53301382
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -507,3 +527,117 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ for(var i = 0; i < pluginList.length; i++) {
+ pluginMap[pluginList[i].id] = pluginList[i].version;
+ }
--- End diff --
Minor nit, but this would be cleaner if you used `pluginList.forEach()` (or even `pluginList.reduce()`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54727076
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
At this point, if `versions` is empty and we have no `upperBound`, should we perhaps just immediately `return null` (since that means there were no valid requirements specified)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54834210
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
"except for pinned plugins, but that is a different matter" - I think that's my point. The current behavior seems inconsistent to me:
* If there's no `cordovaRequirements`, we fall back on pinned.
* If there **is** a `cordovaRequirements`, but we don't match any requirements, we fall back on pinned.
* If there **is** a `cordovaRequirements`, but it is effectively empty, we always get latest.
I don't understand why the latter case should be an exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53298695
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -185,6 +187,31 @@ function listPlatforms(project_dir) {
});
}
+function getInstalledPlatformsWithVersions(project_dir) {
+ var platforms_on_fs = listPlatforms(project_dir);
+
+ return Q.all(platforms_on_fs.map(function(p) {
+ return superspawn.maybeSpawn(path.join(project_dir, 'platforms', p, 'cordova', 'version'), [], { chmod: true })
+ .then(function(v) {
+ return {
+ platform: p,
+ version: v ? v : null
--- End diff --
Minor nit, but `v ? v : null` can be simplified to `v || null`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-183479340
Rebased to master and responded to PR feedback. @TimBarham @vladimir-kotikov can you take a look at this as well?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54723579
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets, opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id)) {
+ return Q(target);
+ } else {
+ events.emit('verbose', 'No version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ return Q(ver);
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ return Q(id + '@' + ver);
+ } else {
+ // If no version is given at all and we are fetching from npm, we
+ // can attempt to use the Cordova dependencies the plugin lists in
+ // their package.json
+ var shouldUseNpmInfo = !fetchOptions.searchpath && !fetchOptions.noregistry;
+
+ if(shouldUseNpmInfo) {
+ events.emit('verbose', 'No version given in config.xml, attempting to use plugin engine info');
+ }
+
+ return (shouldUseNpmInfo ? registry.info([id]) : Q({}))
--- End diff --
This feels a little hokey to me. Perhaps it would be cleaner to pass the plugin `id` and `shouldUseNpmInfo` to `getFetchVersion()`, and let it call `registry.info()` if appropriate. Of course, then you'd have to mock `registry` to test this. So maybe not worth the extra effort since we want to get this change in :smile:. I'll leave it up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54820983
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
Right, they are essentially equivalent and will fetch the same version in both scenarios (except for pinned plugins, but that is a different matter). The only difference will be in that verbose message, which I am okay with because I think that there is a distinction between giving an empty `cordovaDependencies` and ignoring the feature. One says "I have no dependencies" and the other doesn't really say anything. I think it is unnecessary to return the special case of `null`, because returning latest better matches the expected behavior of this function. I can still make it return here if that helps clarify the code. I just think it should return latest rather than `null`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the pull request:
https://github.com/apache/cordova-lib/pull/363#issuecomment-186352413
Responded to most the feedback. Also added warnings and verbose logging which I completely forgot in my original PR. I will rebase this branch soon
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r50916949
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -122,25 +123,6 @@ module.exports = function plugin(command, targets, opts) {
var id = parts[0];
var version = parts[1];
- // If no version is specified, retrieve the version (or source) from config.xml
- if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
- events.emit('verbose', 'no version specified, retrieving version from config.xml');
- var ver = getVersionFromConfigFile(id, cfg);
-
- if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
- target = ver;
- } else {
- //if version exists from config.xml, use that
- if(ver) {
- target = ver ? (id + '@' + ver) : target;
- } else {
- //fetch pinned version from cordova-lib
- var pinnedVer = pkgJson.cordovaPlugins[id];
- target = pinnedVer ? (id + '@' + pinnedVer) : target;
- }
- }
- }
-
// Fetch the plugin first.
events.emit('verbose', 'Calling plugman.fetch on plugin "' + target + '"');
--- End diff --
Yup, this line should have been moved down with the rest
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54819210
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project based
+ * on the plugin's engine information from NPM and the platforms/plugins installed
+ * in the project. The cordovaDependencies object in the package.json's engines
+ * entry takes the form of an object that maps plugin versions to a series of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g. <3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched (e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise} A promise that will resolve to either a string
+ * if there is a version of the plugin that this
+ * project satisfies or null if there is not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+ // Figure out the project requirements
+ if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+ var pluginList = getInstalledPlugins(projectRoot);
+ var pluginMap = {};
+
+ pluginList.forEach(function(plugin) {
+ pluginMap[plugin.id] = plugin.version;
+ });
+
+ return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+ .then(function(platformVersions) {
+ return determinePluginVersionToFetch(
+ pluginInfo.versions,
+ pluginInfo.engines.cordovaDependencies,
+ pluginMap,
+ platformVersions,
+ cordovaVersion);
+ });
+ } else {
+ // If we have no engine, we want to fall back to the default behavior
+ events.emit('verbose', 'No plugin engine info found or not using registry, falling back to latest or pinned version');
+ return Q(null);
+ }
+}
+
+function findVersion(versions, version) {
+ var cleanedVersion = semver.clean(version);
+ for(var i = 0; i < versions.length; i++) {
+ if(semver.clean(versions[i]) === cleanedVersion) {
+ return versions[i];
+ }
+ }
+ return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap, platformMap, cordovaVersion) {
+ // Filters out pre-release versions
+ var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+ var versions = [];
+ var upperBound = null;
+ var upperBoundRange = null;
+
+ for(var version in engine) {
+ if(semver.valid(semver.clean(version)) && !semver.gt(version, latest)) {
+ versions.push(version);
+ } else {
+ // Check if this is an upperbound; validRange() handles whitespace
+ var cleanedRange = semver.validRange(version);
+ if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+ // We only care about the highest upper bound that our project does not support
+ if(getFailedRequirements(engine[version], pluginMap, platformMap, cordovaVersion).length !== 0) {
+ var maxMatchingUpperBound = cleanedRange.substring(1);
+ if (maxMatchingUpperBound && (!upperBound || semver.gt(maxMatchingUpperBound, upperBound))) {
+ upperBound = maxMatchingUpperBound;
+ upperBoundRange = version;
+ }
+ }
+ }
+ }
+ }
+
--- End diff --
Hmmm... In `getFetchVersion()`, if no `cordovaDependencies` are defined, we return null and display the following message (verbose log level):
No plugin engine info found or not using registry, falling back to latest or pinned version
If we get to this in point in `determinePluginVersionToFetch()` with an empty versions array and no `upperBound`, then that is essentially equivalent to there having been no `cordovaDependencies` in the first place, right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53300652
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -154,6 +135,38 @@ module.exports = function plugin(command, targets, opts) {
is_top_level: true
};
+ // If no version is specified, retrieve the version (or source) from config.xml
+ if (!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)) {
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+ target = ver;
+ } else if (ver) {
+ // If version exists in config.xml, use that
+ target = ver ? (id + '@' + ver) : target;
--- End diff --
I know this isn't your code - but would be good to clean this up while we're here... We already know `ver` has a value (since that is the condition of the `if` statement), so this should just be `target = id + '@' + ver`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org