RepoCommand: persist unreadable submodules in .gitmodules

In cases where a manifest file mixes different remotes, a Gerrit
server process may not have access to all remotes, and won't be able
to produce a full submodule tree.

Preserving this information in .gitmodules will let downstream clients
reconstruct the full tree.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I52f5d3f288e771dca0af2b4dd3f3fa0f940dcf15

2 files changed, 73 insertions, 63 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 9afb58ecfb..2df34df30f 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
@@ -213,8 +213,7 @@ public class RepoCommandTest extends RepositoryTestCase {
 		repos.put("platform/base", child);
 
 		RevCommit commit = cmd
-				.setInputStream(new ByteArrayInputStream(
-						xmlContent.toString().getBytes(UTF_8)))
+			.setInputStream(new ByteArrayInputStream(xmlContent.toString().getBytes(UTF_8)))
 			.setRemoteReader(repos)
 			.setURI("platform/")
 			.setTargetURI("platform/superproject")
@@ -236,6 +235,49 @@ public class RepoCommandTest extends RepositoryTestCase {
 		child.close();
 		dest.close();
 	}
+	@Test
+	public void recordUnreachableRemotes() throws Exception {
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\"https://host.com/\" />")
+			.append("<default revision=\"master\" remote=\"remote1\" />")
+			.append("<project path=\"base\" name=\"platform/base\" />")
+			.append("</manifest>");
+
+		Repository dest = Git.cloneRepository()
+				.setURI(db.getDirectory().toURI().toString())
+				.setDirectory(createUniqueTestGitDir(true)).setBare(true).call()
+				.getRepository();
+
+		assertTrue(dest.isBare());
+
+		RevCommit commit = new RepoCommand(dest)
+			.setInputStream(new ByteArrayInputStream(
+				xmlContent.toString().getBytes(UTF_8)))
+			.setRemoteReader(new IndexedRepos())
+			.setURI("platform/")
+			.setTargetURI("platform/superproject")
+			.setRecordRemoteBranch(true)
+			.setIgnoreRemoteFailures(true)
+			.setRecordSubmoduleLabels(true)
+			.call();
+
+		String idStr = commit.getId().name() + ":" + ".gitmodules";
+		ObjectId modId = dest.resolve(idStr);
+
+		try (ObjectReader reader = dest.newObjectReader()) {
+			byte[] bytes = reader.open(modId).getCachedBytes(Integer.MAX_VALUE);
+			Config base = new Config();
+			BlobBasedConfig cfg = new BlobBasedConfig(base, bytes);
+			String subUrl = cfg.getString("submodule", "base", "url");
+			assertEquals(subUrl, "https://host.com/platform/base");
+		}
+
+		dest.close();
+	}
+
+
 
 	@Test
 	public void gerritSetup() throws Exception {
@@ -814,37 +856,6 @@ public class RepoCommandTest extends RepositoryTestCase {
 		assertEquals("submodule content should be as expected",
 				"master world", content);
 	}
-
-	@Test
-	public void testNonDefaultRemotes() throws Exception {
-		StringBuilder xmlContent = new StringBuilder();
-		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
-			.append("<manifest>")
-			.append("<remote name=\"remote1\" fetch=\".\" />")
-			.append("<remote name=\"remote2\" fetch=\"")
-			.append(notDefaultUri)
-			.append("\" />")
-			.append("<default revision=\"master\" remote=\"remote1\" />")
-			.append("<project path=\"foo\" name=\"")
-			.append(defaultUri)
-			.append("\" />")
-			.append("<project path=\"bar\" name=\".\" remote=\"remote2\" />")
-			.append("</manifest>");
-
-		Repository localDb = createWorkRepository();
-		JGitTestUtil.writeTrashFile(
-				localDb, "manifest.xml", xmlContent.toString());
-		RepoCommand command = new RepoCommand(localDb);
-		command
-			.setPath(localDb.getWorkTree().getAbsolutePath() + "/manifest.xml")
-			.setURI(rootUri)
-			.call();
-		File file = new File(localDb.getWorkTree(), "foo/hello.txt");
-		assertTrue("We should have foo", file.exists());
-		file = new File(localDb.getWorkTree(), "bar/world.txt");
-		assertTrue("We should have bar", file.exists());
-	}
-
 	@Test
 	public void testRemoteAlias() throws Exception {
 		StringBuilder xmlContent = new StringBuilder();
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 10bd6005b5..24651b9b6b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
@@ -543,10 +543,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 						objectId = ObjectId.fromString(proj.getRevision());
 					} else {
 						objectId = callback.sha1(nameUri, proj.getRevision());
-						if (objectId == null) {
-							if (ignoreRemoteFailures) {
-								continue;
-							}
+						if (objectId == null && !ignoreRemoteFailures) {
 							throw new RemoteUnavailableException(nameUri);
 						}
 						if (recordRemoteBranch) {
@@ -585,38 +582,40 @@ public class RepoCommand extends GitCommand<RevCommit> {
 					cfg.setString("submodule", path, "url", submodUrl.toString()); //$NON-NLS-1$ //$NON-NLS-2$
 
 					// create gitlink
-					DirCacheEntry dcEntry = new DirCacheEntry(path);
-					dcEntry.setObjectId(objectId);
-					dcEntry.setFileMode(FileMode.GITLINK);
-					builder.add(dcEntry);
-
-					for (CopyFile copyfile : proj.getCopyFiles()) {
-						byte[] src = callback.readFile(
-								nameUri, proj.getRevision(), copyfile.src);
-						objectId = inserter.insert(Constants.OBJ_BLOB, src);
-						dcEntry = new DirCacheEntry(copyfile.dest);
+					if (objectId != null) {
+						DirCacheEntry dcEntry = new DirCacheEntry(path);
 						dcEntry.setObjectId(objectId);
-						dcEntry.setFileMode(FileMode.REGULAR_FILE);
+						dcEntry.setFileMode(FileMode.GITLINK);
 						builder.add(dcEntry);
-					}
-					for (LinkFile linkfile : proj.getLinkFiles()) {
-						String link;
-						if (linkfile.dest.contains("/")) { //$NON-NLS-1$
-							link = FileUtils.relativizeGitPath(
+
+						for (CopyFile copyfile : proj.getCopyFiles()) {
+							byte[] src = callback.readFile(
+								nameUri, proj.getRevision(), copyfile.src);
+							objectId = inserter.insert(Constants.OBJ_BLOB, src);
+							dcEntry = new DirCacheEntry(copyfile.dest);
+							dcEntry.setObjectId(objectId);
+							dcEntry.setFileMode(FileMode.REGULAR_FILE);
+							builder.add(dcEntry);
+						}
+						for (LinkFile linkfile : proj.getLinkFiles()) {
+							String link;
+							if (linkfile.dest.contains("/")) { //$NON-NLS-1$
+								link = FileUtils.relativizeGitPath(
 									linkfile.dest.substring(0,
-											linkfile.dest.lastIndexOf('/')),
+										linkfile.dest.lastIndexOf('/')),
 									proj.getPath() + "/" + linkfile.src); //$NON-NLS-1$
-						} else {
-							link = proj.getPath() + "/" + linkfile.src; //$NON-NLS-1$
-						}
+							} else {
+								link = proj.getPath() + "/" + linkfile.src; //$NON-NLS-1$
+							}
 
-						objectId = inserter.insert(Constants.OBJ_BLOB,
+							objectId = inserter.insert(Constants.OBJ_BLOB,
 								link.getBytes(
-										Constants.CHARACTER_ENCODING));
-						dcEntry = new DirCacheEntry(linkfile.dest);
-						dcEntry.setObjectId(objectId);
-						dcEntry.setFileMode(FileMode.SYMLINK);
-						builder.add(dcEntry);
+									Constants.CHARACTER_ENCODING));
+							dcEntry = new DirCacheEntry(linkfile.dest);
+							dcEntry.setObjectId(objectId);
+							dcEntry.setFileMode(FileMode.SYMLINK);
+							builder.add(dcEntry);
+						}
 					}
 				}
 				String content = cfg.toText();