Merge pull request #1457 from cjhoward92/fix/rebase-ff-merge

Handle fast-forward merges properly in repository.performRebase

lib/repository.js

@@ -204,12 +204,21 @@ function performRebase(
 ) {
   var beforeNextFnResult;
 
-  function readRebaseMetadataFile(fileName) {
+  /* 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, continueOnError) {
     return fse.readFile(
       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) {
@@ -240,9 +249,12 @@ function performRebase(
       })
       .catch(function(error) {
         if (error && error.errno === NodeGit.Error.CODE.ITEROVER) {
-          const calcRewritten = fp.flow([
-            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 ?
@@ -252,7 +264,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,

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";