You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by st...@apache.org on 2017/08/30 06:10:11 UTC

[1/4] cordova-lib git commit: CB-13145: pass full options to plugman uninstall

Repository: cordova-lib
Updated Branches:
  refs/heads/master 6b98390ec -> ad31610fa


CB-13145: pass full options to plugman uninstall


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/295f29c0
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/295f29c0
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/295f29c0

Branch: refs/heads/master
Commit: 295f29c0ae83d9512f7ecbafc115861c76af6460
Parents: 6b98390
Author: Steve Gill <st...@gmail.com>
Authored: Tue Aug 22 21:39:24 2017 -0700
Committer: Steve Gill <st...@gmail.com>
Committed: Tue Aug 29 22:23:30 2017 -0700

----------------------------------------------------------------------
 src/cordova/plugin/add.js    | 24 ++----------------------
 src/cordova/plugin/remove.js | 17 +++++++++++++----
 src/cordova/plugin/util.js   | 29 +++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/295f29c0/src/cordova/plugin/add.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/add.js b/src/cordova/plugin/add.js
index 203cf09..e6f8283 100644
--- a/src/cordova/plugin/add.js
+++ b/src/cordova/plugin/add.js
@@ -99,28 +99,8 @@ function add (projectRoot, hooksRunner, opts) {
                 }).then(function (directory) {
                     return pluginInfoProvider.get(directory);
                 }).then(function (pluginInfo) {
-                    // Validate top-level required variables
-                    var pluginVariables = pluginInfo.getPreferences();
-                    opts.cli_variables = opts.cli_variables || {};
-                    var pluginEntry = cfg.getPlugin(pluginInfo.id);
-                    // Get variables from config.xml
-                    var configVariables = pluginEntry ? pluginEntry.variables : {};
-                    // Add config variable if it's missing in cli_variables
-                    Object.keys(configVariables).forEach(function (variable) {
-                        opts.cli_variables[variable] = opts.cli_variables[variable] || configVariables[variable];
-                    });
-                    var missingVariables = Object.keys(pluginVariables)
-                        .filter(function (variableName) {
-                            // discard variables with default value
-                            return !(pluginVariables[variableName] || opts.cli_variables[variableName]);
-                        });
-
-                    if (missingVariables.length) {
-                        events.emit('verbose', 'Removing ' + pluginInfo.dir + ' because mandatory plugin variables were missing.');
-                        shell.rm('-rf', pluginInfo.dir);
-                        var msg = 'Variable(s) missing (use: --variable ' + missingVariables.join('=value --variable ') + '=value).';
-                        return Q.reject(new CordovaError(msg));
-                    }
+                    
+                    plugin_util.mergeVariables(pluginInfo, cfg, opts);
 
                     // Iterate (in serial!) over all platforms in the project and install the plugin.
                     return chainMap(platformList, function (platform) {

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/295f29c0/src/cordova/plugin/remove.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/remove.js b/src/cordova/plugin/remove.js
index 6cbd930..76ad091 100644
--- a/src/cordova/plugin/remove.js
+++ b/src/cordova/plugin/remove.js
@@ -28,6 +28,7 @@ var metadata = require('../../plugman/util/metadata');
 var Q = require('q');
 var path = require('path');
 var fs = require('fs');
+var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 
 module.exports = remove;
 module.exports.validatePluginId = validatePluginId;
@@ -41,10 +42,14 @@ function remove (projectRoot, targets, hooksRunner, opts) {
     var plugins = cordova_util.findPlugins(pluginPath);
     var platformList = cordova_util.listPlatforms(projectRoot);
     var shouldRunPrepare = false;
+    var xml = cordova_util.projectConfig(projectRoot);
+    var cfg = new ConfigParser(xml);
 
     opts.cordova = { plugins: cordova_util.findPlugins(pluginPath) };
     return hooksRunner.fire('before_plugin_rm', opts)
         .then(function () {
+            var pluginInfoProvider = new PluginInfoProvider();
+            var cli_variables;
             return opts.plugins.reduce(function (soFar, target) {
                 var validatedPluginId = module.exports.validatePluginId(target, plugins);
                 if (!validatedPluginId) {
@@ -59,11 +64,15 @@ function remove (projectRoot, targets, hooksRunner, opts) {
                 return platformList.reduce(function (soFar, platform) {
                     return soFar.then(function () {
                         var platformRoot = path.join(projectRoot, 'platforms', platform);
+                        var directory = path.join(pluginPath, target);
+                        var pluginInfo = pluginInfoProvider.get(directory);
                         events.emit('verbose', 'Calling plugman.uninstall on plugin "' + target + '" for platform "' + platform + '"');
-                        var options = {
-                            force: opts.force || false
-                        };
-                        return plugman.uninstall.uninstallPlatform(platform, platformRoot, target, pluginPath, options)
+                        opts.force = opts.force || false;
+                        cli_variables = opts.cli_variables || {};
+
+                        plugin_util.mergeVariables(pluginInfo, cfg, opts);
+
+                        return plugman.uninstall.uninstallPlatform(platform, platformRoot, target, pluginPath, opts)
                             .then(function (didPrepare) {
                                 // If platform does not returned anything we'll need
                                 // to trigger a prepare after all plugins installed

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/295f29c0/src/cordova/plugin/util.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/util.js b/src/cordova/plugin/util.js
index 0da0fb7..d6bfeec 100644
--- a/src/cordova/plugin/util.js
+++ b/src/cordova/plugin/util.js
@@ -22,6 +22,7 @@ var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 
 module.exports.saveToConfigXmlOn = saveToConfigXmlOn;
 module.exports.getInstalledPlugins = getInstalledPlugins;
+module.exports.mergeVariables = mergeVariables;
 
 function getInstalledPlugins (projectRoot) {
     var pluginsDir = path.join(projectRoot, 'plugins');
@@ -35,3 +36,31 @@ function saveToConfigXmlOn (config_json, options) {
     var autosave = config_json.auto_save_plugins || false;
     return autosave || options.save;
 }
+
+// merges cli variables and config.xml (cfg) variables
+// used when adding and removing
+function mergeVariables (pluginInfo, cfg, opts) {
+    // Validate top-level required variables
+    var pluginVariables = pluginInfo.getPreferences();
+    opts.cli_variables = opts.cli_variables || {};
+    var pluginEntry = cfg.getPlugin(pluginInfo.id);
+    // Get variables from config.xml
+    var configVariables = pluginEntry ? pluginEntry.variables : {};
+    // Add config variable if it's missing in cli_variables
+    Object.keys(configVariables).forEach(function (variable) {
+        opts.cli_variables[variable] = opts.cli_variables[variable] || configVariables[variable];
+    });
+    var missingVariables = Object.keys(pluginVariables)
+        .filter(function (variableName) {
+            // discard variables with default value
+            return !(pluginVariables[variableName] || opts.cli_variables[variableName]);
+        });
+
+    if (missingVariables.length) {
+        events.emit('verbose', 'Removing ' + pluginInfo.dir + ' because mandatory plugin variables were missing.');
+        shell.rm('-rf', pluginInfo.dir);
+        var msg = 'Variable(s) missing (use: --variable ' + missingVariables.join('=value --variable ') + '=value).';
+        return Q.reject(new CordovaError(msg));
+    }
+
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


[4/4] cordova-lib git commit: fixed failing e2e-test due to browser release

Posted by st...@apache.org.
fixed failing e2e-test due to browser release


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/ad31610f
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/ad31610f
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/ad31610f

Branch: refs/heads/master
Commit: ad31610fac93d824ed8fc25761e947fb52894691
Parents: e5b8a3c
Author: Steve Gill <st...@gmail.com>
Authored: Tue Aug 29 23:10:02 2017 -0700
Committer: Steve Gill <st...@gmail.com>
Committed: Tue Aug 29 23:10:02 2017 -0700

----------------------------------------------------------------------
 integration-tests/platform.spec.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/ad31610f/integration-tests/platform.spec.js
----------------------------------------------------------------------
diff --git a/integration-tests/platform.spec.js b/integration-tests/platform.spec.js
index 82f6711..a5d2a93 100644
--- a/integration-tests/platform.spec.js
+++ b/integration-tests/platform.spec.js
@@ -187,7 +187,7 @@ describe('platform add plugin rm end-to-end', function () {
         cordova.create('hello')
             .then(function () {
                 process.chdir(project);
-                return cordova.platform('add', 'browser@latest');
+                return cordova.platform('add', 'browser@latest', {'fetch': true});
             })
             .then(function () {
                 return cordova.plugin('add', 'cordova-plugin-media');


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


[2/4] cordova-lib git commit: CB-13145 : added variable-merge.js to deal with plugin.xml variables for uninstall

Posted by st...@apache.org.
CB-13145 : added variable-merge.js to deal with plugin.xml variables for uninstall


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/263502da
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/263502da
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/263502da

Branch: refs/heads/master
Commit: 263502dae6f362b547f98c2a665d656874b56c26
Parents: 295f29c
Author: Audrey So <au...@apache.org>
Authored: Wed Aug 23 13:27:45 2017 -0700
Committer: Steve Gill <st...@gmail.com>
Committed: Tue Aug 29 22:32:09 2017 -0700

----------------------------------------------------------------------
 src/cordova/plugin/add.js     | 11 ++++---
 src/cordova/plugin/remove.js  | 10 +++---
 src/cordova/plugin/util.js    | 18 +++++++++--
 src/plugman/install.js        | 23 ++------------
 src/plugman/uninstall.js      |  5 +++
 src/plugman/variable-merge.js | 63 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/cordova/plugin/add.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/add.js b/src/cordova/plugin/add.js
index e6f8283..66bec9c 100644
--- a/src/cordova/plugin/add.js
+++ b/src/cordova/plugin/add.js
@@ -29,7 +29,6 @@ var ConfigParser = require('cordova-common').ConfigParser;
 var CordovaError = require('cordova-common').CordovaError;
 var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 var events = require('cordova-common').events;
-var shell = require('shelljs');
 var Q = require('q');
 var path = require('path');
 var fs = require('fs');
@@ -50,7 +49,7 @@ function add (projectRoot, hooksRunner, opts) {
     if (!opts.plugins || !opts.plugins.length) {
         return Q.reject(new CordovaError('No plugin specified. Please specify a plugin to add. See `' + cordova_util.binname + ' plugin search`.'));
     }
-
+    var pluginInfo;
     var shouldRunPrepare = false;
     var pluginPath = path.join(projectRoot, 'plugins');
     var platformList = cordova_util.listPlatforms(projectRoot);
@@ -98,9 +97,11 @@ function add (projectRoot, hooksRunner, opts) {
                     });
                 }).then(function (directory) {
                     return pluginInfoProvider.get(directory);
-                }).then(function (pluginInfo) {
-                    
-                    plugin_util.mergeVariables(pluginInfo, cfg, opts);
+                }).then(function (plugInfoProvider) {
+                    pluginInfo = plugInfoProvider;
+                    return plugin_util.mergeVariables(pluginInfo, cfg, opts);
+                }).then(function (variables) {
+                    opts.cli_variables = variables;
 
                     // Iterate (in serial!) over all platforms in the project and install the plugin.
                     return chainMap(platformList, function (platform) {

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/cordova/plugin/remove.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/remove.js b/src/cordova/plugin/remove.js
index 76ad091..6f45dbd 100644
--- a/src/cordova/plugin/remove.js
+++ b/src/cordova/plugin/remove.js
@@ -49,7 +49,7 @@ function remove (projectRoot, targets, hooksRunner, opts) {
     return hooksRunner.fire('before_plugin_rm', opts)
         .then(function () {
             var pluginInfoProvider = new PluginInfoProvider();
-            var cli_variables;
+            var platformRoot;
             return opts.plugins.reduce(function (soFar, target) {
                 var validatedPluginId = module.exports.validatePluginId(target, plugins);
                 if (!validatedPluginId) {
@@ -63,15 +63,15 @@ function remove (projectRoot, targets, hooksRunner, opts) {
                 // reference from the platform's plugin config JSON.
                 return platformList.reduce(function (soFar, platform) {
                     return soFar.then(function () {
-                        var platformRoot = path.join(projectRoot, 'platforms', platform);
+                        platformRoot = path.join(projectRoot, 'platforms', platform);
                         var directory = path.join(pluginPath, target);
                         var pluginInfo = pluginInfoProvider.get(directory);
                         events.emit('verbose', 'Calling plugman.uninstall on plugin "' + target + '" for platform "' + platform + '"');
                         opts.force = opts.force || false;
-                        cli_variables = opts.cli_variables || {};
-
-                        plugin_util.mergeVariables(pluginInfo, cfg, opts);
 
+                        return plugin_util.mergeVariables(pluginInfo, cfg, opts);
+                    }).then(function (variables) {
+                        opts.cli_variables = variables;
                         return plugman.uninstall.uninstallPlatform(platform, platformRoot, target, pluginPath, opts)
                             .then(function (didPrepare) {
                                 // If platform does not returned anything we'll need

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/cordova/plugin/util.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/util.js b/src/cordova/plugin/util.js
index d6bfeec..aaf7068 100644
--- a/src/cordova/plugin/util.js
+++ b/src/cordova/plugin/util.js
@@ -19,6 +19,10 @@
 
 var path = require('path');
 var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
+var shell = require('shelljs');
+var events = require('cordova-common').events;
+var Q = require('q');
+var CordovaError = require('cordova-common').CordovaError;
 
 module.exports.saveToConfigXmlOn = saveToConfigXmlOn;
 module.exports.getInstalledPlugins = getInstalledPlugins;
@@ -37,8 +41,16 @@ function saveToConfigXmlOn (config_json, options) {
     return autosave || options.save;
 }
 
-// merges cli variables and config.xml (cfg) variables
-// used when adding and removing
+/*
+ * Merges cli and config.xml variables.
+ *
+ * @param   {object}    pluginInfo
+ * @param   {object}    config.xml
+ * @param   {object}    options
+ *
+ * @return  {object}    object containing the new merged variables
+ */
+
 function mergeVariables (pluginInfo, cfg, opts) {
     // Validate top-level required variables
     var pluginVariables = pluginInfo.getPreferences();
@@ -62,5 +74,5 @@ function mergeVariables (pluginInfo, cfg, opts) {
         var msg = 'Variable(s) missing (use: --variable ' + missingVariables.join('=value --variable ') + '=value).';
         return Q.reject(new CordovaError(msg));
     }
-
+    return opts.cli_variables;
 }

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/plugman/install.js
----------------------------------------------------------------------
diff --git a/src/plugman/install.js b/src/plugman/install.js
index c70a99c..51c6baf 100644
--- a/src/plugman/install.js
+++ b/src/plugman/install.js
@@ -39,6 +39,7 @@ var cordovaUtil = require('../cordova/util');
 var superspawn = require('cordova-common').superspawn;
 var PluginInfo = require('cordova-common').PluginInfo;
 var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
+var variableMerge = require('../plugman/variable-merge');
 
 /* INSTALL FLOW
    ------------
@@ -311,27 +312,7 @@ function runInstall (actions, platform, project_dir, plugin_dir, plugins_dir, op
     }).then(function (engines) {
         return checkEngines(engines);
     }).then(function () {
-        var prefs = pluginInfo.getPreferences(platform);
-        var keys = underscore.keys(prefs);
-
-        options.cli_variables = options.cli_variables || {};
-        var missing_vars = underscore.difference(keys, Object.keys(options.cli_variables));
-
-        underscore.each(missing_vars, function (_key) {
-            var def = prefs[_key];
-            if (def) {
-                options.cli_variables[_key] = def;
-            }
-        });
-
-        // test missing vars once again after having default
-        missing_vars = underscore.difference(keys, Object.keys(options.cli_variables));
-
-        if (missing_vars.length > 0) {
-            throw new Error('Variable(s) missing: ' + missing_vars.join(', '));
-        }
-
-        filtered_variables = underscore.pick(options.cli_variables, keys);
+        filtered_variables = variableMerge.mergeVariables(plugin_dir, platform, options);
         install.filtered_variables = filtered_variables;
 
         // Check for dependencies

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/plugman/uninstall.js
----------------------------------------------------------------------
diff --git a/src/plugman/uninstall.js b/src/plugman/uninstall.js
index 7e6874b..562636e 100644
--- a/src/plugman/uninstall.js
+++ b/src/plugman/uninstall.js
@@ -36,6 +36,7 @@ var npmUninstall = require('cordova-fetch').uninstall;
 var superspawn = require('cordova-common').superspawn;
 var PlatformJson = require('cordova-common').PlatformJson;
 var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
+var variableMerge = require('../plugman/variable-merge');
 
 // possible options: cli_variables, www_dir
 // Returns a promise.
@@ -243,6 +244,9 @@ function runUninstallPlatform (actions, platform, project_dir, plugin_dir, plugi
     var pluginInfo = pluginInfoProvider.get(plugin_dir);
     var plugin_id = pluginInfo.id;
 
+    // Merge cli_variables and plugin.xml variables
+    var variables = variableMerge.mergeVariables(plugin_dir, platform, options); // eslint-disable-line
+
     // Deps info can be passed recusively
     var platformJson = PlatformJson.load(plugins_dir, platform);
     var depsInfo = options.depsInfo || dependencies.generateDependencyInfo(platformJson, plugins_dir, pluginInfoProvider);
@@ -300,6 +304,7 @@ function runUninstallPlatform (actions, platform, project_dir, plugin_dir, plugi
         // platform inside of the existing CLI project. This option is usually set by cordova-lib for CLI projects
         // but since we're running this code through plugman, we need to set it here implicitly
         options.usePlatformWww = true;
+        options.cli_variables = variables;
 
         var hooksRunner = new HooksRunner(projectRoot);
 

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/263502da/src/plugman/variable-merge.js
----------------------------------------------------------------------
diff --git a/src/plugman/variable-merge.js b/src/plugman/variable-merge.js
new file mode 100644
index 0000000..ea01722
--- /dev/null
+++ b/src/plugman/variable-merge.js
@@ -0,0 +1,63 @@
+/**
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+*/
+
+var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
+var underscore = require('underscore');
+
+module.exports.mergeVariables = mergeVariables;
+
+/*
+ * At this point, cli and config vars have already merged.
+ * Merges those vars (cli and config) with plugin.xml variables.
+ *
+ * @param   {string}    plugin directory
+ * @param   {string}    platform
+ * @param   {object}    options
+ *
+ * @return  {object}    list of filtered variables
+ */
+function mergeVariables (plugin_dir, platform, options) {
+    options.pluginInfoProvider = options.pluginInfoProvider || new PluginInfoProvider();
+    var pluginInfoProvider = options.pluginInfoProvider;
+    var pluginInfo = pluginInfoProvider.get(plugin_dir);
+    var filtered_variables = {};
+
+    var prefs = pluginInfo.getPreferences(platform);
+    var keys = underscore.keys(prefs);
+
+    options.cli_variables = options.cli_variables || {};
+    var missing_vars = underscore.difference(keys, Object.keys(options.cli_variables));
+
+    underscore.each(missing_vars, function (_key) {
+        var def = prefs[_key];
+        if (def) {
+            options.cli_variables[_key] = def;
+        }
+    });
+
+    // test missing vars once again after having default
+    missing_vars = underscore.difference(keys, Object.keys(options.cli_variables));
+
+    if (missing_vars.length > 0) {
+        throw new Error('Variable(s) missing: ' + missing_vars.join(', '));
+    }
+
+    filtered_variables = underscore.pick(options.cli_variables, keys);
+    return filtered_variables;
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


[3/4] cordova-lib git commit: CB-13145 : added unit tests for mergeVariables from util.js and variable-merge.js and updated after review

Posted by st...@apache.org.
CB-13145 : added unit tests for mergeVariables from util.js and variable-merge.js and updated after review


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/e5b8a3c3
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/e5b8a3c3
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/e5b8a3c3

Branch: refs/heads/master
Commit: e5b8a3c3cf73656552cc25bc0f3a315ac6230521
Parents: 263502d
Author: Audrey So <au...@apache.org>
Authored: Thu Aug 24 14:54:22 2017 -0700
Committer: Steve Gill <st...@gmail.com>
Committed: Tue Aug 29 22:33:11 2017 -0700

----------------------------------------------------------------------
 integration-tests/plugin.spec.js    |  2 +
 spec/cordova/plugin/remove.spec.js  | 34 +++++++++++++----
 spec/cordova/plugin/util.spec.js    | 53 ++++++++++++++++++++++++++-
 spec/plugman/variable-merge.spec.js | 63 ++++++++++++++++++++++++++++++++
 src/cordova/plugin/util.js          |  3 +-
 src/plugman/init-defaults.js        |  1 -
 6 files changed, 145 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/integration-tests/plugin.spec.js
----------------------------------------------------------------------
diff --git a/integration-tests/plugin.spec.js b/integration-tests/plugin.spec.js
index a19f8c7..616e9e2 100644
--- a/integration-tests/plugin.spec.js
+++ b/integration-tests/plugin.spec.js
@@ -169,6 +169,8 @@ describe('plugin end-to-end', function () {
     }, 30000);
 
     it('Test 005 : should respect preference default values', function (done) {
+        var plugin_util = require('../src/cordova/plugin/util');
+        spyOn(plugin_util, 'mergeVariables').and.returnValue({ REQUIRED: 'NO', REQUIRED_ANDROID: 'NO' });
         addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED: 'NO', REQUIRED_ANDROID: 'NO' }}, done)
             .then(function () {
                 var platformJsonPath = path.join(project, 'plugins', helpers.testPlatform + '.json');

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/cordova/plugin/remove.spec.js
----------------------------------------------------------------------
diff --git a/spec/cordova/plugin/remove.spec.js b/spec/cordova/plugin/remove.spec.js
index b8e6720..6721b30 100644
--- a/spec/cordova/plugin/remove.spec.js
+++ b/spec/cordova/plugin/remove.spec.js
@@ -20,9 +20,8 @@
 /* eslint-env jasmine */
 /* globals fail */
 
-var remove = require('../../../src/cordova/plugin/remove');
 var rewire = require('rewire');
-var platform_remove = rewire('../../../src/cordova/plugin/remove');
+var remove = rewire('../../../src/cordova/plugin/remove');
 var Q = require('q');
 var cordova_util = require('../../../src/cordova/util');
 var metadata = require('../../../src/plugman/util/metadata');
@@ -37,8 +36,11 @@ describe('cordova/plugin/remove', function () {
     var projectRoot = '/some/path';
     var hook_mock;
     var cfg_parser_mock = function () {};
-    var cfg_parser_revert_mock; // eslint-disable-line no-unused-vars
+    var cfg_parser_revert_mock;
     var package_json_mock;
+    var plugin_info_provider_mock = function () {};
+    var plugin_info_provider_revert_mock;
+    var plugin_info;
     package_json_mock = jasmine.createSpyObj('package json mock', ['cordova', 'dependencies']);
     package_json_mock.dependencies = {};
     package_json_mock.cordova = {};
@@ -55,7 +57,18 @@ describe('cordova/plugin/remove', function () {
         spyOn(prepare, 'preparePlatforms').and.returnValue(true);
         hook_mock.fire.and.returnValue(Q());
         cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write', 'removeEngine', 'addEngine', 'getHookScripts', 'removePlugin']);
-        cfg_parser_revert_mock = platform_remove.__set__('ConfigParser', cfg_parser_mock);
+        cfg_parser_revert_mock = remove.__set__('ConfigParser', cfg_parser_mock);
+        plugin_info_provider_mock.prototype = jasmine.createSpyObj('plugin info provider mock', ['get', 'getPreferences']);
+        plugin_info_provider_mock.prototype.get = function (directory) {
+            // id version dir getPreferences() engines engines.cordovaDependencies name versions
+            return plugin_info;
+        };
+        plugin_info_provider_revert_mock = remove.__set__('PluginInfoProvider', plugin_info_provider_mock);
+    });
+
+    afterEach(function () {
+        cfg_parser_revert_mock();
+        plugin_info_provider_revert_mock();
     });
 
     describe('error/warning conditions', function () {
@@ -88,6 +101,7 @@ describe('cordova/plugin/remove', function () {
         });
 
         it('should call plugman.uninstall.uninstallPlatform for each platform installed in the project and for each provided plugin', function (done) {
+            spyOn(plugin_util, 'mergeVariables');
             remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
             var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};
             remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
@@ -101,6 +115,7 @@ describe('cordova/plugin/remove', function () {
         });
 
         it('should trigger a prepare if plugman.uninstall.uninstallPlatform returned something falsy', function (done) {
+            spyOn(plugin_util, 'mergeVariables');
             remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
             plugman.uninstall.uninstallPlatform.and.returnValue(Q(false));
             var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};
@@ -113,6 +128,7 @@ describe('cordova/plugin/remove', function () {
         });
 
         it('should call plugman.uninstall.uninstallPlugin once plugin has been uninstalled for each platform', function (done) {
+            spyOn(plugin_util, 'mergeVariables');
             remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
             var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};
             remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
@@ -125,7 +141,7 @@ describe('cordova/plugin/remove', function () {
 
         describe('when save option is provided or autosave config is on', function () {
             beforeEach(function () {
-                spyOn(platform_remove, 'validatePluginId').and.returnValue('cordova-plugin-splashscreen');
+                spyOn(plugin_util, 'mergeVariables');
                 spyOn(plugin_util, 'saveToConfigXmlOn').and.returnValue(true);
                 spyOn(config, 'read').and.returnValue(true);
                 spyOn(cordova_util, 'projectConfig').and.returnValue('config.xml');
@@ -136,8 +152,9 @@ describe('cordova/plugin/remove', function () {
             it('should remove provided plugins from config.xml', function (done) {
                 spyOn(cordova_util, 'requireNoCache').and.returnValue(true);
                 fs.existsSync.and.returnValue(true);
+                remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
                 var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};
-                platform_remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
+                remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
                     expect(cfg_parser_mock.prototype.removePlugin).toHaveBeenCalled();
                     expect(cfg_parser_mock.prototype.write).toHaveBeenCalled();
                     expect(events.emit).toHaveBeenCalledWith('log', jasmine.stringMatching('Removing plugin cordova-plugin-splashscreen from config.xml file'));
@@ -149,9 +166,10 @@ describe('cordova/plugin/remove', function () {
 
             it('should remove provided plugins from package.json (if exists)', function (done) {
                 spyOn(cordova_util, 'requireNoCache').and.returnValue(package_json_mock);
+                remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
                 fs.existsSync.and.returnValue(true);
                 var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};
-                platform_remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
+                remove(projectRoot, 'cordova-plugin-splashscreen', hook_mock, opts).then(function () {
                     expect(fs.writeFileSync).toHaveBeenCalled();
                     expect(events.emit).toHaveBeenCalledWith('log', jasmine.stringMatching('Removing cordova-plugin-splashscreen from package.json'));
                 }).fail(function (e) {
@@ -162,6 +180,8 @@ describe('cordova/plugin/remove', function () {
         });
 
         it('should remove fetch metadata from fetch.json', function (done) {
+            plugin_info_provider_mock.prototype.getPreferences.and.returnValue(true);
+            spyOn(plugin_util, 'mergeVariables');
             spyOn(metadata, 'remove_fetch_metadata').and.callThrough();
             remove.validatePluginId.and.returnValue('cordova-plugin-splashscreen');
             var opts = {important: 'options', plugins: ['cordova-plugin-splashscreen']};

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/cordova/plugin/util.spec.js
----------------------------------------------------------------------
diff --git a/spec/cordova/plugin/util.spec.js b/spec/cordova/plugin/util.spec.js
index 7a7c032..47d0667 100644
--- a/spec/cordova/plugin/util.spec.js
+++ b/spec/cordova/plugin/util.spec.js
@@ -20,16 +20,25 @@
 
 var rewire = require('rewire');
 var plugin_util = rewire('../../../src/cordova/plugin/util');
+var shell = require('shelljs');
+var events = require('cordova-common').events;
 
 describe('cordova/plugin/util', function () {
     var plugin_info_mock = function () {};
     var plugin_info_revert_mock;
+    var cfg_parser_revert_mock;
+    var cfg_parser_mock = function () {};
     beforeEach(function () {
-        plugin_info_mock.prototype = jasmine.createSpyObj('plugin info provider prototype mock', ['getAllWithinSearchPath']);
+        spyOn(shell, 'rm');
+        spyOn(events, 'emit');
+        cfg_parser_mock.prototype = jasmine.createSpyObj('config parser protytpe mock', ['getPlugin']);
+        cfg_parser_revert_mock = plugin_util.__set__('ConfigParser', cfg_parser_mock);
+        plugin_info_mock.prototype = jasmine.createSpyObj('plugin info provider prototype mock', ['getAllWithinSearchPath', 'getPreferences']);
         plugin_info_revert_mock = plugin_util.__set__('PluginInfoProvider', plugin_info_mock);
     });
     afterEach(function () {
         plugin_info_revert_mock();
+        cfg_parser_revert_mock();
     });
     describe('getInstalledPlugins helper method', function () {
         it('should return result of PluginInfoProvider\'s getAllWithinSearchPath method', function () {
@@ -50,4 +59,46 @@ describe('cordova/plugin/util', function () {
             })).toBe(true);
         });
     });
+    describe('mergeVariables happy path', function () {
+        it('should return variable from cli', function () {
+            cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined);
+            plugin_info_mock.prototype.getPreferences.and.returnValue({});
+            var opts = { cli_variables: { FCM_VERSION: '9.0.0' } };
+            expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '9.0.0'});
+        });
+        it('should return empty object if there are no config and no cli variables', function () {
+            cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined);
+            plugin_info_mock.prototype.getPreferences.and.returnValue({});
+            var opts = { cli_variables: {} };
+            expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({});
+        });
+        it('cli variable takes precedence over config.xml', function () {
+            cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined);
+            plugin_info_mock.prototype.getPreferences.and.returnValue({
+                name: 'phonegap-plugin-push',
+                spec: '/Users/auso/cordova/phonegap-plugin-push',
+                variables: { FCM_VERSION: '11.0.1' }
+            });
+            var opts = { cli_variables: {FCM_VERSION: '9.0.0'} };
+            expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '9.0.0'});
+        });
+        it('use config.xml variable if no cli variable is passed in', function () {
+            cfg_parser_mock.prototype.getPlugin.and.returnValue({
+                name: 'phonegap-plugin-push',
+                spec: '/Users/auso/cordova/phonegap-plugin-push',
+                variables: { FCM_VERSION: '11.0.1' }
+            });
+            plugin_info_mock.prototype.getPreferences.and.returnValue({});
+            var opts = { cli_variables: {} };
+            expect(plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts)).toEqual({FCM_VERSION: '11.0.1'});
+        });
+        it('should get missed variables', function () {
+            cfg_parser_mock.prototype.getPlugin.and.returnValue(undefined);
+            plugin_info_mock.prototype.getPreferences.and.returnValue({key: 'FCM_VERSION', value: undefined});
+            var opts = { cli_variables: {} };
+            expect(function () { plugin_util.mergeVariables(plugin_info_mock.prototype, cfg_parser_mock.prototype, opts); }).toThrow();
+            expect(shell.rm).toHaveBeenCalledWith('-rf', undefined);
+            expect(events.emit).toHaveBeenCalledWith('verbose', 'Removing undefined because mandatory plugin variables were missing.');
+        });
+    });
 });

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/spec/plugman/variable-merge.spec.js
----------------------------------------------------------------------
diff --git a/spec/plugman/variable-merge.spec.js b/spec/plugman/variable-merge.spec.js
new file mode 100644
index 0000000..d333ba1
--- /dev/null
+++ b/spec/plugman/variable-merge.spec.js
@@ -0,0 +1,63 @@
+/**
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+*/
+var rewire = require('rewire');
+var variable_merge = rewire('../../src/plugman/variable-merge');
+var underscore = require('underscore');
+
+describe('mergeVariables', function () {
+    var plugin_info_provider_mock = function () {};
+    var plugin_info_provider_revert_mock;
+    var plugin_info;
+
+    beforeEach(function () {
+        plugin_info = jasmine.createSpyObj('pluginInfo', ['getPreferences']);
+        plugin_info.dir = 'some\\plugin\\path';
+        plugin_info.id = 'cordova-plugin-device';
+        plugin_info.version = '1.0.0';
+        plugin_info_provider_mock.prototype = jasmine.createSpyObj('plugin info provider mock', ['get']);
+        plugin_info_provider_mock.prototype.get = function (directory) {
+            return plugin_info;
+        };
+        plugin_info_provider_revert_mock = variable_merge.__set__('PluginInfoProvider', plugin_info_provider_mock);
+    });
+    afterEach(function () {
+        plugin_info_provider_revert_mock();
+    });
+    it('use plugin.xml if no cli/config variables', function () {
+        plugin_info.getPreferences.and.returnValue({FCM_VERSION: '11.0.1'});
+        var opts = { cli_variables: { } };
+        expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({FCM_VERSION: '11.0.1'});
+    });
+    it('cli & config variables take precedence over plugin.xml ', function () {
+        plugin_info.getPreferences.and.returnValue({FCM_VERSION: '11.0.1'});
+        var opts = { cli_variables: {FCM_VERSION: '9.0.0'} };
+        expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({FCM_VERSION: '9.0.0'});
+    });
+    it('should return no variables', function () {
+        plugin_info.getPreferences.and.returnValue({});
+        var opts = { cli_variables: {} };
+        expect(variable_merge.mergeVariables('some/path', 'android', opts)).toEqual({});
+    });
+    it('should throw error if variables are missing', function () {
+        plugin_info.getPreferences.and.returnValue({});
+        spyOn(underscore, 'difference').and.returnValue(['missing variable']);
+        var opts = { cli_variables: {} };
+        expect(function () { variable_merge.mergeVariables('some/path', 'android', opts); }).toThrow();
+    });
+});

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/src/cordova/plugin/util.js
----------------------------------------------------------------------
diff --git a/src/cordova/plugin/util.js b/src/cordova/plugin/util.js
index aaf7068..6dfced9 100644
--- a/src/cordova/plugin/util.js
+++ b/src/cordova/plugin/util.js
@@ -21,7 +21,6 @@ var path = require('path');
 var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
 var shell = require('shelljs');
 var events = require('cordova-common').events;
-var Q = require('q');
 var CordovaError = require('cordova-common').CordovaError;
 
 module.exports.saveToConfigXmlOn = saveToConfigXmlOn;
@@ -72,7 +71,7 @@ function mergeVariables (pluginInfo, cfg, opts) {
         events.emit('verbose', 'Removing ' + pluginInfo.dir + ' because mandatory plugin variables were missing.');
         shell.rm('-rf', pluginInfo.dir);
         var msg = 'Variable(s) missing (use: --variable ' + missingVariables.join('=value --variable ') + '=value).';
-        return Q.reject(new CordovaError(msg));
+        throw new CordovaError(msg);
     }
     return opts.cli_variables;
 }

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/e5b8a3c3/src/plugman/init-defaults.js
----------------------------------------------------------------------
diff --git a/src/plugman/init-defaults.js b/src/plugman/init-defaults.js
index 361968c..f3c5ada 100644
--- a/src/plugman/init-defaults.js
+++ b/src/plugman/init-defaults.js
@@ -150,7 +150,6 @@ if (!pkg.author) {
         : prompt('author');
 }
 /* eslint-enable indent */
-
 var license = pkg.license ||
               defaults.license ||
               config.get('init.license') ||


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org