[GitHub] cordova-lib pull request #512: CB-12284 Include project root as additional r...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-lib/pull/512 CB-12284 Include project root as additional root for static router ### Platforms affected All ### What does this PR do? With this change, cordova-serve includes the project root as an additional root for static routing. This is the final piece a middleware, so will only be utilized when a file isn't found through other means (the default static routing which points to the platform root, or other middleware that may have been injected by a third party). This can be useful in cases where source files that have been transpiled (such as TypeScript files) are located under the project root on a path that mirrors the the transpiled file's path under the platform root, and is pointed to by a map file (otherwise when debugging in a browser, the browser won't be able to find the original source file). ### What testing has been done on this change? Verified is resolved the problem reported in the bug, and lets you debug using original TypeScript sources in a TypeScript project, as long as the path to the TypeScript source relative to the project root mirrors the path to the generated JavaScript and map file relative to the www folder. ### Checklist - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database - [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB- is the JIRA ID & "android" is the platform affected. - [ ] Added automated test coverage as appropriate for this change. You can merge this pull request into a Git repository by running: $ git pull https://github.com/TimBarham/cordova-lib CB-12284 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/512.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 #512 commit 84fa8e39b22252381ed4baa6868ff6f22b7a48e2 Author: TimBarham Date: 2016-12-21T02:18:44Z CB-12284 Include project root as additional root for static router This can be useful in cases where source files that have been transpiled (such as TypeScript files) are located under the project root on a path that mirrors the the transpiled file's path under the platform root, and is pointed to by a map file (otherwise when debugging in a browser, the browser won't be able to find the original source file). --- 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 #498: CB-11985 Check if cached platform/plugin exis...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/498#discussion_r82568768
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -114,19 +113,11 @@ function initThenLoadSettingsWithRestore(promises) {
}
/**
-* @param {Array} with one element - the plugin id or "id@version"
+* @param {string} plugin - the plugin id or "id@version"
* @return {Promise.} Promised path to fetched package.
*/
function fetchPlugin(plugin) {
-return initThenLoadSettingsWithRestore(function () {
-events.emit('log', 'Fetching plugin "' + plugin + '" via npm');
-return Q.ninvoke(npm.commands, 'cache', ['add', plugin])
-.then(function (info) {
-var unpack = require('../../util/unpack');
-var pluginDir = path.resolve(npm.cache, info.name,
info.version, 'package');
-// Unpack the plugin that was added to the cache (CB-8154)
-var package_tgz = path.resolve(npm.cache, info.name,
info.version, 'package.tgz');
-return unpack.unpackTgz(package_tgz, pluginDir);
-});
-});
+events.emit('log', 'Fetching plugin "' + plugin + '" via npm');
+var pluginSplit = plugin.split('@');
+return npmhelper.fetchPackage(pluginSplit[0], pluginSplit[1]);
--- End diff --
Hah, look at that. I didn't know @riknoll had added support for scoped
packages. I'll switch this to use `plugin-spec-parser`.
---
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 issue #498: CB-11985 Check if cached platform/plugin exists befo...
Github user TimBarham commented on the issue: https://github.com/apache/cordova-lib/pull/498 @brodybits - that hasn't changed. At the point where we're looking for a cached version, we have already determined the latest version that matches the requested range (for both platforms and plugins). If a version range is specified, we have to hit the internet to determine the latest version that matches that range (and this change doesn't provide any real benefit). If a specific version is specified (which will be the case in VS), then that is the version we'll get (cached if it is there, otherwise have to download). The only change here from a user's perspective should be that plugins are cached in the same place as platforms (in the `.cordova` directory). --- 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 #498: CB-11985 Check if cached platform/plugin exis...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/498#discussion_r82566198
--- Diff: cordova-lib/src/util/npm-helper.js ---
@@ -72,4 +75,53 @@ function restoreSettings() {
}
}
+/**
+ * Fetches the latest version of a package from NPM that matches the
specified version. Returns a promise that
+ * resolves to the directory the NPM package is located in.
+ * @param packageName - name of an npm package
+ * @param packageVersion - requested version or version range
+ */
+function fetchPackage(packageName, packageVersion) {
+// Get the latest matching version from NPM if a version range is
specified
+return util.getLatestMatchingNpmVersion(packageName,
packageVersion).then(
+function (latestVersion) {
+return cachePackage(packageName, latestVersion);
+}
+);
+}
+
+/**
+ * Invokes "npm cache add," and then returns a promise that resolves to a
directory containing the downloaded,
+ * or cached package.
+ * @param packageName - name of an npm package
+ * @param packageVersion - requested version (not a version range)
+ */
+function cachePackage(packageName, packageVersion) {
+var cacheDir = path.join(util.libDirectory, 'npm_cache');
+
+// If already cached, use that rather than calling 'npm cache add'
again.
+var packageCacheDir = path.resolve(cacheDir, packageName,
packageVersion);
--- End diff --
Sure I'll wrap the whole method in a promise.
---
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 #498: CB-11985 Check if cached platform/plugin exis...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-lib/pull/498 CB-11985 Check if cached platform/plugin exists before 'npm cache' ### What does this PR do? Before calling `npm cache add` when installing a platform or plugin, look for an existing `package.tgz` file in the cache directory. This avoids unnecessary `npm` call with long timeout if not connected to the internet. Background: First time build failures in Cordova tools in Visual Studio are primarily due to `npm` failures. Therefore we are working to avoid any requirement for `npm` to hit the internet in simple build scenarios. This includes pre-installing cached versions of certain platforms and plugins. But we only get the full benefit if Cordova uses these cached versions if it finds them, without calling npm cache add. A couple of things to note: * This change uses shared code for platforms and plugins, and means plugins will also be cached to the `.cordova` directory rather than the global npm cache directory. Is that a concern? * It is likely this change will be meaningless with the proposed changes to installing platforms and plugins in Cordova 7.x. We'll cross the bridge when we come to it ð. ### What testing has been done on this change? Local testing. ### Checklist - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database - [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB- is the JIRA ID & "android" is the platform affected. You can merge this pull request into a Git repository by running: $ git pull https://github.com/TimBarham/cordova-lib use-cached-platform Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/498.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 #498 commit 667e774d46399cf97a9481c3890fa7a862871d46 Author: TimBarham Date: 2016-10-07T22:58:35Z CB-11985 Check if cached platform/plugin exists before 'npm cache' This avoids unnecessary npm call with long timeout if not connected to the internet. --- 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-docs issue #623: CB-11621 Add windows@4.4.2 release blog post
Github user TimBarham commented on the issue: https://github.com/apache/cordova-docs/pull/623 Couple of comments. Other than that looks good to me. --- 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-docs pull request #623: CB-11621 Add windows@4.4.2 release blog post
Github user TimBarham commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/623#discussion_r72483583 --- Diff: www/_posts/2016-07-27-cordova-windows-4.4.2.md --- @@ -0,0 +1,33 @@ +--- +layout: post +author: +name: Vladimir Kotikov +url: https://github.com/vladimir-kotikov +title: "Apache Cordova Windows 4.4.2" +categories: announcements +tags: news releases +--- + +We are happy to announce that `Cordova Windows 4.4.2` has been released! + +This release fixes some issues we've missed in 4.4.1. In particular, we've completely fixed all build issues with +new VS build tools. For the rest of changes see release notes below. + +Cordova CLI 6.3.0 will automatically start using this version of **Cordova-Windows** when creating new projects. --- End diff -- I think this should be "cordova-windows" (no uppercase). --- 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-docs pull request #623: CB-11621 Add windows@4.4.2 release blog post
Github user TimBarham commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/623#discussion_r72483527 --- Diff: www/_posts/2016-07-27-cordova-windows-4.4.2.md --- @@ -0,0 +1,33 @@ +--- +layout: post +author: +name: Vladimir Kotikov +url: https://github.com/vladimir-kotikov +title: "Apache Cordova Windows 4.4.2" +categories: announcements +tags: news releases +--- + +We are happy to announce that `Cordova Windows 4.4.2` has been released! + +This release fixes some issues we've missed in 4.4.1. In particular, we've completely fixed all build issues with +new VS build tools. For the rest of changes see release notes below. --- End diff -- I'm wondering how best to express this. Maybe something like 'In particular, we have fixed build issues experienced with the new install experience in Visual Studio "15" previews'. Do you have any thoughts @mbraude? --- 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-windows issue #188: CB-11548 Fix issues where MSBuild cannot be foun...
Github user TimBarham commented on the issue: https://github.com/apache/cordova-windows/pull/188 Merged in 2d7d219fc1864fa22f87cebadac173fd41091351. Sorry @jicongw, I forgot to tag this PR in the modified commit, so you will have to close the PR manually. --- 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-windows issue #188: Fix issues where MSBuild cannot be found due to ...
Github user TimBarham commented on the issue: https://github.com/apache/cordova-windows/pull/188 Thanks @jicongw - I just had one comment, plus fixing the commit message as I mentioned offline. --- 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-windows pull request #188: Fix issues where MSBuild cannot be found ...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/188#discussion_r71791061
--- Diff: template/cordova/lib/MSBuildTools.js ---
@@ -93,6 +93,20 @@ module.exports.findAllAvailableVersions = function () {
return !!item;
});
});
+}
+
+module.exports.findAllAvailableVersions = function () {
+// CB-11548 use VSINSTALLDIR environment if defined to find MSBuild.
If VSINSTALLDIR
+// is not specified or incorrect - fall back to default discovery
mechanism.
--- End diff --
It is quite likely that `VSINSTALLDIR` is specified and *correct*, but just
doesn't contain the msbuild path we're looking for (this will be the case for
previous versions of VS, for example). So I think it would be clearer to say
here something like "not specified or doesn't contain the MSBuild path we are
looking for".
---
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...
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;
+
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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;
+
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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...
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
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54728499
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project
based
+ * on the plugin's engine information from NPM and the platforms/plugins
installed
+ * in the project. The cordovaDependencies object in the package.json's
engines
+ * entry takes the form of an object that maps plugin versions to a series
of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g.
<3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the
project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched
(e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise}A promise that will resolve to either
a string
+ * if there is a version of the plugin
that this
+ * project satisfies or null if there is
not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+// Figure out the project requirements
+if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+var pluginList = getInstalledPlugins(projectRoot);
+var pluginMap = {};
+
+pluginList.forEach(function(plugin) {
+pluginMap[plugin.id] = plugin.version;
+});
+
+return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+.then(function(platformVersions) {
+return determinePluginVersionToFetch(
+pluginInfo.versions,
+pluginInfo.engines.cordovaDependencies,
+pluginMap,
+platformVersions,
+cordovaVersion);
+});
+} else {
+// If we have no engine, we want to fall back to the default
behavior
+events.emit('verbose', 'No plugin engine info found or not using
registry, falling back to latest or pinned version');
+return Q(null);
+}
+}
+
+function findVersion(versions, version) {
+var cleanedVersion = semver.clean(version);
+for(var i = 0; i < versions.length; i++) {
+if(semver.clean(versions[i]) === cleanedVersion) {
+return versions[i];
+}
+}
+return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap,
platformMap, cordovaVersion) {
+// Filters out pre-release versions
+var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+var versions = [];
+var upperBound = null;
+var upperBoundRange = null;
+
+for(var version in engine) {
+if(semver.valid(semver.clean(version)) && !semver.gt(version,
latest)) {
+versions.push(version);
+} else {
+// Check if this is an upperbound; validRange() handles
whitespace
+var cleanedRange = semver.validRange(version);
+if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+// We only care about the highest upper bound that our
project does not support
+if(getFailedRequirements(engine[version], pluginMap,
platformMap, cordovaVersion).length !== 0) {
+var maxMatchingUpperBound = cleanedRange.substring(1);
+if (maxMatchingUpperBound && (!upperBound ||
semver.gt(maxMatchingUpperBound, upperBound))) {
+upperBound = maxMatchingUpperBound;
+upperBoundRange = version
[GitHub] cordova-lib pull request: New plugin version selection implementat...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/363#issuecomment-191259905 This is looking great **@rikroll**! I have a few comments, but mostly pretty minor stuff. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at 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...
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;
+
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54727076
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -512,3 +543,219 @@ function versionString(version) {
return null;
}
+
+/**
+ * Gets the version of a plugin that should be fetched for a given project
based
+ * on the plugin's engine information from NPM and the platforms/plugins
installed
+ * in the project. The cordovaDependencies object in the package.json's
engines
+ * entry takes the form of an object that maps plugin versions to a series
of
+ * constraints and semver ranges. For example:
+ *
+ * { plugin-version: { constraint: semver-range, ...}, ...}
+ *
+ * Constraint can be a plugin, platform, or cordova version. Plugin-version
+ * can be either a single version (e.g. 3.0.0) or an upper bound (e.g.
<3.0.0)
+ *
+ * @param {string} projectRoot The path to the root directory of the
project
+ * @param {object} pluginInfo The NPM info of the plugin be fetched
(e.g. the
+ * result of calling `registry.info()`)
+ * @param {string} cordovaVersion The semver version of cordova-lib
+ *
+ * @return {Promise}A promise that will resolve to either
a string
+ * if there is a version of the plugin
that this
+ * project satisfies or null if there is
not
+ */
+function getFetchVersion(projectRoot, pluginInfo, cordovaVersion) {
+// Figure out the project requirements
+if (pluginInfo.engines && pluginInfo.engines.cordovaDependencies) {
+var pluginList = getInstalledPlugins(projectRoot);
+var pluginMap = {};
+
+pluginList.forEach(function(plugin) {
+pluginMap[plugin.id] = plugin.version;
+});
+
+return cordova_util.getInstalledPlatformsWithVersions(projectRoot)
+.then(function(platformVersions) {
+return determinePluginVersionToFetch(
+pluginInfo.versions,
+pluginInfo.engines.cordovaDependencies,
+pluginMap,
+platformVersions,
+cordovaVersion);
+});
+} else {
+// If we have no engine, we want to fall back to the default
behavior
+events.emit('verbose', 'No plugin engine info found or not using
registry, falling back to latest or pinned version');
+return Q(null);
+}
+}
+
+function findVersion(versions, version) {
+var cleanedVersion = semver.clean(version);
+for(var i = 0; i < versions.length; i++) {
+if(semver.clean(versions[i]) === cleanedVersion) {
+return versions[i];
+}
+}
+return null;
+}
+
+/*
+ * The engine entry maps plugin versions to constraints like so:
+ * {
+ * '1.0.0' : { 'cordova': '<5.0.0' },
+ * '<2.0.0': {
+ * 'cordova': '>=5.0.0',
+ * 'cordova-ios': '~5.0.0',
+ * 'cordova-plugin-camera': '~5.0.0'
+ * },
+ * '3.0.0' : { 'cordova-ios': '>5.0.0' }
+ * }
+ *
+ * See cordova-spec/plugin_fetch.spec.js for test cases and examples
+ */
+function determinePluginVersionToFetch(allVersions, engine, pluginMap,
platformMap, cordovaVersion) {
+// Filters out pre-release versions
+var latest = semver.maxSatisfying(allVersions, '>=0.0.0');
+
+var versions = [];
+var upperBound = null;
+var upperBoundRange = null;
+
+for(var version in engine) {
+if(semver.valid(semver.clean(version)) && !semver.gt(version,
latest)) {
+versions.push(version);
+} else {
+// Check if this is an upperbound; validRange() handles
whitespace
+var cleanedRange = semver.validRange(version);
+if(cleanedRange && UPPER_BOUND_REGEX.exec(cleanedRange)) {
+// We only care about the highest upper bound that our
project does not support
+if(getFailedRequirements(engine[version], pluginMap,
platformMap, cordovaVersion).length !== 0) {
+var maxMatchingUpperBound = cleanedRange.substring(1);
+if (maxMatchingUpperBound && (!upperBound ||
semver.gt(maxMatchingUpperBound, upperBound))) {
+upperBound = maxMatchingUpperBound;
+upperBoundRange = version;
+
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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...
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...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r54722165
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -305,6 +289,49 @@ module.exports = function plugin(command, targets,
opts) {
});
};
+function determinePluginTarget(projectRoot, cfg, target, fetchOptions) {
+var parts = target.split('@');
+var id = parts[0];
+var version = parts[1];
+
+// If no version is specified, retrieve the version (or source) from
config.xml
+if (version || cordova_util.isUrl(id) || cordova_util.isDirectory(id))
{
+return Q(target);
+} else {
+events.emit('verbose', 'No version specified, retrieving version
from config.xml');
+var ver = getVersionFromConfigFile(id, cfg);
+
+if (cordova_util.isUrl(ver) || cordova_util.isDirectory(ver)) {
+return Q(ver);
+} else if (ver) {
+// If version exists in config.xml, use that
+return Q(id + '@' + ver);
+} else {
--- End diff --
And again :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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...
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...
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-docs pull request: Rewrite of storage guide.
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-docs/pull/531 Rewrite of storage guide. * Provides more details about each of the options. * Provides examples. * Describes pros and cons of each approach. You can merge this pull request into a Git repository by running: $ git pull https://github.com/TimBarham/cordova-docs storage Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-docs/pull/531.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 #531 commit 61a8db59e83835abf5b9b6cd7a35bb58e8fee181 Author: Tim Barham Date: 2016-02-29T08:13:08Z Rewrite of storage guide. * Provides more details about each of the options. * Provides examples. * Describes pros and cons of each approach. --- 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-browser pull request: Switch from adm-zip to archiver.
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-browser/pull/10#issuecomment-191031984 @amikula - this PR was closed by a commit made to `cordova-firefoxos`, which was probably done by mistake. But regarding this problem specifically: since it was last released, `cordova-browser` has been updated to use the `cordova-serve`, which in turn uses ExpressJS for things including handling compression. So this PR is out-of-date, and likely the problem will go away when we do another `cordova-browser` release (which I will kick off the process for). In the meantime, you could try the latest changes to `cordova-browser` (including verifying this problem is now fixed) by adding the browser platform using: cordova add platform browser@https://github.com/apache/cordova-browser --save --- 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: CB-10662 Use project's config.xml as a f...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/398#issuecomment-188730806 Thanks @vladimir-kotikov - looks good. Just a couple of suggestions. --- 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: CB-10662 Use project's config.xml as a f...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/398#discussion_r54079687
--- Diff: cordova-lib/src/plugman/platforms/browser.js ---
@@ -31,25 +29,7 @@ module.exports = {
return path.join(project_dir, 'www');
},
package_name:function(project_dir) {
-// preferred location if cordova >= 3.4
-var preferred_path = path.join(project_dir, 'config.xml');
-var config_path;
-
-if (!fs.existsSync(preferred_path)) {
-// older location
-var old_config_path =
path.join(module.exports.www_dir(project_dir), 'config.xml');
-if (!fs.existsSync(old_config_path)) {
-// output newer location and fail reading
-config_path = preferred_path;
-events.emit('verbose', 'unable to find '+config_path);
-} else {
-config_path = old_config_path;
-}
-} else {
-config_path = preferred_path;
-}
-var widget_doc = xml_helpers.parseElementtreeSync(config_path);
-return widget_doc._root.attrib['id'];
+return common.package_name(project_dir);
--- End diff --
Perhaps should make the same change for `firefoxos` and `webos` (and maybe
`blackberry10`) too?
---
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: CB-10662 Use project's config.xml as a f...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/398#discussion_r54079611
--- Diff: cordova-lib/src/plugman/platforms/common.js ---
@@ -22,7 +22,39 @@ var shell = require('shelljs'),
fs= require('fs'),
common;
+var cordovaUtil = require('../../cordova/util');
+var CordovaError = require('cordova-common').CordovaError;
+var xmlHelpers = require('cordova-common').xmlHelpers;
+
module.exports = common = {
+package_name: function(project_dir) {
+
+var configPaths = [
+// preferred location if cordova >= 3.4
+path.join(project_dir, 'config.xml'),
+// older location
+path.join(project_dir, 'www/config.xml'),
+];
+
+var cordovaRoot = cordovaUtil.isCordova();
+if (cordovaRoot) {
+// CB-10662 If we're in cli project then add project's config
as a fallback
+configPaths.push(path.join(cordovaRoot, 'config.xml'));
+}
+
+for (var i = 0; i < configPaths.length; i++) {
+var configPath = configPaths[i];
+// If no config there try next path
+if (!fs.existsSync(configPath)) continue;
+
+var widget_doc = xmlHelpers.parseElementtreeSync(configPath);
+return widget_doc._root.attrib.id;
+}
+
+// No configs found - fail with meaningful error message
+throw new CordovaError('Unable to find project\'s config in none
of ' +
+'the following directories:\n\t' + configPaths.join('\n\t'));
--- End diff --
I would suggest "any" instead of "none".
---
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: CB-10662 Apply project config to platfor...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/398#issuecomment-188545978 A simpler fix might be: since all we're looking for is the project's `id`, why not fall back on the main Cordova project's `config.xml` if we don't find a platform specific one? We could change this in `cordova-lib\src\plugman\platforms\browser.js`. Or, since this code is fairly common across several platforms, we could provide a single implementation (in `common.js`, perhaps?) for those that need 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...
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...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/363#discussion_r53779104
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -185,6 +187,22 @@ function listPlatforms(project_dir) {
});
}
+function getInstalledPlatformsWithVersions(project_dir) {
+var result = {};
+var platforms_on_fs = listPlatforms(project_dir);
+
+return Q.all(platforms_on_fs.map(function(p) {
+return superspawn.maybeSpawn(path.join(project_dir, 'platforms',
p, 'cordova', 'version'), [], { chmod: true })
+.then(function(v) {
+result[p] = v || null;
+}, function(v) {
+result[p] = v || 'broken';
--- End diff --
Is this what you intended? The parameter `v` here will actually be the
error. In the previous version, `v` was simply ignored (so "version" was always
set to "broken").
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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:
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/commit/3b9face5187857d354d85f3e3f4316cbace100db#commitcomment-16237620 Hey @vladimir-kotikov - this fix causes a crash when adding the browser platform (maybe any platform that doesn't have the new API?)... See [CB-10662](https://issues.apache.org/jira/browse/CB-10662). --- 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...
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) {
+
[GitHub] cordova-lib pull request: CB-10641 Run prepare _after_ plugins wer...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/393#issuecomment-185729885 Yep, that looks right to me. --- 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: CB-10519 Wrap all sync calls inside of `...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/384#issuecomment-185719821 Oops, sorry :smile: ... LGTM. There's nowhere we call these directly internally and expect an actual value in return? --- 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...
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-') ==
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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) {
+
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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...
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.
---
---
[GitHub] cordova-lib pull request: New plugin version selection implementat...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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: CB-10550 Fixed the issue of plugin id ma...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/387#issuecomment-185005103 Thanks @bso-intel. I've merged this in https://github.com/apache/cordova-lib/commit/87d8e171, but I rebased on forgot to add "This closes #387" to the comment, so if you could close this PR manually that'd be great! --- 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: CB-10519 Wrap all sync calls inside of `...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/384#issuecomment-184676141 Thanks @vladimir-kotikov. Looks good on a quick look through. Will take a closer look in the morning. --- 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: CB-10519 Wrap all sync calls inside of `...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/384#discussion_r52993992
--- Diff: cordova-lib/src/cordova/build.js ---
@@ -17,22 +17,26 @@
under the License.
*/
-var cordovaUtil = require('./util'),
-HooksRunner = require('../hooks/HooksRunner');
+var Q = require('q'),
+cordovaUtil = require('./util'),
+HooksRunner = require('../hooks/HooksRunner');
// Returns a promise.
module.exports = function build(options) {
-var projectRoot = cordovaUtil.cdProjectRoot();
-options = cordovaUtil.preProcessOptions(options);
-
-// fire build hooks
-var hooksRunner = new HooksRunner(projectRoot);
-return hooksRunner.fire('before_build', options)
-.then(function() {
-return require('./cordova').raw.prepare(options);
-}).then(function() {
-return require('./cordova').raw.compile(options);
-}).then(function() {
-return hooksRunner.fire('after_build', options);
+return Q().then(function() {
+var opts = cordovaUtil.preProcessOptions(options);
+var projectRoot = cordovaUtil.cdProjectRoot();
+return [new HooksRunner(projectRoot), opts];
+})
+.spread(function (hooksRunner, options) {
--- End diff --
Well, that'd be my preference, as the code ends up much simpler and easy to
read, to my mind. Otherwise we're arbitrarily splitting out the synchronous
part of the process into its own separate promise, when there's no need to. But
ultimately it's up to you :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: CB-10519 Wrap all sync calls inside of `...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/384#discussion_r52984062
--- Diff: cordova-lib/src/cordova/build.js ---
@@ -17,22 +17,26 @@
under the License.
*/
-var cordovaUtil = require('./util'),
-HooksRunner = require('../hooks/HooksRunner');
+var Q = require('q'),
+cordovaUtil = require('./util'),
+HooksRunner = require('../hooks/HooksRunner');
// Returns a promise.
module.exports = function build(options) {
-var projectRoot = cordovaUtil.cdProjectRoot();
-options = cordovaUtil.preProcessOptions(options);
-
-// fire build hooks
-var hooksRunner = new HooksRunner(projectRoot);
-return hooksRunner.fire('before_build', options)
-.then(function() {
-return require('./cordova').raw.prepare(options);
-}).then(function() {
-return require('./cordova').raw.compile(options);
-}).then(function() {
-return hooksRunner.fire('after_build', options);
+return Q().then(function() {
+var opts = cordovaUtil.preProcessOptions(options);
+var projectRoot = cordovaUtil.cdProjectRoot();
+return [new HooksRunner(projectRoot), opts];
+})
+.spread(function (hooksRunner, options) {
--- End diff --
Hey @vladimir-kotikov - what's the purpose of using `spread()` here (and
elsewhere)? Why can't we just essentially wrap the entire method in `return
Q().then(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: CB-10611 fixed the before_plugin_install...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/388#issuecomment-184563301 LGTM. --- 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: CB-10550 Fixed the issue of plugin id ma...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/387#discussion_r52976299
--- Diff: cordova-lib/src/plugman/fetch.js ---
@@ -132,10 +132,14 @@ function fetchPlugin(plugin_src, plugins_dir,
options) {
));
}
// If not found in local search path, fetch from the registry.
-var newID = pluginMapperotn[plugin_src];
+var splitVersion = plugin_src.split('@');
+var newID = pluginMapperotn[splitVersion[0]];
if(newID) {
events.emit('warn', 'Notice: ' + plugin_src + ' has been
automatically converted to ' + newID + ' to be fetched from npm. This is due to
our old plugins registry shutting down.');
--- End diff --
Looks great, thanks @bso-intel!
One small nit - use `splitVersion[0]` in the message instead of
`plugin_src` (so, for example, we get `org.apache.cordova.device has been
automatically converted to cordova-plugin-device to be fetched from npm`
instead of `org.apache.cordova.device@1.1.1 has been automatically converted to
cordova-plugin-device to be fetched from npm`).
---
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: CB-10176 Adds standard logger, based on ...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/375#issuecomment-179513568 :+1: Thanks Vladimir. --- 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: CB-10461: 'cordova platform ls' should l...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/376#issuecomment-179511413 Looks great! Just one comment: I think just a couple of spaces would look better than a tab. Also, looks like there is a test that is sensitive to the output of this command. --- 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: CB-10461: 'cordova platform ls' should l...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/376#issuecomment-179503770 Yeah, and in fact, for consistency maybe "installed platforms" should also be separated by line breaks rather than commas: ``` Installed platforms: android 5.1.0 browser 4.1.0-dev Available platforms: amazon-fireos ~3.6.3 (deprecated) blackberry10 ~3.8.0 firefoxos ~3.6.3 webos ~3.7.0 windows ~4.3.0 windows8 ~4.3.0 wp8 ~3.8.2 (deprecated) ``` --- 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: CB-10461: 'cordova platform ls' should l...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/376#issuecomment-179498720 I like the idea, but find the list is starting to get a bit confusing (to see the actual platform names. We need to make sure the platform names a very easy to see. On option that might help would be to switch to using line breaks instead of commas, something like: ``` Installed platforms: browser 4.1.0-dev Available platforms: amazon-fireos ~3.6.3 (deprecated) android ~5.1.0 blackberry10 ~3.8.0 firefoxos ~3.6.3 webos ~3.7.0 windows ~4.3.0 windows8 ~4.3.0 wp8 ~3.8.2 (deprecated) ``` --- 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-plugin-camera pull request: CB-10113 - Browser - Camera on...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/134#issuecomment-178412993 Thanks for this @aliokan. I've tweaked the commit a bit to store the magic number in a constant, and I'll merge 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-plugin-camera pull request: Chrome , Uncaught TypeError: l...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/154#issuecomment-178404256 Ah sorry @fattalgazi - I just fixed this and missed that you already had a PR open. Fixed now in #161. --- 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-plugin-camera pull request: CB-10502 Fix camera plugin exc...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/161 CB-10502 Fix camera plugin exception in Chrome when click capture. The `MediaStream.stop()` method has been deprecated as of Chrome 47. We were using it, and it was generating an exception. If `stop()` method is not found, instead stop each individual track (the new way of doing it). You can merge this pull request into a Git repository by running: $ git pull https://github.com/TimBarham/cordova-plugin-camera CB-10502 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-camera/pull/161.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 #161 commit e48a7e5c5cffe8d9bab6eeff33feeb2747930ace Author: Tim Barham Date: 2016-02-02T06:25:00Z CB-10502 Fix camera plugin exception in Chrome when click capture. The MediaStream.stop() method has been deprecated as of Chrome 47. We were using it, and it was generating an exception. If stop() method is not found, instead stop each individual track (the new way of doing 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-windows pull request: don't copy resource-files at plugin ...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-windows/pull/139#issuecomment-177957240 I've taken a look too, and I agree this seems the best approach. Changes look good to me, except I agree with your comment @sgrebnov that we should use a relative path rather than absolute. Regarding the red "x" on the folder name: this seems to be a limitation of a `jsproj` (that they don't fully support links, at least in the VS UI). If you create an equivalent link in a `csproj`, you don't get the red "x". If it indeed builds fine, then we can live with that. However, there is one way we can avoid the red "x": actually create any folder defined in the `target` path (if there is one). It would be empty, but would make for less confusion in the VS UI. --- 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-windows pull request: CB-9828 Implements PlatformApi contr...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-windows/pull/132#issuecomment-162867249 Generally looks fine to me apart from some misplaced "Android" references I mention inline. Also, it concerns me that `ConsoleLogger` is duplicated across each platform - isn't this something that can live in `cordova-common`? Why aren't we using the same functionality as `cordova-cli`? --- 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-plugin-file pull request: CB-10023 Fix "proxy not found er...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/152#issuecomment-162708869 Moved `isChrome` to be browser specific (we only ever call it either from browser specific code, or after verifying we're running on browser platform). --- 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-plugin-file pull request: CB-10023 Fix "proxy not found er...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/152#issuecomment-162708771 @omefire - no, I don't think so. I think feature detection is more appropriate for the intent of this code. Also, my goal wasn't to change the existing logic, just to make it consistent. --- 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-plugin-file pull request: CB-10023 Fix "proxy not found er...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-file/pull/152#discussion_r46821107
--- Diff: www/isChrome.js ---
@@ -0,0 +1,26 @@
+/*
+ *
+ * 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.
+ *
+ */
+
+module.exports = function () {
+// window.webkitRequestFileSystem and
window.webkitResolveLocalFileSystemURL are available only in Chrome and
+// possibly a good flag to indicate that we're running in Chrome
+return window.webkitRequestFileSystem &&
window.webkitResolveLocalFileSystemURL;
+};
--- End diff --
Ah, good catch, thanks. Normally I have my editor configured to do this
automatically, but new install and I hadn't turned that on :smile:. Fixed 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-plugin-file pull request: CB-10023 Fix "proxy not found er...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/152#issuecomment-162383919 Hey @stevengill, could you take a look at this? I just want to verify this doesn't do anything to screw with `uglify` (which is the change that I am modifying). And @daserge - does this look right 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-plugin-file pull request: CB-10023 Fix "proxy not found er...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-plugin-file/pull/152 CB-10023 Fix "proxy not found error" on Chrome. We shouldn't be patching `requestFileSystem` and `resolveLocalFileSystem` on Chrome. Recent change broke the existing logic for this. Also, added common logic for detecting Chrome (or really, feature detection) so we are checking in a consistent manner. You can merge this pull request into a Git repository by running: $ git pull https://github.com/MSOpenTech/cordova-plugin-file CB-10023 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-file/pull/152.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 #152 commit adbb97780f2d68fa98e71c0aee17f1cecad8ab71 Author: Tim Barham Date: 2015-12-07T01:08:26Z CB-10023 Fix "proxy not found error" on Chrome. We shouldn't be patching requestFileSystem and resolveLocalFileSystem on Chrome. Recent change broke the existing logic for 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: Cb 9590 - Ubuntu support for the new plu...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/349#issuecomment-161826181 Thanks - LGTM! --- 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: Cb 9590 - Ubuntu support for the new plu...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/349#discussion_r46494465
--- Diff: cordova-lib/src/plugman/platforms/ubuntu.js ---
@@ -29,6 +29,54 @@ function toCamelCase(str) {
}).join('');
}
+function getPluginXml(plugin_dir) {
+var et = require('elementtree'),
+fs = require('fs'),
+path = require('path');
+
+var pluginxml;
+var config_path = path.join(plugin_dir, 'plugin.xml');
+
+if (fs.existsSync(config_path)) {
+// Get the current plugin.xml file
+pluginxml = et.parse(fs.readFileSync(config_path, 'utf-8'));
+}
+
+return pluginxml;
+}
+
+function findClassName(pluginxml, plugin_id) {
+var class_name;
+
+// first check if we have a class-name parameter in the plugin config
+if (pluginxml) {
+ var platform = pluginxml.find("./platform/[@name='ubuntu']/");
+ if (platform) {
+ var param =
platform.find("./config-file/[@target='config.xml']/feature/param/[@name='ubuntu-package']");
+ if (param && param.attrib) {
+ class_name = param.attrib.value;
+ return class_name;
+ }
+ }
+}
+
+// fallback to guess work, based on the plugin package name
+
+if (plugin_id.match(/\.[^.]+$/)) {
+// old-style plugin name
+class_name = plugin_id.match(/\.[^.]+$/)[0].substr(1);
+class_name = toCamelCase(class_name);
+} else {
+ match = plugin_id.match(/cordova\-plugin\-([\w\-]+)$/);
+if (match && match.length > 0)
+ class_name = match[0].substr(15);
+ else
+class_name = toCamelCase(class_name);
+}
+
+return class_name;
+}
--- End diff --
Thanks @david-barth-canonical! Some comments:
* Generally the Cordova codebase uses curly braces even around single line
blocks.
* There are some tabs in there that should be converted to spaces.
* The `match` variable needs a `var`.
* Should be calling `toCamelCase()` for the scenario where we match
`cordova-plugin-...`
* Where we don't match `cordova-plugin-...`, `class_name` is not defined
(should be using `plugin_id`).
* Since we will always end up calling `toCamelCase()`, the logic for the
second half of this method can be simplified to:
```js
// fallback to guess work, based on the plugin package name
if (plugin_id.match(/\.[^.]+$/)) {
// old-style plugin name
class_name = plugin_id.match(/\.[^.]+$/)[0].substr(1);
} else {
var match = plugin_id.match(/cordova\-plugin\-([\w\-]+)$/);
if (match && match.length > 0) {
class_name = match[0].substr(15);
} else {
class_name = plugin_id;
}
}
return toCamelCase(class_name);
```
Also, since the original PR has been merged, can you recreate this as a new
PR which will just include the changes in your last commit (and changes in
response to feedback, of course).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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: CB-9590 - Ubuntu support for the new plu...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/294#discussion_r46228904
--- Diff: cordova-lib/src/plugman/platforms/ubuntu.js ---
@@ -29,6 +29,51 @@ function toCamelCase(str) {
}).join('');
}
+function getPluginXml(plugin_dir) {
+var et = require('elementtree'),
+fs = require('fs'),
+path = require('path');
+
+var pluginxml;
+var config_path = path.join(plugin_dir, 'plugin.xml');
+
+if (fs.existsSync(config_path)) {
+// Get the current plugin.xml file
+pluginxml = et.parse(fs.readFileSync(config_path, 'utf-8'));
+}
+
+return pluginxml;
+}
+
+function findClassName(pluginxml, plugin_id) {
+var class_name;
+
+// first check if we have a class-name parameter in the plugin config
+if (pluginxml) {
+ var platform = pluginxml.find("./platform/[@name='ubuntu']/");
+ if (platform) {
+ var param =
platform.find("./config-file/[@target='config.xml']/feature/param/[@name='ubuntu-package']");
+ if (param && param.attrib) {
+ class_name = param.attrib.value;
+ return class_name;
+ }
+ }
+}
+
+// fallback to guess work, based on the plugin package name
+
+if (plugin_id.match(/\.[^.]+$/)) {
+// old-style plugin name
+class_name = plugin_id.match(/\.[^.]+$/)[0].substr(1);
+class_name = toCamelCase(class_name);
+} else {
+class_name =
plugin_id.match(/cordova\-plugin\-([\w\-]+)$/)[0].substr(15);
--- End diff --
This will fail with a JavaScript error if `plugin_id` does not start with
`cordova-plugin`, since `match()` will return `null`. Instead, should do the
`match()` call separately, then only try to get the first array element if the
return value is not `null`. If the return value *is* `null`, then should use
some fallback (like camel-case the entire `plugin_id`, for example).
---
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-android pull request: CB-9835 Downgrade `properties-parser...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-android/pull/230#issuecomment-157950628 Just an FYI - The node 4.x.x dependency in `node-parser` has just been fixed and the fix released, so we can move to `0.3.1` safely if ever there is a reason to do so. --- 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: CB-9987 Reinstall plugins for platform i...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/344#issuecomment-157203021 Note the JIRA number in the title is incorrect. --- 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-plugin-file pull request: CB-7253: requestFileSystem fails...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-file/pull/145#discussion_r43565677
--- Diff: src/android/DirectoryManager.java ---
@@ -56,45 +56,46 @@ public static boolean testFileExists(String name) {
}
/**
- * Get the free disk space
- *
+ * Get the free space in external storage
+ *
* @return Size in KB or -1 if not available
--- End diff --
I would say 50 years of tradition outweighs an IEC standard that nobody
uses.
---
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: CB-9901 cordova plugin search opens in a...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/334#issuecomment-152612090 Are people likely to be doing a plugin search from a gulp script? --- 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-android pull request: CB-9909 Shouldn't escape spaces in p...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-android/pull/237#issuecomment-152359281 @stevengill - can you take a look at this? Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at 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-android pull request: CB-9909 Shouldn't escape spaces in p...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-android/pull/237 CB-9909 Shouldn't escape spaces in paths on Windows. Instead on windows just wrap in quotes. Note that this value is only never executed directly - just displayed to the user. But this way they can copy and paste successfully on Windows. You can merge this pull request into a Git repository by running: $ git pull https://github.com/MSOpenTech/cordova-android fix-android-cmd Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-android/pull/237.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 #237 commit e6d835df2292be642de16eb64990dc0558f00c97 Author: Tim Barham Date: 2015-10-29T22:30:48Z CB-9909 Shouldn't escape spaces in paths on Windows. --- 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: CB-9872 Fixed save.spec.11 failure
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/332#issuecomment-152242161 :+1: --- 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: CB-9872 Fixed save.spec.11 failure
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/332#discussion_r43331295
--- Diff: cordova-lib/spec-cordova/save.spec.js ---
@@ -244,19 +307,26 @@ describe('(save flag)', function () {
console.log(err);
expect(false).toBe(true);
done();
+}).finally(function () {
+revertDownloadPlatform();
});
-}, timeout);
+}, TIMEOUT);
it('spec.11 should update spec with git url when updating using
git url', function (done) {
helpers.setEngineSpec(appPath, platformName,
platformVersionNew);
-platform('add', platformName + '@' + platformVersionNew)
+mockDownloadPlatform(platformLocalPathOld, platformVersionOld);
+
+platform('add', platformName + '@' + platformVersionOld)
.then(function () {
+revertDownloadPlatform();
var fsExistsSync = fs.existsSync.bind(fs);
spyOn(fs, 'existsSync').andCallFake(function (somePath) {
return (somePath === path.join(appPath, 'platforms',
platformName)) || fsExistsSync(somePath);
});
+mockDownloadPlatform(platformLocalPathNew,
platformVersionNew);
--- End diff --
I think we want to have tests for these scenarios so we can verify things
aren't being broken at PR/commit time rather than waiting until we're about to
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: CB-9872 Fixed save.spec.11 failure
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/332#issuecomment-152024899 @stevengill - ultimately, the point of these tests is to test the `--save` feature, so going forward I don't think we want them to be at risk of being broken by platform changes in master, as long as we add tests to the platforms that cover the e2e scenarios that these tests were "accidentally" covering. --- 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: Dummy PR to trigger a test run - don't m...
Github user TimBarham closed the pull request at: https://github.com/apache/cordova-lib/pull/333 --- 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: CB-9872 Fixed save.spec.11 failure
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/332#discussion_r43292467
--- Diff: cordova-lib/spec-cordova/save.spec.js ---
@@ -244,19 +307,26 @@ describe('(save flag)', function () {
console.log(err);
expect(false).toBe(true);
done();
+}).finally(function () {
+revertDownloadPlatform();
});
-}, timeout);
+}, TIMEOUT);
it('spec.11 should update spec with git url when updating using
git url', function (done) {
helpers.setEngineSpec(appPath, platformName,
platformVersionNew);
-platform('add', platformName + '@' + platformVersionNew)
+mockDownloadPlatform(platformLocalPathOld, platformVersionOld);
+
+platform('add', platformName + '@' + platformVersionOld)
.then(function () {
+revertDownloadPlatform();
var fsExistsSync = fs.existsSync.bind(fs);
spyOn(fs, 'existsSync').andCallFake(function (somePath) {
return (somePath === path.join(appPath, 'platforms',
platformName)) || fsExistsSync(somePath);
});
+mockDownloadPlatform(platformLocalPathNew,
platformVersionNew);
--- End diff --
Using edge for this test has caught 2 `cordova-android` issues in the last
week or so. While ideally we don't want `cordova-lib` tests failing because of
`cordova-android` issues, should we continue using edge in these tests until
we've added `update` tests to `cordova-android`?
---
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: Dummy PR to trigger a test run - don't m...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/333#issuecomment-151920060 Yep, saw that PR just after I sent this one :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: CB-9872 Fixed save.spec.11 failure
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/332#issuecomment-151918673 So Alex - what was the underlying problem? --- 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: Dummy PR to trigger a test run - don't m...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-lib/pull/333 Dummy PR to trigger a test run - don't merge :) You can merge this pull request into a Git repository by running: $ git pull https://github.com/MSOpenTech/cordova-lib tb-temp Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/333.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 #333 commit 1ac255925a8a0f3ae2c6984332102a76ffd1c904 Author: Tim Barham Date: 2015-10-28T17:12:42Z Dummy commit to trigger a test run --- 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-plugin-device pull request: add isSimulator for iOS & Andr...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-plugin-device/pull/35#issuecomment-150308646 Works for me. --- 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-android pull request: CB-9835 Downgrade `properties-parser...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-android/pull/230#issuecomment-149915429 :+1: (to getting this change in, and to then creating tests to guard against this going forwards). --- 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-android pull request: CB-9835 Downgrade `properties-parser...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-android/pull/230#issuecomment-149905998 Looks fine to me, as long as we're sure the older version doesn't lose any functionality we need. --- 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: On Windows, verify browsers installed be...
Github user TimBarham commented on the pull request: https://github.com/apache/cordova-lib/pull/327#issuecomment-149737583 Thanks for taking a look, @vladimir-kotikov. --- 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: On Windows, verify browsers installed be...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/327#discussion_r42565419
--- Diff: cordova-serve/src/browser.js ---
@@ -95,16 +105,93 @@ function getBrowser(target, dataDir) {
'firefox': 'firefox',
'opera': 'opera'
},
-'linux' : {
-'chrome': 'google-chrome' + chromeArgs ,
+'linux': {
+'chrome': 'google-chrome' + chromeArgs,
'chromium': 'chromium-browser' + chromeArgs,
'firefox': 'firefox',
'opera': 'opera'
}
};
-target = target.toLowerCase();
if (target in browsers[process.platform]) {
-return Q(browsers[process.platform][target]);
+var browser = browsers[process.platform][target];
+if (process.platform === 'win32') {
+// Windows displays a dialog if the browser is not installed.
We'd prefer to avoid that.
+return checkBrowserExistsWindows(browser,
target).then(function () {
+return browser;
+});
+} else {
+return Q(browser);
+}
}
-return Q.reject('Browser target not supported: ' + target);
+return Q.reject(NOT_SUPPORTED.replace('%target%', target));
+}
+
+function checkBrowserExistsWindows(browser, target) {
+var promise = target === 'edge' ? edgeSupported() :
browserInstalled(browser);
+return promise.catch(function (error) {
+return Q.reject((error && error.toString() ||
NOT_INSTALLED).replace('%target%', target));
+});
+}
+
+function edgeSupported() {
+var d = Q.defer();
+
+child_process.exec('ver', function (err, stdout, stderr) {
--- End diff --
os.release() doesn't recognize Win10 in fairly common versions of node.
---
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: On Windows, verify browsers installed be...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/327#discussion_r42565322
--- Diff: cordova-serve/src/browser.js ---
@@ -95,16 +105,93 @@ function getBrowser(target, dataDir) {
'firefox': 'firefox',
'opera': 'opera'
},
-'linux' : {
-'chrome': 'google-chrome' + chromeArgs ,
+'linux': {
+'chrome': 'google-chrome' + chromeArgs,
'chromium': 'chromium-browser' + chromeArgs,
'firefox': 'firefox',
'opera': 'opera'
}
};
-target = target.toLowerCase();
if (target in browsers[process.platform]) {
-return Q(browsers[process.platform][target]);
+var browser = browsers[process.platform][target];
+if (process.platform === 'win32') {
+// Windows displays a dialog if the browser is not installed.
We'd prefer to avoid that.
+return checkBrowserExistsWindows(browser,
target).then(function () {
+return browser;
+});
+} else {
+return Q(browser);
+}
}
-return Q.reject('Browser target not supported: ' + target);
+return Q.reject(NOT_SUPPORTED.replace('%target%', target));
+}
+
+function checkBrowserExistsWindows(browser, target) {
+var promise = target === 'edge' ? edgeSupported() :
browserInstalled(browser);
+return promise.catch(function (error) {
+return Q.reject((error && error.toString() ||
NOT_INSTALLED).replace('%target%', target));
+});
+}
+
+function edgeSupported() {
+var d = Q.defer();
+
+child_process.exec('ver', function (err, stdout, stderr) {
--- End diff --
I originally used the exec module and the logic actuallly ended up simpler
this way.
---
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: On Windows, verify browsers installed be...
Github user TimBarham commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/327#discussion_r42562761
--- Diff: cordova-serve/src/browser.js ---
@@ -17,9 +17,15 @@
under the License.
*/
-var exec = require('./exec'),
+var child_process = require('child_process'),
+exec = require('./exec'),
+fs = require('fs'),
+path = require('path'),
Q = require('q');
+var NOT_INSALLED = 'The browser target is not installed: %target%';
--- End diff --
Thanks. Oops. Fixed :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: On Windows, verify browsers installed be...
GitHub user TimBarham opened a pull request: https://github.com/apache/cordova-lib/pull/327 On Windows, verify browsers installed before launching. Do this to avoid dialog that pops up on Windows if browser is not installed, and provide more meaningful error messages. You can merge this pull request into a Git repository by running: $ git pull https://github.com/MSOpenTech/cordova-lib serve-verify-browser Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/327.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 #327 commit 8f330f173edd19c1d50fa5db00368192c609bb59 Author: Tim Barham Date: 2015-10-17T02:17:03Z On Windows, verify browsers installed before launching. Do this to avoid dialog that pops up if browser is not installed. --- 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
