RebaseCommand: return correct status on stash apply conflicts

Ensure that also the fast-forward cases return status
STASH_APPLY_CONFLICTS when applying the stash produces conflicts.

Bug: 582526
Change-Id: Ib989ff431dca6e301eb05156ca054a7115fa6ad5
Signed-off-by: Thomas Wolf <twolf@apache.org>

2 files changed, 71 insertions, 13 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 7f820b0433..16f7cd1eb0 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
@@ -2221,7 +2221,7 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		checkoutBranch("refs/heads/master");
 		writeTrashFile(FILE1, "modified file1");
 		git.add().addFilepattern(FILE1).call();
-		git.commit().setMessage("commit3").call();
+		git.commit().setMessage("commit2").call();
 
 		// checkout topic branch / modify file0
 		checkoutBranch("refs/heads/topic");
@@ -2240,6 +2240,57 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
 	}
 
+	@Test
+	public void testFastForwardRebaseWithAutoStashConflict() throws Exception {
+		// create file0, add and commit
+		db.getConfig().setBoolean(ConfigConstants.CONFIG_REBASE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_AUTOSTASH, true);
+		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
+		createBranch(commit, "refs/heads/topic");
+
+		// checkout master branch / modify file1, add and commit
+		checkoutBranch("refs/heads/master");
+		writeTrashFile(FILE1, "modified file1");
+		git.add().addFilepattern(FILE1).call();
+		RevCommit master = git.commit().setMessage("commit2").call();
+
+		// checkout topic branch / modify file0 and file1
+		checkoutBranch("refs/heads/topic");
+		writeTrashFile("file0", "unstaged modified file0");
+		writeTrashFile(FILE1, "unstaged modified file1");
+
+		// rebase
+		assertEquals(Status.STASH_APPLY_CONFLICTS,
+				git.rebase().setUpstream("refs/heads/master").call()
+						.getStatus());
+		checkFile(new File(db.getWorkTree(), "file0"),
+				"unstaged modified file0");
+		checkFile(new File(db.getWorkTree(), FILE1),
+				"<<<<<<< HEAD\n"
+						+ "modified file1\n"
+						+ "=======\n"
+						+ "unstaged modified file1\n"
+						+ ">>>>>>> stash\n");
+		// If there is a merge conflict, the index is not reset, and thus file0
+		// is staged here. This is the same behavior as in C git.
+		String expected = "[file0, mode:100644, content:unstaged modified file0]"
+				+ "[file1, mode:100644, stage:1, content:file1]"
+				+ "[file1, mode:100644, stage:2, content:modified file1]"
+				+ "[file1, mode:100644, stage:3, content:unstaged modified file1]";
+		assertEquals(expected, indexState(CONTENT));
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+		assertEquals(master, db.resolve(Constants.HEAD));
+		assertEquals(master, db.resolve("refs/heads/topic"));
+	}
+
 	private List<DiffEntry> getStashedDiff() throws AmbiguousObjectException,
 			IncorrectObjectTypeException, IOException, MissingObjectException {
 		ObjectId stashId = db.resolve("stash@{0}");
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
index 859bf93841..0f7b8afe21 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -289,13 +289,17 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 					}
 				}
 				RebaseResult res = initFilesAndRewind();
-				if (stopAfterInitialization)
+				if (stopAfterInitialization) {
 					return RebaseResult.INTERACTIVE_PREPARED_RESULT;
+				}
 				if (res != null) {
-					autoStashApply();
-					if (rebaseState.getDir().exists())
+					if (!autoStashApply()) {
+						res = RebaseResult.STASH_APPLY_CONFLICTS_RESULT;
+					}
+					if (rebaseState.getDir().exists()) {
 						FileUtils.delete(rebaseState.getDir(),
 								FileUtils.RECURSIVE);
+					}
 					return res;
 				}
 			}
@@ -381,7 +385,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 	}
 
 	private boolean autoStashApply() throws IOException, GitAPIException {
-		boolean conflicts = false;
+		boolean success = true;
 		if (rebaseState.getFile(AUTOSTASH).exists()) {
 			String stash = rebaseState.readFile(AUTOSTASH);
 			try (Git git = Git.wrap(repo)) {
@@ -389,7 +393,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 						.ignoreRepositoryState(true).setStrategy(strategy)
 						.call();
 			} catch (StashApplyFailureException e) {
-				conflicts = true;
+				success = false;
 				try (RevWalk rw = new RevWalk(repo)) {
 					ObjectId stashId = repo.resolve(stash);
 					RevCommit commit = rw.parseCommit(stashId);
@@ -398,7 +402,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 				}
 			}
 		}
-		return conflicts;
+		return success;
 	}
 
 	private void updateStashRef(ObjectId commitId, PersonIdent refLogIdent,
@@ -723,13 +727,15 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			boolean lastStepIsForward) throws IOException, GitAPIException {
 		String headName = rebaseState.readFile(HEAD_NAME);
 		updateHead(headName, finalHead, upstreamCommit);
-		boolean stashConflicts = autoStashApply();
+		boolean unstashSuccessful = autoStashApply();
 		getRepository().autoGC(monitor);
 		FileUtils.delete(rebaseState.getDir(), FileUtils.RECURSIVE);
-		if (stashConflicts)
+		if (!unstashSuccessful) {
 			return RebaseResult.STASH_APPLY_CONFLICTS_RESULT;
-		if (lastStepIsForward || finalHead == null)
+		}
+		if (lastStepIsForward || finalHead == null) {
 			return RebaseResult.FAST_FORWARD_RESULT;
+		}
 		return RebaseResult.OK_RESULT;
 	}
 
@@ -1149,7 +1155,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		if (!isInteractive() && walk.isMergedInto(upstream, headCommit))
 			return RebaseResult.UP_TO_DATE_RESULT;
 		else if (!isInteractive() && walk.isMergedInto(headCommit, upstream)) {
-			// head is already merged into upstream, fast-foward
+			// head is already merged into upstream, fast-forward
 			monitor.beginTask(MessageFormat.format(
 					JGitText.get().resettingHead,
 					upstreamCommit.getShortMessage()), ProgressMonitor.UNKNOWN);
@@ -1447,13 +1453,14 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 				throw new JGitInternalException(
 						JGitText.get().abortingRebaseFailed);
 			}
-			boolean stashConflicts = autoStashApply();
+			boolean unstashSuccessful = autoStashApply();
 			// cleanup the files
 			FileUtils.delete(rebaseState.getDir(), FileUtils.RECURSIVE);
 			repo.writeCherryPickHead(null);
 			repo.writeMergeHeads(null);
-			if (stashConflicts)
+			if (!unstashSuccessful) {
 				return RebaseResult.STASH_APPLY_CONFLICTS_RESULT;
+			}
 			return result;
 
 		} finally {