cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bra...@apache.org
Subject git commit: [CB-4809]: Check that dependencies' IDs match the <dependency> tags
Date Thu, 26 Sep 2013 18:57:13 GMT
Updated Branches:
  refs/heads/master d873608f3 -> f240acc53


[CB-4809]: Check that dependencies' IDs match the <dependency> tags


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

Branch: refs/heads/master
Commit: f240acc532f93bdf16761836d9b7418373afebec
Parents: d873608
Author: Braden Shepherdson <braden.shepherdson@gmail.com>
Authored: Thu Sep 26 11:49:36 2013 -0400
Committer: Braden Shepherdson <braden.shepherdson@gmail.com>
Committed: Thu Sep 26 14:54:05 2013 -0400

----------------------------------------------------------------------
 spec/fetch.spec.js   |  44 +++++++++++++++++++-
 spec/install.spec.js |   4 +-
 src/fetch.js         |  27 +++++++++++-
 src/install.js       | 104 +++++++++++++++++++++++-----------------------
 4 files changed, 123 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/f240acc5/spec/fetch.spec.js
----------------------------------------------------------------------
diff --git a/spec/fetch.spec.js b/spec/fetch.spec.js
index b28815a..b2f6dac 100644
--- a/spec/fetch.spec.js
+++ b/spec/fetch.spec.js
@@ -49,9 +49,22 @@ describe('fetch', function() {
                 expect(sym).toHaveBeenCalledWith(test_plugin, path.join(temp, 'id'), 'dir');
             });
         });
+        it('should fail when the expected ID doesn\'t match', function(done) {
+            fetch(test_plugin, temp, { expected_id: 'wrongID' })
+            .then(function() {
+                expect('this call').toBe('fail');
+            }, function(err) {
+                expect(err).toEqual(new Error('Expected fetched plugin to have ID "wrongID"
but got "id".'));
+            }).fin(done);
+        });
+        it('should succeed when the expected ID is correct', function(done) {
+            wrapper(fetch(test_plugin, temp, { expected_id: 'id' }), done, function() {
+                expect(1).toBe(1);
+            });
+        });
     });
     describe('git plugins', function() {
-        var clone, save_metadata, done;
+        var clone, save_metadata, done, xml;
 
         function fetchPromise(f) {
             f.then(function() { done = true; }, function(err) { done = err; });
@@ -61,6 +74,9 @@ describe('fetch', function() {
             clone = spyOn(plugins, 'clonePluginGitRepo').andReturn(Q('somedir'));
             save_metadata = spyOn(metadata, 'save_fetch_metadata');
             done = false;
+            xml = spyOn(xml_helpers, 'parseElementtreeSync').andReturn({
+                getroot:function() { return {attrib:{id:'id'}};}
+            });
         });
         it('should call clonePluginGitRepo for https:// and git:// based urls', function()
{
             var url = "https://github.com/bobeast/GAPlugin.git";
@@ -144,6 +160,19 @@ describe('fetch', function() {
                 expect(done).toEqual(new Error('--link is not supported for git URLs'));
             });
         });
+        it('should fail when the expected ID doesn\'t match', function(done) {
+            fetch('https://github.com/bobeast/GAPlugin.git', temp, { expected_id: 'wrongID'
})
+            .then(function() {
+                expect('this call').toBe('fail');
+            }, function(err) {
+                expect(err).toEqual(new Error('Expected fetched plugin to have ID "wrongID"
but got "id".'));
+            }).fin(done);
+        });
+        it('should succeed when the expected ID is correct', function(done) {
+            wrapper(fetch('https://github.com/bobeast/GAPlugin.git', temp, { expected_id:
'id' }), done, function() {
+                expect(1).toBe(1);
+            });
+        });
     });
     describe('registry plugins', function() {
         var pluginId = 'dummyplugin', sFetch;
@@ -165,5 +194,18 @@ describe('fetch', function() {
                 expect(sFetch).toHaveBeenCalledWith([pluginId], 'plugman');
             });
         });
+        it('should fail when the expected ID doesn\'t match', function(done) {
+            fetch(pluginId, temp, { expected_id: 'wrongID' })
+            .then(function() {
+                expect('this call').toBe('fail');
+            }, function(err) {
+                expect(err).toEqual(new Error('Expected fetched plugin to have ID "wrongID"
but got "id".'));
+            }).fin(done);
+        });
+        it('should succeed when the expected ID is correct', function(done) {
+            wrapper(fetch(pluginId, temp, { expected_id: 'id' }), done, function() {
+                expect(1).toBe(1);
+            });
+        });
     });
 });

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/f240acc5/spec/install.spec.js
----------------------------------------------------------------------
diff --git a/spec/install.spec.js b/spec/install.spec.js
index 9214e61..459a42c 100644
--- a/spec/install.spec.js
+++ b/spec/install.spec.js
@@ -248,7 +248,7 @@ describe('install', function() {
                 });
                 waits(100);
                 runs(function() {
-                    expect(s).toHaveBeenCalledWith('C', deps_dir, { link: false, subdir:
undefined, git_ref: undefined, client: 'plugman' });
+                    expect(s).toHaveBeenCalledWith('C', deps_dir, { link: false, subdir:
undefined, git_ref: undefined, client: 'plugman', expected_id: 'C' });
                     expect(s.calls.length).toEqual(3);
                 });
             });
@@ -264,7 +264,7 @@ describe('install', function() {
                 });
                 waitsFor(function() { return done; }, 'promise never resolved', 500);
                 runs(function() {
-                    expect(s).toHaveBeenCalledWith('D', deps_dir, { link: false, subdir:
undefined, git_ref: undefined, client: 'plugman' });
+                    expect(s).toHaveBeenCalledWith('D', deps_dir, { link: false, subdir:
undefined, git_ref: undefined, client: 'plugman', expected_id: 'D' });
                     expect(s.calls.length).toEqual(2);
                 });
             });

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/f240acc5/src/fetch.js
----------------------------------------------------------------------
diff --git a/src/fetch.js b/src/fetch.js
index 727ff84..0d50187 100644
--- a/src/fetch.js
+++ b/src/fetch.js
@@ -8,7 +8,7 @@ var shell   = require('shelljs'),
     Q       = require('q'),
     registry = require('./registry/registry');
 // XXX: leave the require('../plugman') because jasmine shits itself if you declare it up
top
-// possible options: link, subdir, git_ref
+// possible options: link, subdir, git_ref, client, expected_id
 // Returns a promise.
 module.exports = function fetchPlugin(plugin_dir, plugins_dir, options) {
     require('../plugman').emit('log', 'Fetching plugin from "' + plugin_dir + '"...');
@@ -52,6 +52,9 @@ module.exports = function fetchPlugin(plugin_dir, plugins_dir, options)
{
 
             return plugins.clonePluginGitRepo(plugin_dir, plugins_dir, options.subdir, options.git_ref)
             .then(function(dir) {
+                return checkID(options, dir);
+            })
+            .then(function(dir) {
                 metadata.save_fetch_metadata(dir, data);
                 return dir;
             });
@@ -98,9 +101,29 @@ module.exports = function fetchPlugin(plugin_dir, plugins_dir, options)
{
             .then(function(dir) {
                 linkable = false;
                 return movePlugin(dir);
+            })
+            .then(function(dir) {
+                return checkID(options, dir);
             });
         } else {
-            return Q(movePlugin(plugin_dir));
+            return Q(movePlugin(plugin_dir))
+            .then(function(dir) {
+                return checkID(options, dir);
+            });
         }
     }
 };
+
+// Helper function for checking expected plugin IDs against reality.
+function checkID(options, dir) {
+    // Read the plugin.xml file and check the ID matches options.expected_id if set.
+    if (options.expected_id) {
+        var et = xml_helpers.parseElementtreeSync(path.join(dir, 'plugin.xml'));
+        if (et.getroot().attrib.id == options.expected_id) {
+            return dir;
+        } else {
+            return Q.reject(new Error('Expected Fetched plugin to have ID "' + options.expected_id
+ '" but got "' + et.getroot().attrib.id + '".'));
+        }
+    }
+    return dir;
+}

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/f240acc5/src/install.js
----------------------------------------------------------------------
diff --git a/src/install.js b/src/install.js
index bdb6662..ac70f82 100644
--- a/src/install.js
+++ b/src/install.js
@@ -51,7 +51,7 @@ function possiblyFetch(actions, platform, project_dir, id, plugins_dir,
options)
     // Check that the plugin has already been fetched.
     if (!fs.existsSync(plugin_dir)) {
         // if plugin doesnt exist, use fetch to get it.
-        return require('../plugman').raw.fetch(id, plugins_dir, { link: false, subdir: options.subdir,
git_ref: options.git_ref, client: 'plugman' })
+        return require('../plugman').raw.fetch(id, plugins_dir, { link: false, subdir: options.subdir,
git_ref: options.git_ref, client: 'plugman', expected_id: options.expected_id })
         .then(function(plugin_dir) {
             return runInstall(actions, platform, project_dir, plugin_dir, plugins_dir, options);
         });
@@ -221,58 +221,59 @@ var runInstall = module.exports.runInstall = function runInstall(actions,
platfo
         dependencies = dependencies.concat(plugin_et.findall('./platform[@name="'+platform+'"]/dependency'));
         if (dependencies && dependencies.length) {
             require('../plugman').emit('verbose', 'Dependencies detected, iterating through
them...');
-            return Q.all(dependencies.map(function(dep) {
-                var dep_plugin_id = dep.attrib.id;
-                var dep_subdir = dep.attrib.subdir;
-                var dep_url = dep.attrib.url;
-                var dep_git_ref = dep.attrib.commit;
-                if (dep_subdir) {
-                    dep_subdir = path.join.apply(null, dep_subdir.split('/'));
-                }
-
-                // Handle relative dependency paths by expanding and resolving them.
-                // The easy case of relative paths is to have a URL of '.' and a different
subdir.
-                // TODO: Implement the hard case of different repo URLs, rather than the
special case of
-                // same-repo-different-subdir.
-                var urlPromise;
-                if (dep_url == '.') {
-                    // Look up the parent plugin's fetch metadata and determine the correct
URL.
-                    var fetchdata = require('./util/metadata').get_fetch_metadata(plugin_dir);
-
-                    if (!fetchdata || !(fetchdata.source && fetchdata.source.type))
{
-                        return Q.reject(new Error('No fetch metadata found for ' + plugin_id
+ '. Cannot install relative dependencies.'));
+            var dep_plugin_id, dep_subdir, dep_git_ref;
+            return dependencies.reduce(function(soFar, dep) {
+                return soFar.then(function() {
+                    dep_plugin_id = dep.attrib.id;
+                    dep_subdir = dep.attrib.subdir;
+                    var dep_url = dep.attrib.url;
+                    dep_git_ref = dep.attrib.commit;
+                    if (dep_subdir) {
+                        dep_subdir = path.join.apply(null, dep_subdir.split('/'));
                     }
 
-                    // Now there are two cases here: local directory, and git URL.
-                    if (fetchdata.source.type === 'local') {
-                        dep_url = fetchdata.source.path;
-
-                        var d = Q.defer();
-                        child_process.exec('git rev-parse --show-toplevel', { cwd:dep_url
}, function(err, stdout, stderr) {
-                            if (err) {
-                                if (err.code == 128) {
-                                    return d.reject(new Error('Error: Plugin ' + plugin_id
+ ' is not in git repository. All plugins must be in a git repository.'));
-                                } else {
-                                    return d.reject(new Error('Error trying to locate git
repository for plugin.'));
+                    // Handle relative dependency paths by expanding and resolving them.
+                    // The easy case of relative paths is to have a URL of '.' and a different
subdir.
+                    // TODO: Implement the hard case of different repo URLs, rather than
the special case of
+                    // same-repo-different-subdir.
+                    if (dep_url == '.') {
+                        // Look up the parent plugin's fetch metadata and determine the correct
URL.
+                        var fetchdata = require('./util/metadata').get_fetch_metadata(plugin_dir);
+
+                        if (!fetchdata || !(fetchdata.source && fetchdata.source.type))
{
+                            return Q.reject(new Error('No fetch metadata found for ' + plugin_id
+ '. Cannot install relative dependencies.'));
+                        }
+
+                        // Now there are two cases here: local directory, and git URL.
+                        if (fetchdata.source.type === 'local') {
+                            dep_url = fetchdata.source.path;
+
+                            var d = Q.defer();
+                            child_process.exec('git rev-parse --show-toplevel', { cwd:dep_url
}, function(err, stdout, stderr) {
+                                if (err) {
+                                    if (err.code == 128) {
+                                        return d.reject(new Error('Error: Plugin ' + plugin_id
+ ' is not in git repository. All plugins must be in a git repository.'));
+                                    } else {
+                                        return d.reject(new Error('Error trying to locate
git repository for plugin.'));
+                                    }
                                 }
-                            }
-
-                            return d.resolve(stdout.trim());
-                        });
-                        urlPromise = d.promise.then(function(git_repo) {
-                            //Clear out the subdir since the url now contains it
-                            var url = path.join(git_repo, dep_subdir);
-                            dep_subdir = "";
-                            return url;
-                        });
-                    } else if (fetchdata.source.type === 'git') {
-                        urlPromise = Q(fetchdata.source.url);
-                    }
-                } else {
-                    urlPromise = Q(dep_url);
-                }
 
-                return urlPromise.then(function(dep_url) {
+                                return d.resolve(stdout.trim());
+                            });
+                            return d.promise.then(function(git_repo) {
+                                //Clear out the subdir since the url now contains it
+                                var url = path.join(git_repo, dep_subdir);
+                                dep_subdir = "";
+                                return url;
+                            });
+                        } else if (fetchdata.source.type === 'git') {
+                            return Q(fetchdata.source.url);
+                        }
+                    } else {
+                        return Q(dep_url);
+                    }
+                })
+                .then(function(dep_url) {
                     var dep_plugin_dir = path.join(plugins_dir, dep_plugin_id);
                     if (fs.existsSync(dep_plugin_dir)) {
                         require('../plugman').emit('verbose', 'Dependent plugin "' + dep_plugin_id
+ '" already fetched, using that version.');
@@ -289,7 +290,8 @@ var runInstall = module.exports.runInstall = function runInstall(actions,
platfo
                             www_dir: options.www_dir,
                             is_top_level: false,
                             subdir: dep_subdir,
-                            git_ref: dep_git_ref
+                            git_ref: dep_git_ref,
+                            expected_id: dep_plugin_id
                         };
 
                         // CB-4770: registry fetching
@@ -300,7 +302,7 @@ var runInstall = module.exports.runInstall = function runInstall(actions,
platfo
                         return possiblyFetch(actions, platform, project_dir, dep_url, plugins_dir,
opts);
                     }
                 });
-            }))
+            }, Q())
             .then(function() {
                 return handleInstall(actions, plugin_id, plugin_et, platform, project_dir,
plugins_dir, plugin_basename, plugin_dir, filtered_variables, options.www_dir, options.is_top_level);
             });


Mime
View raw message