Updated Branches: refs/heads/master 648694b84 -> b3f21f377
Refactor install to have a single options parameter The number of parameters kept changing, and there are lots of call sites. Now the number and order is set and should stop moving. Project: http://git-wip-us.apache.org/repos/asf/cordova-plugman/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-plugman/commit/151b854d Tree: http://git-wip-us.apache.org/repos/asf/cordova-plugman/tree/151b854d Diff: http://git-wip-us.apache.org/repos/asf/cordova-plugman/diff/151b854d Branch: refs/heads/master Commit: 151b854d9d0cec0f466cbcc4790be7269c2c06db Parents: 648694b Author: Braden Shepherdson <[email protected]> Authored: Tue May 28 10:51:10 2013 -0400 Committer: Braden Shepherdson <[email protected]> Committed: Tue May 28 14:33:39 2013 -0400 ---------------------------------------------------------------------- main.js | 7 +++++- spec/install.spec.js | 32 +++++++++++++------------- spec/platforms/android.spec.js | 2 +- spec/uninstall.spec.js | 8 +++--- src/install.js | 43 ++++++++++++++++++++++++---------- 5 files changed, 57 insertions(+), 35 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/151b854d/main.js ---------------------------------------------------------------------- diff --git a/main.js b/main.js index 4da1150..4b2f656 100755 --- a/main.js +++ b/main.js @@ -74,7 +74,12 @@ else { if (/^[\w-_]+$/.test(key)) cli_variables[key] = tokens.join('='); }); } - plugman.install(cli_opts.platform, cli_opts.project, cli_opts.plugin, plugins_dir, '.', cli_variables, cli_opts.www); + var opts = { + subdir: '.', + cli_variables: cli_variables, + www_dir: cli_opts.www + }; + plugman.install(cli_opts.platform, cli_opts.project, cli_opts.plugin, plugins_dir, opts); } function printUsage() { http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/151b854d/spec/install.spec.js ---------------------------------------------------------------------- diff --git a/spec/install.spec.js b/spec/install.spec.js index 83c3811..4bda6a5 100644 --- a/spec/install.spec.js +++ b/spec/install.spec.js @@ -1,7 +1,7 @@ var install = require('../src/install'), android = require('../src/platforms/android'), - blackberry = require('../src/platforms/blackberry'), + blackberry = require('../src/platforms/blackberry10'), common = require('../src/platforms/common'), actions = require('../src/util/action-stack'), //ios = require('../src/platforms/ios'), @@ -44,7 +44,7 @@ describe('install', function() { describe('success', function() { it('should properly install assets', function() { var s = spyOn(common, 'copyFile').andCallThrough(); - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); // making sure the right methods were called expect(s).toHaveBeenCalled(); expect(s.calls.length).toEqual(3); @@ -66,7 +66,7 @@ describe('install', function() { shell.cp('-rf', dummyplugin, plugins_dir); shell.rm('-rf', path.join(plugins_dir, 'DummyPlugin', 'www', 'dummyplugin')); expect(function() { - install('android', temp, 'DummyPlugin', plugins_dir, '.', {}); + install('android', temp, 'DummyPlugin', plugins_dir, {}); }).toThrow(); // making sure the right methods were called expect(sCopyFile).toHaveBeenCalled(); @@ -83,7 +83,7 @@ describe('install', function() { it('should properly install assets into a custom www dir', function() { var s = spyOn(common, 'copyFile').andCallThrough(); - install('android', temp, dummyplugin, plugins_dir, '.', {}, path.join(temp, 'staging')); + install('android', temp, dummyplugin, plugins_dir, { www_dir: path.join(temp, 'staging') }); // making sure the right methods were called expect(s).toHaveBeenCalled(); expect(s.calls.length).toEqual(3); @@ -106,7 +106,7 @@ describe('install', function() { shell.cp('-rf', dummyplugin, plugins_dir); shell.rm('-rf', path.join(plugins_dir, 'dummyplugin', 'www', 'dummyplugin')); expect(function() { - install('android', temp, 'DummyPlugin', plugins_dir, '.', {}, path.join(temp, 'staging')); + install('android', temp, 'DummyPlugin', plugins_dir, { www_dir: path.join(temp, 'staging') }); }).toThrow(); // making sure the right methods were called expect(sCopyFile).toHaveBeenCalled(); @@ -123,26 +123,26 @@ describe('install', function() { it('should call prepare after a successful install', function() { var s = spyOn(plugman, 'prepare'); - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); expect(s).toHaveBeenCalled(); }); it('should call fetch if provided plugin cannot be resolved locally', function() { var s = spyOn(plugman, 'fetch'); - install('android', temp, 'CLEANYOURSHORTS', plugins_dir, '.', {}); + install('android', temp, 'CLEANYOURSHORTS', plugins_dir, {}); expect(s).toHaveBeenCalled(); }); it('should call the config-changes module\'s add_installed_plugin_to_prepare_queue method', function() { var spy = spyOn(config_changes, 'add_installed_plugin_to_prepare_queue'); - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); expect(spy).toHaveBeenCalledWith(plugins_dir, dummy_id, 'android', {}, true); }); it('should notify if plugin is already installed into project', function() { expect(function() { - install('android', temp, dummyplugin, plugins_dir,'.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); }).not.toThrow(); var spy = spyOn(console, 'log'); - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); expect(spy).toHaveBeenCalledWith('Plugin "com.phonegap.plugins.dummyplugin" already installed, \'sall good.'); }); @@ -153,7 +153,7 @@ describe('install', function() { shell.cp('-rf', dep_a, plugins_dir); shell.cp('-rf', dep_d, plugins_dir); shell.cp('-rf', dep_c, plugins_dir); - install('android', temp, 'A', plugins_dir, '.', {}); + install('android', temp, 'A', plugins_dir, {}); expect(spy.calls.length).toEqual(3); }); it('should fetch any dependent plugins if missing', function() { @@ -161,7 +161,7 @@ describe('install', function() { shell.mkdir('-p', plugins_dir); shell.cp('-rf', dep_a, plugins_dir); shell.cp('-rf', dep_c, plugins_dir); - install('android', temp, 'A', plugins_dir, '.', {}); + install('android', temp, 'A', plugins_dir, {}); expect(spy).toHaveBeenCalled(); }); }); @@ -178,7 +178,7 @@ describe('install', function() { var target = path.join(temp, 'assets', 'www', 'dummyplugin.js'); fs.writeFileSync(target, 'some bs', 'utf-8'); expect(function() { - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); }).toThrow(); }); it('should throw if platform is unrecognized', function() { @@ -188,7 +188,7 @@ describe('install', function() { }); it('should throw if variables are missing', function() { expect(function() { - install('android', temp, variableplugin, plugins_dir, '.', {}); + install('android', temp, variableplugin, plugins_dir, {}); }).toThrow('Variable(s) missing: API_KEY'); }); it('should throw if a file required for installation cannot be found', function() { @@ -197,7 +197,7 @@ describe('install', function() { shell.rm(path.join(plugins_dir, 'DummyPlugin', 'src', 'android', 'DummyPlugin.java')); expect(function() { - install('android', temp, 'DummyPlugin', plugins_dir, '.', {}); + install('android', temp, 'DummyPlugin', plugins_dir, {}); }).toThrow(); }); it('should pass error into specified callback if a file required for installation cannot be found', function(done) { @@ -205,7 +205,7 @@ describe('install', function() { shell.cp('-rf', dummyplugin, plugins_dir); shell.rm(path.join(plugins_dir, 'DummyPlugin', 'src', 'android', 'DummyPlugin.java')); - install('android', temp, 'DummyPlugin', plugins_dir, '.', {}, null, function(err) { + install('android', temp, 'DummyPlugin', plugins_dir, {}, function(err) { expect(err).toBeDefined(); done(); }); http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/151b854d/spec/platforms/android.spec.js ---------------------------------------------------------------------- diff --git a/spec/platforms/android.spec.js b/spec/platforms/android.spec.js index f65caa3..d665512 100644 --- a/spec/platforms/android.spec.js +++ b/spec/platforms/android.spec.js @@ -109,7 +109,7 @@ describe('android project handler', function() { describe('of <source-file> elements', function() { it('should remove stuff by calling common.deleteJava', function(done) { var s = spyOn(common, 'deleteJava'); - install('android', temp, dummyplugin, plugins_dir, '.', {}, undefined, function() { + install('android', temp, dummyplugin, plugins_dir, {}, function() { var source = copyArray(valid_source); android['source-file'].uninstall(source[0], temp); expect(s).toHaveBeenCalledWith(temp, 'src/com/phonegap/plugins/dummyplugin/DummyPlugin.java'); http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/151b854d/spec/uninstall.spec.js ---------------------------------------------------------------------- diff --git a/spec/uninstall.spec.js b/spec/uninstall.spec.js index c899e93..7e51bc1 100644 --- a/spec/uninstall.spec.js +++ b/spec/uninstall.spec.js @@ -41,7 +41,7 @@ describe('uninstall', function() { describe('success', function() { beforeEach(function() { - install('android', temp, dummyplugin, plugins_dir, '.', {}); + install('android', temp, dummyplugin, plugins_dir, {}); }); it('should properly uninstall assets', function() { var s = spyOn(common, 'removeFile').andCallThrough(); @@ -87,7 +87,7 @@ describe('uninstall', function() { shell.cp('-rf', dep_a, plugins_dir); shell.cp('-rf', dep_d, plugins_dir); shell.cp('-rf', dep_c, plugins_dir); - install('android', temp, 'A', plugins_dir, '.', {}); + install('android', temp, 'A', plugins_dir, {}); var spy = spyOn(actions.prototype, 'process').andCallThrough(); uninstall('android', temp, 'A', plugins_dir, {}); expect(spy.calls.length).toEqual(3); @@ -99,8 +99,8 @@ describe('uninstall', function() { shell.cp('-rf', dep_d, plugins_dir); shell.cp('-rf', dep_c, plugins_dir); shell.cp('-rf', dep_e, plugins_dir); - install('android', temp, 'A', plugins_dir, '.', {}); - install('android', temp, 'B', plugins_dir, '.', {}); + install('android', temp, 'A', plugins_dir, {}); + install('android', temp, 'B', plugins_dir, {}); var spy = spyOn(actions.prototype, 'process').andCallThrough(); uninstall('android', temp, 'A', plugins_dir, {}); expect(spy.calls.length).toEqual(2); http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/151b854d/src/install.js ---------------------------------------------------------------------- diff --git a/src/install.js b/src/install.js index 0c92701..dfb331d 100644 --- a/src/install.js +++ b/src/install.js @@ -9,7 +9,8 @@ var path = require('path'), config_changes = require('./util/config-changes'), platform_modules = require('./platforms'); -module.exports = function installPlugin(platform, project_dir, id, plugins_dir, subdir, cli_variables, www_dir, callback) { +// possible options: subdir, cli_variables, www_dir +module.exports = function installPlugin(platform, project_dir, id, plugins_dir, options, callback) { if (!platform_modules[platform]) { var err = new Error(platform + " not supported."); if (callback) callback(err); @@ -19,30 +20,33 @@ module.exports = function installPlugin(platform, project_dir, id, plugins_dir, var current_stack = new action_stack(); - possiblyFetch(current_stack, platform, project_dir, id, plugins_dir, subdir, cli_variables, www_dir, undefined /* git_ref */, true, callback); + options.is_top_level = true; + possiblyFetch(current_stack, platform, project_dir, id, plugins_dir, options, callback); }; -function possiblyFetch(actions, platform, project_dir, id, plugins_dir, subdir, cli_variables, www_dir, git_ref, is_top_level, callback) { +// possible options: subdir, cli_variables, www_dir, git_ref, is_top_level +function possiblyFetch(actions, platform, project_dir, id, plugins_dir, options, callback) { var plugin_dir = path.join(plugins_dir, id); // Check that the plugin has already been fetched. if (!fs.existsSync(plugin_dir)) { // if plugin doesnt exist, use fetch to get it. // TODO: Actual value for git_ref. - require('../plugman').fetch(id, plugins_dir, false, '.', git_ref, function(err, plugin_dir) { + require('../plugman').fetch(id, plugins_dir, false, '.', options.git_ref, function(err, plugin_dir) { if (err) { callback(err); } else { // update ref to plugin_dir after successful fetch, via fetch callback - runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli_variables, www_dir, is_top_level, callback); + runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, options, callback); } }); } else { - runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli_variables, www_dir, is_top_level, callback); + runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, options, callback); } } -function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli_variables, www_dir, is_top_level, callback) { +// possible options: cli_variables, www_dir, is_top_level +function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, options, callback) { var xml_path = path.join(plugin_dir, 'plugin.xml') , xml_text = fs.readFileSync(xml_path, 'utf-8') , plugin_et = new et.ElementTree(et.XML(xml_text)) @@ -109,10 +113,11 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli var missing_vars = []; prefs.forEach(function (pref) { var key = pref.attrib["name"].toUpperCase(); - if (cli_variables[key] == undefined) + options.cli_variables = options.cli_variables || {}; + if (options.cli_variables[key] == undefined) missing_vars.push(key) else - filtered_variables[key] = cli_variables[key] + filtered_variables[key] = options.cli_variables[key] }); if (missing_vars.length > 0) { var err = new Error('Variable(s) missing: ' + missing_vars.join(", ")); @@ -125,7 +130,7 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli var dependencies = plugin_et.findall('dependency'); if (dependencies && dependencies.length) { var end = n(dependencies.length, function() { - handleInstall(actions, plugin_id, plugin_et, platform, project_dir, plugins_dir, plugin_basename, plugin_dir, filtered_variables, www_dir, is_top_level, callback); + handleInstall(actions, plugin_id, plugin_et, platform, project_dir, plugins_dir, plugin_basename, plugin_dir, filtered_variables, options.www_dir, options.is_top_level, callback); }); dependencies.forEach(function(dep) { var dep_plugin_id = dep.attrib.id; @@ -138,10 +143,22 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli var dep_plugin_dir = path.join(plugins_dir, dep_plugin_id); if (fs.existsSync(dep_plugin_dir)) { console.log('Dependent plugin ' + dep_plugin_id + ' already fetched, using that version.'); - runInstall(actions, platform, project_dir, dep_plugin_dir, plugins_dir, filtered_variables, www_dir, false, end); + var opts = { + cli_variables: filtered_variables, + www_dir: options.www_dir, + is_top_level: false + }; + runInstall(actions, platform, project_dir, dep_plugin_dir, plugins_dir, opts, end); } else { console.log('Dependent plugin ' + dep_plugin_id + ' not fetched, retrieving then installing.'); - possiblyFetch(actions, platform, project_dir, dep_url, plugins_dir, dep_subdir, filtered_variables, www_dir, dep_git_ref, false, function(err) { + var opts = { + cli_variables: filtered_variables, + www_dir: options.www_dir, + is_top_level: false, + git_ref: dep_git_ref + }; + + possiblyFetch(actions, platform, project_dir, dep_url, plugins_dir, opts, function(err) { if (err) { if (callback) callback(err); else throw err; @@ -150,7 +167,7 @@ function runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, cli } }); } else { - handleInstall(actions, plugin_id, plugin_et, platform, project_dir, plugins_dir, plugin_basename, plugin_dir, filtered_variables, www_dir, is_top_level, callback); + handleInstall(actions, plugin_id, plugin_et, platform, project_dir, plugins_dir, plugin_basename, plugin_dir, filtered_variables, options.www_dir, options.is_top_level, callback); } }
