Merge "Remove overlapping submodules from repo manifest."

org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java

@@ -512,6 +512,65 @@ public class RepoCommandTest extends RepositoryTestCase {
 		assertFalse("The foo submodule shouldn't exist", foo);
 	}
 
+	@Test
+	public void testRemoveOverlappingBare() throws Exception {
+		Repository remoteDb = createBareRepository();
+		Repository tempDb = createWorkRepository();
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\".\" />")
+			.append("<default revision=\"master\" remote=\"remote1\" />")
+			.append("<project path=\"foo/bar\" name=\"")
+			.append(groupBUri)
+			.append("\" />")
+			.append("<project path=\"a\" name=\"")
+			.append(groupAUri)
+			.append("\" />")
+			.append("<project path=\"foo\" name=\"")
+			.append(defaultUri)
+			.append("\" />")
+			.append("</manifest>");
+		JGitTestUtil.writeTrashFile(
+				tempDb, "manifest.xml", xmlContent.toString());
+		RepoCommand command = new RepoCommand(remoteDb);
+		command
+			.setPath(tempDb.getWorkTree().getAbsolutePath() + "/manifest.xml")
+			.setURI(rootUri)
+			.call();
+		// Clone it
+		File directory = createTempDirectory("testRemoveOverlappingBare");
+		Repository localDb = Git
+			.cloneRepository()
+			.setDirectory(directory)
+			.setURI(remoteDb.getDirectory().toURI().toString())
+			.call()
+			.getRepository();
+		// The .gitmodules file should have 'submodule "foo"' and shouldn't have
+		// 'submodule "foo/bar"' lines.
+		File dotmodules = new File(localDb.getWorkTree(),
+				Constants.DOT_GIT_MODULES);
+		BufferedReader reader = new BufferedReader(new FileReader(dotmodules));
+		boolean foo = false;
+		boolean foobar = false;
+		boolean a = false;
+		while (true) {
+			String line = reader.readLine();
+			if (line == null)
+				break;
+			if (line.contains("submodule \"foo\""))
+				foo = true;
+			if (line.contains("submodule \"foo/bar\""))
+				foobar = true;
+			if (line.contains("submodule \"a\""))
+				a = true;
+		}
+		reader.close();
+		assertTrue("The foo submodule should exist", foo);
+		assertFalse("The foo/bar submodule shouldn't exist", foobar);
+		assertTrue("The a submodule should exist", a);
+	}
+
 	private void resolveRelativeUris() {
 		// Find the longest common prefix ends with "/" as rootUri.
 		defaultUri = defaultDb.getDirectory().toURI().toString();

org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java

@@ -53,8 +53,10 @@ import java.nio.channels.FileChannel;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -104,6 +106,11 @@ import org.xml.sax.helpers.XMLReaderFactory;
  * If called against a bare repository, it will replace all the existing content
  * of the repository with the contents populated from the manifest.
  *
+ * repo manifest allows projects overlapping, e.g. one project's path is
+ * "foo" and another project's path is "foo/bar". This won't
+ * work in git submodule, so we'll skip all the sub projects
+ * ("foo/bar" in the example) while converting.
+ *
  * @see <a href="https://code.google.com/p/git-repo/">git-repo project page</a>
  * @since 3.4
  */
@@ -249,7 +256,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		}
 	}
 
-	private static class Project {
+	private static class Project implements Comparable<Project> {
 		final String name;
 		final String path;
 		final String revision;
@@ -269,6 +276,31 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		void addCopyFile(CopyFile copyfile) {
 			copyfiles.add(copyfile);
 		}
+
+		String getPathWithSlash() {
+			if (path.endsWith("/")) //$NON-NLS-1$
+				return path;
+			else
+				return path + "/"; //$NON-NLS-1$
+		}
+
+		boolean isAncestorOf(Project that) {
+			return that.getPathWithSlash().startsWith(this.getPathWithSlash());
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			if (o instanceof Project) {
+				Project that = (Project) o;
+				return this.getPathWithSlash().equals(that.getPathWithSlash());
+			}
+			return false;
+		}
+
+		@Override
+		public int compareTo(Project that) {
+			return this.getPathWithSlash().compareTo(that.getPathWithSlash());
+		}
 	}
 
 	private static class XmlManifest extends DefaultHandler {
@@ -277,9 +309,9 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		private final String filename;
 		private final String baseUrl;
 		private final Map<String, String> remotes;
-		private final List<Project> projects;
 		private final Set<String> plusGroups;
 		private final Set<String> minusGroups;
+		private List<Project> projects;
 		private String defaultRemote;
 		private String defaultRevision;
 		private Project currentProject;
@@ -389,14 +421,38 @@ public class RepoCommand extends GitCommand<RevCommit> {
 			} catch (URISyntaxException e) {
 				throw new SAXException(e);
 			}
+			removeNotInGroup();
+			removeOverlaps();
 			for (Project proj : projects) {
-				if (inGroups(proj)) {
-					command.addSubmodule(remoteUrl + proj.name,
-							proj.path,
-							proj.revision == null
-									? defaultRevision : proj.revision,
-							proj.copyfiles);
-				}
+				command.addSubmodule(remoteUrl + proj.name,
+						proj.path,
+						proj.revision == null
+								? defaultRevision : proj.revision,
+						proj.copyfiles);
+			}
+		}
+
+		/** Remove projects that are not in our desired groups. */
+		void removeNotInGroup() {
+			Iterator<Project> iter = projects.iterator();
+			while (iter.hasNext())
+				if (!inGroups(iter.next()))
+					iter.remove();
+		}
+
+		/** Remove projects that sits in a subdirectory of any other project. */
+		void removeOverlaps() {
+			Collections.sort(projects);
+			Iterator<Project> iter = projects.iterator();
+			if (!iter.hasNext())
+				return;
+			Project last = iter.next();
+			while (iter.hasNext()) {
+				Project p = iter.next();
+				if (last.isAncestorOf(p))
+					iter.remove();
+				else
+					last = p;
 			}
 		}