From 2a7cd0086bf1bd614478c5722eac5142326e6f2b Mon Sep 17 00:00:00 2001
From: Mathias Kinzler <mathias.kinzler@sap.com>
Date: Thu, 9 Dec 2010 19:22:11 +0100
Subject: [PATCH] Rebase: fix wrong update if original HEAD after Merge+Skip

Rebase would update the original HEAD to the wrong commit when
"skipping" the last commit after a merged commit.

Includes a test for the specific situation.

Change-Id: I087314b1834a3f11a4561f04ca5c21411d54d993
Signed-off-by: Mathias Kinzler <mathias.kinzler@sap.com>
---
 .../eclipse/jgit/api/RebaseCommandTest.java   | 38 ++++++++++++++++
 .../org/eclipse/jgit/api/RebaseCommand.java   | 44 ++++++++++++-------
 2 files changed, 66 insertions(+), 16 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 35b05e719..dd812ee81 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
@@ -499,6 +499,44 @@ public class RebaseCommandTest extends RepositoryTestCase {
 		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
 	}
 
+	public void testMergeFirstStopOnLastConflictAndSkip() throws Exception {
+		// create file1 on master
+		RevCommit firstInMaster = writeFileAndCommit(FILE1, "Add file1", "1",
+				"2", "3");
+		// change in master
+		writeFileAndCommit(FILE1, "change file1 in master", "1master", "2", "3");
+
+		checkFile(FILE1, "1master", "2", "3");
+		// create a topic branch based on the first commit
+		createBranch(firstInMaster, "refs/heads/topic");
+		checkoutBranch("refs/heads/topic");
+		// we have the old content again
+		checkFile(FILE1, "1", "2", "3");
+
+		// add a line (conflicting)
+		writeFileAndCommit(FILE1, "add a line to file1 in topic", "1topic",
+				"2", "3", "4topic");
+
+		// change first line (conflicting again)
+		writeFileAndCommit(FILE1,
+				"change file1 in topic\n\nThis is conflicting", "1topicagain",
+				"2", "3", "4topic");
+
+		RebaseResult res = git.rebase().setUpstream("refs/heads/master").call();
+		assertEquals(Status.STOPPED, res.getStatus());
+
+		writeFileAndAdd(FILE1, "merged");
+
+		res = git.rebase().setOperation(Operation.CONTINUE).call();
+		assertEquals(Status.STOPPED, res.getStatus());
+
+		res = git.rebase().setOperation(Operation.SKIP).call();
+		assertNotNull(res);
+		assertEquals(Status.OK, res.getStatus());
+		assertEquals(RepositoryState.SAFE, db.getRepositoryState());
+		checkFile(FILE1, "merged");
+	}
+
 	public void testStopOnConflictAndSkipNoConflict() throws Exception {
 		// create file1 on master
 		RevCommit firstInMaster = writeFileAndCommit(FILE1, "Add file1", "1",
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 6629c4f8e..70cf702ec 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -218,14 +218,12 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 			if (this.operation == Operation.CONTINUE)
 				newHead = continueRebase();
 
-			List<Step> steps = loadSteps();
-
-			if (this.operation == Operation.SKIP && !steps.isEmpty())
-				checkoutCurrentHead();
+			if (this.operation == Operation.SKIP)
+				newHead = checkoutCurrentHead();
 
 			ObjectReader or = repo.newObjectReader();
-			int stepsToPop = 0;
 
+			List<Step> steps = loadSteps();
 			for (Step step : steps) {
 				if (step.action != Action.PICK)
 					continue;
@@ -250,22 +248,32 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 				if (newHead == null) {
 					return stop(commitToPick);
 				}
-				stepsToPop++;
 			}
-			if (newHead != null || steps.isEmpty()) {
+			if (newHead != null) {
 				// point the previous head (if any) to the new commit
 				String headName = readFile(rebaseDir, HEAD_NAME);
 				if (headName.startsWith(Constants.R_REFS)) {
 					RefUpdate rup = repo.updateRef(headName);
-					if (newHead != null) {
-						rup.setNewObjectId(newHead);
-						rup.forceUpdate();
+					rup.setNewObjectId(newHead);
+					Result res = rup.forceUpdate();
+					switch (res) {
+					case FAST_FORWARD:
+					case FORCED:
+					case NO_CHANGE:
+						break;
+					default:
+						throw new JGitInternalException("Updating HEAD failed");
 					}
 					rup = repo.updateRef(Constants.HEAD);
-					rup.link(headName);
-				}
-				if (this.operation == Operation.SKIP && steps.isEmpty()) {
-					checkoutCurrentHead();
+					res = rup.link(headName);
+					switch (res) {
+					case FAST_FORWARD:
+					case FORCED:
+					case NO_CHANGE:
+						break;
+					default:
+						throw new JGitInternalException("Updating HEAD failed");
+					}
 				}
 				FileUtils.delete(rebaseDir, FileUtils.RECURSIVE);
 				return new RebaseResult(Status.OK);
@@ -276,8 +284,8 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		}
 	}
 
-	private void checkoutCurrentHead() throws IOException, NoHeadException,
-			JGitInternalException {
+	private RevCommit checkoutCurrentHead() throws IOException,
+			NoHeadException, JGitInternalException {
 		ObjectId headTree = repo.resolve(Constants.HEAD + "^{tree}");
 		if (headTree == null)
 			throw new NoHeadException(
@@ -299,6 +307,10 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		} finally {
 			dc.unlock();
 		}
+		RevWalk rw = new RevWalk(repo);
+		RevCommit commit = rw.parseCommit(repo.resolve(Constants.HEAD));
+		rw.release();
+		return commit;
 	}
 
 	/**