From fe1f1b8f8aba60fdd1ad6f0f72e9c9180978cc60 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 14 Jun 2012 15:07:13 -0700 Subject: [PATCH] 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 --- .../jgit/submodule/SubmoduleWalkTest.java | 144 +++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../eclipse/jgit/submodule/SubmoduleWalk.java | 149 +++++++++++++++++- 4 files changed, 288 insertions(+), 7 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleWalkTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleWalkTest.java index 89843fc24..fdb67d266 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleWalkTest.java +++ b/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.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 testDb; + + @Before + public void setUp() throws Exception { + super.setUp(); + testDb = new TestRepository(db); + } @Test public void repositoryWithNoSubmodules() throws IOException { @@ -269,4 +287,130 @@ public void apply(DirCacheEntry ent) { 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()); + } } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 1131c1560..118c9c887 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/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 diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 539f83756..008e1540d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -265,6 +265,7 @@ public static JGitText get() { /***/ public String flagNotFromThis; /***/ public String flagsAlreadyCreated; /***/ public String funnyRefname; + /***/ public String gitmodulesNotFound; /***/ public String headRequiredToStash; /***/ public String hoursAgo; /***/ public String hugeIndexesAreNotSupportedByJgitYet; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java index f037a7c52..040ea2687 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/submodule/SubmoduleWalk.java @@ -46,6 +46,7 @@ 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.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.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 static SubmoduleWalk forIndex(Repository repository) * * @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 static SubmoduleWalk forPath(Repository repository, 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 static SubmoduleWalk forPath(Repository repository, * * @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 static SubmoduleWalk forPath(Repository repository, 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 @@ else if (submoduleUrl.startsWith("../")) { 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 SubmoduleWalk(final Repository repository) throws IOException { 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}. + *

+ * The root tree is not read until the first submodule is encountered by the + * walk. + *

+ * 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}. + *

+ * The root tree is not read until the first submodule is encountered by the + * walk. + *

+ * 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}. + *

+ * Uses the root tree if {@link #setRootTree(AbstractTreeIterator)} was + * previously called, otherwise uses the working tree. + *

+ * 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 @@ private void loadModulesConfig() throws IOException, ConfigInvalidException { config.load(); modulesConfig = config; } + return this; + } + + private void lazyLoadModulesConfig() throws IOException, ConfigInvalidException { + if (modulesConfig == null) + loadModulesConfig(); } /** @@ -412,7 +547,7 @@ public ObjectId getObjectId() { * @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 String getConfigUrl() throws IOException, ConfigInvalidException { * @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 String getConfigUpdate() throws IOException, ConfigInvalidException { * @throws IOException */ public String getModulesUpdate() throws IOException, ConfigInvalidException { - loadModulesConfig(); + lazyLoadModulesConfig(); return modulesConfig.getString( ConfigConstants.CONFIG_SUBMODULE_SECTION, path, ConfigConstants.CONFIG_KEY_UPDATE);