From cc4f88e67dc8c6d28f8022f0c2d814d8fad23392 Mon Sep 17 00:00:00 2001 From: tyler wanek Date: Tue, 14 Feb 2017 14:47:12 -0700 Subject: [PATCH 1/3] Provide rebase details on finish When using the convenience methods for rebase, we should return all the details about the rebase before finishing. This includes the sha translation map for the rebase operation, and other meta data (old head sha, branch names, new head sha) --- lib/repository.js | 103 ++++++++++++++- test/tests/rebase.js | 308 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 404 insertions(+), 7 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 1a4e6eaa8..cc1459de7 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -1,5 +1,6 @@ var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); +var fp = require("lodash/fp"); var NodeGit = require("../"); var Blob = NodeGit.Blob; var Checkout = NodeGit.Checkout; @@ -184,12 +185,35 @@ function getPathHunks(repo, index, filePath, isStaged, additionalDiffOptions) { * invocation of next(). If the callback * returns a promise, the next() will be * called when the promise resolves. + * @param {Function} beforeFinishFn Callback called before the invocation + * of finish(). If the callback returns a + * promise, finish() will be called when the + * promise resolves. This callback will be + * provided a detailed overview of the rebase * @return {Int|Index} An error code for an unsuccesful rebase or an index for * a rebase with conflicts */ -function performRebase(repository, rebase, signature, beforeNextFn) { +function performRebase( + repository, + rebase, + signature, + beforeNextFn, + beforeFinishFn +) { var beforeNextFnResult; + function readRebaseMetadataFile(fileName) { + return fse.readFile( + path.join(repository.path(), "rebase-merge", fileName), + { encoding: "utf8" } + ) + .then(fp.trim); + } + + function calcHeadName(input) { + return input.replace(/refs\/heads\/(.*)/, "$1"); + } + function getPromise() { return rebase.next() .then(function() { @@ -201,11 +225,51 @@ function performRebase(repository, rebase, signature, beforeNextFn) { rebase.commit(null, signature); - return performRebase(repository, rebase, signature, beforeNextFn); + return performRebase( + repository, + rebase, + signature, + beforeNextFn, + beforeFinishFn + ); }); }, function(error) { if (error && error.errno === NodeGit.Error.CODE.ITEROVER) { - return rebase.finish(signature); + const calcRewritten = fp.flow([ + fp.split("\n"), + fp.map(fp.split(" ")) + ]); + + const beforeFinishFnPromise = !beforeFinishFn ? + Promise.resolve() : + Promise.all([ + readRebaseMetadataFile("onto_name"), + readRebaseMetadataFile("onto"), + readRebaseMetadataFile("head-name").then(calcHeadName), + readRebaseMetadataFile("orig-head"), + readRebaseMetadataFile("rewritten").then(calcRewritten) + ]) + .then(function([ + ontoName, + ontoSha, + originalHeadName, + originalHeadSha, + rewritten + ]) { + return beforeFinishFn({ + ontoName, + ontoSha, + originalHeadName, + originalHeadSha, + rebase, + rewritten + }); + }); + + return beforeFinishFnPromise + .then(function() { + return rebase.finish(signature); + }); } else { throw error; } @@ -354,10 +418,19 @@ Repository.prototype.checkoutRef = function(reference, opts) { * promise, the rebase will resume when the * promise resolves. The rebase object is * is passed to the callback. + * @param {Function} beforeFinishFn Callback called before the invocation + * of finish(). If the callback returns a + * promise, finish() will be called when the + * promise resolves. This callback will be + * provided a detailed overview of the rebase * @return {Oid|Index} A commit id for a succesful merge or an index for a * rebase with conflicts */ -Repository.prototype.continueRebase = function(signature, beforeNextFn) { +Repository.prototype.continueRebase = function( + signature, + beforeNextFn, + beforeFinishFn +) { var repo = this; signature = signature || repo.defaultSignature(); @@ -373,7 +446,13 @@ Repository.prototype.continueRebase = function(signature, beforeNextFn) { .then(function(rebase) { rebase.commit(null, signature); - return performRebase(repo, rebase, signature, beforeNextFn); + return performRebase( + repo, + rebase, + signature, + beforeNextFn, + beforeFinishFn + ); }) .then(function(error) { if (error) { @@ -1218,6 +1297,11 @@ Repository.prototype.isReverting = function() { * promise, the rebase will resume when the * promise resolves. The rebase object is * is passed to the callback. + * @param {Function} beforeFinishFn Callback called before the invocation + * of finish(). If the callback returns a + * promise, finish() will be called when the + * promise resolves. This callback will be + * provided a detailed overview of the rebase * @return {Oid|Index} A commit id for a succesful merge or an index for a * rebase with conflicts */ @@ -1227,6 +1311,7 @@ Repository.prototype.rebaseBranches = function( onto, signature, beforeNextFn, + beforeFinishFn, rebaseOptions ) { @@ -1278,7 +1363,13 @@ Repository.prototype.rebaseBranches = function( rebaseOptions ) .then(function(rebase) { - return performRebase(repo, rebase, signature, beforeNextFn); + return performRebase( + repo, + rebase, + signature, + beforeNextFn, + beforeFinishFn + ); }) .then(function(error) { if (error) { diff --git a/test/tests/rebase.js b/test/tests/rebase.js index 70c8e8ee6..56030d197 100644 --- a/test/tests/rebase.js +++ b/test/tests/rebase.js @@ -4,7 +4,7 @@ var local = path.join.bind(path, __dirname); var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); -describe("Rebase", function() { +describe.only("Rebase", function() { var NodeGit = require("../../"); var RepoUtils = require("../utils/repository_setup"); @@ -915,6 +915,312 @@ describe("Rebase", function() { }); }); + it("beforeFinishFn sync callback receives correct rebase data", function() { + var baseFileName = "baseNewFile.txt"; + var ourFileName = "ourNewFile.txt"; + var theirFileName = "theirNewFile.txt"; + + var baseFileContent = "How do you feel about Toll Roads?"; + var ourFileContent = "I like Toll Roads. I have an EZ-Pass!"; + var theirFileContent = "I'm skeptical about Toll Roads"; + + var ourSignature = NodeGit.Signature.create + ("Ron Paul", "RonPaul@TollRoadsRBest.info", 123456789, 60); + var theirSignature = NodeGit.Signature.create + ("Greg Abbott", "Gregggg@IllTollYourFace.us", 123456789, 60); + + var repository = this.repository; + var ourCommit; + var ourBranch; + var theirBranch; + var ourBranchShaPreRebase; + var ourBranchShaPostRebase = "b937100ee0ea17ef20525306763505a7fe2be29e"; + var theirBranchSha; + + var nextCalls = 0; + + var calledBeforeFinishFn = false; + + return fse.writeFile(path.join(repository.workdir(), baseFileName), + baseFileContent) + // Load up the repository index and make our initial commit to HEAD + .then(function() { + return RepoUtils.addFileToIndex(repository, baseFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "b5cdc109d437c4541a13fb7509116b5f03d5039a"); + + return repository.createCommit("HEAD", ourSignature, + ourSignature, "initial commit", oid, []); + }) + .then(function(commitOid) { + assert.equal(commitOid.toString(), + "be03abdf0353d05924c53bebeb0e5bb129cda44a"); + + return repository.getCommit(commitOid).then(function(commit) { + ourCommit = commit; + }).then(function() { + return repository.createBranch(ourBranchName, commitOid) + .then(function(branch) { + ourBranch = branch; + return repository.createBranch(theirBranchName, commitOid); + }); + }); + }) + .then(function(branch) { + theirBranch = branch; + return fse.writeFile(path.join(repository.workdir(), theirFileName), + theirFileContent); + }) + .then(function() { + return RepoUtils.addFileToIndex(repository, theirFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "be5f0fd38a39a67135ad68921c93cd5c17fefb3d"); + + return repository.createCommit(theirBranch.name(), theirSignature, + theirSignature, "they made a commit", oid, [ourCommit]); + }) + .then(function(commitOid) { + theirBranchSha = commitOid.toString(); + assert.equal(theirBranchSha, + "e9ebd92f2f4778baf6fa8e92f0c68642f931a554"); + + return removeFileFromIndex(repository, theirFileName); + }) + .then(function() { + return fse.remove(path.join(repository.workdir(), theirFileName)); + }) + .then(function() { + return fse.writeFile(path.join(repository.workdir(), ourFileName), + ourFileContent); + }) + .then(function() { + return RepoUtils.addFileToIndex(repository, ourFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "77867fc0bfeb3f80ab18a78c8d53aa3a06207047"); + + return repository.createCommit(ourBranch.name(), ourSignature, + ourSignature, "we made a commit", oid, [ourCommit]); + }) + .then(function(commitOid) { + ourBranchShaPreRebase = commitOid.toString(); + assert.equal(ourBranchShaPreRebase, + "e7f37ee070837052937e24ad8ba66f6d83ae7941"); + + return removeFileFromIndex(repository, ourFileName); + }) + .then(function() { + return fse.remove(path.join(repository.workdir(), ourFileName)); + }) + .then(function() { + return repository.checkoutBranch(ourBranchName); + }) + .then(function() { + return repository.rebaseBranches(ourBranchName, theirBranchName, + null, ourSignature, function(rebase) { + assert.ok(rebase instanceof NodeGit.Rebase); + + nextCalls++; + + return Promise.resolve(); + }, function({ + ontoName, + ontoSha, + originalHeadName, + originalHeadSha, + rebase, + rewritten + }) { + calledBeforeFinishFn = true; + + assert.equal(ontoName, theirBranchName); + assert.equal(ontoSha, theirBranchSha); + assert.equal(originalHeadName, ourBranchName); + assert.equal(originalHeadSha, ourBranchShaPreRebase); + assert.deepEqual( + rewritten, + [[ourBranchShaPreRebase, ourBranchShaPostRebase]] + ); + }); + }) + .then(function(commit) { + // verify that the beforeNextFn callback was called + assert.equal(nextCalls, 2); + + // verify that the beforeFinishFn callback was called + assert(calledBeforeFinishFn, "beforeFinishFn was not called"); + + // verify that the "ours" branch has moved to the correct place + assert.equal(commit.id().toString(), ourBranchShaPostRebase); + + return commit.parent(0); + }) + .then(function(commit) { + // verify that we are on top of "their commit" + assert.equal(commit.id().toString(), + "e9ebd92f2f4778baf6fa8e92f0c68642f931a554"); + }); + }); + + it("beforeFinishFn async callback receives correct rebase data", function() { + var baseFileName = "baseNewFile.txt"; + var ourFileName = "ourNewFile.txt"; + var theirFileName = "theirNewFile.txt"; + + var baseFileContent = "How do you feel about Toll Roads?"; + var ourFileContent = "I like Toll Roads. I have an EZ-Pass!"; + var theirFileContent = "I'm skeptical about Toll Roads"; + + var ourSignature = NodeGit.Signature.create + ("Ron Paul", "RonPaul@TollRoadsRBest.info", 123456789, 60); + var theirSignature = NodeGit.Signature.create + ("Greg Abbott", "Gregggg@IllTollYourFace.us", 123456789, 60); + + var repository = this.repository; + var ourCommit; + var ourBranch; + var theirBranch; + var ourBranchShaPreRebase; + var ourBranchShaPostRebase = "b937100ee0ea17ef20525306763505a7fe2be29e"; + var theirBranchSha; + + var nextCalls = 0; + + var calledBeforeFinishFn = false; + + return fse.writeFile(path.join(repository.workdir(), baseFileName), + baseFileContent) + // Load up the repository index and make our initial commit to HEAD + .then(function() { + return RepoUtils.addFileToIndex(repository, baseFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "b5cdc109d437c4541a13fb7509116b5f03d5039a"); + + return repository.createCommit("HEAD", ourSignature, + ourSignature, "initial commit", oid, []); + }) + .then(function(commitOid) { + assert.equal(commitOid.toString(), + "be03abdf0353d05924c53bebeb0e5bb129cda44a"); + + return repository.getCommit(commitOid).then(function(commit) { + ourCommit = commit; + }).then(function() { + return repository.createBranch(ourBranchName, commitOid) + .then(function(branch) { + ourBranch = branch; + return repository.createBranch(theirBranchName, commitOid); + }); + }); + }) + .then(function(branch) { + theirBranch = branch; + return fse.writeFile(path.join(repository.workdir(), theirFileName), + theirFileContent); + }) + .then(function() { + return RepoUtils.addFileToIndex(repository, theirFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "be5f0fd38a39a67135ad68921c93cd5c17fefb3d"); + + return repository.createCommit(theirBranch.name(), theirSignature, + theirSignature, "they made a commit", oid, [ourCommit]); + }) + .then(function(commitOid) { + theirBranchSha = commitOid.toString(); + assert.equal(theirBranchSha, + "e9ebd92f2f4778baf6fa8e92f0c68642f931a554"); + + return removeFileFromIndex(repository, theirFileName); + }) + .then(function() { + return fse.remove(path.join(repository.workdir(), theirFileName)); + }) + .then(function() { + return fse.writeFile(path.join(repository.workdir(), ourFileName), + ourFileContent); + }) + .then(function() { + return RepoUtils.addFileToIndex(repository, ourFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "77867fc0bfeb3f80ab18a78c8d53aa3a06207047"); + + return repository.createCommit(ourBranch.name(), ourSignature, + ourSignature, "we made a commit", oid, [ourCommit]); + }) + .then(function(commitOid) { + ourBranchShaPreRebase = commitOid.toString(); + assert.equal(ourBranchShaPreRebase, + "e7f37ee070837052937e24ad8ba66f6d83ae7941"); + + return removeFileFromIndex(repository, ourFileName); + }) + .then(function() { + return fse.remove(path.join(repository.workdir(), ourFileName)); + }) + .then(function() { + return repository.checkoutBranch(ourBranchName); + }) + .then(function() { + return repository.rebaseBranches(ourBranchName, theirBranchName, + null, ourSignature, function(rebase) { + assert.ok(rebase instanceof NodeGit.Rebase); + + nextCalls++; + + return Promise.resolve(); + }, function({ + ontoName, + ontoSha, + originalHeadName, + originalHeadSha, + rebase, + rewritten + }) { + calledBeforeFinishFn = true; + + assert.equal(ontoName, theirBranchName); + assert.equal(ontoSha, theirBranchSha); + assert.equal(originalHeadName, ourBranchName); + assert.equal(originalHeadSha, ourBranchShaPreRebase); + assert.deepEqual( + rewritten, + [[ourBranchShaPreRebase, ourBranchShaPostRebase]] + ); + + return Promise.resolve(); + }); + }) + .then(function(commit) { + // verify that the beforeNextFn callback was called + assert.equal(nextCalls, 2); + + // verify that the beforeFinishFn callback was called + assert(calledBeforeFinishFn, "beforeFinishFn was not called"); + + // verify that the "ours" branch has moved to the correct place + assert.equal(commit.id().toString(), ourBranchShaPostRebase); + + return commit.parent(0); + }) + .then(function(commit) { + // verify that we are on top of "their commit" + assert.equal(commit.id().toString(), + "e9ebd92f2f4778baf6fa8e92f0c68642f931a554"); + }); + }); + it("can rebase with conflicts using the convenience methods", function() { var fileName = "everyonesFile.txt"; From b5e2dc008420c7424b22c7d37bf635fabf526c0c Mon Sep 17 00:00:00 2001 From: tyler wanek Date: Wed, 15 Feb 2017 08:03:31 -0700 Subject: [PATCH 2/3] coverage utility is breaking over destructuring --- test/tests/rebase.js | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/test/tests/rebase.js b/test/tests/rebase.js index 56030d197..177c972ae 100644 --- a/test/tests/rebase.js +++ b/test/tests/rebase.js @@ -1028,22 +1028,18 @@ describe.only("Rebase", function() { nextCalls++; return Promise.resolve(); - }, function({ - ontoName, - ontoSha, - originalHeadName, - originalHeadSha, - rebase, - rewritten - }) { + }, function(rebaseMetadata) { calledBeforeFinishFn = true; - assert.equal(ontoName, theirBranchName); - assert.equal(ontoSha, theirBranchSha); - assert.equal(originalHeadName, ourBranchName); - assert.equal(originalHeadSha, ourBranchShaPreRebase); + assert.equal(rebaseMetadata.ontoName, theirBranchName); + assert.equal(rebaseMetadata.ontoSha, theirBranchSha); + assert.equal(rebaseMetadata.originalHeadName, ourBranchName); + assert.equal( + rebaseMetadata.originalHeadSha, + ourBranchShaPreRebase + ); assert.deepEqual( - rewritten, + rebaseMetadata.rewritten, [[ourBranchShaPreRebase, ourBranchShaPostRebase]] ); }); @@ -1180,22 +1176,18 @@ describe.only("Rebase", function() { nextCalls++; return Promise.resolve(); - }, function({ - ontoName, - ontoSha, - originalHeadName, - originalHeadSha, - rebase, - rewritten - }) { + }, function(rebaseMetadata) { calledBeforeFinishFn = true; - assert.equal(ontoName, theirBranchName); - assert.equal(ontoSha, theirBranchSha); - assert.equal(originalHeadName, ourBranchName); - assert.equal(originalHeadSha, ourBranchShaPreRebase); + assert.equal(rebaseMetadata.ontoName, theirBranchName); + assert.equal(rebaseMetadata.ontoSha, theirBranchSha); + assert.equal(rebaseMetadata.originalHeadName, ourBranchName); + assert.equal( + rebaseMetadata.originalHeadSha, + ourBranchShaPreRebase + ); assert.deepEqual( - rewritten, + rebaseMetadata.rewritten, [[ourBranchShaPreRebase, ourBranchShaPostRebase]] ); From 4141760652a425786e3b7978c83473af783ccfff Mon Sep 17 00:00:00 2001 From: tyler wanek Date: Tue, 28 Feb 2017 08:22:04 -0700 Subject: [PATCH 3/3] Should test everything, not just rebase --- test/tests/rebase.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/rebase.js b/test/tests/rebase.js index 177c972ae..731884f82 100644 --- a/test/tests/rebase.js +++ b/test/tests/rebase.js @@ -4,7 +4,7 @@ var local = path.join.bind(path, __dirname); var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); -describe.only("Rebase", function() { +describe("Rebase", function() { var NodeGit = require("../../"); var RepoUtils = require("../utils/repository_setup");