From f613b52fe6629433c56219e6862c9e3f66b719ff Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sat, 21 Mar 2015 12:13:03 -0700 Subject: [PATCH 1/4] Checkout branch with tree instead of head Checkout.head futzes with the index, and that's not what we want in a branch checkout --- lib/repository.js | 23 ++++++++++++++--------- test/tests/checkout.js | 16 +++++++++------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 37e9833ff..4da999de1 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -804,24 +804,29 @@ Repository.prototype.getStatusExt = function(opts) { */ Repository.prototype.checkoutBranch = function(branch, opts) { var repo = this; - + var reference; opts = opts || {}; - opts.checkoutStrategy = opts.checkoutStrategy || Checkout.STRATEGY.SAFE; - + opts.checkoutStrategy = opts.checkoutStrategy || + Checkout.STRATEGY.SAFE_CREATE; return repo.getReference(branch) .then(function(ref) { if (!ref.isBranch()) { return false; } - - var name = ref.name(); - + reference = ref; + return repo.getBranchCommit(ref.name()); + }) + .then(function(commit) { + return commit.getTree(); + }) + .then(function(tree) { + return Checkout.tree(repo, tree, opts); + }) + .then(function() { + var name = reference.name(); return repo.setHead(name, repo.defaultSignature(), "Switch HEAD to " + name); - }) - .then(function() { - return Checkout.head(repo, opts); }); }; diff --git a/test/tests/checkout.js b/test/tests/checkout.js index fbe565125..99c63d35b 100644 --- a/test/tests/checkout.js +++ b/test/tests/checkout.js @@ -83,21 +83,23 @@ describe("Checkout", function() { }); }); - it("can checkout a branch", function() { + it.only("can checkout a branch", function() { var test = this; - return test.repository.checkoutBranch(checkoutBranchName, { - checkoutStrategy: Checkout.STRATEGY.FORCE - }) + return test.repository.checkoutBranch(checkoutBranchName) .then(function() { var packageContent = fse.readFileSync(packageJsonPath, "utf-8"); assert.ok(!~packageContent.indexOf("\"ejs\": \"~1.0.0\",")); }) .then(function() { - return test.repository.checkoutBranch("master", { - checkoutStrategy: Checkout.STRATEGY.FORCE - }); + return test.repository.getStatus(); + }) + .then(function(statuses) { + assert.equal(statuses.length, 0); + }) + .then(function() { + return test.repository.checkoutBranch("master"); }) .then(function() { var packageContent = fse.readFileSync(packageJsonPath, "utf-8"); From a4a4c8b66bac9feba25bd00e3c0c17efc5e4d203 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Thu, 19 Mar 2015 11:48:48 -0400 Subject: [PATCH 2/4] Properly update the index on Merge FF --- lib/repository.js | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 4da999de1..64b75d7be 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -661,15 +661,18 @@ Repository.prototype.mergeBranches = function(to, from, signature) { var repo = this; var fromBranch; var toBranch; + var headCommit; signature = signature || repo.defaultSignature(); return Promise.all([ repo.getBranch(to), - repo.getBranch(from) - ]).then(function(branches) { - toBranch = branches[0]; - fromBranch = branches[1]; + repo.getBranch(from), + repo.getHeadCommit() + ]).then(function(objects) { + toBranch = objects[0]; + fromBranch = objects[1]; + headCommit = objects[2]; return Promise.all([ repo.getBranchCommit(toBranch), @@ -694,13 +697,26 @@ Repository.prototype.mergeBranches = function(to, from, signature) { " to branch " + fromBranch.shorthand(); - return toBranch.setTarget( - fromCommitOid, - signature, - message) + return branchCommits[1].getTree() + .then(function(tree) { + if (headCommit.toString() == toCommitOid) { + // Checkout the tree if we're on the branch + var opts = {checkoutStrategy: NodeGit.Checkout.STRATEGY.SAFE}; + return NodeGit.Checkout.tree(repo, tree, opts); + } else { + // Otherwise, just point the ref + return; + } + }) .then(function() { - return fromCommitOid; - }); + return toBranch.setTarget( + fromCommitOid, + signature, + message) + .then(function() { + return fromCommitOid; + }); + }) } else { // We have to merge. Lets do it! From 5bffb8d629994dabe0dbdaac288ef33c074652bc Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sat, 21 Mar 2015 12:14:03 -0700 Subject: [PATCH 3/4] Remove only from tests --- test/tests/checkout.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/checkout.js b/test/tests/checkout.js index 99c63d35b..05c3105a9 100644 --- a/test/tests/checkout.js +++ b/test/tests/checkout.js @@ -83,7 +83,7 @@ describe("Checkout", function() { }); }); - it.only("can checkout a branch", function() { + it("can checkout a branch", function() { var test = this; return test.repository.checkoutBranch(checkoutBranchName) From 7775aa70c1dd296996054af71a14b2b50d52c210 Mon Sep 17 00:00:00 2001 From: Steve Smith Date: Sun, 22 Mar 2015 09:26:55 -0700 Subject: [PATCH 4/4] Fast forward checking out the tree instead of the head Don't need to manipulate the index in this scenario --- lib/repository.js | 21 +++++++++------------ test/tests/merge.js | 11 +++++++++++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 64b75d7be..729bb1415 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -661,24 +661,22 @@ Repository.prototype.mergeBranches = function(to, from, signature) { var repo = this; var fromBranch; var toBranch; - var headCommit; signature = signature || repo.defaultSignature(); return Promise.all([ - repo.getBranch(to), - repo.getBranch(from), - repo.getHeadCommit() + repo.getBranch(to), + repo.getBranch(from) ]).then(function(objects) { toBranch = objects[0]; fromBranch = objects[1]; - headCommit = objects[2]; return Promise.all([ repo.getBranchCommit(toBranch), repo.getBranchCommit(fromBranch) ]); - }).then(function(branchCommits) { + }) + .then(function(branchCommits) { var toCommitOid = branchCommits[0].toString(); var fromCommitOid = branchCommits[1].toString(); @@ -699,13 +697,12 @@ Repository.prototype.mergeBranches = function(to, from, signature) { return branchCommits[1].getTree() .then(function(tree) { - if (headCommit.toString() == toCommitOid) { + if (toBranch.isHead()) { // Checkout the tree if we're on the branch - var opts = {checkoutStrategy: NodeGit.Checkout.STRATEGY.SAFE}; + var opts = { + checkoutStrategy: NodeGit.Checkout.STRATEGY.SAFE_CREATE + }; return NodeGit.Checkout.tree(repo, tree, opts); - } else { - // Otherwise, just point the ref - return; } }) .then(function() { @@ -716,7 +713,7 @@ Repository.prototype.mergeBranches = function(to, from, signature) { .then(function() { return fromCommitOid; }); - }) + }); } else { // We have to merge. Lets do it! diff --git a/test/tests/merge.js b/test/tests/merge.js index 14f4b4a2f..b08de4bc3 100644 --- a/test/tests/merge.js +++ b/test/tests/merge.js @@ -214,6 +214,10 @@ describe("Merge", function() { theirCommit = commit; }); }) + .then(function() { + var opts = {checkoutStrategy: NodeGit.Checkout.STRATEGY.FORCE}; + return repository.checkoutBranch(ourBranchName, opts); + }) .then(function() { return repository.mergeBranches( ourBranchName, @@ -228,6 +232,13 @@ describe("Merge", function() { .then(function(branchCommit) { assert.equal(oid.toString(), branchCommit.toString()); }); + }) + .then(function() { + return repository.getStatus(); + }) + .then(function(statuses) { + // make sure we didn't change the index + assert.equal(statuses.length, 0); }); });