Some changes to the resolver API, add more tests.

lib/resolve/Resolver.js

@@ -39,25 +39,51 @@ Resolver.prototype.getTempDir = function () {
 };
 
 Resolver.prototype.hasNew = function (canonicalPkg) {
-    return Q.resolve(true);
+    var that = this,
+        promise,
+        metaFile;
+
+    // If already working, error out
+    if (this._working) {
+        return Q.reject(createError('Already working', 'EWORKING'));
+    }
+
+    this._working = true;
+
+    // Avoid reading the package meta if _hasNew was not rewritten
+    if (this._hasNew === Resolver.prototype._hasNew) {
+        promise = this._hasNew();
+    // Otherwise call _hasNew with both the package meta and the canonical package
+    } else {
+        metaFile = path.join(canonicalPkg, '.bower.json');
+        promise = Q.nfcall(fs.readFile, metaFile)
+        .then(function (contents) {
+            var pkgMeta = JSON.parse(contents.toString());
+            return that._hasNew(pkgMeta, canonicalPkg);
+        }, function () {
+            return true;  // Simply resolve to true if there was an error reading the meta
+        });
+    }
+
+    return promise.fin(function () {
+        that._working = false;
+    });
 };
 
 Resolver.prototype.resolve = function () {
     var that = this;
 
-    // If already resolving, throw an error
-    // We don't actually reject a promise because this is a
-    // serious logic error
-    if (this._resolving) {
-        throw new Error('Already resolving');
+    // If already working, error out
+    if (this._working) {
+        return Q.reject(createError('Already working', 'EWORKING'));
     }
 
-    this._resolving = true;
+    this._working = true;
 
     // Create temporary dir
     return this._createTempDir()
     // Resolve self
-    .then(this._resolveSelf.bind(this))
+    .then(this._resolve.bind(this))
     // Read json, generating the package meta
     .then(function () {
         return that._readJson(that._tempDir);
@@ -71,14 +97,15 @@ Resolver.prototype.resolve = function () {
         ]);
     })
     .then(function () {
-        that._resolving = false;
         // Resolve with the folder
         return that._tempDir;
     }, function (err) {
-        that._resolving = false;
         // If something went wrong, unset the temporary dir
         that._tempDir = null;
         throw err;
+    })
+    .fin(function () {
+        that._working = false;
     });
 };
 
@@ -93,12 +120,16 @@ Resolver.clearRuntimeCache = function () {};
 // -----------------
 
 // Abstract function that should be implemented by concrete resolvers
-Resolver.prototype._resolveSelf = function () {
-    throw new Error('_resolveSelf not implemented');
+Resolver.prototype._resolve = function () {
+    throw new Error('_resolve not implemented');
 };
 
 // -----------------
 
+Resolver.prototype._hasNew = function (pkgMeta, canonicalPkg) {
+    return Q.resolve(true);
+};
+
 Resolver.prototype._createTempDir = function () {
     var baseDir = path.join(tmp.tmpdir, 'bower');
 

lib/resolve/resolvers/FsResolver.js

@@ -13,6 +13,11 @@ var FsResolver = function (source, options) {
     source = path.resolve(source);
 
     Resolver.call(this, source, options);
+
+    // If target was specified, simply reject the promise
+    if (this._target !== '*') {
+        throw createError('File system sources can\'t resolve targets', 'ENORESTARGET');
+    }
 };
 
 util.inherits(FsResolver, Resolver);
@@ -20,23 +25,10 @@ mout.object.mixIn(FsResolver, Resolver);
 
 // -----------------
 
-FsResolver.prototype.hasNew = function (canonicalPkg) {
-    // If target was specified, simply reject the promise
-    if (this._target !== '*') {
-        return Q.reject(createError('File system sources can\'t resolve targets', 'ENORESTARGET'));
-    }
-
-    // TODO: should we store latest mtimes in the resolution and compare?
-    //       this would be beneficial when copying big files/folders
-    return Q.resolve(true);
-};
-
-FsResolver.prototype._resolveSelf = function () {
-    // If target was specified, simply reject the promise
-    if (this._target !== '*') {
-        return Q.reject(createError('File system sources can\'t resolve targets', 'ENORESTARGET'));
-    }
+// TODO: should we store latest mtimes in the resolution and compare?
+//       this would be beneficial when copying big files/folders
 
+FsResolver.prototype._resolve = function () {
     return this._readJson(this._source)
     .then(this._copy.bind(this))
     .then(this._extract.bind(this))
@@ -108,8 +100,6 @@ FsResolver.prototype._rename = function () {
 };
 
 FsResolver.prototype._savePkgMeta = function (meta) {
-    // TODO: store mtime into the package meta
-
     // Store main if is a single file
     if (this._singleFile) {
         meta.main = this._singleFile;

lib/resolve/resolvers/GitFsResolver.js

@@ -32,7 +32,7 @@ GitFsResolver.prototype._checkout = function () {
     // Copy files to the temporary directory first
     return this._copy()
     .then(cmd.bind(cmd, 'git', ['checkout', '-f', resolution.tag || resolution.branch || resolution.commit], { cwd: this._tempDir }))
-    // Cleanup unstagged files
+    // Cleanup unstaged files
     .then(cmd.bind(cmd, 'git', ['clean', '-f', '-d'], { cwd: this._tempDir }));
 };
 
@@ -44,6 +44,7 @@ GitFsResolver.fetchRefs = function (source) {
         return Q.resolve(this._refs[source]);
     }
 
+    // TODO: should reuse promises!
     return cmd('git', ['show-ref', '--tags', '--heads'], { cwd : source })
     .then(function (stdout) {
         // Make them an array

lib/resolve/resolvers/GitRemoteResolver.js

@@ -42,6 +42,7 @@ GitRemoteResolver.fetchRefs = function (source) {
         return Q.resolve(this._refs[source]);
     }
 
+    // TODO: should reuse promises!
     return cmd('git', ['ls-remote', '--tags', '--heads', source])
     .then(function (stdout) {
         // Make them an array

lib/resolve/resolvers/GitResolver.js

@@ -17,25 +17,10 @@ mout.object.mixIn(GitResolver, Resolver);
 
 // -----------------
 
-GitResolver.prototype._resolveSelf = function () {
+GitResolver.prototype._hasNew = function (pkgMeta) {
+    var oldResolution = pkgMeta._resolution || {};
+
     return this._findResolution()
-    .then(function () {
-        return this._checkout()
-        // Always run cleanup after checkout to ensure that .git is removed!
-        // If it's not removed, problems might arrise when the "tmp" module attemps
-        // to delete the temporary folder
-        .fin(this._cleanup.bind(this));
-    }.bind(this));
-};
-
-GitResolver.prototype.hasNew = function (canonicalPkg) {
-    var oldResolution;
-
-    return this._readJson(canonicalPkg)
-    .then(function (meta) {
-        oldResolution = meta._resolution || {};
-    })
-    .then(this._findResolution.bind(this))
     .then(function (resolution) {
         // Check if resolution types are different
         if (oldResolution.type !== resolution.type) {
@@ -52,6 +37,17 @@ GitResolver.prototype.hasNew = function (canonicalPkg) {
     });
 };
 
+GitResolver.prototype._resolve = function () {
+    return this._findResolution()
+    .then(function () {
+        return this._checkout()
+        // Always run cleanup after checkout to ensure that .git is removed!
+        // If it's not removed, problems might arrise when the "tmp" module attemps
+        // to delete the temporary folder
+        .fin(this._cleanup.bind(this));
+    }.bind(this));
+};
+
 // -----------------
 
 // Abstract functions that should be implemented by concrete git resolvers