FetchCommand: Fix detection of submodule recursion mode

The submodule.name.fetchRecurseSubmodules value was being read from the
configuration of the submodule, but it should be read from the config
of the parent repository.

Also, the fetch.recurseSubmodules value from the parent repository's
configuration was not being considered at all.

Fix both of these and add tests. Now the precedence of the recurse mode
is determined as follows:

 1. Value passed to the API
 2. Value configured in submodule.name.fetchRecurseSubmodules
 3. Value configured in fetch.recurseSubmodules
 4. Default to "on demand"

Change-Id: Ic23b7c40b5f39135fb3fd754c597dd4bcc94240c

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandRecurseSubmodulesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandRecurseSubmodulesTest.java
index fbd3e1a0573e9a4649b591ef29fd7611222b6a99..9b2496ab2fa06e81fa2f2f9ee25ec2f9b384b09c 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandRecurseSubmodulesTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandRecurseSubmodulesTest.java
@@ -51,9 +51,11 @@ import java.io.File;
 import org.eclipse.jgit.api.ResetCommand.ResetType;
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.RepositoryTestCase;
+import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.lib.SubmoduleConfig.FetchRecurseSubmodulesMode;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.submodule.SubmoduleStatus;
@@ -85,6 +87,8 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
 
        private final String REMOTE = "origin";
 
+       private final String PATH = "sub";
+
        @Before
        public void setUpSubmodules()
                        throws Exception {
@@ -100,7 +104,6 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
                addRepoToClose(sub1);
 
                String file = "file.txt";
-               String path = "sub";
 
                write(new File(sub1.getWorkTree(), file), "content");
                sub1Git.add().addFilepattern(file).call();
@@ -122,7 +125,7 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
                assertNotNull(sub2Head);
 
                // Add submodule 2 to submodule 1
-               Repository r2 = sub1Git.submoduleAdd().setPath(path)
+               Repository r2 = sub1Git.submoduleAdd().setPath(PATH)
                                .setURI(sub2.getDirectory().toURI().toString()).call();
                assertNotNull(r2);
                addRepoToClose(r2);
@@ -131,7 +134,7 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
                assertNotNull(sub1Head);
 
                // Add submodule 1 to default repository
-               Repository r1 = git.submoduleAdd().setPath(path)
+               Repository r1 = git.submoduleAdd().setPath(PATH)
                                .setURI(sub1.getDirectory().toURI().toString()).call();
                assertNotNull(r1);
                addRepoToClose(r1);
@@ -193,6 +196,90 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
        @Test
        public void shouldFetchSubmodulesWhenOnDemandAndRevisionChanged()
                        throws Exception {
+               RevCommit update = updateSubmoduleRevision();
+               FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
+
+               // The first submodule should have been updated
+               assertTrue(result.submoduleResults().containsKey("sub"));
+               FetchResult subResult = result.submoduleResults().get("sub");
+
+               // The second submodule should not get updated
+               assertTrue(subResult.submoduleResults().isEmpty());
+               assertSubmoduleFetchHeads(commit1, submodule2Head);
+
+               // After fetch the parent repo's fetch head should be the commit
+               // that updated the submodule.
+               assertEquals(update,
+                               git2.getRepository().resolve(Constants.FETCH_HEAD));
+       }
+
+       @Test
+       public void shouldNotFetchSubmodulesWhenOnDemandAndRevisionNotChanged()
+                       throws Exception {
+               FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
+               assertTrue(result.submoduleResults().isEmpty());
+               assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
+       }
+
+       @Test
+       public void shouldNotFetchSubmodulesWhenSubmoduleConfigurationSetToNo()
+                       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);
+               assertTrue(result.submoduleResults().isEmpty());
+               assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
+       }
+
+       @Test
+       public void shouldFetchSubmodulesWhenSubmoduleConfigurationSetToYes()
+                       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);
+               assertTrue(result.submoduleResults().containsKey("sub"));
+               FetchResult subResult = result.submoduleResults().get("sub");
+               assertTrue(subResult.submoduleResults().containsKey("sub"));
+               assertSubmoduleFetchHeads(commit1, commit2);
+       }
+
+       @Test
+       public void shouldNotFetchSubmodulesWhenFetchConfigurationSetToNo()
+                       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);
+               assertTrue(result.submoduleResults().isEmpty());
+               assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
+       }
+
+       @Test
+       public void shouldFetchSubmodulesWhenFetchConfigurationSetToYes()
+                       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);
+               assertTrue(result.submoduleResults().containsKey("sub"));
+               FetchResult subResult = result.submoduleResults().get("sub");
+               assertTrue(subResult.submoduleResults().containsKey("sub"));
+               assertSubmoduleFetchHeads(commit1, commit2);
+       }
+
+       private RevCommit updateSubmoduleRevision() throws Exception {
                // Fetch the submodule in the original git and reset it to
                // the commit that was created
                try (SubmoduleWalk w = SubmoduleWalk.forIndex(git.getRepository())) {
@@ -221,28 +308,7 @@ public class FetchCommandRecurseSubmodulesTest extends RepositoryTestCase {
                assertEquals(commit1, subStatus.getHeadId());
                assertEquals(SubmoduleStatusType.INITIALIZED, subStatus.getType());
 
-               FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
-
-               // The first submodule should have been updated
-               assertTrue(result.submoduleResults().containsKey("sub"));
-               FetchResult subResult = result.submoduleResults().get("sub");
-
-               // The second submodule should not get updated
-               assertTrue(subResult.submoduleResults().isEmpty());
-               assertSubmoduleFetchHeads(commit1, submodule2Head);
-
-               // After fetch the parent repo's fetch head should be the commit
-               // that updated the submodule.
-               assertEquals(update,
-                               git2.getRepository().resolve(Constants.FETCH_HEAD));
-       }
-
-       @Test
-       public void shouldNotFetchSubmodulesWhenOnDemandAndRevisionNotChanged()
-                       throws Exception {
-               FetchResult result = fetch(FetchRecurseSubmodulesMode.ON_DEMAND);
-               assertTrue(result.submoduleResults().isEmpty());
-               assertSubmoduleFetchHeads(submodule1Head, submodule2Head);
+               return update;
        }
 
        private FetchResult fetch(FetchRecurseSubmodulesMode mode)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java
index b365087888f594836ed6751e1705592ac1661931..cc3302b486f54f490cb5f98edb630c806888b46a 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/FetchCommand.java
@@ -60,6 +60,7 @@ import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
@@ -106,15 +107,14 @@ public class FetchCommand extends TransportCommand<FetchCommand, FetchResult> {
                refSpecs = new ArrayList<>(3);
        }
 
-       private FetchRecurseSubmodulesMode getRecurseMode(Repository repository,
-                       String path) {
+       private FetchRecurseSubmodulesMode getRecurseMode(String path) {
                // Use the caller-specified mode, if set
                if (submoduleRecurseMode != null) {
                        return submoduleRecurseMode;
                }
 
-               // Fall back to submodule config, if set
-               FetchRecurseSubmodulesMode mode = repository.getConfig().getEnum(
+               // Fall back to submodule.name.fetchRecurseSubmodules, if set
+               FetchRecurseSubmodulesMode mode = repo.getConfig().getEnum(
                                FetchRecurseSubmodulesMode.values(),
                                ConfigConstants.CONFIG_SUBMODULE_SECTION, path,
                                ConfigConstants.CONFIG_KEY_FETCH_RECURSE_SUBMODULES, null);
@@ -122,22 +122,29 @@ public class FetchCommand extends TransportCommand<FetchCommand, FetchResult> {
                        return mode;
                }
 
+               // Fall back to fetch.recurseSubmodules, if set
+               mode = repo.getConfig().getEnum(FetchRecurseSubmodulesMode.values(),
+                               ConfigConstants.CONFIG_FETCH_SECTION, null,
+                               ConfigConstants.CONFIG_KEY_RECURSE_SUBMODULES, null);
+               if (mode != null) {
+                       return mode;
+               }
+
                // Default to on-demand mode
                return FetchRecurseSubmodulesMode.ON_DEMAND;
        }
 
-       private boolean isRecurseSubmodules() {
-               return submoduleRecurseMode != null
-                               && submoduleRecurseMode != FetchRecurseSubmodulesMode.NO;
-       }
-
        private void fetchSubmodules(FetchResult results)
                        throws org.eclipse.jgit.api.errors.TransportException,
                        GitAPIException, InvalidConfigurationException {
                try (SubmoduleWalk walk = new SubmoduleWalk(repo);
                                RevWalk revWalk = new RevWalk(repo)) {
                        // Walk over submodules in the parent repository's FETCH_HEAD.
-                       walk.setTree(revWalk.parseTree(repo.resolve(Constants.FETCH_HEAD)));
+                       ObjectId fetchHead = repo.resolve(Constants.FETCH_HEAD);
+                       if (fetchHead == null) {
+                               return;
+                       }
+                       walk.setTree(revWalk.parseTree(fetchHead));
                        while (walk.next()) {
                                Repository submoduleRepo = walk.getRepository();
 
@@ -150,7 +157,7 @@ public class FetchCommand extends TransportCommand<FetchCommand, FetchResult> {
                                }
 
                                FetchRecurseSubmodulesMode recurseMode = getRecurseMode(
-                                               submoduleRepo, walk.getPath());
+                                               walk.getPath());
 
                                // When the fetch mode is "yes" we always fetch. When the mode
                                // is "on demand", we only fetch if the submodule's revision was
@@ -204,8 +211,7 @@ public class FetchCommand extends TransportCommand<FetchCommand, FetchResult> {
                        configure(transport);
 
                        FetchResult result = transport.fetch(monitor, refSpecs);
-                       if (!repo.isBare() && (!result.getTrackingRefUpdates().isEmpty()
-                                       || isRecurseSubmodules())) {
+                       if (!repo.isBare()) {
                                fetchSubmodules(result);
                        }
 

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 ff0d811ba90cbdf74fdca566c42a39ff78027aff..74fc7067a1401aed4a991c5a862d6222b4d8db08 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -380,4 +380,10 @@ public class ConfigConstants {
         * @since 4.7
         */
        public static final String CONFIG_KEY_FETCH_RECURSE_SUBMODULES = "fetchRecurseSubmodules";
+
+       /**
+        * The "recurseSubmodules" key
+        * @since 4.7
+        */
+       public static final String CONFIG_KEY_RECURSE_SUBMODULES = "recurseSubmodules";
 }