Read .gitmodules config from the tree in SubmoduleWalk

It is not always appropriate to use the .gitmodules file from the
working tree, for example if reading the modules at a specific commit.
And sometimes it is impossible, as in a bare repository.

When using the static factory methods, automatically set up the
appropriate root tree so lazy loading of the config file reads from
the appropriate place. Leave the current behavior of looking in the
working tree as a fallback for the case where walking the index.

Change-Id: I71b7ed3ba16c80b0adb8c5fd85b5c37fd4aef8eb

org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleWalkTest.java

@@ -42,6 +42,10 @@
  */
 package org.eclipse.jgit.submodule;
 
+import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_PATH;
+import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_URL;
+import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_SUBMODULE_SECTION;
+import static org.eclipse.jgit.lib.Constants.DOT_GIT_MODULES;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -57,19 +61,33 @@ import org.eclipse.jgit.dircache.DirCacheEditor;
 import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
 import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryTestCase;
+import org.eclipse.jgit.revwalk.RevBlob;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
+import org.eclipse.jgit.treewalk.CanonicalTreeParser;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
+import org.junit.Before;
 import org.junit.Test;
 
 /**
  * Unit tests of {@link SubmoduleWalk}
  */
 public class SubmoduleWalkTest extends RepositoryTestCase {
+	private TestRepository<FileRepository> testDb;
+
+	@Before
+	public void setUp() throws Exception {
+		super.setUp();
+		testDb = new TestRepository<FileRepository>(db);
+	}
 
 	@Test
 	public void repositoryWithNoSubmodules() throws IOException {
@@ -269,4 +287,130 @@ public class SubmoduleWalkTest extends RepositoryTestCase {
 		assertEquals(id1, gen.getObjectId());
 		assertFalse(gen.next());
 	}
+
+	@Test
+	public void indexWithGitmodules() throws Exception {
+		final ObjectId subId = ObjectId
+				.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+		final String path = "sub";
+
+		final Config gitmodules = new Config();
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_PATH,
+				"sub");
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_URL,
+				"git://example.com/sub");
+		final RevBlob gitmodulesBlob = testDb.blob(gitmodules.toText());
+
+		// Different config in the working tree.
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_URL,
+				"git://example.com/bad");
+		writeTrashFile(DOT_GIT_MODULES, gitmodules.toText());
+
+		DirCache cache = db.lockDirCache();
+		DirCacheEditor editor = cache.editor();
+		editor.add(new PathEdit(path) {
+
+			public void apply(DirCacheEntry ent) {
+				ent.setFileMode(FileMode.GITLINK);
+				ent.setObjectId(subId);
+			}
+		});
+		editor.add(new PathEdit(DOT_GIT_MODULES) {
+
+			public void apply(DirCacheEntry ent) {
+				ent.setFileMode(FileMode.REGULAR_FILE);
+				ent.setObjectId(gitmodulesBlob);
+			}
+		});
+		editor.commit();
+
+		SubmoduleWalk gen = SubmoduleWalk.forIndex(db);
+		assertTrue(gen.next());
+		assertEquals(path, gen.getPath());
+		assertEquals(subId, gen.getObjectId());
+		assertEquals(new File(db.getWorkTree(), path), gen.getDirectory());
+		assertNull(gen.getConfigUpdate());
+		assertNull(gen.getConfigUrl());
+		assertEquals("sub", gen.getModulesPath());
+		assertNull(gen.getModulesUpdate());
+		assertEquals("git://example.com/sub", gen.getModulesUrl());
+		assertNull(gen.getRepository());
+		assertFalse(gen.next());
+	}
+
+	@Test
+	public void treeIdWithGitmodules() throws Exception {
+		final ObjectId subId = ObjectId
+				.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+		final String path = "sub";
+
+		final Config gitmodules = new Config();
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_PATH,
+				"sub");
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_URL,
+				"git://example.com/sub");
+
+		RevCommit commit = testDb.getRevWalk().parseCommit(testDb.commit()
+				.noParents()
+				.add(DOT_GIT_MODULES, gitmodules.toText())
+				.edit(new PathEdit(path) {
+
+							public void apply(DirCacheEntry ent) {
+								ent.setFileMode(FileMode.GITLINK);
+								ent.setObjectId(subId);
+							}
+						})
+				.create());
+
+		SubmoduleWalk gen = SubmoduleWalk.forPath(db, commit.getTree(), "sub");
+		assertEquals(path, gen.getPath());
+		assertEquals(subId, gen.getObjectId());
+		assertEquals(new File(db.getWorkTree(), path), gen.getDirectory());
+		assertNull(gen.getConfigUpdate());
+		assertNull(gen.getConfigUrl());
+		assertEquals("sub", gen.getModulesPath());
+		assertNull(gen.getModulesUpdate());
+		assertEquals("git://example.com/sub", gen.getModulesUrl());
+		assertNull(gen.getRepository());
+		assertFalse(gen.next());
+	}
+
+	@Test
+	public void testTreeIteratorWithGitmodules() throws Exception {
+		final ObjectId subId = ObjectId
+				.fromString("abcd1234abcd1234abcd1234abcd1234abcd1234");
+		final String path = "sub";
+
+		final Config gitmodules = new Config();
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_PATH,
+				"sub");
+		gitmodules.setString(CONFIG_SUBMODULE_SECTION, path, CONFIG_KEY_URL,
+				"git://example.com/sub");
+
+		RevCommit commit = testDb.getRevWalk().parseCommit(testDb.commit()
+				.noParents()
+				.add(DOT_GIT_MODULES, gitmodules.toText())
+				.edit(new PathEdit(path) {
+
+							public void apply(DirCacheEntry ent) {
+								ent.setFileMode(FileMode.GITLINK);
+								ent.setObjectId(subId);
+							}
+						})
+				.create());
+
+		final CanonicalTreeParser p = new CanonicalTreeParser();
+		p.reset(testDb.getRevWalk().getObjectReader(), commit.getTree());
+		SubmoduleWalk gen = SubmoduleWalk.forPath(db, p, "sub");
+		assertEquals(path, gen.getPath());
+		assertEquals(subId, gen.getObjectId());
+		assertEquals(new File(db.getWorkTree(), path), gen.getDirectory());
+		assertNull(gen.getConfigUpdate());
+		assertNull(gen.getConfigUrl());
+		assertEquals("sub", gen.getModulesPath());
+		assertNull(gen.getModulesUpdate());
+		assertEquals("git://example.com/sub", gen.getModulesUrl());
+		assertNull(gen.getRepository());
+		assertFalse(gen.next());
+	}
 }

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -205,6 +205,7 @@ flagIsDisposed={0} is disposed.
 flagNotFromThis={0} not from this.
 flagsAlreadyCreated={0} flags already created.
 funnyRefname=funny refname
+gitmodulesNotFound=.gitmodules not found in tree.
 headRequiredToStash=HEAD required to stash local changes
 hoursAgo={0} hours ago
 hugeIndexesAreNotSupportedByJgitYet=Huge indexes are not supported by jgit, yet

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -265,6 +265,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String flagNotFromThis;
 	/***/ public String flagsAlreadyCreated;
 	/***/ public String funnyRefname;
+	/***/ public String gitmodulesNotFound;
 	/***/ public String headRequiredToStash;
 	/***/ public String hoursAgo;
 	/***/ public String hugeIndexesAreNotSupportedByJgitYet;

org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java

@@ -46,6 +46,7 @@ import java.io.File;
 import java.io.IOException;
 import java.text.MessageFormat;
 
+import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.CorruptObjectException;
@@ -54,6 +55,8 @@ import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
@@ -64,6 +67,7 @@ import org.eclipse.jgit.lib.RepositoryBuilder;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.treewalk.AbstractTreeIterator;
+import org.eclipse.jgit.treewalk.CanonicalTreeParser;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
 import org.eclipse.jgit.treewalk.filter.TreeFilter;
@@ -78,6 +82,8 @@ public class SubmoduleWalk {
 	 * Create a generator to walk over the submodule entries currently in the
 	 * index
 	 *
+	 * The {@code .gitmodules} file is read from the index.
+	 *
 	 * @param repository
 	 * @return generator over submodule index entries
 	 * @throws IOException
@@ -85,7 +91,15 @@ public class SubmoduleWalk {
 	public static SubmoduleWalk forIndex(Repository repository)
 			throws IOException {
 		SubmoduleWalk generator = new SubmoduleWalk(repository);
-		generator.setTree(new DirCacheIterator(repository.readDirCache()));
+		try {
+			DirCache index = repository.readDirCache();
+			generator.setTree(new DirCacheIterator(index));
+			generator.setRootTree(new DirCacheIterator(index));
+			generator.useWorkingTree = true;
+		} catch (IOException e) {
+			generator.release();
+			throw e;
+		}
 		return generator;
 	}
 
@@ -95,6 +109,8 @@ public class SubmoduleWalk {
 	 *
 	 * @param repository
 	 * @param treeId
+	 *            the root of a tree containing both a submodule at the given path
+	 *            and .gitmodules at the root.
 	 * @param path
 	 * @return generator at given path, null if no submodule at given path
 	 * @throws IOException
@@ -106,6 +122,7 @@ public class SubmoduleWalk {
 			generator.setTree(treeId);
 			PathFilter filter = PathFilter.create(path);
 			generator.setFilter(filter);
+			generator.setRootTree(treeId);
 			while (generator.next())
 				if (filter.isDone(generator.walk))
 					return generator;
@@ -123,6 +140,8 @@ public class SubmoduleWalk {
 	 *
 	 * @param repository
 	 * @param iterator
+	 *            the root of a tree containing both a submodule at the given path
+	 *            and .gitmodules at the root.
 	 * @param path
 	 * @return generator at given path, null if no submodule at given path
 	 * @throws IOException
@@ -134,6 +153,7 @@ public class SubmoduleWalk {
 			generator.setTree(iterator);
 			PathFilter filter = PathFilter.create(path);
 			generator.setFilter(filter);
+			generator.setRootTree(iterator);
 			while (generator.next())
 				if (filter.isDone(generator.walk))
 					return generator;
@@ -278,10 +298,14 @@ public class SubmoduleWalk {
 
 	private StoredConfig repoConfig;
 
-	private FileBasedConfig modulesConfig;
+	private AbstractTreeIterator rootTree;
+
+	private Config modulesConfig;
 
 	private String path;
 
+	private boolean useWorkingTree;
+
 	/**
 	 * Create submodule generator
 	 *
@@ -295,8 +319,113 @@ public class SubmoduleWalk {
 		walk.setRecursive(true);
 	}
 
-	private void loadModulesConfig() throws IOException, ConfigInvalidException {
+	/**
-		if (modulesConfig == null) {
+	 * Set the config used by this walk.
+	 *
+	 * This method need only be called if constructing a walk manually instead of
+	 * with one of the static factory methods above.
+	 *
+	 * @param config
+	 *            .gitmodules config object
+	 * @return this generator
+	 */
+	public SubmoduleWalk setModulesConfig(final Config config) {
+		modulesConfig = config;
+		return this;
+	}
+
+	/**
+	 * Set the tree used by this walk for finding {@code .gitmodules}.
+	 * <p>
+	 * The root tree is not read until the first submodule is encountered by the
+	 * walk.
+	 * <p>
+	 * This method need only be called if constructing a walk manually instead of
+	 * with one of the static factory methods above.
+	 *
+	 * @param tree
+	 *            tree containing .gitmodules
+	 * @return this generator
+	 */
+	public SubmoduleWalk setRootTree(final AbstractTreeIterator tree) {
+		rootTree = tree;
+		modulesConfig = null;
+		return this;
+	}
+
+	/**
+	 * Set the tree used by this walk for finding {@code .gitmodules}.
+	 * <p>
+	 * The root tree is not read until the first submodule is encountered by the
+	 * walk.
+	 * <p>
+	 * This method need only be called if constructing a walk manually instead of
+	 * with one of the static factory methods above.
+	 *
+	 * @param id
+	 *            ID of a tree containing .gitmodules
+	 * @return this generator
+	 * @throws IOException
+	 */
+	public SubmoduleWalk setRootTree(final AnyObjectId id) throws IOException {
+		final CanonicalTreeParser p = new CanonicalTreeParser();
+		p.reset(walk.getObjectReader(), id);
+		rootTree = p;
+		modulesConfig = null;
+		return this;
+	}
+
+	/**
+	 * Load the config for this walk from {@code .gitmodules}.
+	 * <p>
+	 * Uses the root tree if {@link #setRootTree(AbstractTreeIterator)} was
+	 * previously called, otherwise uses the working tree.
+	 * <p>
+	 * If no submodule config is found, loads an empty config.
+	 *
+	 * @return this generator
+	 * @throws IOException if an error occurred, or if the repository is bare
+	 * @throws ConfigInvalidException
+	 */
+	public SubmoduleWalk loadModulesConfig() throws IOException, ConfigInvalidException {
+		if (rootTree != null) {
+			TreeWalk configWalk = new TreeWalk(repository);
+			try {
+				configWalk.addTree(rootTree);
+
+				// The root tree may be part of the submodule walk, so we need to revert
+				// it after this walk.
+				int idx;
+				for (idx = 0; !rootTree.first(); idx++) {
+					rootTree.back(1);
+				}
+
+				try {
+					configWalk.setRecursive(false);
+					PathFilter filter = PathFilter.create(Constants.DOT_GIT_MODULES);
+					configWalk.setFilter(filter);
+					while (configWalk.next()) {
+						if (filter.isDone(configWalk)) {
+							modulesConfig = new BlobBasedConfig(null, repository,
+									configWalk.getObjectId(0));
+							return this;
+						}
+					}
+					if (!useWorkingTree) {
+						modulesConfig = new Config();
+						return this;
+					}
+				} finally {
+					if (idx > 0)
+						rootTree.next(idx);
+				}
+			} finally {
+				configWalk.release();
+			}
+		}
+		if (repository.isBare()) {
+			modulesConfig = new Config();
+		} else {
 			File modulesFile = new File(repository.getWorkTree(),
 					Constants.DOT_GIT_MODULES);
 			FileBasedConfig config = new FileBasedConfig(modulesFile,
@@ -304,6 +433,12 @@ public class SubmoduleWalk {
 			config.load();
 			modulesConfig = config;
 		}
+		return this;
+	}
+
+	private void lazyLoadModulesConfig() throws IOException, ConfigInvalidException {
+		if (modulesConfig == null)
+			loadModulesConfig();
 	}
 
 	/**
@@ -412,7 +547,7 @@ public class SubmoduleWalk {
 	 * @throws IOException
 	 */
 	public String getModulesPath() throws IOException, ConfigInvalidException {
-		loadModulesConfig();
+		lazyLoadModulesConfig();
 		return modulesConfig.getString(
 				ConfigConstants.CONFIG_SUBMODULE_SECTION, path,
 				ConfigConstants.CONFIG_KEY_PATH);
@@ -440,7 +575,7 @@ public class SubmoduleWalk {
 	 * @throws IOException
 	 */
 	public String getModulesUrl() throws IOException, ConfigInvalidException {
-		loadModulesConfig();
+		lazyLoadModulesConfig();
 		return modulesConfig.getString(
 				ConfigConstants.CONFIG_SUBMODULE_SECTION, path,
 				ConfigConstants.CONFIG_KEY_URL);
@@ -468,7 +603,7 @@ public class SubmoduleWalk {
 	 * @throws IOException
 	 */
 	public String getModulesUpdate() throws IOException, ConfigInvalidException {
-		loadModulesConfig();
+		lazyLoadModulesConfig();
 		return modulesConfig.getString(
 				ConfigConstants.CONFIG_SUBMODULE_SECTION, path,
 				ConfigConstants.CONFIG_KEY_UPDATE);