Interactive Rebase: Do actions if there were conflicts

If a commit was marked for edit, reword, squash or fixup, but the
interactive rebase stopped because of a conflict, the step was not done
after conflict resolution. This is done now.

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

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

@@ -2327,6 +2327,213 @@ public class RebaseCommandTest extends RepositoryTestCase {
 
 	}
 
+	@Test
+	public void testRebaseShouldStopForEditInCaseOfConflict()
+			throws Exception {
+		// create file1 on master
+		writeTrashFile(FILE1, FILE1);
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Add file1\nnew line").call();
+		assertTrue(new File(db.getWorkTree(), FILE1).exists());
+
+		//change file1
+		writeTrashFile(FILE1, FILE1 + "a");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		//change file1
+		writeTrashFile(FILE1, FILE1 + "b");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		RebaseResult result = git.rebase().setUpstream("HEAD~2")
+				.runInteractively(new InteractiveHandler() {
+
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.remove(0);
+						steps.get(0).setAction(Action.EDIT);
+					}
+
+					public String modifyCommitMessage(String commit) {
+						return commit;
+					}
+				}).call();
+		assertEquals(Status.STOPPED, result.getStatus());
+		git.add().addFilepattern(FILE1).call();
+		result = git.rebase().setOperation(Operation.CONTINUE).call();
+		assertEquals(Status.EDIT, result.getStatus());
+
+	}
+
+	@Test
+	public void testRebaseShouldStopForRewordInCaseOfConflict()
+			throws Exception {
+		// create file1 on master
+		writeTrashFile(FILE1, FILE1);
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Add file1\nnew line").call();
+		assertTrue(new File(db.getWorkTree(), FILE1).exists());
+
+		// change file1
+		writeTrashFile(FILE1, FILE1 + "a");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		// change file1
+		writeTrashFile(FILE1, FILE1 + "b");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		RebaseResult result = git.rebase().setUpstream("HEAD~2")
+				.runInteractively(new InteractiveHandler() {
+
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.remove(0);
+						steps.get(0).setAction(Action.REWORD);
+					}
+
+					public String modifyCommitMessage(String commit) {
+						return "rewritten commit message";
+					}
+				}).call();
+		assertEquals(Status.STOPPED, result.getStatus());
+		git.add().addFilepattern(FILE1).call();
+		result = git.rebase().runInteractively(new InteractiveHandler() {
+
+			public void prepareSteps(List<RebaseTodoLine> steps) {
+				steps.remove(0);
+				steps.get(0).setAction(Action.REWORD);
+			}
+
+			public String modifyCommitMessage(String commit) {
+				return "rewritten commit message";
+			}
+		}).setOperation(Operation.CONTINUE).call();
+		assertEquals(Status.OK, result.getStatus());
+		Iterator<RevCommit> logIterator = git.log().all().call().iterator();
+		String actualCommitMag = logIterator.next().getShortMessage();
+		assertEquals("rewritten commit message", actualCommitMag);
+
+	}
+
+	@Test
+	public void testRebaseShouldSquashInCaseOfConflict() throws Exception {
+		// create file1 on master
+		writeTrashFile(FILE1, FILE1);
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Add file1\nnew line").call();
+		assertTrue(new File(db.getWorkTree(), FILE1).exists());
+
+		// change file2
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("Change file2").call();
+
+		// change file1
+		writeTrashFile(FILE1, FILE1 + "a");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		// change file1
+		writeTrashFile(FILE1, FILE1 + "b");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		RebaseResult result = git.rebase().setUpstream("HEAD~3")
+				.runInteractively(new InteractiveHandler() {
+
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.PICK);
+						steps.remove(1);
+						steps.get(1).setAction(Action.SQUASH);
+					}
+
+					public String modifyCommitMessage(String commit) {
+						return "squashed message";
+					}
+				}).call();
+		assertEquals(Status.STOPPED, result.getStatus());
+		git.add().addFilepattern(FILE1).call();
+		result = git.rebase().runInteractively(new InteractiveHandler() {
+
+			public void prepareSteps(List<RebaseTodoLine> steps) {
+				steps.get(0).setAction(Action.PICK);
+				steps.remove(1);
+				steps.get(1).setAction(Action.SQUASH);
+			}
+
+			public String modifyCommitMessage(String commit) {
+				return "squashed message";
+			}
+		}).setOperation(Operation.CONTINUE).call();
+		assertEquals(Status.OK, result.getStatus());
+		Iterator<RevCommit> logIterator = git.log().all().call().iterator();
+		String actualCommitMag = logIterator.next().getShortMessage();
+		assertEquals("squashed message", actualCommitMag);
+	}
+
+	@Test
+	public void testRebaseShouldFixupInCaseOfConflict() throws Exception {
+		// create file1 on master
+		writeTrashFile(FILE1, FILE1);
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Add file1").call();
+		assertTrue(new File(db.getWorkTree(), FILE1).exists());
+
+		// change file2
+		writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("Change file2").call();
+
+		// change file1
+		writeTrashFile(FILE1, FILE1 + "a");
+		git.add().addFilepattern(FILE1).call();
+		git.commit().setMessage("Change file1").call();
+
+		// change file1, add file3
+		writeTrashFile(FILE1, FILE1 + "b");
+		writeTrashFile("file3", "file3");
+		git.add().addFilepattern(FILE1).call();
+		git.add().addFilepattern("file3").call();
+		git.commit().setMessage("Change file1, add file3").call();
+
+		RebaseResult result = git.rebase().setUpstream("HEAD~3")
+				.runInteractively(new InteractiveHandler() {
+
+					public void prepareSteps(List<RebaseTodoLine> steps) {
+						steps.get(0).setAction(Action.PICK);
+						steps.remove(1);
+						steps.get(1).setAction(Action.FIXUP);
+					}
+
+					public String modifyCommitMessage(String commit) {
+						return commit;
+					}
+				}).call();
+		assertEquals(Status.STOPPED, result.getStatus());
+		git.add().addFilepattern(FILE1).call();
+		result = git.rebase().runInteractively(new InteractiveHandler() {
+
+			public void prepareSteps(List<RebaseTodoLine> steps) {
+				steps.get(0).setAction(Action.PICK);
+				steps.remove(1);
+				steps.get(1).setAction(Action.FIXUP);
+			}
+
+			public String modifyCommitMessage(String commit) {
+				return "commit";
+			}
+		}).setOperation(Operation.CONTINUE).call();
+		assertEquals(Status.OK, result.getStatus());
+		Iterator<RevCommit> logIterator = git.log().all().call().iterator();
+		String actualCommitMsg = logIterator.next().getShortMessage();
+		assertEquals("Change file2", actualCommitMsg);
+		actualCommitMsg = logIterator.next().getShortMessage();
+		assertEquals("Add file1", actualCommitMsg);
+		assertTrue(new File(db.getWorkTree(), "file3").exists());
+
+	}
+
 	private File getTodoFile() {
 		File todoFile = new File(db.getDirectory(), GIT_REBASE_TODO);
 		return todoFile;

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

@@ -61,11 +61,13 @@ import java.util.regex.Pattern;
 import org.eclipse.jgit.api.RebaseResult.Status;
 import org.eclipse.jgit.api.ResetCommand.ResetType;
 import org.eclipse.jgit.api.errors.CheckoutConflictException;
+import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.InvalidRebaseStepException;
 import org.eclipse.jgit.api.errors.InvalidRefNameException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.api.errors.NoHeadException;
+import org.eclipse.jgit.api.errors.NoMessageException;
 import org.eclipse.jgit.api.errors.RefAlreadyExistsException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.api.errors.UnmergedPathsException;
@@ -75,6 +77,7 @@ import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheCheckout;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -197,6 +200,10 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 
 	private boolean stopAfterInitialization = false;
 
+	private RevCommit newHead;
+
+	private boolean lastStepWasForward;
+
 	/**
 	 * @param repo
 	 */
@@ -220,8 +227,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 	 */
 	public RebaseResult call() throws GitAPIException, NoHeadException,
 			RefNotFoundException, WrongRepositoryStateException {
-		RevCommit newHead = null;
+		newHead = null;
-		boolean lastStepWasForward = false;
+		lastStepWasForward = false;
 		checkCallable();
 		checkParameters();
 		try {
@@ -261,7 +268,19 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 
 			if (operation == Operation.CONTINUE) {
 				newHead = continueRebase();
-
+				List<RebaseTodoLine> doneLines = repo.readRebaseTodo(
+						rebaseState.getPath(DONE), true);
+				RebaseTodoLine step = doneLines.get(doneLines.size() - 1);
+				if (newHead != null
+						&& step.getAction() != Action.PICK) {
+					RebaseTodoLine newStep = new RebaseTodoLine(
+							step.getAction(),
+							AbbreviatedObjectId.fromObjectId(newHead),
+							step.getShortMessage());
+					RebaseResult result = processStep(newStep, false);
+					if (result != null)
+						return result;
+				}
 				File amendFile = rebaseState.getFile(AMEND);
 				boolean amendExists = amendFile.exists();
 				if (amendExists) {
@@ -280,8 +299,6 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			if (operation == Operation.SKIP)
 				newHead = checkoutCurrentHead();
 
-			ObjectReader or = repo.newObjectReader();
-
 			List<RebaseTodoLine> steps = repo.readRebaseTodo(
 					rebaseState.getPath(GIT_REBASE_TODO), false);
 			if (steps.size() == 0) {
@@ -296,21 +313,79 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			for (int i = 0; i < steps.size(); i++) {
 				RebaseTodoLine step = steps.get(i);
 				popSteps(1);
+				RebaseResult result = processStep(step, true);
+				if (result != null) {
+					return result;
+				}
+			}
+			return finishRebase(newHead, lastStepWasForward);
+		} catch (CheckoutConflictException cce) {
+			return new RebaseResult(cce.getConflictingPaths());
+		} catch (IOException ioe) {
+			throw new JGitInternalException(ioe.getMessage(), ioe);
+		}
+	}
+
+	private RebaseResult processStep(RebaseTodoLine step, boolean shouldPick)
+			throws IOException, GitAPIException {
 		if (Action.COMMENT.equals(step.getAction()))
-					continue;
+			return null;
+		ObjectReader or = repo.newObjectReader();
+
 		Collection<ObjectId> ids = or.resolve(step.getCommit());
 		if (ids.size() != 1)
 			throw new JGitInternalException(
 					"Could not resolve uniquely the abbreviated object ID");
-				RevCommit commitToPick = walk
+		RevCommit commitToPick = walk.parseCommit(ids.iterator().next());
-						.parseCommit(ids.iterator().next());
+		if (shouldPick) {
 			if (monitor.isCancelled())
 				return new RebaseResult(commitToPick, Status.STOPPED);
+			RebaseResult result = cherryPickCommit(commitToPick);
+			if (result != null)
+				return result;
+		}
+		boolean isSquash = false;
+		switch (step.getAction()) {
+		case PICK:
+			return null; // continue rebase process on pick command
+		case REWORD:
+			String oldMessage = commitToPick.getFullMessage();
+			String newMessage = interactiveHandler
+					.modifyCommitMessage(oldMessage);
+			newHead = new Git(repo).commit().setMessage(newMessage)
+					.setAmend(true).call();
+			return null;
+		case EDIT:
+			rebaseState.createFile(AMEND, commitToPick.name());
+			return stop(commitToPick, Status.EDIT);
+		case COMMENT:
+			break;
+		case SQUASH:
+			isSquash = true;
+			//$FALL-THROUGH$
+		case FIXUP:
+			resetSoftToParent();
+			List<RebaseTodoLine> steps = repo.readRebaseTodo(
+					rebaseState.getPath(GIT_REBASE_TODO), false);
+			RebaseTodoLine nextStep = steps.size() > 0 ? steps.get(0) : null;
+			File messageFixupFile = rebaseState.getFile(MESSAGE_FIXUP);
+			File messageSquashFile = rebaseState.getFile(MESSAGE_SQUASH);
+			if (isSquash && messageFixupFile.exists())
+				messageFixupFile.delete();
+			newHead = doSquashFixup(isSquash, commitToPick, nextStep,
+					messageFixupFile, messageSquashFile);
+		}
+		return null;
+	}
+
+	private RebaseResult cherryPickCommit(RevCommit commitToPick)
+			throws IOException, GitAPIException, NoMessageException,
+			UnmergedPathsException, ConcurrentRefUpdateException,
+			WrongRepositoryStateException, NoHeadException {
 		try {
 			monitor.beginTask(MessageFormat.format(
 					JGitText.get().applyingCommit,
-							commitToPick.getShortMessage()),
+					commitToPick.getShortMessage()), ProgressMonitor.UNKNOWN);
-							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
@@ -321,9 +396,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 				// here we should skip this step in order to avoid
 				// confusing pseudo-changed
 				String ourCommitName = getOurCommitName();
-						CherryPickResult cherryPickResult = new Git(repo)
+				CherryPickResult cherryPickResult = new Git(repo).cherryPick()
-								.cherryPick().include(commitToPick)
+						.include(commitToPick).setOurCommitName(ourCommitName)
-								.setOurCommitName(ourCommitName)
 						.setReflogPrefix("rebase:").call(); //$NON-NLS-1$
 				switch (cherryPickResult.getStatus()) {
 				case FAILED:
@@ -338,48 +412,11 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 					newHead = cherryPickResult.getNewHead();
 				}
 			}
-					boolean isSquash = false;
+			return null;
-					switch (step.getAction()) {
-					case PICK:
-						continue; // continue rebase process on pick command
-					case REWORD:
-						String oldMessage = commitToPick.getFullMessage();
-						String newMessage = interactiveHandler
-								.modifyCommitMessage(oldMessage);
-						newHead = new Git(repo).commit().setMessage(newMessage)
-								.setAmend(true).call();
-						continue;
-					case EDIT:
-						rebaseState.createFile(AMEND, commitToPick.name());
-						return stop(commitToPick, Status.EDIT);
-					case COMMENT:
-						break;
-					case SQUASH:
-						isSquash = true;
-						//$FALL-THROUGH$
-					case FIXUP:
-						resetSoftToParent();
-						RebaseTodoLine nextStep = (i >= steps.size() - 1 ? null
-								: steps.get(i + 1));
-						File messageFixupFile = rebaseState.getFile(MESSAGE_FIXUP);
-						File messageSquashFile = rebaseState
-								.getFile(MESSAGE_SQUASH);
-						if (isSquash && messageFixupFile.exists())
-								messageFixupFile.delete();
-						newHead = doSquashFixup(isSquash, commitToPick,
-								nextStep, messageFixupFile, messageSquashFile);
-					}
 		} finally {
 			monitor.endTask();
 		}
 	}
-			return finishRebase(newHead, lastStepWasForward);
-		} catch (CheckoutConflictException cce) {
-			return new RebaseResult(cce.getConflictingPaths());
-		} catch (IOException ioe) {
-			throw new JGitInternalException(ioe.getMessage(), ioe);
-		}
-	}
 
 	private RebaseResult finishRebase(RevCommit newHead,
 			boolean lastStepWasForward) throws IOException {