PullCommand: Add support for recursing into submodules

Add a new API method to set the recurse mode, and pass the mode into
the fetch command.

Extend the existing FetchCommandRecurseSubmodulesTest to also perform
the same tests for fetch. Rename the test class accordingly.

Change-Id: I12553af47774b4778f7011e1018bd575a7909bd0
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

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

@@ -64,9 +64,16 @@ import org.eclipse.jgit.submodule.SubmoduleWalk;
 import org.eclipse.jgit.transport.FetchResult;
 import org.eclipse.jgit.transport.RefSpec;
 import org.junit.Before;
-import org.junit.Test;
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.experimental.theories.Theory;
+import org.junit.runner.RunWith;
+
+@RunWith(Theories.class)
+public class FetchAndPullCommandsRecurseSubmodulesTest extends RepositoryTestCase {
+	@DataPoints
+	public static boolean[] useFetch = { true, false };
 
-public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 	private Git git;
 
 	private Git git2;
@@ -90,8 +97,7 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 	private final String PATH = "sub";
 
 	@Before
-	public void setUpSubmodules()
+	public void setUpSubmodules() throws Exception {
-			throws Exception {
 		git = new Git(db);
 
 		// Create submodule 1
@@ -177,27 +183,28 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 		commit2 = sub2Git.commit().setMessage("new commit").call();
 	}
 
-	@Test
+	@Theory
-	public void shouldNotFetchSubmodulesWhenNo() throws Exception {
+	public void shouldNotFetchSubmodulesWhenNo(boolean fetch) throws Exception {
-		FetchResult result = fetch(FetchRecurseSubmodulesMode.NO);
+		FetchResult result = execute(FetchRecurseSubmodulesMode.NO, fetch);
 		assertTrue(result.submoduleResults().isEmpty());
 		assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
 	}
 
-	@Test
+	@Theory
-	public void shouldFetchSubmodulesWhenYes() throws Exception {
+	public void shouldFetchSubmodulesWhenYes(boolean fetch) throws Exception {
-		FetchResult result = fetch(FetchRecurseSubmodulesMode.YES);
+		FetchResult result = execute(FetchRecurseSubmodulesMode.YES, fetch);
 		assertTrue(result.submoduleResults().containsKey("sub"));
 		FetchResult subResult = result.submoduleResults().get("sub");
 		assertTrue(subResult.submoduleResults().containsKey("sub"));
 		assertSubmoduleFetchHeads(commit1, commit2);
 	}
 
-	@Test
+	@Theory
-	public void shouldFetchSubmodulesWhenOnDemandAndRevisionChanged()
+	public void shouldFetchSubmodulesWhenOnDemandAndRevisionChanged(
-			throws Exception {
+			boolean fetch) throws Exception {
 		RevCommit update = updateSubmoduleRevision();
-		FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
+		FetchResult result = execute(FetchRecurseSubmodulesMode.ON_DEMAND,
+				fetch);
 
 		// The first submodule should have been updated
 		assertTrue(result.submoduleResults().containsKey("sub"));
@@ -213,66 +220,67 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 				git2.getRepository().resolve(Constants.FETCH_HEAD));
 	}
 
-	@Test
+	@Theory
-	public void shouldNotFetchSubmodulesWhenOnDemandAndRevisionNotChanged()
+	public void shouldNotFetchSubmodulesWhenOnDemandAndRevisionNotChanged(
-			throws Exception {
+			boolean fetch) throws Exception {
-		FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
+		FetchResult result = execute(FetchRecurseSubmodulesMode.ON_DEMAND,
+				fetch);
 		assertTrue(result.submoduleResults().isEmpty());
 		assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
 	}
 
-	@Test
+	@Theory
-	public void shouldNotFetchSubmodulesWhenSubmoduleConfigurationSetToNo()
+	public void shouldNotFetchSubmodulesWhenSubmoduleConfigurationSetToNo(
-			throws Exception {
+			boolean fetch) throws Exception {
 		StoredConfig config = git2.getRepository().getConfig();
 		config.setEnum(ConfigConstants.CONFIG_SUBMODULE_SECTION, PATH,
 				ConfigConstants.CONFIG_KEY_FETCH_RECURSE_SUBMODULES,
 				FetchRecurseSubmodulesMode.NO);
 		config.save();
 		updateSubmoduleRevision();
-		FetchResult result = fetch(null);
+		FetchResult result = execute(null, fetch);
 		assertTrue(result.submoduleResults().isEmpty());
 		assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
 	}
 
-	@Test
+	@Theory
-	public void shouldFetchSubmodulesWhenSubmoduleConfigurationSetToYes()
+	public void shouldFetchSubmodulesWhenSubmoduleConfigurationSetToYes(
-			throws Exception {
+			boolean fetch) throws Exception {
 		StoredConfig config = git2.getRepository().getConfig();
 		config.setEnum(ConfigConstants.CONFIG_SUBMODULE_SECTION, PATH,
 				ConfigConstants.CONFIG_KEY_FETCH_RECURSE_SUBMODULES,
 				FetchRecurseSubmodulesMode.YES);
 		config.save();
-		FetchResult result = fetch(null);
+		FetchResult result = execute(null, fetch);
 		assertTrue(result.submoduleResults().containsKey("sub"));
 		FetchResult subResult = result.submoduleResults().get("sub");
 		assertTrue(subResult.submoduleResults().containsKey("sub"));
 		assertSubmoduleFetchHeads(commit1, commit2);
 	}
 
-	@Test
+	@Theory
-	public void shouldNotFetchSubmodulesWhenFetchConfigurationSetToNo()
+	public void shouldNotFetchSubmodulesWhenFetchConfigurationSetToNo(
-			throws Exception {
+			boolean fetch) throws Exception {
 		StoredConfig config = git2.getRepository().getConfig();
 		config.setEnum(ConfigConstants.CONFIG_FETCH_SECTION, null,
 				ConfigConstants.CONFIG_KEY_RECURSE_SUBMODULES,
 				FetchRecurseSubmodulesMode.NO);
 		config.save();
 		updateSubmoduleRevision();
-		FetchResult result = fetch(null);
+		FetchResult result = execute(null, fetch);
 		assertTrue(result.submoduleResults().isEmpty());
 		assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
 	}
 
-	@Test
+	@Theory
-	public void shouldFetchSubmodulesWhenFetchConfigurationSetToYes()
+	public void shouldFetchSubmodulesWhenFetchConfigurationSetToYes(
-			throws Exception {
+			boolean fetch) throws Exception {
 		StoredConfig config = git2.getRepository().getConfig();
 		config.setEnum(ConfigConstants.CONFIG_FETCH_SECTION, null,
 				ConfigConstants.CONFIG_KEY_RECURSE_SUBMODULES,
 				FetchRecurseSubmodulesMode.YES);
 		config.save();
-		FetchResult result = fetch(null);
+		FetchResult result = execute(null, fetch);
 		assertTrue(result.submoduleResults().containsKey("sub"));
 		FetchResult subResult = result.submoduleResults().get("sub");
 		assertTrue(subResult.submoduleResults().containsKey("sub"));
@@ -311,10 +319,24 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 		return update;
 	}
 
-	private FetchResult fetch(FetchRecurseSubmodulesMode mode)
+	private FetchResult execute(FetchRecurseSubmodulesMode mode, boolean fetch)
 			throws Exception {
-		FetchResult result = git2.fetch().setRemote(REMOTE).setRefSpecs(REFSPEC)
+		FetchResult result;
-				.setRecurseSubmodules(mode).call();
+
+		if (fetch) {
+			result = git2.fetch().setRemote(REMOTE).setRefSpecs(REFSPEC)
+					.setRecurseSubmodules(mode).call();
+		} else {
+			// For the purposes of this test we don't need to care about the
+			// pull result, or the result of pull with merge. We are only
+			// interested in checking whether or not the submodules were updated
+			// as expected. Setting to rebase makes it easier to assert about
+			// the state of the parent repository head, i.e. we know it should
+			// be at the submodule update commit, and don't need to consider a
+			// merge commit created by the pull.
+			result = git2.pull().setRemote(REMOTE).setRebase(true)
+					.setRecurseSubmodules(mode).call().getFetchResult();
+		}
 		assertNotNull(result);
 		return result;
 	}

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

@@ -69,6 +69,7 @@ import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryState;
+import org.eclipse.jgit.lib.SubmoduleConfig.FetchRecurseSubmodulesMode;
 import org.eclipse.jgit.merge.MergeStrategy;
 import org.eclipse.jgit.transport.FetchResult;
 import org.eclipse.jgit.transport.TagOpt;
@@ -95,6 +96,8 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 
 	private TagOpt tagOption;
 
+	private FetchRecurseSubmodulesMode submoduleRecurseMode = null;
+
 	/**
 	 * @param repo
 	 */
@@ -277,7 +280,8 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 						JGitText.get().pullTaskName));
 
 			FetchCommand fetch = new FetchCommand(repo).setRemote(remote)
-					.setProgressMonitor(monitor).setTagOpt(tagOption);
+					.setProgressMonitor(monitor).setTagOpt(tagOption)
+					.setRecurseSubmodules(submoduleRecurseMode);
 			configure(fetch);
 
 			fetchRes = fetch.call();
@@ -427,6 +431,19 @@ public class PullCommand extends TransportCommand<PullCommand, PullResult> {
 		return this;
 	}
 
+	/**
+	 * Set the mode to be used for recursing into submodules.
+	 *
+	 * @param recurse
+	 * @return {@code this}
+	 * @since 4.7
+	 */
+	public PullCommand setRecurseSubmodules(
+			FetchRecurseSubmodulesMode recurse) {
+		this.submoduleRecurseMode = recurse;
+		return this;
+	}
+
 	/**
 	 * Reads the rebase mode to use for a pull command from the repository
 	 * configuration. This is the value defined for the configurations