cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From agri...@apache.org
Subject [13/16] git commit: config-changes.js: Address review comments.
Date Wed, 19 Feb 2014 20:32:59 GMT
config-changes.js: Address review comments.

 - Use ConfigKeeper for plugin.xml
 - Don't write out platform JSON from get_platform_json.
 - Pretty print platform JSON.
 - Use "exports" instead of "package" and "module.exports".

Rebased on top of current master where another caching mechanism for iOS project files
was introduced in commit 1655ada. Keeping both of them for new because for now
ConfigKeeper doesn't help to speed up adding multiple plugins. Added a TODO
comment to remove caching from ios parser once multiple plugins can be
installed with a single instance of PlatformMunger.


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

Branch: refs/heads/master
Commit: e5fa81490cfa0d10e39f5133d8c56aff8fbac4e2
Parents: 8ba299e
Author: Mark Koudritsky <kamrik@chromium.org>
Authored: Fri Feb 14 10:53:31 2014 -0500
Committer: Andrew Grieve <agrieve@chromium.org>
Committed: Wed Feb 19 15:32:26 2014 -0500

----------------------------------------------------------------------
 spec/util/config-changes.spec.js |   6 +-
 src/platforms/ios.js             |  14 ++--
 src/util/config-changes.js       | 144 +++++++++++++++++++---------------
 3 files changed, 91 insertions(+), 73 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/e5fa8149/spec/util/config-changes.spec.js
----------------------------------------------------------------------
diff --git a/spec/util/config-changes.spec.js b/spec/util/config-changes.spec.js
index 10f0af5..3c3c918 100644
--- a/spec/util/config-changes.spec.js
+++ b/spec/util/config-changes.spec.js
@@ -108,15 +108,13 @@ describe('config-changes module', function() {
     });
 
     describe('get_platform_json method', function() {
-        it('should write out and return an empty config json file if it doesn\'t exist',
function() {
+        it('should return an empty config json object if file doesn\'t exist', function()
{
             var filepath = path.join(plugins_dir, 'android.json');
             var cfg = configChanges.get_platform_json(plugins_dir, 'android');
             expect(cfg).toBeDefined();
             expect(cfg.prepare_queue).toBeDefined();
             expect(cfg.config_munge).toBeDefined();
             expect(cfg.installed_plugins).toBeDefined();
-            expect(fs.existsSync(filepath)).toBe(true);
-            expect(fs.readFileSync(filepath, 'utf-8')).toEqual(JSON.stringify(cfg));
         });
         it('should return the json file if it exists', function() {
             var filepath = path.join(plugins_dir, 'android.json');
@@ -133,7 +131,7 @@ describe('config-changes module', function() {
             var cfg = {poop:true};
             configChanges.save_platform_json(cfg, plugins_dir, 'android');
             expect(fs.existsSync(filepath)).toBe(true);
-            expect(fs.readFileSync(filepath, 'utf-8')).toEqual(JSON.stringify(cfg));
+            expect(JSON.parse(fs.readFileSync(filepath, 'utf-8'))).toEqual(cfg);
         });
     });
 

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/e5fa8149/src/platforms/ios.js
----------------------------------------------------------------------
diff --git a/src/platforms/ios.js b/src/platforms/ios.js
index 8a059dd..7d540e4 100644
--- a/src/platforms/ios.js
+++ b/src/platforms/ios.js
@@ -72,9 +72,9 @@ module.exports = {
                 project.xcode.removeFromLibrarySearchPaths({path:project_ref});
             }
             shell.rm('-rf', destFile);
-            
+
             if(fs.existsSync(targetDir) && fs.readdirSync(targetDir).length>0){
-                shell.rm('-rf', targetDir); 
+                shell.rm('-rf', targetDir);
             }
         }
     },
@@ -97,7 +97,7 @@ module.exports = {
             project.xcode.removeHeaderFile(path.join('Plugins', path.relative(project.plugins_dir,
destFile)));
             shell.rm('-rf', destFile);
             if(fs.existsSync(targetDir) && fs.readdirSync(targetDir).length>0){
-                shell.rm('-rf', targetDir); 
+                shell.rm('-rf', targetDir);
             }
         }
     },
@@ -140,13 +140,17 @@ module.exports = {
         }
     },
     parseProjectFile:function(project_dir) {
+        // TODO: With ConfigKeeper introduced in config-changes.js
+        // there is now double caching of iOS project files.
+        // Remove the cache here when install can handle
+        // a list of plugins at once.
         if (cachedProjectFiles[project_dir]) {
             return cachedProjectFiles[project_dir];
         }
         // grab and parse pbxproj
         // we don't want CordovaLib's xcode project
         var project_files = glob.sync(path.join(project_dir, '*.xcodeproj', 'project.pbxproj'));
-        
+
         if (project_files.length === 0) {
             throw new Error("does not appear to be an xcode project (no xcode project file)");
         }
@@ -155,7 +159,7 @@ module.exports = {
         xcodeproj.parseSync();
 
         // grab and parse plist file or config.xml
-        var config_files = (glob.sync(path.join(project_dir, '**', '{PhoneGap,Cordova}.plist')).length
== 0 ? 
+        var config_files = (glob.sync(path.join(project_dir, '**', '{PhoneGap,Cordova}.plist')).length
== 0 ?
                             glob.sync(path.join(project_dir, '**', 'config.xml')) :
                             glob.sync(path.join(project_dir, '**', '{PhoneGap,Cordova}.plist'))
                            );

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/e5fa8149/src/util/config-changes.js
----------------------------------------------------------------------
diff --git a/src/util/config-changes.js b/src/util/config-changes.js
index 8e1796b..989a582 100644
--- a/src/util/config-changes.js
+++ b/src/util/config-changes.js
@@ -39,7 +39,6 @@ var fs   = require('fs'),
     bplist = require('bplist-parser'),
     xcode = require('xcode'),
     et   = require('elementtree'),
-    underscore = require('underscore'),
     xml_helpers = require('./../util/xml-helpers'),
     platforms = require('./../platforms'),
     events = require('./../events'),
@@ -55,60 +54,57 @@ var keep_these_frameworks = [
 ];
 
 
-var package = module.exports = {};
-
-package.PlatformMunger = PlatformMunger;
+exports.PlatformMunger = PlatformMunger;
 
 /******************************************************************************
 Adapters to keep the current refactoring effort to within this file
 ******************************************************************************/
-package.add_plugin_changes = function(platform, project_dir, plugins_dir, plugin_id, plugin_vars,
is_top_level, should_increment, cache) {
+exports.add_plugin_changes = function(platform, project_dir, plugins_dir, plugin_id, plugin_vars,
is_top_level, should_increment, cache) {
     var munger = new PlatformMunger(platform, project_dir, plugins_dir);
     munger.add_plugin_changes(plugin_id, plugin_vars, is_top_level, should_increment, cache);
     munger.save_all();
 };
 
-package.remove_plugin_changes = function(platform, project_dir, plugins_dir, plugin_name,
plugin_id, is_top_level, should_decrement) {
-    // TODO: should_decrement paramenter is never used, remove it here and wherever called
+exports.remove_plugin_changes = function(platform, project_dir, plugins_dir, plugin_name,
plugin_id, is_top_level, should_decrement) {
+    // TODO: should_decrement parameter is never used, remove it here and wherever called
     var munger = new PlatformMunger(platform, project_dir, plugins_dir);
     munger.remove_plugin_changes(plugin_name, plugin_id, is_top_level);
     munger.save_all();
 };
 
-package.process = function(plugins_dir, project_dir, platform) {
+exports.process = function(plugins_dir, project_dir, platform) {
     var munger = new PlatformMunger(platform, project_dir, plugins_dir);
     munger.process();
     munger.save_all();
 };
-
 /******************************************************************************/
 
 
-package.add_installed_plugin_to_prepare_queue = add_installed_plugin_to_prepare_queue;
+exports.add_installed_plugin_to_prepare_queue = add_installed_plugin_to_prepare_queue;
 function add_installed_plugin_to_prepare_queue(plugins_dir, plugin, platform, vars, is_top_level)
{
     checkPlatform(platform);
-    var config = module.exports.get_platform_json(plugins_dir, platform);
+    var config = exports.get_platform_json(plugins_dir, platform);
     config.prepare_queue.installed.push({'plugin':plugin, 'vars':vars, 'topLevel':is_top_level});
-    module.exports.save_platform_json(config, plugins_dir, platform);
+    exports.save_platform_json(config, plugins_dir, platform);
 }
 
-package.add_uninstalled_plugin_to_prepare_queue = add_uninstalled_plugin_to_prepare_queue;
+exports.add_uninstalled_plugin_to_prepare_queue = add_uninstalled_plugin_to_prepare_queue;
 function add_uninstalled_plugin_to_prepare_queue(plugins_dir, plugin, platform, is_top_level)
{
     checkPlatform(platform);
 
     var plugin_xml = xml_helpers.parseElementtreeSync(path.join(plugins_dir, plugin, 'plugin.xml'));
-    var config = module.exports.get_platform_json(plugins_dir, platform);
-    config.prepare_queue.uninstalled.push({'plugin':plugin, 'id':plugin_xml._root.attrib['id'],
'topLevel':is_top_level});
-    module.exports.save_platform_json(config, plugins_dir, platform);
+    var config = exports.get_platform_json(plugins_dir, platform);
+    config.prepare_queue.uninstalled.push({'plugin':plugin, 'id':plugin_xml.getroot().attrib['id'],
'topLevel':is_top_level});
+    exports.save_platform_json(config, plugins_dir, platform);
 }
 
 
 /******************************************************************************
 * PlatformMunger class
 *
-* Can deal with config file of a single porject.
+* Can deal with config file of a single project.
+* Parsed config files are cached in a ConfigKeeper object.
 ******************************************************************************/
-
 function PlatformMunger(platform, project_dir, plugins_dir) {
     checkPlatform(platform);
     this.platform = platform;
@@ -124,8 +120,8 @@ function PlatformMunger_save_all() {
     this.config_keeper.save_all();
 }
 
-// Deal with a single file munge.
-// Theoretically, since files are independent several of those can run in parallel.
+// Apply a munge object to a single config file.
+// The remove parameter tells whether to add the change or remove it.
 PlatformMunger.prototype.apply_file_munge = PlatformMunger_apply_file_munge;
 function PlatformMunger_apply_file_munge(file, munge, remove) {
     var self = this;
@@ -167,7 +163,7 @@ function PlatformMunger_apply_file_munge(file, munge, remove) {
 PlatformMunger.prototype.remove_plugin_changes = remove_plugin_changes;
 function remove_plugin_changes(plugin_name, plugin_id, is_top_level) {
     var self = this;
-    var platform_config = module.exports.get_platform_json(self.plugins_dir, self.platform);
+    var platform_config = exports.get_platform_json(self.plugins_dir, self.platform);
     var plugin_dir = path.join(self.plugins_dir, plugin_name);
     var plugin_vars = (is_top_level ? platform_config.installed_plugins[plugin_id] : platform_config.dependent_plugins[plugin_id]);
 
@@ -198,17 +194,18 @@ function remove_plugin_changes(plugin_name, plugin_id, is_top_level)
{
     }
 
     // save
-    module.exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
+    exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
 }
 
 
 PlatformMunger.prototype.add_plugin_changes = add_plugin_changes;
 function add_plugin_changes(plugin_id, plugin_vars, is_top_level, should_increment) {
     var self = this;
-    var platform_config = module.exports.get_platform_json(self.plugins_dir, self.platform);
+    var platform_config = exports.get_platform_json(self.plugins_dir, self.platform);
     var plugin_dir = path.join(self.plugins_dir, plugin_id);
 
-    plugin_id = xml_helpers.parseElementtreeSync(path.join(plugin_dir, 'plugin.xml'), 'utf-8')._root.attrib['id'];
+    var plugin_config = self.config_keeper.get(plugin_dir, '', 'plugin.xml');
+    plugin_id = plugin_config.data.getroot().attrib.id;
 
     // get config munge, aka how should this plugin change various config files
     var config_munge = self.generate_plugin_config_munge(plugin_dir, plugin_vars);
@@ -247,7 +244,7 @@ function add_plugin_changes(plugin_id, plugin_vars, is_top_level, should_increme
     }
 
     // save
-    module.exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
+    exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
 }
 
 
@@ -258,7 +255,7 @@ PlatformMunger.prototype.reapply_global_munge = reapply_global_munge ;
 function reapply_global_munge () {
     var self = this;
 
-    var platform_config = module.exports.get_platform_json(self.plugins_dir, self.platform);
+    var platform_config = exports.get_platform_json(self.plugins_dir, self.platform);
     var global_munge = platform_config.config_munge;
     for (var file in global_munge) {
         // TODO: remove this warning some time after 3.4 is out.
@@ -277,6 +274,7 @@ function reapply_global_munge () {
 
 
 // generate_plugin_config_munge
+// Generate the munge object from plugin.xml + vars
 PlatformMunger.prototype.generate_plugin_config_munge = generate_plugin_config_munge;
 function generate_plugin_config_munge(plugin_dir, vars) {
     var self = this;
@@ -288,7 +286,8 @@ function generate_plugin_config_munge(plugin_dir, vars) {
     }
 
     var munge = {};
-    var plugin_xml = xml_helpers.parseElementtreeSync(path.join(plugin_dir, 'plugin.xml'));
+    var plugin_config = self.config_keeper.get(plugin_dir, '', 'plugin.xml');
+    var plugin_xml = plugin_config.data;
 
     var platformTag = plugin_xml.find('platform[@name="' + self.platform + '"]');
     var changes = [];
@@ -351,13 +350,13 @@ function generate_plugin_config_munge(plugin_dir, vars) {
 }
 
 
-// Go over the prepare queue an apply the config munges for all plugins
-// that have been (un)installed.
+// Go over the prepare queue an apply the config munges for each plugin
+// that has been (un)installed.
 PlatformMunger.prototype.process = PlatformMunger_process;
 function PlatformMunger_process() {
     var self = this;
 
-    var platform_config = module.exports.get_platform_json(self.plugins_dir, self.platform);
+    var platform_config = exports.get_platform_json(self.plugins_dir, self.platform);
 
     // Uninstallation first
     platform_config.prepare_queue.uninstalled.forEach(function(u) {
@@ -369,25 +368,26 @@ function PlatformMunger_process() {
         self.add_plugin_changes(u.plugin, u.vars, u.topLevel, true);
     });
 
-    platform_config = module.exports.get_platform_json(self.plugins_dir, self.platform);
+    platform_config = exports.get_platform_json(self.plugins_dir, self.platform);
 
-    // Empty out uninstalled queue.
+    // Empty out installed/ uninstalled queues.
     platform_config.prepare_queue.uninstalled = [];
-    // Empty out installed queue.
     platform_config.prepare_queue.installed = [];
-    // save
-    module.exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
+    // save platform json
+    exports.save_platform_json(platform_config, self.plugins_dir, self.platform);
 }
-
 /**** END of PlatformMunger ****/
 
 
-
 /******************************************************************************
 * ConfigKeeper class
 *
-* Used to load and store config files to avoid reparsing
-* and writing them out multiple times.
+* Used to load and store config files to avoid re-parsing and writing them out
+* multiple times.
+*
+* The config files are referred to by a fake path constructed as
+* project_dir/platform/file
+* where file is the name used for the file in config munges.
 ******************************************************************************/
 function ConfigKeeper() {
     this._cached = {};
@@ -406,6 +406,7 @@ function ConfigKeeper_get(project_dir, platform, file) {
     return config_file;
 }
 
+
 ConfigKeeper.prototype.save_all = ConfigKeeper_save_all;
 function ConfigKeeper_save_all() {
     var self = this;
@@ -414,12 +415,12 @@ function ConfigKeeper_save_all() {
         if (config_file.is_changed) config_file.save();
     });
 }
+/**** END of ConfigKeeper ****/
 
-// TODO: move save/get_platform_json those to be part of ConfigKeeper
-// But they are used in many places in plugman and cordova-cli
-// and can save the file bypassing the ConfigKeeper's cache.
-// Must change in all those places as well.
-package.get_platform_json = get_platform_json;
+// TODO: move save/get_platform_json to be part of ConfigKeeper or ConfigFile
+// For now they are used in many places in plugman and cordova-cli and can
+// save the file bypassing the ConfigKeeper's cache.
+exports.get_platform_json = get_platform_json;
 function get_platform_json(plugins_dir, platform) {
     checkPlatform(platform);
 
@@ -433,17 +434,15 @@ function get_platform_json(plugins_dir, platform) {
             installed_plugins:{},
             dependent_plugins:{}
         };
-        fs.writeFileSync(filepath, JSON.stringify(config), 'utf-8');
         return config;
     }
 }
 
-package.save_platform_json = save_platform_json;
+exports.save_platform_json = save_platform_json;
 function save_platform_json(config, plugins_dir, platform) {
     checkPlatform(platform);
-
     var filepath = path.join(plugins_dir, platform + '.json');
-    fs.writeFileSync(filepath, JSON.stringify(config), 'utf-8');
+    fs.writeFileSync(filepath, JSON.stringify(config, null, 4), 'utf-8');
 }
 
 /**** END of ConfigKeeper ****/
@@ -451,14 +450,24 @@ function save_platform_json(config, plugins_dir, platform) {
 
 /******************************************************************************
 * ConfigFile class
+*
+* Can load and keep various types of config files. Provides some functionality
+* specific to some file types such as grafting XML children. In most cases it
+* should be instantiated by ConfigKeeper.
+*
+* For plugin.xml files use as:
+* plugin_config = self.config_keeper.get(plugin_dir, '', 'plugin.xml');
+*
+* TODO: Consider moving it out to a separate file and maybe partially with
+* overrides in platform handlers.
 ******************************************************************************/
 function ConfigFile(project_dir, platform, file_tag) {
     this.project_dir = project_dir;
     this.platform = platform;
     this.file_tag = file_tag;
+    this.is_changed = false;
 
     this.load();
-    this.is_changed = false;
 }
 
 // ConfigFile.load()
@@ -488,7 +497,7 @@ function ConfigFile_load() {
         // plist file
         self.type = 'plist';
         // TODO: isBinaryPlist() reads the file and then parse re-reads it again.
-        //       We always write out text plist, not bianray.
+        //       We always write out text plist, not binaray.
         //       Do we still need to support binary plist?
         //       If yes, use plist.parseStringSync() and read the file once.
         self.plist_module = (isBinaryPlist(filepath) ? bplist : plist);
@@ -575,30 +584,36 @@ function isBinaryPlist(filename) {
 }
 
 // Find out the real name of an iOS project
-// TODO: glob is slow, need a better way or caching, or avoid using.
-function getIOSProjectname(project_dir){
+// TODO: glob is slow, need a better way or caching, or avoid using more than once.
+function getIOSProjectname(project_dir) {
     var matches = glob.sync(path.join(project_dir, '*.xcodeproj'));
-    var iospath= project_dir; // TODO: Do we ever want to return project dir here? I wont
work in resolveConfigFilePath().
-    if (matches.length) {
+    var iospath;
+    if (matches.length === 1) {
         iospath = path.basename(matches[0],'.xcodeproj');
+    } else {
+        var msg;
+        if (matches.length === 0) {
+            msg = 'Does not appear to be an xcode project, no xcode project file in ' + project_dir;
+        }
+        else {
+            msg = 'There are multiple *.xcodeproj dirs in ' + project_dir;
+        }
+        throw new Error(msg);
     }
     return iospath;
 }
 
 // Some config-file target attributes are not qualified with a full leading directory, or
contain wildcards.
 // Resolve to a real path in this function.
-// TODO: some globs are very slow, try to get rid of as many of them as possible.
+// TODO: getIOSProjectname is slow because of glob, try to avoid calling it several times
per project.
 function resolveConfigFilePath(project_dir, platform, file) {
     var filepath = path.join(project_dir, file);
     var matches;
 
     // .pbxproj file
     if (file === 'framework') {
-        var project_files = glob.sync(path.join(project_dir, '*.xcodeproj', 'project.pbxproj'));
-        if (project_files.length === 0) {
-            throw new Error("does not appear to be an xcode project (no xcode project file)");
-        }
-        filepath = project_files[0];
+        var proj_name = getIOSProjectname(project_dir);
+        filepath = path.join(project_dir, proj_name + '.xcodeproj', 'project.pbxproj');
         return filepath;
     }
 
@@ -609,7 +624,8 @@ function resolveConfigFilePath(project_dir, platform, file) {
         return filepath;
     }
 
-    // special-case config.xml target that is just "config.xml". this should be resolved
to the real location of the file.
+    // special-case config.xml target that is just "config.xml". This should be resolved
to the real location of the file.
+    // TODO: move the logic that contains the locations of config.xml from cordova CLI into
plugman.
     if (file == 'config.xml') {
         if (platform == 'ubuntu') {
             filepath = path.join(project_dir, 'config.xml');
@@ -637,7 +653,7 @@ function resolveConfigFilePath(project_dir, platform, file) {
 // Increment obj[key1][key2]...[keyN] by val. If it
 // didn't exist, set it to val.
 function deep_add(obj, val, keys /* or key1, key2 .... */ ) {
-    if ( !underscore.isArray(keys) ) {
+    if ( !Array.isArray(keys) ) {
         keys = Array.prototype.slice.call(arguments, 2);
     }
     var k = keys[0];
@@ -677,8 +693,8 @@ function increment_munge(base, munge) {
 
 // Update the base munge object as
 // base[file][selector][child] -= base[file][selector][child]
-// nodes that reached zero value are removed from base and the
-// returned munge object.
+// nodes that reached zero value are removed from base and the returned munge
+// object.
 function decrement_munge(base, munge) {
     var zeroed = {};
 


Mime
View raw message