Do not allow non-ff-rebase if there are uncommitted changes

With this change jgit checks for uncommitted changes before a rebase is
started. This is also done by native git. One reason is that an abort
would override such changes. The check is skipped for a non-interactive
rebase when it will result in a fast-forward. In this case there can be
only checkout conflicts but no merge conflicts, so there cannot be an
abort which overrides uncommitted changes.

Bug: 422352
Change-Id: I1e0b59b2a4d80a686b67a6729e441924362b1236
Signed-off-by: Stefan Lay <stefan.lay@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 152 insertions, 28 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
index 2c9fce8186..b33ad6ba5b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java
@@ -80,7 +80,6 @@ import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.ReflogEntry;
 import org.eclipse.jgit.lib.RepositoryState;
 import org.eclipse.jgit.merge.MergeStrategy;
-import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.util.FileUtils;
@@ -1200,9 +1199,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// rebase
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.CONFLICTS, result.getStatus());
-		assertEquals(1, result.getConflicts().size());
-		assertEquals("file2", result.getConflicts().get(0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
+		assertEquals(1, result.getUncommittedChanges().size());
+		assertEquals("file2", result.getUncommittedChanges().get(0));
 	}
 
 	@Test
@@ -1233,9 +1232,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.CONFLICTS, result.getStatus());
-		assertEquals(1, result.getConflicts().size());
-		assertEquals("file2", result.getConflicts().get(0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
+		assertEquals(1, result.getUncommittedChanges().size());
+		assertEquals("file2", result.getUncommittedChanges().get(0));
 
 		checkFile(uncommittedFile, "uncommitted file2");
 		assertEquals(RepositoryState.SAFE, git.getRepository().getRepositoryState());
@@ -1268,9 +1267,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// rebase
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.CONFLICTS, result.getStatus());
-		assertEquals(1, result.getConflicts().size());
-		assertEquals(FILE1, result.getConflicts().get(0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
+		assertEquals(1, result.getUncommittedChanges().size());
+		assertEquals(FILE1, result.getUncommittedChanges().get(0));
 	}
 
 	@Test
@@ -1302,9 +1301,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// rebase
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.CONFLICTS, result.getStatus());
-		assertEquals(1, result.getConflicts().size());
-		assertEquals(FILE1, result.getConflicts().get(0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
+		assertEquals(1, result.getUncommittedChanges().size());
+		assertEquals(FILE1, result.getUncommittedChanges().get(0));
 	}
 
 	@Test
@@ -1333,7 +1332,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		writeTrashFile("file0", "unstaged modified file0");
 
 		// rebase
-		assertEquals(Status.OK, git.rebase().setUpstream("refs/heads/master")
+		assertEquals(Status.UNCOMMITTED_CHANGES,
+				git.rebase().setUpstream("refs/heads/master")
 				.call().getStatus());
 	}
 
@@ -1371,12 +1371,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// rebase
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.FAILED, result.getStatus());
-		// staged file0 causes DIRTY_INDEX
-		assertEquals(1, result.getFailingPaths().size());
-		assertEquals(MergeFailureReason.DIRTY_INDEX, result.getFailingPaths()
-				.get("file0"));
-		assertEquals("unstaged modified file0", read(file0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
+		assertEquals(1, result.getUncommittedChanges().size());
 		// index shall be unchanged
 		assertEquals(indexState, indexState(CONTENT));
 		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
@@ -1412,7 +1408,8 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		writeTrashFile("file0", "unstaged modified file0");
 
 		// rebase
-		assertEquals(Status.OK, git.rebase().setUpstream("refs/heads/master")
+		assertEquals(Status.UNCOMMITTED_CHANGES,
+				git.rebase().setUpstream("refs/heads/master")
 				.call().getStatus());
 	}
 
@@ -1453,11 +1450,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// rebase
 		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
 				.call();
-		assertEquals(Status.FAILED, result.getStatus());
+		assertEquals(Status.UNCOMMITTED_CHANGES, result.getStatus());
 		// staged file0 causes DIRTY_INDEX
-		assertEquals(1, result.getFailingPaths().size());
-		assertEquals(MergeFailureReason.DIRTY_INDEX, result.getFailingPaths()
-				.get("file0"));
+		assertEquals(1, result.getUncommittedChanges().size());
 		assertEquals("unstaged modified file0", read(file0));
 		// index shall be unchanged
 		assertEquals(indexState, indexState(CONTENT));
@@ -1465,6 +1460,82 @@ public class RebaseCommandTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void testFastForwardRebaseWithModification() throws Exception {
+		// create file0 + file1, add and commit
+		writeTrashFile("file0", "file0");
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch
+		createBranch(commit, "refs/heads/topic");
+
+		// still on master / modify file1, add and commit
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout topic branch / modify file0 and add to index
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file0", "modified file0 in index");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		// modify once more
+		writeTrashFile("file0", "modified file0");
+
+		// rebase
+		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+				.call();
+		assertEquals(Status.FAST_FORWARD, result.getStatus());
+		checkFile(new File(db.getWorkTree(), "file0"), "modified file0");
+		checkFile(new File(db.getWorkTree(), FILE1), "modified file1");
+		assertEquals("[file0, mode:100644, content:modified file0 in index]"
+				+ "[file1, mode:100644, content:modified file1]",
+				indexState(CONTENT));
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+	}
+
+	@Test
+	public void testRebaseWithModificationShouldNotDeleteData()
+			throws Exception {
+		// create file0 + file1, add and commit
+		writeTrashFile("file0", "file0");
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch
+		createBranch(commit, "refs/heads/topic");
+
+		// still on master / modify file1, add and commit
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout topic branch / modify file1, add and commit
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile(FILE1, "modified file1 on topic");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		writeTrashFile("file0", "modified file0");
+
+		RebaseResult result = git.rebase().setUpstream("refs/heads/master")
+				.call();
+		// the following condition was true before commit 83b6ab233:
+		// jgit started the rebase and deleted the change on abort
+		// This test should verify that content was deleted
+		if (result.getStatus() == Status.STOPPED)
+			git.rebase().setOperation(Operation.ABORT).call();
+
+		checkFile(new File(db.getWorkTree(), "file0"), "modified file0");
+		checkFile(new File(db.getWorkTree(), FILE1),
+				"modified file1 on topic");
+		assertEquals("[file0, mode:100644, content:file0]"
+				+ "[file1, mode:100644, content:modified file1 on topic]",
+				indexState(CONTENT));
+	}
+
+	@Test
 	public void testRebaseWithUncommittedDelete() throws Exception {
 		// create file0 + file1, add and commit
 		File file0 = writeTrashFile("file0", "file0");
@@ -1595,9 +1666,9 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// and attempt to rebase
 		RebaseResult rebaseResult = git.rebase()
 					.setUpstream("refs/heads/master").call();
-		assertEquals(Status.CONFLICTS, rebaseResult.getStatus());
-		assertEquals(1, rebaseResult.getConflicts().size());
-		assertEquals(FILE1, rebaseResult.getConflicts().get(0));
+		assertEquals(Status.UNCOMMITTED_CHANGES, rebaseResult.getStatus());
+		assertEquals(1, rebaseResult.getUncommittedChanges().size());
+		assertEquals(FILE1, rebaseResult.getUncommittedChanges().get(0));
 
 		checkFile(theFile, "dirty the file");
 
@@ -2585,6 +2656,59 @@ public class RebaseCommandTest extends RepositoryTestCase {
 
 	}
 
+	@Test
+	public void testInteractiveRebaseWithModificationShouldNotDeleteDataOnAbort()
+			throws Exception {
+		// create file0 + file1, add and commit
+		writeTrashFile("file0", "file0");
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		git.commit().setMessage("commit1").call();
+
+		// modify file1, add and commit
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit2").call();
+
+		// modify file1, add and commit
+		writeTrashFile(FILE1, "modified file1 a second time");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// modify file0, but do not commit
+		writeTrashFile("file0", "modified file0 in index");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		// do not commit
+		writeTrashFile("file0", "modified file0");
+
+		// start rebase
+		RebaseResult result = git.rebase().setUpstream("HEAD~2")
+				.runInteractively(new InteractiveHandler() {
+
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.EDIT);
+						steps.get(1).setAction(Action.PICK);
+					}
+
+					public String modifyCommitMessage(String commit) {
+						return commit;
+					}
+				}).call();
+		// the following condition was true before commit 83b6ab233:
+		// jgit started the rebase and deleted the change on abort
+		// This test should verify that content was deleted
+		if (result.getStatus() == Status.EDIT)
+			git.rebase().setOperation(Operation.ABORT).call();
+
+		checkFile(new File(db.getWorkTree(), "file0"), "modified file0");
+		checkFile(new File(db.getWorkTree(), "file1"),
+				"modified file1 a second time");
+		assertEquals("[file0, mode:100644, content:modified file0 in index]"
+				+ "[file1, mode:100644, content:modified file1 a second time]",
+				indexState(CONTENT));
+
+	}
+
 	private File getTodoFile() {
 		File todoFile = new File(db.getDirectory(), GIT_REBASE_TODO);
 		return todoFile;