From e5c4391900348da691be3c8196ac44a4dd983354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Cruz?= Date: Sat, 4 May 2013 16:32:31 +0100 Subject: [PATCH] Fix some bugs. --- lib/resolve/Manager.js | 16 +++++++ lib/resolve/PackageRepository.js | 9 ++-- lib/resolve/resolverFactory.js | 54 +++++++++++----------- lib/resolve/resolvers/FsResolver.js | 9 ++-- lib/resolve/resolvers/GitFsResolver.js | 6 +-- lib/resolve/resolvers/GitRemoteResolver.js | 10 +++- lib/resolve/resolvers/UrlResolver.js | 4 ++ lib/util/copy.js | 23 ++------- lib/util/decomposeEndpoint.js | 18 ++++++++ lib/util/extract.js | 6 +++ 10 files changed, 98 insertions(+), 57 deletions(-) create mode 100644 lib/util/decomposeEndpoint.js diff --git a/lib/resolve/Manager.js b/lib/resolve/Manager.js index b36cd11a..9947deb9 100644 --- a/lib/resolve/Manager.js +++ b/lib/resolve/Manager.js @@ -9,4 +9,20 @@ var Manager = function (options) { // ----------------- +Manager.prototype.install = function (endpoints) { + this._packages = {}; + + // If some endpoints were passed, use those + // Otherwise grab the ones specified in the json + + // Check which packages are already installed + // and not install those if the target range is matched + + // Query the PackageRepository +}; + +Manager.prototype._onResolve = function () { + +}; + module.exports = Manager; \ No newline at end of file diff --git a/lib/resolve/PackageRepository.js b/lib/resolve/PackageRepository.js index 14aa2eea..bdb27803 100644 --- a/lib/resolve/PackageRepository.js +++ b/lib/resolve/PackageRepository.js @@ -5,15 +5,16 @@ var PackageRepository = function (options) { options = options || {}; this._offline = !!options.offline; + this._force = !!options.force; this._config = options.config || config; }; // ----------------- -PackageRepository.prototype.get = function (endpoint) { - // TODO: should query cache as soon as it is done! - - return resolverFactory(endpoint, this._options) +PackageRepository.prototype.get = function (decEndpoint) { + return resolverFactory(decEndpoint, { + skipCache: this._force + }) .then(function (resolver) { return resolver.resolve(); }); diff --git a/lib/resolve/resolverFactory.js b/lib/resolve/resolverFactory.js index 3bbb560b..f9dc31e6 100644 --- a/lib/resolve/resolverFactory.js +++ b/lib/resolve/resolverFactory.js @@ -9,45 +9,41 @@ var UrlResolver = require('./resolvers/UrlResolver'); var config = require('../config'); var createError = require('../util/createError'); -function createResolver(endpoint, options) { - var split = endpoint.split('#'), - source, - target; +function createResolver(decEndpoint, options) { + var resOptions, + source = decEndpoint.source; - // TODO: extract name from the endpoint and set it up in the options.name - // not sure about the @ being used to specify it because it may conflict - // with git@ .. - - // Extract the source and target from the endpoint - source = split[0]; - target = split[1]; - - // Setup options options = options || {}; options.config = options.config || config; - options.target = target; + + // Setup resolver options + resOptions = { + target: decEndpoint.target, + name: decEndpoint.name, + config: options.config + }; // Git case: git git+ssh, git+http, git+https - if (/^git(\+(ssh|https?))?:\/\//i.test(source)) { + // .git at the end (probably ssh shorthand) + if (/^git(\+(ssh|https?))?:\/\//i.test(source) || /\.git\/?$/i.test(source)) { source = source.replace(/^git\+/, ''); - return Q.resolve(new GitRemoteResolver(source, options)); - } - - // Git case: .git at the end (probably ssh shorthand) - if (/\.git$/i.test(source)) { - return Q.resolve(new GitRemoteResolver(source, options)); + return Q.fcall(function () { + return new GitRemoteResolver(source, resOptions); + }); } // URL case if (/^https?:\/\//i.exec(source)) { - return Q.resolve(new UrlResolver(source, options)); + return Q.fcall(function () { + return new UrlResolver(source, resOptions); + }); } // Check if source is a git repository return Q.nfcall(fs.stat, path.join(source, '.git')) .then(function (stats) { if (stats.isDirectory()) { - return new GitFsResolver(source, options); + return GitFsResolver; } throw new Error('Not a Git repository'); @@ -56,7 +52,7 @@ function createResolver(endpoint, options) { .fail(function () { return Q.nfcall(fs.stat, source) .then(function () { - return new FsResolver(source, options); + return FsResolver; }); }) // If not, check if is a shorthand and expand it @@ -70,7 +66,7 @@ function createResolver(endpoint, options) { package: parts[1] }); - return new GitRemoteResolver(source, options); + return { resolver: GitRemoteResolver, source: source }; } throw err; @@ -78,7 +74,13 @@ function createResolver(endpoint, options) { // TODO: if not, check against the registry // note that the registry should also have a persistent cache for offline usage // Finally throw a meaningful error - .fail(function () { + .then(function (ConcreteResolver) { + if (typeof ConcreteResolver === 'function') { + return new ConcreteResolver(source, resOptions); + } + + return new ConcreteResolver.resolver(source, resOptions); + }, function () { throw new createError('Could not find appropriate resolver for source "' + source + '"', 'ENORESOLVER'); }); } diff --git a/lib/resolve/resolvers/FsResolver.js b/lib/resolve/resolvers/FsResolver.js index 5154a351..26f19f69 100644 --- a/lib/resolve/resolvers/FsResolver.js +++ b/lib/resolve/resolvers/FsResolver.js @@ -10,11 +10,11 @@ var createError = require('../../util/createError'); var osJunk = require('../../util/osJunk'); var FsResolver = function (source, options) { - // Ensure absolute path - source = path.resolve(source); - Resolver.call(this, source, options); + // Ensure absolute path + this._source = path.resolve(this._config.cwd, source); + // If target was specified, simply reject the promise if (this._target !== '*') { throw createError('File system sources can\'t resolve targets', 'ENORESTARGET'); @@ -29,6 +29,9 @@ mout.object.mixIn(FsResolver, Resolver); // TODO: should we store latest mtimes in the resolution and compare? // this would be beneficial when copying big files/folders +// TODO: there's room for improvement by using streams if the source +// is an archive file, by piping read stream to the zip extractor +// this will likely increase the complexity of code but might worth it FsResolver.prototype._resolve = function () { return this._readJson(this._source) .then(this._copy.bind(this)) diff --git a/lib/resolve/resolvers/GitFsResolver.js b/lib/resolve/resolvers/GitFsResolver.js index b9d5ef6d..a49cdecb 100644 --- a/lib/resolve/resolvers/GitFsResolver.js +++ b/lib/resolve/resolvers/GitFsResolver.js @@ -7,10 +7,10 @@ var cmd = require('../../util/cmd'); var path = require('path'); var GitFsResolver = function (source, options) { - // Ensure absolute path - source = path.resolve(source); - GitResolver.call(this, source, options); + + // Ensure absolute path + this._source = path.resolve(this._config.cwd, source); }; util.inherits(GitFsResolver, GitResolver); diff --git a/lib/resolve/resolvers/GitRemoteResolver.js b/lib/resolve/resolvers/GitRemoteResolver.js index 0a98cf0a..7dfd1edf 100644 --- a/lib/resolve/resolvers/GitRemoteResolver.js +++ b/lib/resolve/resolvers/GitRemoteResolver.js @@ -5,8 +5,14 @@ var GitResolver = require('./GitResolver'); var cmd = require('../../util/cmd'); var GitRemoteResolver = function (source, options) { - if (!mout.string.startsWith(source, 'file://') && !mout.string.endsWith(source, '.git')) { - source += '.git'; + if (!mout.string.startsWith(source, 'file://')) { + // Trim trailing slashes + source = source.replace(/\/+$/, ''); + + // Ensure trailing .git + if (!mout.string.endsWith(source, '.git')) { + source += '.git'; + } } GitResolver.call(this, source, options); diff --git a/lib/resolve/resolvers/UrlResolver.js b/lib/resolve/resolvers/UrlResolver.js index 3074a647..b82ad5c3 100644 --- a/lib/resolve/resolvers/UrlResolver.js +++ b/lib/resolve/resolvers/UrlResolver.js @@ -73,6 +73,10 @@ UrlResolver.prototype._hasNew = function (pkgMeta) { }); }; +// TODO: there's room for improvement by using streams if the url +// is an archive file, by piping read stream to the zip extractor +// this will likely increase the complexity of code but might worth it + UrlResolver.prototype._resolve = function () { // If target was specified, simply reject the promise if (this._target !== '*') { diff --git a/lib/util/copy.js b/lib/util/copy.js index 0d4d52d8..c1ac8de8 100644 --- a/lib/util/copy.js +++ b/lib/util/copy.js @@ -5,22 +5,7 @@ var Q = require('q'); function copy(reader, writer) { var deferred = Q.defer(), - ignore, - finish; - - finish = function (err) { - // Ensure that all listeners are removed - writer.removeAllListeners(); - reader.removeAllListeners(); - - // If we got an error, simply reject the deferred - // Otherwise resolve it - if (err) { - deferred.reject(err); - } else { - deferred.resolve(); - } - }; + ignore; // Reader if (reader.type === 'Directory' && reader.ignore) { @@ -32,12 +17,12 @@ function copy(reader, writer) { } reader - .on('error', finish); + .on('error', deferred.reject); // Writer writer = fstream.Writer(writer) - .on('error', finish) - .on('close', finish); + .on('error', deferred.reject) + .on('close', deferred.resolve); // Finally pipe reader to writer reader.pipe(writer); diff --git a/lib/util/decomposeEndpoint.js b/lib/util/decomposeEndpoint.js new file mode 100644 index 00000000..5d7e132e --- /dev/null +++ b/lib/util/decomposeEndpoint.js @@ -0,0 +1,18 @@ +var createError = require('./createError'); + +function decomposeEndpoint(endpoint) { + var regExp = /^(?:([\w\-]|(?:[\w\.\-]+[\w\-])?)\|)?([^\|#]+)(?:#(.*))?$/; + var matches = endpoint.match(regExp); + + if (!matches) { + throw createError('Invalid endpoint: "' + endpoint + '"', 'EINVEND'); + } + + return { + name: matches[1], + source: matches[2], + target: matches[3] + }; +} + +module.exports = decomposeEndpoint; \ No newline at end of file diff --git a/lib/util/extract.js b/lib/util/extract.js index 7fe2fab1..1aee3ebf 100644 --- a/lib/util/extract.js +++ b/lib/util/extract.js @@ -31,6 +31,7 @@ function extractZip(archive, dest) { var deferred = Q.defer(); fs.createReadStream(archive) + .on('error', deferred.reject) .pipe(unzip.Extract({ path: dest })) .on('error', deferred.reject) .on('close', deferred.resolve.bind(deferred, dest)); @@ -42,6 +43,7 @@ function extractTar(archive, dest) { var deferred = Q.defer(); fs.createReadStream(archive) + .on('error', deferred.reject) .pipe(tar.Extract({ path: dest })) .on('error', deferred.reject) .on('close', deferred.resolve.bind(deferred, dest)); @@ -53,7 +55,9 @@ function extractTarGz(archive, dest) { var deferred = Q.defer(); fs.createReadStream(archive) + .on('error', deferred.reject) .pipe(zlib.createGunzip()) + .on('error', deferred.reject) .pipe(tar.Extract({ path: dest })) .on('error', deferred.reject) .on('close', deferred.resolve.bind(deferred, dest)); @@ -65,7 +69,9 @@ function extractGz(archive, dest) { var deferred = Q.defer(); fs.createReadStream(archive) + .on('error', deferred.reject) .pipe(zlib.createGunzip()) + .on('error', deferred.reject) .pipe(fs.createWriteStream(dest)) .on('error', deferred.reject) .on('close', deferred.resolve.bind(deferred, dest));