From b15b9d5df25b375108ca125b31338df637454865 Mon Sep 17 00:00:00 2001 From: Mathias Kinzler Date: Mon, 31 Jan 2011 12:12:48 +0100 Subject: [PATCH] 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 --- .../jgit/api/PullCommandWithRebaseTest.java | 9 +-- .../org/eclipse/jgit/JGitText.properties | 1 + .../src/org/eclipse/jgit/JGitText.java | 1 + .../src/org/eclipse/jgit/api/PullCommand.java | 71 +++++++++---------- .../org/eclipse/jgit/api/RebaseCommand.java | 17 +++++ 5 files changed, 56 insertions(+), 43 deletions(-) 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 2847bbe1f..f131c9079 100644 --- 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 c03c64929..2f480194a 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 4e3cb2035..71fa26de4 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 8bc2103bc..db0791865 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 { 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 { 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 { 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 7241042c0..72e92b476 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; @@ -828,6 +829,22 @@ public class RebaseCommand extends GitCommand { 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