From 22c58ce31e398ca4446ffd0585cc64479123aa6f Mon Sep 17 00:00:00 2001 From: John Haley Date: Fri, 5 Dec 2014 10:05:04 -0700 Subject: [PATCH 1/4] Fixed fetch to be async and use callbacks --- example/fetch.js | 9 ++++-- generate/input/descriptor.json | 41 +++++++++++++++++++++++-- lib/repository.js | 55 ++++++++++++++-------------------- 3 files changed, 66 insertions(+), 39 deletions(-) diff --git a/example/fetch.js b/example/fetch.js index 4b5471874..29bf0a454 100644 --- a/example/fetch.js +++ b/example/fetch.js @@ -2,8 +2,11 @@ var nodegit = require('../'); var path = require('path'); nodegit.Repository.open(path.resolve(__dirname, '../.git')).then(function(repo) { - return repo.fetch("origin"); -}) -.done(function() { + return repo.fetch("origin", { + credentials: function(url, userName) { + return nodegit.Cred.sshKeyFromAgent(userName); + } + }); +}).done(function() { console.log("It worked!"); }); diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index eeff8f1ba..cca202762 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1073,18 +1073,47 @@ "reflog_message": { "isOptional": true } + }, + "isAsync": true, + "return": { + "isErrorCode": true } }, "git_remote_get_fetch_refspecs": { - "ignore": true + "args": { + "array": { + "isReturn": true, + "shouldAlloc": true, + "cppClassName": "Array", + "jsClassName": "Array", + "size": "count", + "key": "strings" + } + }, + "isAsync": true }, "git_remote_get_push_refspecs": { - "ignore": true + "args": { + "array": { + "isReturn": true, + "shouldAlloc": true, + "cppClassName": "Array", + "jsClassName": "Array", + "size": "count", + "key": "strings" + } + }, + "isAsync": true }, "git_remote_list": { "args": { "out": { - "shouldAlloc": true + "isReturn": true, + "shouldAlloc": true, + "cppClassName": "Array", + "jsClassName": "Array", + "size": "count", + "key": "strings" } } }, @@ -1093,6 +1122,12 @@ }, "git_remote_rename": { "ignore": true + }, + "git_remote_set_fetch_refspecs": { + "ignore": true + }, + "git_remote_set_push_refspecs": { + "ignore": true } } }, diff --git a/lib/repository.js b/lib/repository.js index 42e91419d..00c4e8bea 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -53,37 +53,16 @@ function(name, commit, force, signature, logMessage) { }; /** - * Look up a branch + * Look up a refs's commit. * - * @param {String|Ref} name Branch name, e.g. "master" or Branch Ref - * @param {Function} callback - * @return {Ref} - */ -Repository.prototype.getBranch = function(name, callback) { - name = (name instanceof Reference || - ~name.indexOf("refs/heads/")) ? name - : "refs/heads/" + name; - - return this.getReference(name).then(function(reference) { - if (typeof callback === "function") { - callback(null, reference); - } - - return reference; - }, callback); -}; - -/** - * Look up a branch's most recent commit. - * - * @param {String|Ref} name Branch name, e.g. "master" or Branch Ref + * @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" or Branch Ref * @param {Function} callback * @return {Commit} */ -Repository.prototype.getBranchCommit = function(name, callback) { +Repository.prototype.getReferenceCommit = function(name, callback) { var repository = this; - return this.getBranch(name).then(function(reference) { + return this.getReference(name).then(function(reference) { return repository.getCommit(reference.target()).then(function(commit) { if (typeof callback === "function") { callback(null, commit); @@ -112,8 +91,11 @@ Repository.prototype.getCurrentBranch = function() { */ Repository.prototype.getReference = function(name, callback) { var repository = this; + var lookup = name.indexOf("refs/") === 0 + ? Reference.lookup(this, name) + : Reference.dwim(this, name); - return Reference.lookup(this, name).then(function(reference) { + return lookup.then(function(reference) { if (reference.isSymbolic()) { return reference.resolve(function (error, reference) { reference.repo = repository; @@ -507,29 +489,36 @@ Repository.prototype.getRemote = function(remote, callback) { * * @param {String|Remote} remote */ -Repository.prototype.fetch = function(remote) { +Repository.prototype.fetch = function(remote, remoteCallbacks, callback) { var repo = this; return repo.getRemote(remote).then(function(remote) { - return remote.fetch(repo.defaultSignature()); - }); + remote.setCallbacks(remoteCallbacks); + + return remote.fetch(repo.defaultSignature(), "Fetch from " + remote) + .then(function() { + if (typeof callback === "function") { + callback(); + } + }); + }, callback); }; /** * Fetches from all remotes */ -Repository.prototype.fetchAll = function() { +Repository.prototype.fetchAll = function(remoteCallbacks, callback) { var repo = this; - return repo.getRemotes(function(remotes) { + return repo.getRemotes().then(function(remotes) { var fetchPromises = []; remotes.forEach(function(remote) { - fetchPromises.push(repo.fetch(remote)); + fetchPromises.push(repo.fetch(remote, remoteCallbacks, callback)); }); return Promise.all(fetchPromises); - }); + }, callback); }; /** From edf5bb2bd420692bfb5a65e62949334b64fcdc42 Mon Sep 17 00:00:00 2001 From: John Haley Date: Fri, 5 Dec 2014 10:42:55 -0700 Subject: [PATCH 2/4] Fixed tests and reimplemented missing functions on Repository --- lib/reference.js | 9 +++++++++ lib/repository.js | 35 +++++++++++++++++++++++++++++------ test/tests/remote.js | 9 ++++----- test/tests/repository.js | 4 ++-- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/lib/reference.js b/lib/reference.js index cb308c017..c772239fd 100644 --- a/lib/reference.js +++ b/lib/reference.js @@ -13,6 +13,15 @@ var Branch = NodeGit.Branch; */ Reference.lookup = LookupWrapper(Reference); +/** +* Retrieves the reference by it's short name +* @param {Repository} repo The repo that the reference lives in +* @param {String|Reference} id The reference to lookup +* @param {Function} callback +* @return {Reference} +*/ +Reference.dwim = LookupWrapper(Reference, Reference.dwim); + /** * Returns true if this reference is valid * @return {Boolean} diff --git a/lib/repository.js b/lib/repository.js index 00c4e8bea..b2b1e60f3 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -55,7 +55,8 @@ function(name, commit, force, signature, logMessage) { /** * Look up a refs's commit. * - * @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" or Branch Ref + * @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" + * or Branch Ref * @param {Function} callback * @return {Commit} */ @@ -73,6 +74,30 @@ Repository.prototype.getReferenceCommit = function(name, callback) { }, callback); }; +/** +* Look up a branch. Alias for `getReference` +* +* @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" +* or Branch Ref +* @param {Function} callback +* @return {Ref} +*/ +Repository.prototype.getBranch = function(name, callback) { + return this.getReference(name, callback); +}; + +/** +* Look up a branch's most recent commit. Alias to `getReferenceCommit` +* +* @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" +* or Branch Ref +* @param {Function} callback +* @return {Commit} +*/ +Repository.prototype.getBranchCommit = function(name, callback) { + return this.getReferenceCommit(name, callback); +}; + /** * Gets the branch that HEAD currently points to * Is an alias to head() @@ -85,17 +110,15 @@ Repository.prototype.getCurrentBranch = function() { /** * Lookup the reference with the given name. * - * @param {String} name + * @param {String|Ref} name Ref name, e.g. "master", "refs/heads/master" + * or Branch Ref * @param {Function} callback * @return {Reference} */ Repository.prototype.getReference = function(name, callback) { var repository = this; - var lookup = name.indexOf("refs/") === 0 - ? Reference.lookup(this, name) - : Reference.dwim(this, name); - return lookup.then(function(reference) { + return Reference.dwim(this, name).then(function(reference) { if (reference.isSymbolic()) { return reference.resolve(function (error, reference) { reference.repo = repository; diff --git a/test/tests/remote.js b/test/tests/remote.js index b1bd807d8..dccbc03b3 100644 --- a/test/tests/remote.js +++ b/test/tests/remote.js @@ -104,11 +104,10 @@ describe("Remote", function() { }); it("can fetch from a remote", function() { - return this.repository.fetch("origin") - .then(function() { - assert(true); - }, function() { - assert(false); + return this.repository.fetch("origin", { + credentials: function(url, userName) { + return NodeGit.Cred.sshKeyFromAgent(userName); + } }); }); }); diff --git a/test/tests/repository.js b/test/tests/repository.js index ef45a630a..ed9650625 100644 --- a/test/tests/repository.js +++ b/test/tests/repository.js @@ -49,8 +49,8 @@ describe("Repository", function() { it("can list remotes", function() { return this.repository.getRemotes().then(function(remotes) { - assert.equal(remotes.count(), 1); - assert.equal(remotes.strings(), "origin"); + assert.equal(remotes.length, 1); + assert.equal(remotes[0], "origin"); }); }); From c1ff5faec096b56970b3804c5fdf84a7b0fde0ee Mon Sep 17 00:00:00 2001 From: John Haley Date: Fri, 5 Dec 2014 11:04:42 -0700 Subject: [PATCH 3/4] Added in optional ignore cert param for fetching --- lib/repository.js | 17 ++++++++++++++--- test/tests/remote.js | 11 ++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index b2b1e60f3..c6d813ae9 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -512,11 +512,17 @@ Repository.prototype.getRemote = function(remote, callback) { * * @param {String|Remote} remote */ -Repository.prototype.fetch = function(remote, remoteCallbacks, callback) { +Repository.prototype.fetch = function( + remote, + remoteCallbacks, + ignoreCertErrors, + callback) +{ var repo = this; return repo.getRemote(remote).then(function(remote) { remote.setCallbacks(remoteCallbacks); + remote.checkCert(ignoreCertErrors ? 0 : 1); return remote.fetch(repo.defaultSignature(), "Fetch from " + remote) .then(function() { @@ -530,14 +536,19 @@ Repository.prototype.fetch = function(remote, remoteCallbacks, callback) { /** * Fetches from all remotes */ -Repository.prototype.fetchAll = function(remoteCallbacks, callback) { +Repository.prototype.fetchAll = function( + remoteCallbacks, + ignoreCertErrors, + callback) +{ var repo = this; return repo.getRemotes().then(function(remotes) { var fetchPromises = []; remotes.forEach(function(remote) { - fetchPromises.push(repo.fetch(remote, remoteCallbacks, callback)); + fetchPromises.push( + repo.fetch(remote, remoteCallbacks, ignoreCertErrors, callback)); }); return Promise.all(fetchPromises); diff --git a/test/tests/remote.js b/test/tests/remote.js index dccbc03b3..daa3dd7de 100644 --- a/test/tests/remote.js +++ b/test/tests/remote.js @@ -108,6 +108,15 @@ describe("Remote", function() { credentials: function(url, userName) { return NodeGit.Cred.sshKeyFromAgent(userName); } - }); + }, true); }); + + it("can fetch from all remotes", function() { + return this.repository.fetchAll({ + credentials: function(url, userName) { + return NodeGit.Cred.sshKeyFromAgent(userName); + } + }, true); + }); + }); From d6f2b93fd63d44dfe13cbd3c7545c951a10076b7 Mon Sep 17 00:00:00 2001 From: John Haley Date: Fri, 5 Dec 2014 11:13:07 -0700 Subject: [PATCH 4/4] Set a higher timeout for the fetchAll test --- test/tests/remote.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/tests/remote.js b/test/tests/remote.js index daa3dd7de..af2354491 100644 --- a/test/tests/remote.js +++ b/test/tests/remote.js @@ -112,6 +112,9 @@ describe("Remote", function() { }); it("can fetch from all remotes", function() { + // Set a reasonable timeout here for the fetchAll test + this.timeout(15000); + return this.repository.fetchAll({ credentials: function(url, userName) { return NodeGit.Cred.sshKeyFromAgent(userName);