Proper handling of rebase during pull

After consulting with Christian Halstrick, it turned out that the
handling of rebase during pull was implemented incorrectly.

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

4 files changed, 51 insertions, 39 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
index c03c64929d..2f480194a7 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
@@ -122,6 +122,7 @@ couldNotDeleteTemporaryIndexFileShouldNotHappen=Could not delete temporary index
 couldNotGetAdvertisedRef=Could not get advertised Ref for branch {0}
 couldNotLockHEAD=Could not lock HEAD
 couldNotReadIndexInOneGo=Could not read index in one go, only {0} out of {1} read
+couldNotReadObjectWhileParsingCommit=Could not read an object while parsing commit {0}
 couldNotRenameDeleteOldIndex=Could not rename delete old index
 couldNotRenameTemporaryFile=Could not rename temporary file {0} to new location {1}
 couldNotRenameTemporaryIndexFileToIndex=Could not rename temporary index file to index
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
index 4e3cb2035e..71fa26de48 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/JGitText.java
@@ -182,6 +182,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String couldNotGetAdvertisedRef;
 	/***/ public String couldNotLockHEAD;
 	/***/ public String couldNotReadIndexInOneGo;
+	/***/ public String couldNotReadObjectWhileParsingCommit;
 	/***/ public String couldNotRenameDeleteOldIndex;
 	/***/ public String couldNotRenameTemporaryFile;
 	/***/ public String couldNotRenameTemporaryIndexFileToIndex;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
index 8bc2103bc2..db07918655 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java
@@ -181,16 +181,10 @@ public class PullCommand extends GitCommand<PullResult> {
 		String remoteBranchName = repoConfig.getString(
 				ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
 				ConfigConstants.CONFIG_KEY_MERGE);
-		boolean doRebase = false;
-		if (remoteBranchName == null) {
-			// check if the branch is configured for pull-rebase
-			remoteBranchName = repoConfig.getString(
-					ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
-					ConfigConstants.CONFIG_KEY_REBASE);
-			if (remoteBranchName != null) {
-				doRebase = true;
-			}
-		}
+		// check if the branch is configured for pull-rebase
+		boolean doRebase = repoConfig.getBoolean(
+				ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
+				ConfigConstants.CONFIG_KEY_REBASE, false);
 
 		if (remoteBranchName == null) {
 			String missingKey = ConfigConstants.CONFIG_BRANCH_SECTION + DOT
@@ -237,11 +231,38 @@ public class PullCommand extends GitCommand<PullResult> {
 					JGitText.get().operationCanceled,
 					JGitText.get().pullTaskName));
 
+		// we check the updates to see which of the updated branches
+		// corresponds
+		// to the remote branch name
+		AnyObjectId commitToMerge;
+		if (isRemote) {
+			Ref r = null;
+			if (fetchRes != null) {
+				r = fetchRes.getAdvertisedRef(remoteBranchName);
+				if (r == null)
+					r = fetchRes.getAdvertisedRef(Constants.R_HEADS
+							+ remoteBranchName);
+			}
+			if (r == null)
+				throw new JGitInternalException(MessageFormat.format(JGitText
+						.get().couldNotGetAdvertisedRef, remoteBranchName));
+			else
+				commitToMerge = r.getObjectId();
+		} else {
+			try {
+				commitToMerge = repo.resolve(remoteBranchName);
+			} catch (IOException e) {
+				throw new JGitInternalException(
+						JGitText.get().exceptionCaughtDuringExecutionOfPullCommand,
+						e);
+			}
+		}
+
 		PullResult result;
 		if (doRebase) {
 			RebaseCommand rebase = new RebaseCommand(repo);
 			try {
-				RebaseResult rebaseRes = rebase.setUpstream(remoteBranchName)
+				RebaseResult rebaseRes = rebase.setUpstream(commitToMerge)
 						.setProgressMonitor(monitor).setOperation(
 								Operation.BEGIN).call();
 				result = new PullResult(fetchRes, remote, rebaseRes);
@@ -255,34 +276,6 @@ public class PullCommand extends GitCommand<PullResult> {
 				throw new JGitInternalException(e.getMessage(), e);
 			}
 		} else {
-			// we check the updates to see which of the updated branches
-			// corresponds
-			// to the remote branch name
-			AnyObjectId commitToMerge;
-
-			if (isRemote) {
-				Ref r = null;
-				if (fetchRes != null) {
-					r = fetchRes.getAdvertisedRef(remoteBranchName);
-					if (r == null)
-						r = fetchRes.getAdvertisedRef(Constants.R_HEADS
-								+ remoteBranchName);
-				}
-				if (r == null)
-					throw new JGitInternalException(MessageFormat.format(
-							JGitText.get().couldNotGetAdvertisedRef,
-							remoteBranchName));
-				else
-					commitToMerge = r.getObjectId();
-			} else {
-				try {
-					commitToMerge = repo.resolve(remoteBranchName);
-				} catch (IOException e) {
-					throw new JGitInternalException(
-							JGitText.get().exceptionCaughtDuringExecutionOfPullCommand,
-							e);
-				}
-			}
 			MergeCommand merge = new MergeCommand(repo);
 			merge.include(
 					"branch \'" + remoteBranchName + "\' of " + remoteUri,
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 7241042c09..72e92b476e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/RebaseCommand.java
@@ -75,6 +75,7 @@ import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheCheckout;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
+import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
@@ -830,6 +831,22 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 
 	/**
 	 * @param upstream
+	 *            id of the upstream commit
+	 * @return {@code this}
+	 */
+	public RebaseCommand setUpstream(AnyObjectId upstream) {
+		try {
+			this.upstreamCommit = walk.parseCommit(upstream);
+		} catch (IOException e) {
+			throw new JGitInternalException(MessageFormat.format(
+					JGitText.get().couldNotReadObjectWhileParsingCommit,
+					upstream.name()), e);
+		}
+		return this;
+	}
+
+	/**
+	 * @param upstream
 	 *            the upstream branch
 	 * @return {@code this}
 	 * @throws RefNotFoundException