RebaseCommand: detect and handle fast-forward properly

This bug was hidden by an incomplete test: the current Rebase
implementation using the "git rebase -i" pattern does not work
correctly if fast-forwarding is involved. The reason for this is that
the log command does not return any commits in this case.
In addition, a check for already merged commits was introduced to
avoid spurious conflicts.

Change-Id: Ib9898fe0f982fa08e41f1dca9452c43de715fdb6
Signed-off-by: Mathias Kinzler <mathias.kinzler@sap.com>

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

@@ -132,7 +132,7 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		// create a topic branch
 		createBranch(first, "refs/heads/topic");
 		// create file2 on master
-		writeTrashFile("file2", "file2");
+		File file2 = writeTrashFile("file2", "file2");
 		git.add().addFilepattern("file2").call();
 		git.commit().setMessage("Add file2").call();
 		assertTrue(new File(db.getWorkTree(), "file2").exists());
@@ -141,7 +141,38 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		assertFalse(new File(db.getWorkTree(), "file2").exists());
 
 		RebaseResult res = git.rebase().setUpstream("refs/heads/master").call();
-		assertEquals(Status.UP_TO_DATE, res.getStatus());
+		assertTrue(new File(db.getWorkTree(), "file2").exists());
+		checkFile(file2, "file2");
+		assertEquals(Status.FAST_FORWARD, res.getStatus());
+	}
+
+	@Test
+	public void testFastForwardWithMultipleCommits() throws Exception {
+		// create file1 on master
+		writeTrashFile(FILE1, FILE1);
+		git.add().addFilepattern(FILE1).call();
+		RevCommit first = git.commit().setMessage("Add file1").call();
+
+		assertTrue(new File(db.getWorkTree(), FILE1).exists());
+		// create a topic branch
+		createBranch(first, "refs/heads/topic");
+		// create file2 on master
+		File file2 = writeTrashFile("file2", "file2");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("Add file2").call();
+		assertTrue(new File(db.getWorkTree(), "file2").exists());
+		// write a second commit
+		writeTrashFile("file2", "file2 new content");
+		git.add().addFilepattern("file2").call();
+		git.commit().setMessage("Change content of file2").call();
+
+		checkoutBranch("refs/heads/topic");
+		assertFalse(new File(db.getWorkTree(), "file2").exists());
+
+		RebaseResult res = git.rebase().setUpstream("refs/heads/master").call();
+		assertTrue(new File(db.getWorkTree(), "file2").exists());
+		checkFile(file2, "file2 new content");
+		assertEquals(Status.FAST_FORWARD, res.getStatus());
 	}
 
 	@Test

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

@@ -63,8 +63,10 @@ import java.util.Map;
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.api.RebaseResult.Status;
 import org.eclipse.jgit.api.errors.GitAPIException;
+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.RefAlreadyExistsException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.api.errors.UnmergedPathsException;
 import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
@@ -189,6 +191,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 	public RebaseResult call() throws NoHeadException, RefNotFoundException,
 			JGitInternalException, GitAPIException {
 		RevCommit newHead = null;
+		boolean lastStepWasForward = false;
 		checkCallable();
 		checkParameters();
 		try {
@@ -237,11 +240,17 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 				monitor.beginTask(MessageFormat.format(
 						JGitText.get().applyingCommit, commitToPick
 								.getShortMessage()), ProgressMonitor.UNKNOWN);
-				// TODO if the first parent of commitToPick is the current HEAD,
+				// if the first parent of commitToPick is the current HEAD,
-				// we should fast-forward instead of cherry-pick to avoid
+				// we do a fast-forward instead of cherry-pick to avoid
 				// unnecessary object rewriting
-				newHead = new Git(repo).cherryPick().include(commitToPick)
+				newHead = tryFastForward(commitToPick);
-						.call();
+				lastStepWasForward = newHead != null;
+				if (!lastStepWasForward)
+					// TODO if the content of this commit is already merged here
+					// we should skip this step in order to avoid confusing
+					// pseudo-changed
+					newHead = new Git(repo).cherryPick().include(commitToPick)
+							.call();
 				monitor.endTask();
 				if (newHead == null) {
 					return stop(commitToPick);
@@ -274,6 +283,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 					}
 				}
 				FileUtils.delete(rebaseDir, FileUtils.RECURSIVE);
+				if (lastStepWasForward)
+					return new RebaseResult(Status.FAST_FORWARD);
 				return new RebaseResult(Status.OK);
 			}
 			return new RebaseResult(Status.UP_TO_DATE);
@@ -496,6 +507,7 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		RevCommit headCommit = walk.lookupCommit(headId);
 		monitor.beginTask(JGitText.get().obtainingCommitsForCherryPick,
 				ProgressMonitor.UNKNOWN);
+
 		LogCommand cmd = new Git(repo).log().addRange(upstreamCommit,
 				headCommit);
 		Iterable<RevCommit> commitsToUse = cmd.call();
@@ -503,6 +515,22 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			cherryPickList.add(commit);
 		}
 
+		// if the upstream commit is in a direct line to the current head,
+		// the log command will not report any commits; in this case,
+		// we create the cherry-pick list ourselves
+		if (cherryPickList.isEmpty()) {
+			Iterable<RevCommit> parents = new Git(repo).log().add(
+					upstreamCommit).call();
+			for (RevCommit parent : parents) {
+				if (parent.equals(headCommit))
+					break;
+				if (parent.getParentCount() != 1)
+					throw new JGitInternalException(
+							JGitText.get().canOnlyCherryPickCommitsWithOneParent);
+				cherryPickList.add(parent);
+			}
+		}
+
 		// nothing to do: return with UP_TO_DATE_RESULT
 		if (cherryPickList.isEmpty())
 			return RebaseResult.UP_TO_DATE_RESULT;
@@ -548,6 +576,75 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		return null;
 	}
 
+	/**
+	 * checks if we can fast-forward and returns the new head if it is possible
+	 *
+	 * @param newCommit
+	 * @return the new head, or null
+	 * @throws RefNotFoundException
+	 * @throws IOException
+	 */
+	public RevCommit tryFastForward(RevCommit newCommit)
+			throws RefNotFoundException, IOException {
+		Ref head = repo.getRef(Constants.HEAD);
+		if (head == null || head.getObjectId() == null)
+			throw new RefNotFoundException(MessageFormat.format(
+					JGitText.get().refNotResolved, Constants.HEAD));
+
+		ObjectId headId = head.getObjectId();
+		if (headId == null)
+			throw new RefNotFoundException(MessageFormat.format(
+					JGitText.get().refNotResolved, Constants.HEAD));
+		RevCommit headCommit = walk.lookupCommit(headId);
+		if (walk.isMergedInto(newCommit, headCommit))
+			return newCommit;
+
+		String headName;
+		if (head.isSymbolic())
+			headName = head.getTarget().getName();
+		else
+			headName = "detached HEAD";
+		return tryFastForward(headName, headCommit, newCommit);
+	}
+
+	private RevCommit tryFastForward(String headName, RevCommit oldCommit,
+			RevCommit newCommit) throws IOException, JGitInternalException {
+		boolean tryRebase = false;
+		for (RevCommit parentCommit : newCommit.getParents())
+			if (parentCommit.equals(oldCommit))
+				tryRebase = true;
+		if (!tryRebase)
+			return null;
+
+		CheckoutCommand co = new CheckoutCommand(repo);
+		try {
+			co.setName(newCommit.name()).call();
+			if (headName.startsWith(Constants.R_HEADS)) {
+				RefUpdate rup = repo.updateRef(headName);
+				rup.setExpectedOldObjectId(oldCommit);
+				rup.setNewObjectId(newCommit);
+				rup.setRefLogMessage("Fast-foward from " + oldCommit.name()
+						+ " to " + newCommit.name(), false);
+				Result res = rup.update(walk);
+				switch (res) {
+				case FAST_FORWARD:
+				case NO_CHANGE:
+				case FORCED:
+					break;
+				default:
+					throw new IOException("Could not fast-forward");
+				}
+			}
+			return newCommit;
+		} catch (RefAlreadyExistsException e) {
+			throw new JGitInternalException(e.getMessage(), e);
+		} catch (RefNotFoundException e) {
+			throw new JGitInternalException(e.getMessage(), e);
+		} catch (InvalidRefNameException e) {
+			throw new JGitInternalException(e.getMessage(), e);
+		}
+	}
+
 	private void checkParameters() throws WrongRepositoryStateException {
 		if (this.operation != Operation.BEGIN) {
 			// these operations are only possible while in a rebasing state

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

@@ -67,7 +67,11 @@ public class RebaseResult {
 		/**
 		 * Already up-to-date
 		 */
-		UP_TO_DATE;
+		UP_TO_DATE,
+		/**
+		 * Fast-forward, HEAD points to the new commit
+		 */
+		FAST_FORWARD;
 	}
 
 	static final RebaseResult UP_TO_DATE_RESULT = new RebaseResult(