Handle all values of branch.[name].rebase

BranchConfig treated this config property as a boolean, but git also
allows the values "preserve" and "interactive". Config property
pull.rebase also allows the same values.

Replace private enum PullCommand.PullRebaseMode by new public enum
BranchConfig.BranchRebaseMode and adapt all uses. Add a new setter to
PullCommand.

Note: PullCommand will treat "interactive" like "true", i.e., as a
non-interactive rebase. Not sure how "interactive" should be handled.
At least it won't balk on it.

Bug: 499482
Change-Id: I7309360f5662b2c2efa1bd8ea6f112c63cf064af
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

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

@@ -62,6 +62,7 @@ import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
@@ -605,11 +606,10 @@ public class CloneCommandTest extends RepositoryTestCase {
 		command.setURI(fileUri());
 		Git git2 = command.call();
 		addRepoToClose(git2.getRepository());
-		assertFalse(git2
-				.getRepository()
-				.getConfig()
-				.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION, "test",
-						ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertNull(git2.getRepository().getConfig().getEnum(
+				BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, "test",
+				ConfigConstants.CONFIG_KEY_REBASE, null));
 
 		FileBasedConfig userConfig = SystemReader.getInstance().openUserConfig(
 				null, git.getRepository().getFS());
@@ -623,11 +623,12 @@ public class CloneCommandTest extends RepositoryTestCase {
 		command.setURI(fileUri());
 		git2 = command.call();
 		addRepoToClose(git2.getRepository());
-		assertTrue(git2
-				.getRepository()
-				.getConfig()
-				.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION, "test",
-						ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertEquals(BranchRebaseMode.REBASE,
+				git2.getRepository().getConfig().getEnum(
+						BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, "test",
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
 
 		userConfig.setString(ConfigConstants.CONFIG_BRANCH_SECTION, null,
 				ConfigConstants.CONFIG_KEY_AUTOSETUPREBASE,
@@ -639,11 +640,12 @@ public class CloneCommandTest extends RepositoryTestCase {
 		command.setURI(fileUri());
 		git2 = command.call();
 		addRepoToClose(git2.getRepository());
-		assertTrue(git2
-				.getRepository()
-				.getConfig()
-				.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION, "test",
-						ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertEquals(BranchRebaseMode.REBASE,
+				git2.getRepository().getConfig().getEnum(
+						BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, "test",
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
 
 	}
 

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

@@ -43,11 +43,14 @@
 package org.eclipse.jgit.api;
 
 import static org.junit.Assert.assertArrayEquals;
+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;
 
 import org.eclipse.jgit.junit.RepositoryTestCase;
+import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.StoredConfig;
@@ -98,32 +101,40 @@ public class RenameBranchCommandTest extends RepositoryTestCase {
 	@Test
 	public void renameBranchSingleConfigValue() throws Exception {
 		StoredConfig config = git.getRepository().getConfig();
-		config.setBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, true);
+		config.setEnum(ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.REBASE);
 		config.save();
 
 		String branch = "b1";
 
-		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, true));
-		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				branch, ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertEquals(BranchRebaseMode.REBASE,
+				config.getEnum(BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
+		assertNull(config.getEnum(BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, branch,
+				ConfigConstants.CONFIG_KEY_REBASE, null));
 
 		assertNotNull(git.branchRename().setNewName(branch).call());
 
 		config = git.getRepository().getConfig();
-		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, false));
-		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				branch, ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertNull(config.getEnum(BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REBASE, null));
+		assertEquals(BranchRebaseMode.REBASE,
+				config.getEnum(BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, branch,
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
 	}
 
 	@Test
 	public void renameBranchExistingSection() throws Exception {
 		String branch = "b1";
 		StoredConfig config = git.getRepository().getConfig();
-		config.setBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, true);
+		config.setEnum(ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.REBASE);
 		config.setString(ConfigConstants.CONFIG_BRANCH_SECTION,
 				Constants.MASTER, "a", "a");
 		config.setString(ConfigConstants.CONFIG_BRANCH_SECTION, branch, "a",
@@ -140,18 +151,22 @@ public class RenameBranchCommandTest extends RepositoryTestCase {
 	@Test
 	public void renameBranchMultipleConfigValues() throws Exception {
 		StoredConfig config = git.getRepository().getConfig();
-		config.setBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, true);
+		config.setEnum(ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.REBASE);
 		config.setBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
 				Constants.MASTER, ConfigConstants.CONFIG_KEY_MERGE, true);
 		config.save();
 
 		String branch = "b1";
 
-		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, true));
-		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				branch, ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertEquals(BranchRebaseMode.REBASE,
+				config.getEnum(BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
+		assertNull(config.getEnum(BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, branch,
+				ConfigConstants.CONFIG_KEY_REBASE, null));
 		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
 				Constants.MASTER, ConfigConstants.CONFIG_KEY_MERGE, true));
 		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
@@ -160,10 +175,14 @@ public class RenameBranchCommandTest extends RepositoryTestCase {
 		assertNotNull(git.branchRename().setNewName(branch).call());
 
 		config = git.getRepository().getConfig();
-		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				Constants.MASTER, ConfigConstants.CONFIG_KEY_REBASE, false));
-		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				branch, ConfigConstants.CONFIG_KEY_REBASE, false));
+		assertNull(config.getEnum(BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, Constants.MASTER,
+				ConfigConstants.CONFIG_KEY_REBASE, null));
+		assertEquals(BranchRebaseMode.REBASE,
+				config.getEnum(BranchRebaseMode.values(),
+						ConfigConstants.CONFIG_BRANCH_SECTION, branch,
+						ConfigConstants.CONFIG_KEY_REBASE,
+						BranchRebaseMode.NONE));
 		assertFalse(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
 				Constants.MASTER, ConfigConstants.CONFIG_KEY_MERGE, false));
 		assertTrue(config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,

org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java

@@ -58,6 +58,7 @@ import org.eclipse.jgit.dircache.DirCacheCheckout;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -326,9 +327,9 @@ public class CloneCommand extends TransportCommand<CloneCommand, Git> {
 				ConfigConstants.CONFIG_KEY_AUTOSETUPREBASE);
 		if (ConfigConstants.CONFIG_KEY_ALWAYS.equals(autosetupRebase)
 				|| ConfigConstants.CONFIG_KEY_REMOTE.equals(autosetupRebase))
-			clonedRepo.getConfig().setBoolean(
+			clonedRepo.getConfig().setEnum(
 					ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
-					ConfigConstants.CONFIG_KEY_REBASE, true);
+					ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.REBASE);
 		clonedRepo.getConfig().save();
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/api/PullCommand.java

@@ -60,6 +60,7 @@ import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.BranchConfig.BranchRebaseMode;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
@@ -83,7 +84,7 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 
 	private ProgressMonitor monitor = NullProgressMonitor.INSTANCE;
 
-	private PullRebaseMode pullRebaseMode = null;
+	private BranchRebaseMode pullRebaseMode = null;
 
 	private String remote;
 
@@ -91,33 +92,6 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 
 	private MergeStrategy strategy = MergeStrategy.RECURSIVE;
 
-	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);
-		}
-	}
-
 	/**
 	 * @param repo
 	 */
@@ -158,7 +132,46 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 	 */
 	public PullCommand setRebase(boolean useRebase) {
 		checkCallable();
-		pullRebaseMode = useRebase ? PullRebaseMode.REBASE : PullRebaseMode.NO_REBASE;
+		pullRebaseMode = useRebase ? BranchRebaseMode.REBASE
+				: BranchRebaseMode.NONE;
+		return this;
+	}
+
+	/**
+	 * Sets the {@link BranchRebaseMode} to use after fetching.
+	 *
+	 * <dl>
+	 * <dt>BranchRebaseMode.REBASE</dt>
+	 * <dd>Equivalent to {@code --rebase} on the command line: use rebase
+	 * instead of merge after fetching.</dd>
+	 * <dt>BranchRebaseMode.PRESERVE</dt>
+	 * <dd>Equivalent to {@code --preserve-merges} on the command line: rebase
+	 * preserving local merge commits.</dd>
+	 * <dt>BranchRebaseMode.INTERACTIVE</dt>
+	 * <dd>Equivalent to {@code --interactive} on the command line: use
+	 * interactive rebase.</dd>
+	 * <dt>BranchRebaseMode.NONE</dt>
+	 * <dd>Equivalent to {@code --no-rebase}: merge instead of rebasing.
+	 * <dt>{@code null}</dt>
+	 * <dd>Use the setting defined in the git configuration, either {@code
+	 * branch.[name].rebase} or, if not set, {@code pull.rebase}</dd>
+	 * </dl>
+	 *
+	 * This setting overrides the settings in the configuration file. By
+	 * default, the setting in the repository configuration file is used.
+	 * <p>
+	 * A branch can be configured to use rebase by default. See
+	 * {@code branch.[name].rebase}, {@code branch.autosetuprebase}, and
+	 * {@code pull.rebase}.
+	 *
+	 * @param rebaseMode
+	 *            the {@link BranchRebaseMode} to use
+	 * @return {@code this}
+	 * @since 4.5
+	 */
+	public PullCommand setRebase(BranchRebaseMode rebaseMode) {
+		checkCallable();
+		pullRebaseMode = rebaseMode;
 		return this;
 	}
 
@@ -315,12 +328,13 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 				Repository.shortenRefName(remoteBranchName), remoteUri);
 
 		PullResult result;
-		if (pullRebaseMode.rebase) {
+		if (pullRebaseMode != BranchRebaseMode.NONE) {
 			RebaseCommand rebase = new RebaseCommand(repo);
 			RebaseResult rebaseRes = rebase.setUpstream(commitToMerge)
 					.setUpstreamName(upstreamName).setProgressMonitor(monitor)
 					.setOperation(Operation.BEGIN).setStrategy(strategy)
-					.setPreserveMerges(pullRebaseMode.preserveMerges)
+					.setPreserveMerges(
+							pullRebaseMode == BranchRebaseMode.PRESERVE)
 					.call();
 			result = new PullResult(fetchRes, remote, rebaseRes);
 		} else {
@@ -397,13 +411,29 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 		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(),
+	/**
+	 * Reads the rebase mode to use for a pull command from the repository
+	 * configuration. This is the value defined for the configurations
+	 * {@code branch.[branchName].rebase}, or,if not set, {@code pull.rebase}.
+	 * If neither is set, yields {@link BranchRebaseMode#NONE}.
+	 *
+	 * @param branchName
+	 *            name of the local branch
+	 * @param config
+	 *            the {@link Config} to read the value from
+	 * @return the {@link BranchRebaseMode}
+	 * @since 4.5
+	 */
+	public static BranchRebaseMode getRebaseMode(String branchName,
+			Config config) {
+		BranchRebaseMode mode = config.getEnum(BranchRebaseMode.values(),
 				ConfigConstants.CONFIG_BRANCH_SECTION,
-				branchName, ConfigConstants.CONFIG_KEY_REBASE, mode);
+				branchName, ConfigConstants.CONFIG_KEY_REBASE, null);
+		if (mode == null) {
+			mode = config.getEnum(BranchRebaseMode.values(),
+					ConfigConstants.CONFIG_PULL_SECTION, null,
+					ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.NONE);
+		}
 		return mode;
 	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/lib/BranchConfig.java

@@ -54,6 +54,39 @@ import org.eclipse.jgit.transport.RemoteConfig;
  */
 public class BranchConfig {
 
+	/**
+	 * Config values for branch.[name].rebase (and pull.rebase).
+	 *
+	 * @since 4.5
+	 */
+	public enum BranchRebaseMode implements Config.ConfigEnum {
+
+		/** Value for rebasing */
+		REBASE("true"), //$NON-NLS-1$
+		/** Value for rebasing preserving local merge commits */
+		PRESERVE("preserve"), //$NON-NLS-1$
+		/** Value for rebasing interactively */
+		INTERACTIVE("interactive"), //$NON-NLS-1$
+		/** Value for not rebasing at all but merging */
+		NONE("false"); //$NON-NLS-1$
+
+		private final String configValue;
+
+		private BranchRebaseMode(String configValue) {
+			this.configValue = configValue;
+		}
+
+		@Override
+		public String toConfigValue() {
+			return configValue;
+		}
+
+		@Override
+		public boolean matchConfigValue(String s) {
+			return configValue.equals(s);
+		}
+	}
+
 	/**
 	 * The value that means "local repository" for {@link #getRemote()}:
 	 * {@value}
@@ -143,8 +176,19 @@ public class BranchConfig {
 	 * @since 3.5
 	 */
 	public boolean isRebase() {
-		return config.getBoolean(ConfigConstants.CONFIG_BRANCH_SECTION,
-				branchName, ConfigConstants.CONFIG_KEY_REBASE, false);
+		return getRebaseMode() != BranchRebaseMode.NONE;
+	}
+
+	/**
+	 * Retrieves the config value of branch.[name].rebase.
+	 *
+	 * @return the {@link BranchRebaseMode}
+	 * @since 4.5
+	 */
+	public BranchRebaseMode getRebaseMode() {
+		return config.getEnum(BranchRebaseMode.values(),
+				ConfigConstants.CONFIG_BRANCH_SECTION, branchName,
+				ConfigConstants.CONFIG_KEY_REBASE, BranchRebaseMode.NONE);
 	}
 
 	/**