Merge "Use project names instead of paths for the submodule name"

2 files changed, 123 insertions, 62 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
index df31ab0869..1eca587bfb 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
@@ -268,7 +268,8 @@ public class RepoCommandTest extends RepositoryTestCase {
 						.getCachedBytes(Integer.MAX_VALUE);
 				Config base = new Config();
 				BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
-				String subUrl = cfg.getString("submodule", "base", "url");
+				String subUrl = cfg.getString("submodule", "platform/base",
+						"url");
 				assertEquals(subUrl, "../base");
 			}
 		}
@@ -301,7 +302,8 @@ public class RepoCommandTest extends RepositoryTestCase {
 						.getCachedBytes(Integer.MAX_VALUE);
 				Config base = new Config();
 				BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
-				String subUrl = cfg.getString("submodule", "base", "url");
+				String subUrl = cfg.getString("submodule", "platform/base",
+						"url");
 				assertEquals(subUrl, "https://host.com/platform/base");
 			}
 		}
@@ -387,8 +389,8 @@ public class RepoCommandTest extends RepositoryTestCase {
 								.getCachedBytes(Integer.MAX_VALUE);
 						Config base = new Config();
 						BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
-						String subUrl = cfg.getString("submodule", "src",
-								"url");
+						String subUrl = cfg.getString("submodule",
+								"chromium/src", "url");
 						assertEquals(
 								"https://chromium.googlesource.com/chromium/src",
 								subUrl);
@@ -443,8 +445,8 @@ public class RepoCommandTest extends RepositoryTestCase {
 								.getCachedBytes(Integer.MAX_VALUE);
 						Config base = new Config();
 						BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
-						String subUrl = cfg.getString("submodule", "src",
-								"url");
+						String subUrl = cfg.getString("submodule",
+								"chromium/src", "url");
 						assertEquals("../chromium/src", subUrl);
 					}
 					fetchSlash = !fetchSlash;
@@ -613,7 +615,7 @@ public class RepoCommandTest extends RepositoryTestCase {
 				String content = reader.readLine();
 				assertEquals(
 						"The first line of .gitmodules file should be as expected",
-						"[submodule \"foo\"]", content);
+						"[submodule \"" + defaultUri + "\"]", content);
 			}
 			// The gitlink should be the same as remote head sha1
 			String gitlink = localDb.resolve(Constants.HEAD + ":foo").name();
@@ -801,9 +803,9 @@ public class RepoCommandTest extends RepositoryTestCase {
 				.append("<manifest>")
 				.append("<remote name=\"remote1\" fetch=\".\" />")
 				.append("<default revision=\"master\" remote=\"remote1\" />")
-				.append("<project path=\"bar\" name=\"").append(defaultUri)
-				.append("\" revision=\"").append(BRANCH).append("\" >")
-				.append("<copyfile src=\"hello.txt\" dest=\"Hello.txt\" />")
+				.append("<project path=\"bar\" name=\"").append(notDefaultUri)
+				.append("\" >")
+				.append("<copyfile src=\"world.txt\" dest=\"World.txt\" />")
 				.append("</project>").append("</manifest>");
 		JGitTestUtil.writeTrashFile(tempDb, "new.xml", xmlContent.toString());
 		command = new RepoCommand(remoteDb);
@@ -819,8 +821,8 @@ public class RepoCommandTest extends RepositoryTestCase {
 			File hello = new File(localDb.getWorkTree(), "Hello");
 			assertFalse("The Hello file shouldn't exist", hello.exists());
 			// The Hello.txt file should exist
-			File hellotxt = new File(localDb.getWorkTree(), "Hello.txt");
-			assertTrue("The Hello.txt file should exist", hellotxt.exists());
+			File hellotxt = new File(localDb.getWorkTree(), "World.txt");
+			assertTrue("The World.txt file should exist", hellotxt.exists());
 			dotmodules = new File(localDb.getWorkTree(),
 					Constants.DOT_GIT_MODULES);
 		}
@@ -835,9 +837,9 @@ public class RepoCommandTest extends RepositoryTestCase {
 				String line = reader.readLine();
 				if (line == null)
 					break;
-				if (line.contains("submodule \"foo\""))
+				if (line.contains("submodule \"" + defaultUri + "\""))
 					foo = true;
-				if (line.contains("submodule \"bar\""))
+				if (line.contains("submodule \"" + notDefaultUri + "\""))
 					bar = true;
 			}
 			assertTrue("The bar submodule should exist", bar);
@@ -876,9 +878,7 @@ public class RepoCommandTest extends RepositoryTestCase {
 				Constants.DOT_GIT_MODULES);
 		}
 
-		// The .gitmodules file should have 'submodule "foo"' and shouldn't
-		// have
-		// 'submodule "foo/bar"' lines.
+		// Check .gitmodules file
 		try (BufferedReader reader = new BufferedReader(
 				new FileReader(dotmodules))) {
 			boolean foo = false;
@@ -888,16 +888,17 @@ public class RepoCommandTest extends RepositoryTestCase {
 				String line = reader.readLine();
 				if (line == null)
 					break;
-				if (line.contains("submodule \"foo\""))
+				if (line.contains("submodule \"" + defaultUri + "\""))
 					foo = true;
-				if (line.contains("submodule \"foo/bar\""))
+				if (line.contains("submodule \"" + groupBUri + "\""))
 					foobar = true;
-				if (line.contains("submodule \"a\""))
+				if (line.contains("submodule \"" + groupAUri + "\""))
 					a = true;
 			}
-			assertTrue("The foo submodule should exist", foo);
-			assertFalse("The foo/bar submodule shouldn't exist", foobar);
-			assertTrue("The a submodule should exist", a);
+			assertTrue("The " + defaultUri + " submodule should exist", foo);
+			assertFalse("The " + groupBUri + " submodule shouldn't exist",
+					foobar);
+			assertTrue("The " + groupAUri + " submodule should exist", a);
 		}
 	}
 
@@ -1033,11 +1034,11 @@ public class RepoCommandTest extends RepositoryTestCase {
 			assertEquals(
 					"Recording remote branches should work for short branch descriptions",
 					"master",
-					c.getString("submodule", "with-branch", "branch"));
+					c.getString("submodule", notDefaultUri, "branch"));
 			assertEquals(
 					"Recording remote branches should work for full ref specs",
 					"refs/heads/master",
-					c.getString("submodule", "with-long-branch", "branch"));
+					c.getString("submodule", defaultUri, "branch"));
 		}
 	}
 
@@ -1095,7 +1096,7 @@ public class RepoCommandTest extends RepositoryTestCase {
 				.append("<project path=\"shallow-please\" ").append("name=\"")
 				.append(defaultUri).append("\" ").append("clone-depth=\"1\" />")
 				.append("<project path=\"non-shallow\" ").append("name=\"")
-				.append(defaultUri).append("\" />").append("</manifest>");
+				.append(notDefaultUri).append("\" />").append("</manifest>");
 		JGitTestUtil.writeTrashFile(tempDb, "manifest.xml",
 				xmlContent.toString());
 
@@ -1115,9 +1116,9 @@ public class RepoCommandTest extends RepositoryTestCase {
 			FileBasedConfig c = new FileBasedConfig(gitmodules, FS.DETECTED);
 			c.load();
 			assertEquals("Recording shallow configuration should work", "true",
-					c.getString("submodule", "shallow-please", "shallow"));
+					c.getString("submodule", defaultUri, "shallow"));
 			assertNull("Recording non shallow configuration should work",
-					c.getString("submodule", "non-shallow", "shallow"));
+					c.getString("submodule", notDefaultUri, "shallow"));
 		}
 	}
 
@@ -1176,6 +1177,43 @@ public class RepoCommandTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void testTwoPathUseTheSameName() 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=\"path1\" ").append("name=\"")
+				.append(defaultUri).append("\" />")
+				.append("<project path=\"path2\" ").append("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).setRecommendShallow(true).call();
+		File directory = createTempDirectory("testBareRepo");
+		try (Repository localDb = Git.cloneRepository().setDirectory(directory)
+				.setURI(remoteDb.getDirectory().toURI().toString()).call()
+				.getRepository();) {
+			File gitmodules = new File(localDb.getWorkTree(), ".gitmodules");
+			assertTrue("The .gitmodules file should exist",
+					gitmodules.exists());
+			FileBasedConfig c = new FileBasedConfig(gitmodules, FS.DETECTED);
+			c.load();
+			assertEquals("A module should exist for path1", "path1",
+					c.getString("submodule", defaultUri + "/path1", "path"));
+			assertEquals("A module should exist for path2", "path2",
+					c.getString("submodule", defaultUri + "/path2", "path"));
+		}
+	}
+
 	private void resolveRelativeUris() {
 		// Find the longest common prefix ends with "/" as rootUri.
 		defaultUri = defaultDb.getDirectory().toURI().toString();
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
index b3eee07fb6..2046ba7b43 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
@@ -52,10 +52,10 @@ import java.io.InputStream;
 import java.net.URI;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.StringJoiner;
 
 import org.eclipse.jgit.annotations.Nullable;
@@ -124,7 +124,6 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	private IncludedFileReader includedReader;
 	private boolean ignoreRemoteFailures = false;
 
-	private List<RepoProject> bareProjects;
 	private ProgressMonitor monitor;
 
 	/**
@@ -519,37 +518,33 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		}
 
 		if (repo.isBare()) {
-			bareProjects = new ArrayList<>();
 			if (author == null)
 				author = new PersonIdent(repo);
 			if (callback == null)
 				callback = new DefaultRemoteReader();
-			for (RepoProject proj : filteredProjects) {
-				addSubmoduleBare(proj.getUrl(), proj.getPath(),
-						proj.getRevision(), proj.getCopyFiles(),
-						proj.getLinkFiles(), proj.getGroups(),
-						proj.getRecommendShallow());
-			}
+			List<RepoProject> renamedProjects = renameProjects(filteredProjects);
+
 			DirCache index = DirCache.newInCore();
 			DirCacheBuilder builder = index.builder();
 			ObjectInserter inserter = repo.newObjectInserter();
 			try (RevWalk rw = new RevWalk(repo)) {
 				Config cfg = new Config();
 				StringBuilder attributes = new StringBuilder();
-				for (RepoProject proj : bareProjects) {
+				for (RepoProject proj : renamedProjects) {
+					String name = proj.getName();
 					String path = proj.getPath();
-					String nameUri = proj.getName();
+					String url = proj.getUrl();
 					ObjectId objectId;
 					if (ObjectId.isId(proj.getRevision())) {
 						objectId = ObjectId.fromString(proj.getRevision());
 					} else {
-						objectId = callback.sha1(nameUri, proj.getRevision());
+						objectId = callback.sha1(url, proj.getRevision());
 						if (objectId == null && !ignoreRemoteFailures) {
-							throw new RemoteUnavailableException(nameUri);
+							throw new RemoteUnavailableException(url);
 						}
 						if (recordRemoteBranch) {
 							// can be branch or tag
-							cfg.setString("submodule", path, "branch", //$NON-NLS-1$ //$NON-NLS-2$
+							cfg.setString("submodule", name, "branch", //$NON-NLS-1$ //$NON-NLS-2$
 									proj.getRevision());
 						}
 
@@ -559,7 +554,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 							// depth in the 'clone-depth' field, while
 							// git core only uses a binary 'shallow = true/false'
 							// hint, we'll map any depth to 'shallow = true'
-							cfg.setBoolean("submodule", path, "shallow", //$NON-NLS-1$ //$NON-NLS-2$
+							cfg.setBoolean("submodule", name, "shallow", //$NON-NLS-1$ //$NON-NLS-2$
 									true);
 						}
 					}
@@ -575,12 +570,13 @@ public class RepoCommand extends GitCommand<RevCommit> {
 						attributes.append(rec.toString());
 					}
 
-					URI submodUrl = URI.create(nameUri);
+					URI submodUrl = URI.create(url);
 					if (targetUri != null) {
 						submodUrl = relativize(targetUri, submodUrl);
 					}
-					cfg.setString("submodule", path, "path", path); //$NON-NLS-1$ //$NON-NLS-2$
-					cfg.setString("submodule", path, "url", submodUrl.toString()); //$NON-NLS-1$ //$NON-NLS-2$
+					cfg.setString("submodule", name, "path", path); //$NON-NLS-1$ //$NON-NLS-2$
+					cfg.setString("submodule", name, "url", //$NON-NLS-1$ //$NON-NLS-2$
+							submodUrl.toString());
 
 					// create gitlink
 					if (objectId != null) {
@@ -591,7 +587,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 
 						for (CopyFile copyfile : proj.getCopyFiles()) {
 							byte[] src = callback.readFile(
-								nameUri, proj.getRevision(), copyfile.src);
+								url, proj.getRevision(), copyfile.src);
 							objectId = inserter.insert(Constants.OBJ_BLOB, src);
 							dcEntry = new DirCacheEntry(copyfile.dest);
 							dcEntry.setObjectId(objectId);
@@ -691,7 +687,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		} else {
 			try (Git git = new Git(repo)) {
 				for (RepoProject proj : filteredProjects) {
-					addSubmodule(proj.getUrl(), proj.getPath(),
+					addSubmodule(proj.getName(), proj.getUrl(), proj.getPath(),
 							proj.getRevision(), proj.getCopyFiles(),
 							proj.getLinkFiles(), git);
 				}
@@ -703,9 +699,9 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		}
 	}
 
-	private void addSubmodule(String url, String path, String revision,
-			List<CopyFile> copyfiles, List<LinkFile> linkfiles, Git git)
-			throws GitAPIException, IOException {
+	private void addSubmodule(String name, String url, String path,
+			String revision, List<CopyFile> copyfiles, List<LinkFile> linkfiles,
+			Git git) throws GitAPIException, IOException {
 		assert (!repo.isBare());
 		assert (git != null);
 		if (!linkfiles.isEmpty()) {
@@ -713,7 +709,8 @@ public class RepoCommand extends GitCommand<RevCommit> {
 					JGitText.get().nonBareLinkFilesNotSupported);
 		}
 
-		SubmoduleAddCommand add = git.submoduleAdd().setPath(path).setURI(url);
+		SubmoduleAddCommand add = git.submoduleAdd().setName(name).setPath(path)
+				.setURI(url);
 		if (monitor != null)
 			add.setProgressMonitor(monitor);
 
@@ -731,16 +728,42 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		}
 	}
 
-	private void addSubmoduleBare(String url, String path, String revision,
-			List<CopyFile> copyfiles, List<LinkFile> linkfiles,
-			Set<String> groups, String recommendShallow) {
-		assert (repo.isBare());
-		assert (bareProjects != null);
-		RepoProject proj = new RepoProject(url, path, revision, null, groups,
-				recommendShallow);
-		proj.addCopyFiles(copyfiles);
-		proj.addLinkFiles(linkfiles);
-		bareProjects.add(proj);
+	/**
+	 * Rename the projects if there's a conflict when converted to submodules.
+	 *
+	 * @param projects
+	 *            parsed projects
+	 * @return projects that are renamed if necessary
+	 */
+	private List<RepoProject> renameProjects(List<RepoProject> projects) {
+		Map<String, List<RepoProject>> m = new HashMap<>();
+		for (RepoProject proj : projects) {
+			List<RepoProject> l = m.get(proj.getName());
+			if (l == null) {
+				l = new ArrayList<>();
+				m.put(proj.getName(), l);
+			}
+			l.add(proj);
+		}
+
+		List<RepoProject> ret = new ArrayList<>();
+		for (List<RepoProject> ps : m.values()) {
+			boolean nameConflict = ps.size() != 1;
+			for (RepoProject proj : ps) {
+				String name = proj.getName();
+				if (nameConflict) {
+					name += SLASH + proj.getPath();
+				}
+				RepoProject p = new RepoProject(name,
+						proj.getPath(), proj.getRevision(), null,
+						proj.getGroups(), proj.getRecommendShallow());
+				p.setUrl(proj.getUrl());
+				p.addCopyFiles(proj.getCopyFiles());
+				p.addLinkFiles(proj.getLinkFiles());
+				ret.add(p);
+			}
+		}
+		return ret;
 	}
 
 	/*