Preserve merges during pull if configured to do so

Setting branch.<name>.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 <swamblumat-eclipsebugs@yahoo.de>
11 years ago · 48544e27ae
3 changed files with 256 additions and 23 deletions
--- 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 @@ package org.eclipse.jgit.api;

 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.File;
 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.Repository;
 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 class PullCommandTest extends RepositoryTestCase {
 		assertEquals(message, mergeCommit.getShortMessage());
 	}

+	private enum TestPullMode {
+		MERGE, REBASE, REBASE_PREASERVE
+	}
+
+	@Test
+	/** global rebase config should be respected */
+	public void testPullWithRebasePreserve1Config() throws Exception {
+		Callable<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> setup = new Callable<PullResult>() {
+			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<PullResult> 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 {
--- 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<PullCommand, PullResult> {

 	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<PullCommand, PullResult> {

 	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);
+		}
 	}

 	/**
@ -206,21 +227,8 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 					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;
+		if (pullRebaseMode == null) {
+			pullRebaseMode = getRebaseMode(branchName, repoConfig);
 		}

 		if (remoteBranchName == null)
@ -299,11 +307,12 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 				+ 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 class PullCommand extends TransportCommand<PullCommand, PullResult> {
 		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;
+	}
 }
--- 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";