From 48544e27ae3808807dc1960083d8e42c05e62af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konrad=20K=C3=BCgler?= Date: Fri, 23 May 2014 23:09:17 +0200 Subject: [PATCH] Preserve merges during pull if configured to do so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting branch..rebase or pull.rebase to 'preserve' will preserve merges during rebase. Also, pull.rebase is now consulted if there is no branch-specific configuration. Bug: 429664 Change-Id: I345fa295c7e774e0d0a8e6aba30fbfc3552e0084 Signed-off-by: Konrad Kügler --- .../org/eclipse/jgit/api/PullCommandTest.java | 207 ++++++++++++++++++ .../src/org/eclipse/jgit/api/PullCommand.java | 65 ++++-- .../org/eclipse/jgit/lib/ConfigConstants.java | 7 + 3 files changed, 256 insertions(+), 23 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java index 245219d88..d4805d0aa 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PullCommandTest.java @@ -44,6 +44,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -52,6 +53,7 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.util.concurrent.Callable; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; import org.eclipse.jgit.api.MergeResult.MergeStatus; @@ -64,6 +66,7 @@ import org.eclipse.jgit.lib.RepositoryState; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RemoteConfig; @@ -297,6 +300,210 @@ public void testPullMergeProgrammaticConfigurationImpliedTargetBranch() assertEquals(message, mergeCommit.getShortMessage()); } + private enum TestPullMode { + MERGE, REBASE, REBASE_PREASERVE + } + + @Test + /** global rebase config should be respected */ + public void testPullWithRebasePreserve1Config() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "preserve"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE_PREASERVE); + } + + @Test + /** the branch-local config should win over the global config */ + public void testPullWithRebasePreserveConfig2() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "false"); + config.setString("branch", "master", "rebase", "preserve"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE_PREASERVE); + } + + @Test + /** the branch-local config should be respected */ + public void testPullWithRebasePreserveConfig3() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("branch", "master", "rebase", "preserve"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE_PREASERVE); + } + + @Test + /** global rebase config should be respected */ + public void testPullWithRebaseConfig1() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "true"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE); + } + + @Test + /** the branch-local config should win over the global config */ + public void testPullWithRebaseConfig2() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "preserve"); + config.setString("branch", "master", "rebase", "true"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE); + } + + @Test + /** the branch-local config should be respected */ + public void testPullWithRebaseConfig3() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("branch", "master", "rebase", "true"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.REBASE); + } + + @Test + /** without config it should merge */ + public void testPullWithoutConfig() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.MERGE); + } + + @Test + /** the branch local config should win over the global config */ + public void testPullWithMergeConfig() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "true"); + config.setString("branch", "master", "rebase", "false"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.MERGE); + } + + @Test + /** the branch local config should win over the global config */ + public void testPullWithMergeConfig2() throws Exception { + Callable setup = new Callable() { + public PullResult call() throws Exception { + StoredConfig config = dbTarget.getConfig(); + config.setString("pull", null, "rebase", "false"); + config.save(); + return target.pull().call(); + } + }; + doTestPullWithRebase(setup, TestPullMode.MERGE); + } + + private void doTestPullWithRebase(Callable pullSetup, + TestPullMode expectedPullMode) throws Exception { + // simple upstream change + writeToFile(sourceFile, "content"); + source.add().addFilepattern(sourceFile.getName()).call(); + RevCommit sourceCommit = source.commit().setMessage("source commit") + .call(); + + // create a merge commit in target + File loxalFile = new File(dbTarget.getWorkTree(), "local.txt"); + writeToFile(loxalFile, "initial\n"); + target.add().addFilepattern("local.txt").call(); + RevCommit t1 = target.commit().setMessage("target commit 1").call(); + + target.checkout().setCreateBranch(true).setName("side").call(); + + String newContent = "initial\n" + "and more\n"; + writeToFile(loxalFile, newContent); + target.add().addFilepattern("local.txt").call(); + RevCommit t2 = target.commit().setMessage("target commit 2").call(); + + target.checkout().setName("master").call(); + + MergeResult mergeResult = target.merge() + .setFastForward(MergeCommand.FastForwardMode.NO_FF).include(t2) + .call(); + assertEquals(MergeStatus.MERGED, mergeResult.getMergeStatus()); + assertFileContentsEqual(loxalFile, newContent); + ObjectId merge = mergeResult.getNewHead(); + + // pull + PullResult res = pullSetup.call(); + assertNotNull(res.getFetchResult()); + + if (expectedPullMode == TestPullMode.MERGE) { + assertEquals(MergeStatus.MERGED, res.getMergeResult() + .getMergeStatus()); + assertNull(res.getRebaseResult()); + } else { + assertNull(res.getMergeResult()); + assertEquals(RebaseResult.OK_RESULT, res.getRebaseResult()); + } + assertFileContentsEqual(sourceFile, "content"); + + RevWalk rw = new RevWalk(dbTarget); + rw.sort(RevSort.TOPO); + rw.markStart(rw.parseCommit(dbTarget.resolve("refs/heads/master"))); + + RevCommit next; + if (expectedPullMode == TestPullMode.MERGE) { + next = rw.next(); + assertEquals(2, next.getParentCount()); + assertEquals(merge, next.getParent(0)); + assertEquals(sourceCommit, next.getParent(1)); + // since both parents are known do no further checks here + } else { + if (expectedPullMode == TestPullMode.REBASE_PREASERVE) { + next = rw.next(); + assertEquals(2, next.getParentCount()); + } + next = rw.next(); + assertEquals(t2.getShortMessage(), next.getShortMessage()); + next = rw.next(); + assertEquals(t1.getShortMessage(), next.getShortMessage()); + next = rw.next(); + assertEquals(sourceCommit, next); + next = rw.next(); + assertEquals("Initial commit for source", next.getShortMessage()); + next = rw.next(); + assertNull(next); + } + rw.release(); + } + @Override @Before public void setUp() throws Exception { 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 7634957eb..cdaf3ec97 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java @@ -81,7 +81,7 @@ public class PullCommand extends TransportCommand { private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; - private PullRebaseMode pullRebaseMode = PullRebaseMode.USE_CONFIG; + private PullRebaseMode pullRebaseMode = null; private String remote; @@ -89,10 +89,31 @@ public class PullCommand extends TransportCommand { private MergeStrategy strategy = MergeStrategy.RECURSIVE; - private enum PullRebaseMode { - USE_CONFIG, - REBASE, - NO_REBASE + private enum PullRebaseMode implements Config.ConfigEnum { + REBASE_PRESERVE("preserve", true, true), //$NON-NLS-1$ + REBASE("true", true, false), //$NON-NLS-1$ + NO_REBASE("false", false, false); //$NON-NLS-1$ + + private final String configValue; + + private final boolean rebase; + + private final boolean preserveMerges; + + PullRebaseMode(String configValue, boolean rebase, + boolean preserveMerges) { + this.configValue = configValue; + this.rebase = rebase; + this.preserveMerges = preserveMerges; + } + + public String toConfigValue() { + return configValue; + } + + public boolean matchConfigValue(String in) { + return in.equals(configValue); + } } /** @@ -205,23 +226,10 @@ public PullResult call() throws GitAPIException, ConfigConstants.CONFIG_BRANCH_SECTION, branchName, ConfigConstants.CONFIG_KEY_MERGE); - // determines whether rebase should be used after fetching - boolean doRebase = false; - switch (pullRebaseMode) { - case REBASE: - doRebase = true; - break; - case NO_REBASE: - doRebase = false; - break; - case USE_CONFIG: - default: - // check if the branch is configured for pull-rebase - doRebase = repoConfig.getBoolean( - ConfigConstants.CONFIG_BRANCH_SECTION, branchName, - ConfigConstants.CONFIG_KEY_REBASE, false); - break; - } + // determines whether rebase should be used after fetching + if (pullRebaseMode == null) { + pullRebaseMode = getRebaseMode(branchName, repoConfig); + } if (remoteBranchName == null) remoteBranchName = branchName; @@ -299,11 +307,12 @@ public PullResult call() throws GitAPIException, + remoteUri; PullResult result; - if (doRebase) { + if (pullRebaseMode.rebase) { RebaseCommand rebase = new RebaseCommand(repo); RebaseResult rebaseRes = rebase.setUpstream(commitToMerge) .setUpstreamName(upstreamName).setProgressMonitor(monitor) .setOperation(Operation.BEGIN).setStrategy(strategy) + .setPreserveMerges(pullRebaseMode.preserveMerges) .call(); result = new PullResult(fetchRes, remote, rebaseRes); } else { @@ -378,4 +387,14 @@ public PullCommand setStrategy(MergeStrategy strategy) { this.strategy = strategy; return this; } + + private static PullRebaseMode getRebaseMode(String branchName, Config config) { + PullRebaseMode mode = config.getEnum(PullRebaseMode.values(), + ConfigConstants.CONFIG_PULL_SECTION, null, + ConfigConstants.CONFIG_KEY_REBASE, PullRebaseMode.NO_REBASE); + mode = config.getEnum(PullRebaseMode.values(), + ConfigConstants.CONFIG_BRANCH_SECTION, + branchName, ConfigConstants.CONFIG_KEY_REBASE, mode); + return mode; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java index 1a0945926..b52ecd815 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -97,6 +97,13 @@ public class ConfigConstants { */ public static final String CONFIG_FETCH_SECTION = "fetch"; + /** + * The "pull" section + * + * @since 3.5 + */ + public static final String CONFIG_PULL_SECTION = "pull"; + /** The "algorithm" key */ public static final String CONFIG_KEY_ALGORITHM = "algorithm";