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>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandWithRebaseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandWithRebaseTest.java
index 2847bbe1f086c09019d04b00bc35d7415226b15d..f131c9079c11e0797d1a22abfd7b33b4b9f91523 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandWithRebaseTest.java
+++ b/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",
-                               "refs/remotes/origin/master");
-               targetConfig.unset("branch", "master", "merge");
+               targetConfig
+                               .setString("branch", "master", "merge", "refs/heads/master");
+               targetConfig.setBoolean("branch", "master", "rebase", true);
                targetConfig.save();
 
                assertFileContentsEqual(targetFile, "Hello world");

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/JGitText.properties
index c03c64929d4d52e1b62faece2366070c2d33db04..2f480194a7381734dfe199df42a78f26d6abb27c 100644 (file)
--- 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 4e3cb2035efa9c5a81a480497c49f290c9ac9735..71fa26de4899f6bda5b0304c88b8e7b68ff7252f 100644 (file)
--- 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 8bc2103bc290aa088e550fa0db4327949419a0ac..db07918655248f830222ccff29ff1cba150b58d8 100644 (file)
--- 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 7241042c099f1d618a735d2e98b77d8d984ab137..72e92b476e6ca2e0b0f789a3bb41c77ae4f08028 100644 (file)
--- 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;
@@ -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