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>

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

@@ -140,8 +140,9 @@ public class PullCommandWithRebaseTest extends RepositoryTestCase {
 				.call();
 		StoredConfig config = target.getRepository().getConfig();
 		config.setString("branch", "basedOnMaster", "remote", ".");
-		config.setString("branch", "basedOnMaster", "rebase",
+		config.setString("branch", "basedOnMaster", "merge",
 				"refs/heads/master");
+		config.setBoolean("branch", "basedOnMaster", "rebase", true);
 		config.save();
 		target.getRepository().updateRef(Constants.HEAD).link(
 				"refs/heads/basedOnMaster");
@@ -212,9 +213,9 @@ public class PullCommandWithRebaseTest extends RepositoryTestCase {
 		target.checkout().setStartPoint("refs/remotes/origin/master").setName(
 				"master").call();
 
-		targetConfig.setString("branch", "master", "rebase",
+		targetConfig
-				"refs/remotes/origin/master");
+				.setString("branch", "master", "merge", "refs/heads/master");
-		targetConfig.unset("branch", "master", "merge");
+		targetConfig.setBoolean("branch", "master", "rebase", true);
 		targetConfig.save();
 
 		assertFileContentsEqual(targetFile, "Hello world");

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

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;

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(
+		boolean doRebase = repoConfig.getBoolean(
 				ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
-					ConfigConstants.CONFIG_KEY_REBASE);
+				ConfigConstants.CONFIG_KEY_REBASE, false);
-			if (remoteBranchName != null) {
-				doRebase = true;
-			}
-		}
 
 		if (remoteBranchName == null) {
 			String missingKey = ConfigConstants.CONFIG_BRANCH_SECTION + DOT
@@ -237,29 +231,10 @@ public class PullCommand extends GitCommand<PullResult> {
 					JGitText.get().operationCanceled,
 					JGitText.get().pullTaskName));
 
-		PullResult result;
-		if (doRebase) {
-			RebaseCommand rebase = new RebaseCommand(repo);
-			try {
-				RebaseResult rebaseRes = rebase.setUpstream(remoteBranchName)
-						.setProgressMonitor(monitor).setOperation(
-								Operation.BEGIN).call();
-				result = new PullResult(fetchRes, remote, rebaseRes);
-			} catch (NoHeadException e) {
-				throw new JGitInternalException(e.getMessage(), e);
-			} catch (RefNotFoundException e) {
-				throw new JGitInternalException(e.getMessage(), e);
-			} catch (JGitInternalException e) {
-				throw new JGitInternalException(e.getMessage(), e);
-			} catch (GitAPIException e) {
-				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) {
@@ -269,9 +244,8 @@ public class PullCommand extends GitCommand<PullResult> {
 							+ remoteBranchName);
 			}
 			if (r == null)
-					throw new JGitInternalException(MessageFormat.format(
+				throw new JGitInternalException(MessageFormat.format(JGitText
-							JGitText.get().couldNotGetAdvertisedRef,
+						.get().couldNotGetAdvertisedRef, remoteBranchName));
-							remoteBranchName));
 			else
 				commitToMerge = r.getObjectId();
 		} else {
@@ -283,6 +257,25 @@ public class PullCommand extends GitCommand<PullResult> {
 						e);
 			}
 		}
+
+		PullResult result;
+		if (doRebase) {
+			RebaseCommand rebase = new RebaseCommand(repo);
+			try {
+				RebaseResult rebaseRes = rebase.setUpstream(commitToMerge)
+						.setProgressMonitor(monitor).setOperation(
+								Operation.BEGIN).call();
+				result = new PullResult(fetchRes, remote, rebaseRes);
+			} catch (NoHeadException e) {
+				throw new JGitInternalException(e.getMessage(), e);
+			} catch (RefNotFoundException e) {
+				throw new JGitInternalException(e.getMessage(), e);
+			} catch (JGitInternalException e) {
+				throw new JGitInternalException(e.getMessage(), e);
+			} catch (GitAPIException e) {
+				throw new JGitInternalException(e.getMessage(), e);
+			}
+		} else {
 			MergeCommand merge = new MergeCommand(repo);
 			merge.include(
 					"branch \'" + remoteBranchName + "\' of " + remoteUri,

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;
@@ -828,6 +829,22 @@ public class RebaseCommand extends GitCommand<RebaseResult> {
 		return this;
 	}
 
+	/**
+	 * @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