From fd31f247a623cf466b684a621814d05d67858770 Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Thu, 11 Oct 2012 13:40:54 +0100 Subject: [PATCH 01/13] Unit of work implementation. A unit of work is a simple storage with write lock/unlock. The manager/package now share a unit of work instance. The unit of work is used to prevent shared dependencies from being cloned/copied "at the same time" fixing issue #81. The prune and version resolving algorithm was also not correct. It now resolves versions correctly, fixing issue #57. - Fix I/O errors caused by copying/clone repos simultaneously to the same dest. - Optimize the clone/copy step by avoiding it if the last resolved resource is the same. - Fix failing test (at least on my windows machine) - Add some more tests - Fix CS. --- bin/bower | 2 +- lib/commands/help.js | 4 +- lib/commands/install.js | 2 +- lib/commands/list.js | 16 ++-- lib/commands/uninstall.js | 2 +- lib/core/config.js | 4 +- lib/core/manager.js | 19 ++--- lib/core/package.js | 120 ++++++++++++++++++++++++----- lib/core/unit_work.js | 64 +++++++++++++++ lib/util/hogan-colors.js | 10 +-- lib/util/prune.js | 18 +++-- lib/util/read-json.js | 4 +- lib/util/save.js | 2 +- test/assets/project/component.json | 2 +- test/manager.js | 68 ++++++++++++++++ test/package.js | 2 +- 16 files changed, 279 insertions(+), 60 deletions(-) create mode 100644 lib/core/unit_work.js create mode 100644 test/manager.js diff --git a/bin/bower b/bin/bower index f491e48a..d12ff97e 100755 --- a/bin/bower +++ b/bin/bower @@ -37,4 +37,4 @@ bower.commands[bower.command || 'help'].line(input) .on('error', function (err) { if (options.verbose) throw err; else template('error', { message: err.message }).on('data', function (d) { console.log (d); }); - }) \ No newline at end of file + }); \ No newline at end of file diff --git a/lib/commands/help.js b/lib/commands/help.js index f4c6bc57..8f0776bb 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -23,10 +23,10 @@ module.exports = function (name) { var templateName = name ? 'help-' + name : 'help'; if (!name) context = { commands: Object.keys(commands).join(', ') }; - _.extend(context, config) + _.extend(context, config); template(templateName, context).on('data', emitter.emit.bind(emitter, 'end')); return emitter; -} +}; module.exports.line = function (argv) { var options = nopt({}, {}, argv); diff --git a/lib/commands/install.js b/lib/commands/install.js index 25f43cb6..b537887c 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -44,4 +44,4 @@ module.exports.line = function (argv) { if (options.help) return help('install'); return module.exports(paths, options); -}; \ No newline at end of file +}; diff --git a/lib/commands/list.js b/lib/commands/list.js index f5336ab9..bac409d0 100644 --- a/lib/commands/list.js +++ b/lib/commands/list.js @@ -24,7 +24,7 @@ var shorthand = { 'h': ['--help'] }; var optionTypes = { help: Boolean, paths: Boolean, map: Boolean }; var getTree = function (packages, subPackages, result) { - var result = result || {}; + result = result || {}; _.each(subPackages || packages, function (pkg, name) { @@ -50,10 +50,10 @@ var generatePath = function (name, main) { if (typeof main == 'string') { return path.join(config.directory, name, main); } else { - var main = main.map(function (main) { return generatePath(name, main); }); + main = main.map(function (main) { return generatePath(name, main); }); return main.length == 1 ? main[0] : main; } -} +}; var buildSource = function (pkg, shallow) { var result = {}; @@ -72,7 +72,7 @@ var buildSource = function (pkg, shallow) { } return result; -} +}; var shallowTree = function (packages, tree) { var result = {}; @@ -82,7 +82,7 @@ var shallowTree = function (packages, tree) { }); return result; -} +}; var deepTree = function (packages, tree) { @@ -91,7 +91,7 @@ var deepTree = function (packages, tree) { Object.keys(tree).forEach(function (packageName) { result[packageName] = {}; - result[packageName].source = buildSource(packages[packageName]) + result[packageName].source = buildSource(packages[packageName]); if (Object.keys(tree[packageName]).length) { result[packageName].dependencies = deepTree(packages, tree[packageName]); @@ -100,7 +100,7 @@ var deepTree = function (packages, tree) { }); return result; -} +}; var getNodes = function (packages, tree) { return Object.keys(tree).map(function (key) { @@ -125,7 +125,7 @@ var getNodes = function (packages, tree) { return template('tree-branch', { package: key, version: version, upgrade: upgrade }, true); } }); -} +}; var cliTree = function (emitter, packages, tree) { emitter.emit('data', archy({ diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js index 9b92b4ba..a16e73f1 100644 --- a/lib/commands/uninstall.js +++ b/lib/commands/uninstall.js @@ -77,5 +77,5 @@ module.exports.line = function (argv) { if (options.help) return help('uninstall'); var names = options.argv.remain.slice(1); - return module.exports(names, options) + return module.exports(names, options); }; \ No newline at end of file diff --git a/lib/core/config.js b/lib/core/config.js index 53c4cd9b..d61f28e5 100644 --- a/lib/core/config.js +++ b/lib/core/config.js @@ -1,4 +1,4 @@ module.exports = { - directory: 'components', + directory: 'components', json: 'component.json' -} \ No newline at end of file +}; \ No newline at end of file diff --git a/lib/core/manager.js b/lib/core/manager.js index a101b238..61fe62dd 100644 --- a/lib/core/manager.js +++ b/lib/core/manager.js @@ -13,14 +13,15 @@ // - end: fired when finished installing // ========================================== -var Package = require('./package'); -var config = require('./config'); -var prune = require('../util/prune'); -var events = require('events'); -var async = require('async'); -var path = require('path'); -var glob = require('glob'); -var fs = require('fs'); +var Package = require('./package'); +var UnitWork = require('./unit_work'); +var config = require('./config'); +var prune = require('../util/prune'); +var events = require('events'); +var async = require('async'); +var path = require('path'); +var glob = require('glob'); +var fs = require('fs'); // read local dependencies (with versions) // read json dependencies (resolving along the way into temp dir) @@ -31,6 +32,7 @@ var Manager = function (endpoints) { this.dependencies = {}; this.cwd = process.cwd(); this.endpoints = endpoints || []; + this.unitWork = new UnitWork; }; Manager.prototype = Object.create(events.EventEmitter.prototype); @@ -82,7 +84,6 @@ Manager.prototype.resolveEndpoints = function () { Manager.prototype.loadJSON = function () { var json = path.join(this.cwd, config.json); - fs.exists(json, function (exists) { if (!exists) return this.emit('error', new Error('Could not find local ' + config.json)); fs.readFile(json, 'utf8', function (err, json) { diff --git a/lib/core/package.js b/lib/core/package.js index 504abc7b..377f9b54 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -31,6 +31,7 @@ var config = require('./config'); var source = require('./source'); var template = require('../util/template'); var readJSON = require('../util/read-json'); +var UnitWork = require('./unit_work'); var temp = process.env.TMPDIR || process.env.TMP @@ -43,13 +44,14 @@ var home = (process.platform === "win32" var cache = process.platform === "win32" ? path.resolve(process.env.APPDATA || home || temp, "bower-cache") - : path.resolve(home || temp, ".bower") + : path.resolve(home || temp, ".bower"); var Package = function (name, endpoint, manager) { - this.dependencies = {}; - this.json = {}; - this.name = name; - this.manager = manager; + this.dependencies = {}; + this.json = {}; + this.name = name; + this.manager = manager; + this.unitWork = manager ? manager.unitWork : new UnitWork; if (endpoint) { @@ -69,6 +71,7 @@ var Package = function (name, endpoint, manager) { this.tag = endpoint; } else if (/^[\.\/~]\.?[^.]*\.(js|css)/.test(endpoint) && fs.statSync(endpoint).isFile()) { + this.path = path.resolve(endpoint); this.assetType = path.extname(endpoint); this.name = this.name.replace(this.assetType, ''); @@ -82,15 +85,33 @@ var Package = function (name, endpoint, manager) { this.name = this.name.replace(this.assetType, ''); } else { - this.tag = endpoint.split('#', 2)[1]; + try { + fs.statSync(endpoint); + this.path = path.resolve(endpoint); + } catch (e) { + this.tag = endpoint.split('#', 2)[1]; + } } + this.endpoint = endpoint; + this.originalTag = this.tag; + + // Generate an id and a resourceId + // The id is an unique id that describes this package + // The resourceId is an unique id that describes the resource of this package + this.id = new Buffer(this.name + '%' + this.tag + '%' + this.gitUrl + '%' + this.path + '%' + this.assetUrl).toString('base64'); + this.resourceId = new Buffer(this.gitUrl + '%' + this.path + '%' + this.assetUrl).toString('base64'); } if (this.manager) { this.on('data', this.manager.emit.bind(this.manager, 'data')); this.on('error', this.manager.emit.bind(this.manager, 'error')); } + + // Cache a self bound function + this.waitUnlock = this.waitUnlock.bind(this); + + this.setMaxListeners(30); // Increase the number of listeners because a package can have more than the default 10 dependencies }; Package.prototype = Object.create(events.EventEmitter.prototype); @@ -98,6 +119,31 @@ Package.prototype = Object.create(events.EventEmitter.prototype); Package.prototype.constructor = Package; Package.prototype.resolve = function () { + if (this.unitWork) { + // Ensure that nobody is resolving the same dep at the same time + // If there is, we wait for the unlock event + if (this.unitWork.isLocked(this.name)) return this.unitWork.on('unlock', this.waitUnlock); + else { + var data = this.unitWork.retrieve(this.name); + if (data) { + // Check if this exact package is the last resolved one + // If so, we copy the resolved result and we don't need to do anything else + if (data.id === this.id) { + this.unserialize(data); + return this.emit('resolve'); + // Check if this exact package resource is the last resolved one + // This is to prevent it from being downloaded over and over again in such case + } else if (data.resourceId === this.resourceId) { + this.path = data.path; + this.unitWork.lock(this.name, this); + return this.once('loadJSON', this.saveUnit).checkout(); + } + } + } + + // If not, we lock and resolve it + this.unitWork.lock(this.name, this); + } if (this.assetUrl) { this.download(); @@ -154,8 +200,8 @@ Package.prototype.generateAssetJSON = function () { main: 'index' + this.assetType, version: semverParser.exec(this.assetUrl) ? RegExp.$1 : "0.0.0", repository: { type: "asset", url: this.assetUrl } - } -} + }; +}; Package.prototype.uninstall = function () { template('action', { name: 'uninstalling', shizzle: this.path }) @@ -176,15 +222,16 @@ Package.prototype.loadJSON = function (name) { if (!name) return this.loadJSON('package.json'); return this.assetUrl ? this.emit('loadJSON') : this.path && this.on('describeTag', function (tag) { this.version = this.tag = semver.clean(tag); - this.emit('loadJSON') + this.emit('loadJSON'); }.bind(this)).describeTag(); } this.json = json; + this.name = this.json.name; this.version = this.json.version; this.emit('loadJSON'); }.bind(this), this); -} +}; Package.prototype.download = function () { template('action', { name: 'downloading', shizzle: this.assetUrl }) @@ -218,13 +265,13 @@ Package.prototype.download = function () { res.on('end', function () { file.end(); - this.once('loadJSON', this.addDependencies).loadJSON(); + this.once('loadJSON', this.saveUnit).loadJSON(); }.bind(this)); }.bind(this)).on('error', this.emit.bind(this, 'error')); }.bind(this)); -} +}; Package.prototype.copy = function () { template('action', { name: 'copying', shizzle: this.path }).on('data', this.emit.bind(this, 'data')); @@ -236,7 +283,7 @@ Package.prototype.copy = function () { if (this.assetType) { return fs.readFile(this.path, function (err, data) { fs.writeFile(path.join((this.path = tmpPath), 'index' + this.assetType), data, function () { - this.once('loadJSON', this.addDependencies).loadJSON(); + this.once('loadJSON', this.saveUnit).loadJSON(); }.bind(this)); }.bind(this)); } @@ -248,7 +295,7 @@ Package.prototype.copy = function () { }) ); - this.once('loadJSON', this.addDependencies); + this.once('loadJSON', this.saveUnit); reader.on('error', this.emit.bind(this, 'error')); reader.on('end', this.loadJSON.bind(this)); @@ -257,14 +304,20 @@ Package.prototype.copy = function () { }; Package.prototype.getDeepDependencies = function (result) { - var result = result || []; + result = result || []; for (var name in this.dependencies) { - result.push(this.dependencies[name]) + result.push(this.dependencies[name]); this.dependencies[name].getDeepDependencies(result); } return result; }; +Package.prototype.saveUnit = function () { + this.unitWork.store(this.name, this.serialize(), this); + this.unitWork.unlock(this.name, this); + this.addDependencies(); +}; + Package.prototype.addDependencies = function () { var dependencies = this.json.dependencies || {}; var callbacks = Object.keys(dependencies).map(function (name) { @@ -287,7 +340,7 @@ Package.prototype.clone = function () { this.once('cache', function () { this.once('loadJSON', this.copy.bind(this)).checkout(); }.bind(this)).cache(); -} +}; Package.prototype.cache = function () { mkdirp(cache, function (err) { @@ -376,7 +429,7 @@ Package.prototype.describeTag = function () { else if (code != 0) return this.emit('error', new Error('Git status: ' + code)); this.emit('describeTag', tag.replace(/\n$/, '')); }.bind(this)); -} +}; Package.prototype.versions = function () { this.on('fetch', function () { @@ -418,6 +471,37 @@ Package.prototype.fetchURL = function () { } }; +Package.prototype.waitUnlock = function (name) { + if (this.name === name) { + this.unitWork.removeListener('unlock', this.waitUnlock); + this.resolve(); + } +}; + +Package.prototype.serialize = function () { + return { + id: this.id, + resourceId: this.resourceId, + path: this.path, + tag: this.tag, + originalTag: this.originalTag, + assetUrl: this.assetUrl, + assetType: this.assetType, + json: this.json, + endpoint: this.endpoint, + gitUrl: this.gitUrl, + dependencies: this.dependencies + }; +}; + +Package.prototype.unserialize = function (obj) { + for (var key in obj) { + this[key] = obj[key]; + } + + this.version = this.tag; +}; + Package.prototype.__defineGetter__('localPath', function () { return path.join(process.cwd(), config.directory, this.name); }); diff --git a/lib/core/unit_work.js b/lib/core/unit_work.js new file mode 100644 index 00000000..91a05e55 --- /dev/null +++ b/lib/core/unit_work.js @@ -0,0 +1,64 @@ +// ========================================== +// BOWER: Package Object Definition +// ========================================== +// Copyright 2012 Twitter, Inc +// Licensed under The MIT License +// http://opensource.org/licenses/MIT +// ========================================== +// Events: +// - lock: fired when a lock write over a key is acquired +// - unlock: fired when an unlock write over a key is acquired +// ========================================== + +var events = require('events'); + +var UnitWork = function () { + this.locks = []; + this.data = []; + + this.setMaxListeners(100); // Increase the number of listeners because this is a central storage +}; + +UnitWork.prototype = Object.create(events.EventEmitter.prototype); + +UnitWork.prototype.constructor = UnitWork; + +UnitWork.prototype.lock = function (key, owner) { + if (this.locks[key]) throw new Error('A write lock for "' + key + '" was already acquired.'); + if (!owner) throw new Error('A lock requires an owner.'); + this.locks[key] = owner; + + return this.emit('lock', key); +}; + +UnitWork.prototype.unlock = function (key, owner) { + if (!owner) throw new Error('A write lock requires an owner.'); + if (this.locks[key]) { + if (this.locks[key] !== owner) throw new Error('Lock owner for "' + key + '" mismatch.'); + delete this.locks[key]; + this.emit('unlock', key); + } + + return this; +}; + +UnitWork.prototype.isLocked = function (key) { + return !!this.locks[key]; +}; + +UnitWork.prototype.store = function (key, data, owner) { + if (this.locks[key] && owner !== this.locks[key]) throw new Error('A write lock for "' + key + '" is acquired therefore only its owner can write to it.'); + this.data[key] = data; + + return this; +}; + +UnitWork.prototype.retrieve = function (key) { + return this.data[key]; +}; + +UnitWork.prototype.keys = function () { + return Object.keys(this.data); +}; + +module.exports = UnitWork; \ No newline at end of file diff --git a/lib/util/hogan-colors.js b/lib/util/hogan-colors.js index 19551f08..52d6815a 100644 --- a/lib/util/hogan-colors.js +++ b/lib/util/hogan-colors.js @@ -12,11 +12,11 @@ var _ = require('lodash'); module.exports = hogan.Template.prototype.renderWithColors = function (context, partials, indent) { context = _.extend({ - yellow : function (s) { return s.yellow }, - green : function (s) { return s.green }, - cyan : function (s) { return s.cyan }, - grey : function (s) { return s.grey }, - red : function (s) { return s.red } + yellow : function (s) { return s.yellow; }, + green : function (s) { return s.green; }, + cyan : function (s) { return s.cyan; }, + grey : function (s) { return s.grey; }, + red : function (s) { return s.red; } }, context); return this.ri([context], partials || {}, indent); }; \ No newline at end of file diff --git a/lib/util/prune.js b/lib/util/prune.js index 02f9f067..48da8568 100644 --- a/lib/util/prune.js +++ b/lib/util/prune.js @@ -7,16 +7,15 @@ // ========================================== var semver = require('semver'); -var _ = require('lodash'); var versionRequirements = function (dependencyMap) { - var result = [] + var result = {}; for (var name in dependencyMap) { dependencyMap[name].forEach(function (pkg) { - for (var dep in pkg.json.dependencies) { - result[dep] = result[dep] || []; - result[dep].concat(pkg.json.dependencies[dep]); + result[name] = result[name] || []; + if (pkg.originalTag && result[name].indexOf(pkg.originalTag) === -1) { + result[name].push(pkg.originalTag); } }); } @@ -27,12 +26,15 @@ var versionRequirements = function (dependencyMap) { var validVersions = function (versions, dependency) { if (!versions || !versions.length) return true; + // If a non resolved dependency is passed, we simply ignore it + if (!dependency.version) return false; + if (!semver.valid(dependency.version)) { - throw new Error('Invalid semver version "' + dependency.version + '" specified in ' + dependency.name); + throw new Error('Invalid semver version ' + dependency.version + ' specified in ' + dependency.name); } - return _.find(versions, function (version) { - return !semver.satisfies(dependency.version, version) + return versions.every(function (version) { + return semver.satisfies(dependency.version, version); }); }; diff --git a/lib/util/read-json.js b/lib/util/read-json.js index dfd2da4f..3c3a6949 100644 --- a/lib/util/read-json.js +++ b/lib/util/read-json.js @@ -19,6 +19,6 @@ var read = module.exports = function (path, cb, obj) { .on('data', obj.emit.bind(obj, 'data')); } } - } + }; readJSON(path, cb); -} \ No newline at end of file +}; \ No newline at end of file diff --git a/lib/util/save.js b/lib/util/save.js index 33ce9051..85e0821f 100644 --- a/lib/util/save.js +++ b/lib/util/save.js @@ -40,7 +40,7 @@ function save (eventType, modifier, emitter, manager, paths) { }).loadJSON.bind(manager)); -}; +} function addDependency(pkg) { var path = (pkg.gitUrl || pkg.assetUrl || pkg.path || ''); diff --git a/test/assets/project/component.json b/test/assets/project/component.json index cdd7695d..d9dc7197 100644 --- a/test/assets/project/component.json +++ b/test/assets/project/component.json @@ -3,6 +3,6 @@ "version": "1.0.0", "dependencies": { "package-bootstrap": "git://github.com/fat/package-bootstrap.git#~2.0.0", - "jquery": "git://github.com/maccman/package-jquery.git#v1.7.2" + "jquery": "git://github.com/maccman/package-jquery.git#v1.8.1" } } \ No newline at end of file diff --git a/test/manager.js b/test/manager.js new file mode 100644 index 00000000..df6cf959 --- /dev/null +++ b/test/manager.js @@ -0,0 +1,68 @@ +var assert = require('assert'); +var path = require('path'); +// var fs = require('fs'); +// var _ = require('lodash'); +var Manager = require('../lib/core/manager'); +var rimraf = require('rimraf'); +var config = require('../lib/core/config'); + +describe('manager', function () { + beforeEach(function (done) { + rimraf(config.directory, function (err) { + if (err) { + throw new Error('Unable to delete local directory.'); + } + done(); + }); + }); + + it('Should resolve JSON dependencies', function (next) { + var manager = new Manager([]); + manager.cwd = __dirname + '/assets/project'; + + manager.on('resolve', function () { + assert.deepEqual(manager.dependencies["jquery-ui"][0].version, "1.8.23"); + assert.deepEqual(manager.dependencies["jquery"][0].version, "1.8.1"); + assert.deepEqual(manager.dependencies["package-bootstrap"][0].version, "2.0.6"); + + rimraf(config.directory, function (err) { + next(); + }); + }); + + manager.resolve(); + + }); + + it('Should resolve nested JSON dependencies', function (next) { + var manager = new Manager([]); + manager.cwd = __dirname + '/assets/other-project'; + + manager.on('resolve', function () { + assert.deepEqual(manager.dependencies["jquery"][0].version, "1.7.2"); + assert.deepEqual(manager.dependencies["jquery-pjax"][0].version, "1.0.0"); + + rimraf(config.directory, function (err) { + next(); + }); + }); + + manager.resolve(); + }); + + it('Should detect unresolvable packages in nested JSON dependencies', function (next) { + var manager = new Manager([]); + manager.cwd = __dirname + '/assets/conflict-project'; + + var detected = false; + manager.on('error', function (err) { + if (/no resolvable.* jquery$/i) detected = true; + }); + manager.on('resolve', function () { + if (!detected) throw new Error('A conflict in jquery should have been detected.'); + next(); + }); + + manager.resolve(); + }); +}); diff --git a/test/package.js b/test/package.js index f1466297..acfcda93 100644 --- a/test/package.js +++ b/test/package.js @@ -149,4 +149,4 @@ describe('package', function () { }); pkg.clone(); }); -}); +}); \ No newline at end of file From 71a7b8e8f0572a01b3a852b1e1fafc632d3cb27f Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Thu, 11 Oct 2012 15:23:47 +0100 Subject: [PATCH 02/13] Add missing tests. --- test/assets/project-nested-conflict/component.json | 8 ++++++++ test/assets/project-nested/component.json | 8 ++++++++ test/manager.js | 4 ++-- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 test/assets/project-nested-conflict/component.json create mode 100644 test/assets/project-nested/component.json diff --git a/test/assets/project-nested-conflict/component.json b/test/assets/project-nested-conflict/component.json new file mode 100644 index 00000000..0a78680f --- /dev/null +++ b/test/assets/project-nested-conflict/component.json @@ -0,0 +1,8 @@ +{ + "name": "myproject", + "version": "1.0.0", + "dependencies": { + "jquery": "1.6.0", + "jquery-pjax": "1.0.0" + } +} \ No newline at end of file diff --git a/test/assets/project-nested/component.json b/test/assets/project-nested/component.json new file mode 100644 index 00000000..62263548 --- /dev/null +++ b/test/assets/project-nested/component.json @@ -0,0 +1,8 @@ +{ + "name": "myproject", + "version": "1.0.0", + "dependencies": { + "jquery": "1.7.2", + "jquery-pjax": "1.0.0" + } +} \ No newline at end of file diff --git a/test/manager.js b/test/manager.js index df6cf959..cc982b48 100644 --- a/test/manager.js +++ b/test/manager.js @@ -36,7 +36,7 @@ describe('manager', function () { it('Should resolve nested JSON dependencies', function (next) { var manager = new Manager([]); - manager.cwd = __dirname + '/assets/other-project'; + manager.cwd = __dirname + '/assets/project-nested'; manager.on('resolve', function () { assert.deepEqual(manager.dependencies["jquery"][0].version, "1.7.2"); @@ -52,7 +52,7 @@ describe('manager', function () { it('Should detect unresolvable packages in nested JSON dependencies', function (next) { var manager = new Manager([]); - manager.cwd = __dirname + '/assets/conflict-project'; + manager.cwd = __dirname + '/assets/project-nested-conflict'; var detected = false; manager.on('error', function (err) { From e8b473b5506108781f2d8757e2817f596272f9a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Thu, 11 Oct 2012 19:26:52 +0100 Subject: [PATCH 03/13] Ensure that an unit of work is always present, remove unnecessary if statement. --- lib/core/package.js | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/lib/core/package.js b/lib/core/package.js index 377f9b54..03b6ffce 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -51,7 +51,7 @@ var Package = function (name, endpoint, manager) { this.json = {}; this.name = name; this.manager = manager; - this.unitWork = manager ? manager.unitWork : new UnitWork; + this.unitWork = manager && manager.unitWork ? manager.unitWork : new UnitWork; if (endpoint) { @@ -119,32 +119,30 @@ Package.prototype = Object.create(events.EventEmitter.prototype); Package.prototype.constructor = Package; Package.prototype.resolve = function () { - if (this.unitWork) { - // Ensure that nobody is resolving the same dep at the same time - // If there is, we wait for the unlock event - if (this.unitWork.isLocked(this.name)) return this.unitWork.on('unlock', this.waitUnlock); - else { - var data = this.unitWork.retrieve(this.name); - if (data) { - // Check if this exact package is the last resolved one - // If so, we copy the resolved result and we don't need to do anything else - if (data.id === this.id) { - this.unserialize(data); - return this.emit('resolve'); - // Check if this exact package resource is the last resolved one - // This is to prevent it from being downloaded over and over again in such case - } else if (data.resourceId === this.resourceId) { - this.path = data.path; - this.unitWork.lock(this.name, this); - return this.once('loadJSON', this.saveUnit).checkout(); - } + // Ensure that nobody is resolving the same dep at the same time + // If there is, we wait for the unlock event + if (this.unitWork.isLocked(this.name)) return this.unitWork.on('unlock', this.waitUnlock); + else { + var data = this.unitWork.retrieve(this.name); + if (data) { + // Check if this exact package is the last resolved one + // If so, we copy the resolved result and we don't need to do anything else + if (data.id === this.id) { + this.unserialize(data); + return this.emit('resolve'); + // Check if this exact package resource is the last resolved one + // This is to prevent it from being downloaded over and over again in such case + } else if (data.resourceId === this.resourceId) { + this.path = data.path; + this.unitWork.lock(this.name, this); + return this.once('loadJSON', this.saveUnit).checkout(); } } - - // If not, we lock and resolve it - this.unitWork.lock(this.name, this); } + // If not, we lock and resolve it + this.unitWork.lock(this.name, this); + if (this.assetUrl) { this.download(); } else if (this.gitUrl) { From 0a4a484f3ddef485abf117d66b1be0e975c92ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Thu, 11 Oct 2012 20:29:22 +0100 Subject: [PATCH 04/13] Remove unused property, fix CS again. --- lib/commands/install.js | 2 +- lib/core/package.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/commands/install.js b/lib/commands/install.js index b537887c..97a8b7d3 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -25,7 +25,7 @@ var shorthand = { 'h': ['--help'], 'S': ['--save'] }; module.exports = function (paths, options) { var emitter = new Emitter; - var manager = new Manager(paths) + var manager = new Manager(paths); if (options && options.save) save(emitter, manager, paths); diff --git a/lib/core/package.js b/lib/core/package.js index 03b6ffce..5471c87a 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -93,7 +93,9 @@ var Package = function (name, endpoint, manager) { } } - this.endpoint = endpoint; + // Store a reference to the original tag + // This is because the tag gets rewriten later and the original tag + // must be used by the manager later on this.originalTag = this.tag; // Generate an id and a resourceId @@ -486,7 +488,6 @@ Package.prototype.serialize = function () { assetUrl: this.assetUrl, assetType: this.assetType, json: this.json, - endpoint: this.endpoint, gitUrl: this.gitUrl, dependencies: this.dependencies }; From b0bc14a75e355a14d1812b37666a06deeed597b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Thu, 11 Oct 2012 23:27:59 +0100 Subject: [PATCH 05/13] Little code style tweak. --- lib/core/package.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/core/package.js b/lib/core/package.js index 5471c87a..7eb9bd9f 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -124,21 +124,21 @@ Package.prototype.resolve = function () { // Ensure that nobody is resolving the same dep at the same time // If there is, we wait for the unlock event if (this.unitWork.isLocked(this.name)) return this.unitWork.on('unlock', this.waitUnlock); - else { - var data = this.unitWork.retrieve(this.name); - if (data) { - // Check if this exact package is the last resolved one - // If so, we copy the resolved result and we don't need to do anything else - if (data.id === this.id) { - this.unserialize(data); - return this.emit('resolve'); - // Check if this exact package resource is the last resolved one - // This is to prevent it from being downloaded over and over again in such case - } else if (data.resourceId === this.resourceId) { - this.path = data.path; - this.unitWork.lock(this.name, this); - return this.once('loadJSON', this.saveUnit).checkout(); - } + + var data = this.unitWork.retrieve(this.name); + if (data) { + // Check if this exact package is the last resolved one + // If so, we copy the resolved result and we don't need to do anything else + if (data.id === this.id) { + this.unserialize(data); + return this.emit('resolve'); + } + // Check if this exact package resource is the last resolved one + // This is to prevent it from being downloaded over and over again in such case + if (data.resourceId === this.resourceId) { + this.path = data.path; + this.unitWork.lock(this.name, this); + return this.once('loadJSON', this.saveUnit).checkout(); } } From 8434acf9ed8f70adeac8059f61fb519ea2e99362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Fri, 12 Oct 2012 09:40:31 +0200 Subject: [PATCH 06/13] Doc improvement. --- lib/core/package.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/package.js b/lib/core/package.js index 7eb9bd9f..a893436f 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -134,7 +134,7 @@ Package.prototype.resolve = function () { return this.emit('resolve'); } // Check if this exact package resource is the last resolved one - // This is to prevent it from being downloaded over and over again in such case + // This is to prevent it from being downloaded or copied over and over again in such case if (data.resourceId === this.resourceId) { this.path = data.path; this.unitWork.lock(this.name, this); From 6bd00a0e8e4ad339b4f31c3b19d70e8bb0f09177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Sat, 13 Oct 2012 14:22:08 +0100 Subject: [PATCH 07/13] Tweak tests so they don't break in the future. --- test/manager.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/manager.js b/test/manager.js index cc982b48..d587c804 100644 --- a/test/manager.js +++ b/test/manager.js @@ -5,6 +5,7 @@ var path = require('path'); var Manager = require('../lib/core/manager'); var rimraf = require('rimraf'); var config = require('../lib/core/config'); +var semver = require('semver'); describe('manager', function () { beforeEach(function (done) { @@ -21,10 +22,9 @@ describe('manager', function () { manager.cwd = __dirname + '/assets/project'; manager.on('resolve', function () { - assert.deepEqual(manager.dependencies["jquery-ui"][0].version, "1.8.23"); - assert.deepEqual(manager.dependencies["jquery"][0].version, "1.8.1"); - assert.deepEqual(manager.dependencies["package-bootstrap"][0].version, "2.0.6"); - + assert.ok(semver.gte(manager.dependencies["jquery"][0].version, "1.8.1")); + assert.ok(semver.gte(manager.dependencies["package-bootstrap"][0].version, "2.0.0")); + assert.ok(semver.gte(manager.dependencies["jquery-ui"][0].version, "1.8.0")); rimraf(config.directory, function (err) { next(); }); From 5cd8dd709430bec401c88b3df3933508fe947a2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Sat, 13 Oct 2012 14:22:08 +0100 Subject: [PATCH 08/13] Tweak tests so they don't break in the future. --- test/assets/project/component.json | 2 +- test/manager.js | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/assets/project/component.json b/test/assets/project/component.json index d9dc7197..3be5db75 100644 --- a/test/assets/project/component.json +++ b/test/assets/project/component.json @@ -3,6 +3,6 @@ "version": "1.0.0", "dependencies": { "package-bootstrap": "git://github.com/fat/package-bootstrap.git#~2.0.0", - "jquery": "git://github.com/maccman/package-jquery.git#v1.8.1" + "jquery": "~1.8.1" } } \ No newline at end of file diff --git a/test/manager.js b/test/manager.js index cc982b48..54fa6e0a 100644 --- a/test/manager.js +++ b/test/manager.js @@ -5,6 +5,7 @@ var path = require('path'); var Manager = require('../lib/core/manager'); var rimraf = require('rimraf'); var config = require('../lib/core/config'); +var semver = require('semver'); describe('manager', function () { beforeEach(function (done) { @@ -21,10 +22,9 @@ describe('manager', function () { manager.cwd = __dirname + '/assets/project'; manager.on('resolve', function () { - assert.deepEqual(manager.dependencies["jquery-ui"][0].version, "1.8.23"); - assert.deepEqual(manager.dependencies["jquery"][0].version, "1.8.1"); - assert.deepEqual(manager.dependencies["package-bootstrap"][0].version, "2.0.6"); - + assert.ok(semver.gte(manager.dependencies['jquery'][0].version, '1.8.1')); + assert.ok(semver.gte(manager.dependencies['package-bootstrap'][0].version, '2.0.0')); + assert.ok(semver.gte(manager.dependencies['jquery-ui'][0].version, '1.8.0')); rimraf(config.directory, function (err) { next(); }); @@ -39,8 +39,8 @@ describe('manager', function () { manager.cwd = __dirname + '/assets/project-nested'; manager.on('resolve', function () { - assert.deepEqual(manager.dependencies["jquery"][0].version, "1.7.2"); - assert.deepEqual(manager.dependencies["jquery-pjax"][0].version, "1.0.0"); + assert.deepEqual(manager.dependencies['jquery'][0].version, '1.7.2'); + assert.deepEqual(manager.dependencies['jquery-pjax'][0].version, '1.0.0'); rimraf(config.directory, function (err) { next(); From bf371acc4104b294cc21843181cc0e126e37ba67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Sun, 14 Oct 2012 19:48:23 +0100 Subject: [PATCH 09/13] Remove unnecessary code, fix beforeEach in some cases. --- test/manager.js | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/test/manager.js b/test/manager.js index d587c804..3ec354ae 100644 --- a/test/manager.js +++ b/test/manager.js @@ -1,7 +1,6 @@ var assert = require('assert'); var path = require('path'); -// var fs = require('fs'); -// var _ = require('lodash'); +var fs = require('fs'); var Manager = require('../lib/core/manager'); var rimraf = require('rimraf'); var config = require('../lib/core/config'); @@ -9,12 +8,14 @@ var semver = require('semver'); describe('manager', function () { beforeEach(function (done) { - rimraf(config.directory, function (err) { - if (err) { - throw new Error('Unable to delete local directory.'); - } - done(); - }); + if (fs.existsSync(config.directory)) { + rimraf(config.directory, function (err) { + if (err) { + throw new Error('Unable to delete local directory.'); + } + done(); + }); + } }); it('Should resolve JSON dependencies', function (next) { @@ -25,9 +26,7 @@ describe('manager', function () { assert.ok(semver.gte(manager.dependencies["jquery"][0].version, "1.8.1")); assert.ok(semver.gte(manager.dependencies["package-bootstrap"][0].version, "2.0.0")); assert.ok(semver.gte(manager.dependencies["jquery-ui"][0].version, "1.8.0")); - rimraf(config.directory, function (err) { - next(); - }); + next(); }); manager.resolve(); @@ -41,10 +40,7 @@ describe('manager', function () { manager.on('resolve', function () { assert.deepEqual(manager.dependencies["jquery"][0].version, "1.7.2"); assert.deepEqual(manager.dependencies["jquery-pjax"][0].version, "1.0.0"); - - rimraf(config.directory, function (err) { - next(); - }); + next(); }); manager.resolve(); From 612084fb57690996b2e96bb058ab3df38729653b Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Tue, 16 Oct 2012 01:02:03 +0000 Subject: [PATCH 10/13] Fix download() emitting the 'download' event several times. --- lib/core/package.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/package.js b/lib/core/package.js index a893436f..0ac4ec65 100644 --- a/lib/core/package.js +++ b/lib/core/package.js @@ -256,7 +256,7 @@ Package.prototype.download = function () { template('action', { name: 'redirect detected', shizzle: this.assetUrl }) .on('data', this.emit.bind(this, 'data')); this.assetUrl = res.headers.location; - this.download(); + return this.download(); } res.on('data', function (data) { From c5a0f0ddcf23ef717d98827402ba96fd67f1195d Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Tue, 16 Oct 2012 01:02:31 +0000 Subject: [PATCH 11/13] Fix redirect test not being async, cache and local files are now flushed before each test. --- test/manager.js | 35 ++++++++++++++++++++++------------- test/package.js | 35 ++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/test/manager.js b/test/manager.js index 0b038152..4b2f58d5 100644 --- a/test/manager.js +++ b/test/manager.js @@ -1,6 +1,4 @@ var assert = require('assert'); -var path = require('path'); -var fs = require('fs'); var Manager = require('../lib/core/manager'); var rimraf = require('rimraf'); var config = require('../lib/core/config'); @@ -8,14 +6,17 @@ var semver = require('semver'); describe('manager', function () { beforeEach(function (done) { - if (fs.existsSync(config.directory)) { - rimraf(config.directory, function (err) { - if (err) { - throw new Error('Unable to delete local directory.'); - } - done(); - }); - } else done(); + var del = 0; + + rimraf(config.directory, function (err) { + // Ignore the error if the local directory was not actually deleted + if (++del >= 2) done(); + }); + + rimraf(config.cache, function (err) { + // Ignore the error if the cache directory was not actually deleted + if (++del >= 2) done(); + }); }); it('Should resolve JSON dependencies', function (next) { @@ -23,12 +24,16 @@ describe('manager', function () { manager.cwd = __dirname + '/assets/project'; manager.on('resolve', function () { - assert.ok(semver.gte(manager.dependencies["jquery"][0].version, "1.8.1")); - assert.ok(semver.gte(manager.dependencies["package-bootstrap"][0].version, "2.0.0")); - assert.ok(semver.gte(manager.dependencies["jquery-ui"][0].version, "1.8.0")); + assert.ok(semver.gte(manager.dependencies['jquery'][0].version, '1.8.1')); + assert.ok(semver.gte(manager.dependencies['package-bootstrap'][0].version, '2.0.0')); + assert.ok(semver.gte(manager.dependencies['jquery-ui'][0].version, '1.8.0')); next(); }); + manager.on('error', function (err) { + throw new Error(err); + }); + manager.resolve(); }); @@ -43,6 +48,10 @@ describe('manager', function () { next(); }); + manager.on('error', function (err) { + throw new Error(err); + }); + manager.resolve(); }); diff --git a/test/package.js b/test/package.js index acfcda93..ac50cc6c 100644 --- a/test/package.js +++ b/test/package.js @@ -8,6 +8,20 @@ var config = require('../lib/core/config'); var Package = require('../lib/core/package'); describe('package', function () { + beforeEach(function (done) { + var del = 0; + + rimraf(config.directory, function (err) { + // Ignore the error if the local directory was not actually deleted + if (++del >= 2) done(); + }); + + rimraf(config.cache, function (err) { + // Ignore the error if the cache directory was not actually deleted + if (++del >= 2) done(); + }); + }); + it('Should resolve git URLs properly', function () { var pkg = new Package('jquery', 'git://github.com/jquery/jquery.git'); assert.equal(pkg.gitUrl, 'git://github.com/jquery/jquery.git'); @@ -34,7 +48,7 @@ describe('package', function () { assert.equal(pkg.gitUrl, 'git@github.com:twitter/flight.git'); }); - it('Should resolve url when we got redirected', function() { + it('Should resolve url when we got redirected', function (next) { var redirecting_url = 'http://redirecting-url.com'; var redirecting_to_url = 'http://redirected-to-url.com'; @@ -52,6 +66,11 @@ describe('package', function () { pkg.on('resolve', function () { assert(pkg.assetUrl); assert.equal(pkg.assetUrl, redirecting_to_url + '/jquery.zip'); + next(); + }); + + pkg.on('error', function (err) { + throw new Error(err); }); pkg.download(); @@ -109,6 +128,10 @@ describe('package', function () { next(); }); + pkg.on('error', function (err) { + throw new Error(err); + }); + pkg.loadJSON(); }); @@ -121,6 +144,10 @@ describe('package', function () { next(); }); + pkg.on('error', function (err) { + throw new Error(err); + }); + pkg.resolve(); }); @@ -141,12 +168,18 @@ describe('package', function () { pkg.on('resolve', function () { pkg.install(); }); + + pkg.on('error', function (err) { + throw new Error(err); + }); + pkg.on('install',function () { assert(fs.existsSync(pkg.localPath)); rimraf(config.directory, function(err){ next(); }); }); + pkg.clone(); }); }); \ No newline at end of file From 2abde5539d3ccc14085e93151dbe00040407cbc2 Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Tue, 16 Oct 2012 01:02:42 +0000 Subject: [PATCH 12/13] Fix CS again. --- test/help.js | 2 +- test/manager.js | 2 ++ test/package.js | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/help.js b/test/help.js index 373a816b..ccffd164 100644 --- a/test/help.js +++ b/test/help.js @@ -25,5 +25,5 @@ describe('help', function () { next(); }); }); - + }); \ No newline at end of file diff --git a/test/manager.js b/test/manager.js index 4b2f58d5..b8ed5a8f 100644 --- a/test/manager.js +++ b/test/manager.js @@ -5,6 +5,7 @@ var config = require('../lib/core/config'); var semver = require('semver'); describe('manager', function () { + beforeEach(function (done) { var del = 0; @@ -70,4 +71,5 @@ describe('manager', function () { manager.resolve(); }); + }); diff --git a/test/package.js b/test/package.js index ac50cc6c..40529d2a 100644 --- a/test/package.js +++ b/test/package.js @@ -8,6 +8,7 @@ var config = require('../lib/core/config'); var Package = require('../lib/core/package'); describe('package', function () { + beforeEach(function (done) { var del = 0; @@ -182,4 +183,5 @@ describe('package', function () { pkg.clone(); }); + }); \ No newline at end of file From df4ee497265e3bbff57e50a73138d852d057aa9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Tue, 16 Oct 2012 23:47:59 +0100 Subject: [PATCH 13/13] Remove unnecessary tear down code in test because it is already being done in the beforeEach. --- test/package.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/package.js b/test/package.js index 4866028f..735f912d 100644 --- a/test/package.js +++ b/test/package.js @@ -205,9 +205,7 @@ describe('package', function () { async.map([pkg.localPath, cachePath], fs.stat, function (err, results) { if (err) throw new Error(err); assert.equal(results[0].mode, results[1].mode); - rimraf(config.directory, function(err){ - next(); - }); + next(); }); });