You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by br...@apache.org on 2013/09/26 20:57:13 UTC
git commit: [CB-4809]: Check that dependencies' IDs match the
tags
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 <br...@gmail.com>
Authored: Thu Sep 26 11:49:36 2013 -0400
Committer: Braden Shepherdson <br...@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);
});