Introduce FAILED result for RebaseCommand

In case an underlying cherry-pick fails due to uncommitted changes, a
RebaseCommand shall fail and roll-back changes.

Change-Id: Ic22eb047fb03ac2c8391f777036b7dbf22a1b061
Signed-off-by: Philipp Thun <philipp.thun@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/api/RebaseCommandTest.java

@@ -57,6 +57,7 @@
 import org.eclipse.jgit.api.RebaseCommand.Action;
 import org.eclipse.jgit.api.RebaseCommand.Operation;
 import org.eclipse.jgit.api.RebaseResult.Status;
+import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.api.errors.UnmergedPathsException;
 import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
@@ -67,6 +68,7 @@
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.RepositoryState;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.junit.Before;
@@ -878,6 +880,344 @@ public void testRepositoryStateChecks() throws Exception {
 		}
 	}
 
+	@Test
+	public void testRebaseWithUntrackedFile() throws Exception {
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / create untracked file3
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file3", "untracked file3");
+
+		// rebase
+		assertEquals(Status.OK, git.rebase().setUpstream("refs/heads/master")
+				.call().getStatus());
+	}
+
+	@Test
+	@SuppressWarnings("null")
+	public void testRebaseWithUnstagedTopicChange() throws Exception {
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file2
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "unstaged file2");
+
+		// rebase
+		JGitInternalException exception = null;
+		try {
+			git.rebase().setUpstream("refs/heads/master").call();
+		} catch (JGitInternalException e) {
+			exception = e;
+		}
+		assertNotNull(exception);
+		assertEquals("Checkout conflict with files: \nfile2",
+				exception.getMessage());
+	}
+
+	@Test
+	@SuppressWarnings("null")
+	public void testRebaseWithUncommittedTopicChange() throws Exception {
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file2 and add
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "uncommitted file2");
+		git.add().addFilepattern("file2").call();
+		// do not commit
+
+		// rebase
+		JGitInternalException exception = null;
+		try {
+			git.rebase().setUpstream("refs/heads/master").call();
+		} catch (JGitInternalException e) {
+			exception = e;
+		}
+		assertNotNull(exception);
+		assertEquals("Checkout conflict with files: \nfile2",
+				exception.getMessage());
+	}
+
+	@Test
+	@SuppressWarnings("null")
+	public void testRebaseWithUnstagedMasterChange() throws Exception {
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file1
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile(FILE1, "unstaged modified file1");
+
+		// rebase
+		JGitInternalException exception = null;
+		try {
+			git.rebase().setUpstream("refs/heads/master").call();
+		} catch (JGitInternalException e) {
+			exception = e;
+		}
+		assertNotNull(exception);
+		assertEquals("Checkout conflict with files: \nfile1",
+				exception.getMessage());
+	}
+
+	@Test
+	@SuppressWarnings("null")
+	public void testRebaseWithUncommittedMasterChange() throws Exception {
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file1 and add
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile(FILE1, "uncommitted modified file1");
+		git.add().addFilepattern(FILE1).call();
+		// do not commit
+
+		// rebase
+		JGitInternalException exception = null;
+		try {
+			git.rebase().setUpstream("refs/heads/master").call();
+		} catch (JGitInternalException e) {
+			exception = e;
+		}
+		assertNotNull(exception);
+		assertEquals("Checkout conflict with files: \nfile1",
+				exception.getMessage());
+	}
+
+	@Test
+	public void testRebaseWithUnstagedMasterChangeBaseCommit() 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 and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file0
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file0", "unstaged modified file0");
+
+		// rebase
+		assertEquals(Status.OK, git.rebase().setUpstream("refs/heads/master")
+				.call().getStatus());
+	}
+
+	@Test
+	public void testRebaseWithUncommittedMasterChangeBaseCommit()
+			throws Exception {
+		// create file0 + file1, add and commit
+		File file0 = writeTrashFile("file0", "file0");
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern("file0").addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file0 and add
+		checkoutBranch("refs/heads/topic");
+		write(file0, "unstaged modified file0");
+		git.add().addFilepattern("file0").call();
+		// do not commit
+
+		// get current index state
+		String indexState = indexState(CONTENT);
+
+		// 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));
+		// index shall be unchanged
+		assertEquals(indexState, indexState(CONTENT));
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+	}
+
+	@Test
+	public void testRebaseWithUnstagedMasterChangeOtherCommit()
+			throws Exception {
+		// create file0, add and commit
+		writeTrashFile("file0", "file0");
+		git.add().addFilepattern("file0").call();
+		git.commit().setMessage("commit0").call();
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file0
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file0", "unstaged modified file0");
+
+		// rebase
+		assertEquals(Status.OK, git.rebase().setUpstream("refs/heads/master")
+				.call().getStatus());
+	}
+
+	@Test
+	public void testRebaseWithUncommittedMasterChangeOtherCommit()
+			throws Exception {
+		// create file0, add and commit
+		File file0 = writeTrashFile("file0", "file0");
+		git.add().addFilepattern("file0").call();
+		git.commit().setMessage("commit0").call();
+		// create file1, add and commit
+		writeTrashFile(FILE1, "file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit commit = git.commit().setMessage("commit1").call();
+
+		// create topic branch and checkout / create file2, add and commit
+		createBranch(commit, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("commit2").call();
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("commit3").call();
+
+		// checkout topic branch / modify file0 and add
+		checkoutBranch("refs/heads/topic");
+		write(file0, "unstaged modified file0");
+		git.add().addFilepattern("file0").call();
+		// do not commit
+
+		// get current index state
+		String indexState = indexState(CONTENT);
+
+		// 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));
+		// index shall be unchanged
+		assertEquals(indexState, indexState(CONTENT));
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+	}
+
 	private int countPicks() throws IOException {
 		int count = 0;
 		File todoFile = new File(db.getDirectory(),

org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java

@@ -199,7 +199,7 @@ public RebaseResult call() throws NoHeadException, RefNotFoundException,
 			switch (operation) {
 			case ABORT:
 				try {
-					return abort();
+					return abort(new RebaseResult(Status.ABORTED));
 				} catch (IOException ioe) {
 					throw new JGitInternalException(ioe.getMessage(), ioe);
 				}
@@ -217,12 +217,12 @@ public RebaseResult call() throws NoHeadException, RefNotFoundException,
 			}
 
 			if (monitor.isCancelled())
-				return abort();
+				return abort(new RebaseResult(Status.ABORTED));
 
-			if (this.operation == Operation.CONTINUE)
+			if (operation == Operation.CONTINUE)
 				newHead = continueRebase();
 
-			if (this.operation == Operation.SKIP)
+			if (operation == Operation.SKIP)
 				newHead = checkoutCurrentHead();
 
 			ObjectReader or = repo.newObjectReader();
@@ -238,23 +238,37 @@ public RebaseResult call() throws NoHeadException, RefNotFoundException,
 						.parseCommit(ids.iterator().next());
 				if (monitor.isCancelled())
 					return new RebaseResult(commitToPick);
+				try {
 					monitor.beginTask(MessageFormat.format(
-						JGitText.get().applyingCommit, commitToPick
+							JGitText.get().applyingCommit,
-								.getShortMessage()), ProgressMonitor.UNKNOWN);
+							commitToPick.getShortMessage()),
+							ProgressMonitor.UNKNOWN);
 					// if the first parent of commitToPick is the current HEAD,
 					// we do a fast-forward instead of cherry-pick to avoid
 					// unnecessary object rewriting
 					newHead = tryFastForward(commitToPick);
 					lastStepWasForward = newHead != null;
-				if (!lastStepWasForward)
+					if (!lastStepWasForward) {
-					// TODO if the content of this commit is already merged here
+						// TODO if the content of this commit is already merged
-					// we should skip this step in order to avoid confusing
+						// here we should skip this step in order to avoid
-					// pseudo-changed
+						// confusing pseudo-changed
-					newHead = new Git(repo).cherryPick().include(commitToPick)
+						CherryPickResult cherryPickResult = new Git(repo)
-							.call().getNewHead();
+								.cherryPick().include(commitToPick).call();
-				monitor.endTask();
+						switch (cherryPickResult.getStatus()) {
-				if (newHead == null) {
+						case FAILED:
+							if (operation == Operation.BEGIN)
+								return abort(new RebaseResult(
+										cherryPickResult.getFailingPaths()));
+							else
 								return stop(commitToPick);
+						case CONFLICTING:
+							return stop(commitToPick);
+						case OK:
+							newHead = cherryPickResult.getNewHead();
+						}
+					}
+				} finally {
+					monitor.endTask();
 				}
 			}
 			if (newHead != null) {
@@ -685,17 +699,23 @@ private void createFile(File parentDir, String name, String content)
 		}
 	}
 
-	private RebaseResult abort() throws IOException {
+	private RebaseResult abort(RebaseResult result) throws IOException {
 		try {
 			String commitId = readFile(repo.getDirectory(), Constants.ORIG_HEAD);
 			monitor.beginTask(MessageFormat.format(
 					JGitText.get().abortingRebase, commitId),
 					ProgressMonitor.UNKNOWN);
 
+			DirCacheCheckout dco;
 			RevCommit commit = walk.parseCommit(repo.resolve(commitId));
-			// no head in order to reset --hard
+			if (result.getStatus().equals(Status.FAILED)) {
-			DirCacheCheckout dco = new DirCacheCheckout(repo, repo
+				RevCommit head = walk.parseCommit(repo.resolve(Constants.HEAD));
-					.lockDirCache(), commit.getTree());
+				dco = new DirCacheCheckout(repo, head.getTree(),
+						repo.lockDirCache(), commit.getTree());
+			} else {
+				dco = new DirCacheCheckout(repo, repo.lockDirCache(),
+						commit.getTree());
+			}
 			dco.setFailOnConflict(false);
 			dco.checkout();
 			walk.release();
@@ -724,7 +744,7 @@ private RebaseResult abort() throws IOException {
 			}
 			// cleanup the files
 			FileUtils.delete(rebaseDir, FileUtils.RECURSIVE);
-			return new RebaseResult(Status.ABORTED);
+			return result;
 
 		} finally {
 			monitor.endTask();

org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseResult.java

@@ -42,6 +42,10 @@
  */
 package org.eclipse.jgit.api;
 
+import java.util.Map;
+
+import org.eclipse.jgit.merge.ResolveMerger;
+import org.eclipse.jgit.merge.ResolveMerger.MergeFailureReason;
 import org.eclipse.jgit.revwalk.RevCommit;
 
 /**
@@ -64,6 +68,10 @@ public enum Status {
 		 * Stopped due to a conflict; must either abort or resolve or skip
 		 */
 		STOPPED,
+		/**
+		 * Failed; the original HEAD was restored
+		 */
+		FAILED,
 		/**
 		 * Already up-to-date
 		 */
@@ -81,16 +89,36 @@ public enum Status {
 
 	private final RevCommit currentCommit;
 
+	private Map<String, MergeFailureReason> failingPaths;
+
 	RebaseResult(Status status) {
 		this.mySatus = status;
 		currentCommit = null;
 	}
 
+	/**
+	 * Create <code>RebaseResult</code> with status {@link Status#STOPPED}
+	 *
+	 * @param commit
+	 *            current commit
+	 */
 	RebaseResult(RevCommit commit) {
-		this.mySatus = Status.STOPPED;
+		mySatus = Status.STOPPED;
 		currentCommit = commit;
 	}
 
+	/**
+	 * Create <code>RebaseResult</code> with status {@link Status#FAILED}
+	 * 
+	 * @param failingPaths
+	 *            list of paths causing this rebase to fail abnormally
+	 */
+	RebaseResult(Map<String, MergeFailureReason> failingPaths) {
+		mySatus = Status.FAILED;
+		currentCommit = null;
+		this.failingPaths = failingPaths;
+	}
+
 	/**
 	 * @return the overall status
 	 */
@@ -105,4 +133,13 @@ public Status getStatus() {
 	public RevCommit getCurrentCommit() {
 		return currentCommit;
 	}
+
+	/**
+	 * @return the list of paths causing this rebase to fail abnormally (see
+	 *         {@link ResolveMerger#getFailingPaths()} for details) if status is
+	 *         {@link Status#FAILED}, otherwise <code>null</code>
+	 */
+	public Map<String, MergeFailureReason> getFailingPaths() {
+		return failingPaths;
+	}
 }