From 72644a6628a679896c3d081b2698eb20898e0a03 Mon Sep 17 00:00:00 2001 From: Carson Howard Date: Mon, 12 Mar 2018 15:19:52 -0700 Subject: [PATCH 1/3] Handle fast-forward merges in repository.performRebase --- lib/repository.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 56d2f5a6f..6b7422114 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -204,9 +204,18 @@ function performRebase( ) { var beforeNextFnResult; - function readRebaseMetadataFile(fileName) { + /* In the case of FF merges, this will fail + * when looking for 'rewritten' so we need + * to handle that case. + */ + function readRebaseMetadataFile(fileName, determineExistance) { + const fullPath = path.join(repository.path(), "rebase-merge", fileName); + if (determineExistance && !fse.existsSync(fullPath)) { + return Promise.resolve(null); + } + return fse.readFile( - path.join(repository.path(), "rebase-merge", fileName), + fullPath, { encoding: "utf8" } ) .then(fp.trim); @@ -241,6 +250,10 @@ function performRebase( .catch(function(error) { if (error && error.errno === NodeGit.Error.CODE.ITEROVER) { const calcRewritten = fp.flow([ + fp.cond([ + [fp.isEmpty, fp.constant("")], + [fp.stubTrue, fp.identity] + ]), fp.split("\n"), fp.map(fp.split(" ")) ]); @@ -252,7 +265,7 @@ function performRebase( readRebaseMetadataFile("onto"), readRebaseMetadataFile("head-name").then(calcHeadName), readRebaseMetadataFile("orig-head"), - readRebaseMetadataFile("rewritten").then(calcRewritten) + readRebaseMetadataFile("rewritten", true).then(calcRewritten) ]) .then(function([ ontoName, From 1c334c67e462e7c615516a768a665ec0e85998ca Mon Sep 17 00:00:00 2001 From: Carson Howard Date: Tue, 13 Mar 2018 10:02:29 -0700 Subject: [PATCH 2/3] Added a unit test and better comments --- lib/repository.js | 18 +++-- test/tests/rebase.js | 164 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 10 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 6b7422114..3a94b38a9 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -204,9 +204,8 @@ function performRebase( ) { var beforeNextFnResult; - /* In the case of FF merges, this will fail - * when looking for 'rewritten' so we need - * to handle that case. + /* In the case of FF merges and a beforeFinishFn, this will fail + * when looking for 'rewritten' so we need to handle that case. */ function readRebaseMetadataFile(fileName, determineExistance) { const fullPath = path.join(repository.path(), "rebase-merge", fileName); @@ -249,13 +248,12 @@ function performRebase( }) .catch(function(error) { if (error && error.errno === NodeGit.Error.CODE.ITEROVER) { - const calcRewritten = fp.flow([ - fp.cond([ - [fp.isEmpty, fp.constant("")], - [fp.stubTrue, fp.identity] - ]), - fp.split("\n"), - fp.map(fp.split(" ")) + const calcRewritten = fp.cond([ + [fp.isEmpty, fp.constant(null)], + [fp.stubTrue, fp.flow([ + fp.split("\n"), + fp.map(fp.split(" ")) + ])] ]); const beforeFinishFnPromise = !beforeFinishFn ? diff --git a/test/tests/rebase.js b/test/tests/rebase.js index d121a6c0f..f9dd3d2a2 100644 --- a/test/tests/rebase.js +++ b/test/tests/rebase.js @@ -6,6 +6,8 @@ var fse = promisify(require("fs-extra")); describe("Rebase", function() { var NodeGit = require("../../"); + var Checkout = NodeGit.Checkout; + var Merge = NodeGit.Merge; var RepoUtils = require("../utils/repository_setup"); var repoPath = local("../repos/rebase"); @@ -790,6 +792,168 @@ describe("Rebase", function() { }); }); + it( + "can fast-forward a merge commit via rebase using the " + + "convenience methods that has a beforeFinishFn", + function() { + var ourFileName = "ourNewFile.txt"; + var theirFileName = "theirNewFile.txt"; + var theirOtherFileName = "antherNewFile.txt"; + + var ourFileContent = "I like Toll Roads. I have an EZ-Pass!"; + var theirFileContent = "I'm skeptical about Toll Roads"; + var theirOtherFileContent = "This is some more content, guys!"; + + 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 theirOtherSignature = NodeGit.Signature.create + ("Greg Abbott", "Gregggg@IllTollYourFace.us", 123456999, 60); + var ourMergeSignature = NodeGit.Signature.create + ("Ron Paul", "RonPaul@TollRoadsRBest.info", 123456889, 60); + + var repository = this.repository; + var ourCommit; + var theirCommit; + var theirBranch; + var ourBranch; + + return fse.writeFile(path.join(repository.workdir(), ourFileName), + ourFileContent) + // Load up the repository index and make our initial commit to HEAD + .then(function() { + return RepoUtils.addFileToIndex(repository, ourFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "11ead82b1135b8e240fb5d61e703312fb9cc3d6a"); + + return repository.createCommit("HEAD", ourSignature, + ourSignature, "we made a commit", oid, []); + }) + .then(function(commitOid) { + assert.equal(commitOid.toString(), + "91a183f87842ebb7a9b08dad8bc2473985796844"); + + 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(), + "76631cb5a290dafe2959152626bb90f2a6d8ec94"); + + return repository.createCommit(theirBranch.name(), theirSignature, + theirSignature, "they made a commit", oid, [ourCommit]); + }) + .then(function(commitOid) { + theirCommit = commitOid; + assert.equal(commitOid.toString(), + "0e9231d489b3f4303635fc4b0397830da095e7e7"); + }) + .then(function() { + return repository.checkoutBranch( + ourBranch, + { checkoutStrategy: Checkout.STRATEGY.FORCE } + ); + }) + .then(function() { + return repository.mergeBranches( + ourBranchName, + theirBranchName, + ourMergeSignature, + Merge.PREFERENCE.NO_FASTFORWARD + ); + }) + .then(function() { + return repository.getHeadCommit(); + }) + .then(function(headCommit) { + assert.notEqual(ourCommit.id().toString(), headCommit.id().toString()); + }) + .then(function() { + return repository.checkoutBranch( + theirBranch, + { checkoutStrategy: Checkout.STRATEGY.FORCE } + ); + }) + .then(function() { + return fse.writeFile( + path.join(repository.workdir(), theirOtherFileName), + theirOtherFileContent + ); + }) + .then(function() { + return RepoUtils.addFileToIndex(repository, theirOtherFileName); + }) + .then(function(oid) { + assert.equal(oid.toString(), + "c242b53f2c9446544cf9bdac7e8ed6ce583226cb"); + + return repository.createCommit(theirBranch.name(), theirOtherSignature, + theirOtherSignature, "they made another commit", oid, [theirCommit]); + }) + .then(function(commitOid) { + assert.equal(commitOid.toString(), + "8fa0ce25a2accf464b004ddeeb63add7b816b627"); + }) + .then(function() { + return Promise.all([ + repository.getReference(ourBranchName), + repository.getReference(theirBranchName) + ]); + }) + .then(function(refs) { + assert.equal(refs.length, 2); + + return Promise.all([ + NodeGit.AnnotatedCommit.fromRef(repository, refs[0]), + NodeGit.AnnotatedCommit.fromRef(repository, refs[1]) + ]); + }) + .then(function(annotatedCommits) { + assert.equal(annotatedCommits.length, 2); + + var ourAnnotatedCommit = annotatedCommits[0]; + var theirAnnotatedCommit = annotatedCommits[1]; + + assert.equal(ourAnnotatedCommit.id().toString(), + "0d1d322b59df68bac6eea6a2a189f974cb590368"); + assert.equal(theirAnnotatedCommit.id().toString(), + "8fa0ce25a2accf464b004ddeeb63add7b816b627"); + + return repository.rebaseBranches( + ourBranchName, + theirBranchName, + null, + ourSignature, + null, + function(rebaseData) { + assert.equal(rebaseData.rewritten, null); + } + ); + }) + .then(function(commit) { + assert.equal(commit.id().toString(), + "8fa0ce25a2accf464b004ddeeb63add7b816b627"); + }); + }); + it("can rebase using the convenience method", function() { var baseFileName = "baseNewFile.txt"; var ourFileName = "ourNewFile.txt"; From 62daba3b075441b48f5fefc3b4b2438e14b31e85 Mon Sep 17 00:00:00 2001 From: Carson Howard Date: Thu, 15 Mar 2018 13:35:18 -0700 Subject: [PATCH 3/3] Replace fse.existsSync with a catch on fse.readFile --- lib/repository.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/repository.js b/lib/repository.js index 3a94b38a9..d876b1241 100644 --- a/lib/repository.js +++ b/lib/repository.js @@ -207,17 +207,18 @@ function performRebase( /* In the case of FF merges and a beforeFinishFn, this will fail * when looking for 'rewritten' so we need to handle that case. */ - function readRebaseMetadataFile(fileName, determineExistance) { - const fullPath = path.join(repository.path(), "rebase-merge", fileName); - if (determineExistance && !fse.existsSync(fullPath)) { - return Promise.resolve(null); - } - + function readRebaseMetadataFile(fileName, continueOnError) { return fse.readFile( - fullPath, + path.join(repository.path(), "rebase-merge", fileName), { encoding: "utf8" } ) - .then(fp.trim); + .then(fp.trim) + .catch(function(err) { + if (continueOnError) { + return null; + } + throw err; + }); } function calcHeadName(input) {