[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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: 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[stevengill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[vladimir-kotikov]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[vladimir-kotikov]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[vladimir-kotikov]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[nikhilkh]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[nikhilkh]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[nikhilkh]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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, 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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' : { 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[dblotsky]

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' : { 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 = 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: 

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 `

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[TimBarham]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[dblotsky]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[dblotsky]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[dblotsky]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[stevengill]

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: New plugin version selection implementat...

[riknoll]

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 
Date:   2016-01-15T21:35:26Z

Adding new version choosing logic for plugin fetching

commit b71e0df52db052bb3b05d9386561f32197456879
Author: riknoll 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org