Fix RepoCommand to allow for relative URLs

This is necessary for deploying submodules on android.googlesource.com.

* Allow an empty base URL. This is useful if the 'fetch' field is "."
  and all names are relative to some host root.

* The URLs in the resulting superproject are relative to the
  superproject's URL. Add RepoCommand#setDestinationURI to
  set this. If unset, the existing behavior is maintained.

* Add two tests for the Android and Gerrit case, checking the URL
  format in .gitmodules; the tests use a custom RemoteReader which is
  representative of the use of this class in Gerrit's Supermanifest
  plugin.

Change-Id: Ia75530226120d75aa0017c5410fd65d0563e91b
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>

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

@@ -46,12 +46,14 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
@@ -183,6 +185,107 @@ public class RepoCommandTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void androidSetup() throws Exception {
+		Repository child = Git.cloneRepository()
+				.setURI(groupADb.getDirectory().toURI().toString())
+				.setDirectory(createUniqueTestGitDir(true)).setBare(true).call()
+				.getRepository();
+
+		Repository dest = Git.cloneRepository()
+				.setURI(db.getDirectory().toURI().toString())
+				.setDirectory(createUniqueTestGitDir(true)).setBare(true).call()
+				.getRepository();
+
+		assertTrue(dest.isBare());
+		assertTrue(child.isBare());
+
+		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=\"base\" name=\"platform/base\" />")
+			.append("</manifest>");
+		RepoCommand cmd = new RepoCommand(dest);
+
+		IndexedRepos repos = new IndexedRepos();
+		repos.put("platform/base", child);
+
+		RevCommit commit = cmd
+			.setInputStream(new ByteArrayInputStream(xmlContent.toString().getBytes(StandardCharsets.UTF_8)))
+			.setRemoteReader(repos)
+			.setURI("platform/")
+			.setTargetURI("platform/superproject")
+			.setRecordRemoteBranch(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, "../base");
+		}
+
+		child.close();
+		dest.close();
+	}
+
+	@Test
+	public void gerritSetup() throws Exception {
+		Repository child =
+			Git.cloneRepository().setURI(groupADb.getDirectory().toURI().toString())
+				.setDirectory(createUniqueTestGitDir(true))
+				.setBare(true).call().getRepository();
+
+		Repository dest = Git.cloneRepository()
+			.setURI(db.getDirectory().toURI().toString()).setDirectory(createUniqueTestGitDir(true))
+			.setBare(true).call().getRepository();
+
+		assertTrue(dest.isBare());
+		assertTrue(child.isBare());
+
+		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=\"plugins/cookbook\" name=\"plugins/cookbook\" />")
+			.append("</manifest>");
+		RepoCommand cmd = new RepoCommand(dest);
+
+		IndexedRepos repos = new IndexedRepos();
+		repos.put("plugins/cookbook", child);
+
+		RevCommit commit = cmd
+			.setInputStream(new ByteArrayInputStream(xmlContent.toString().getBytes(StandardCharsets.UTF_8)))
+			.setRemoteReader(repos)
+			.setURI("")
+			.setTargetURI("gerrit")
+			.setRecordRemoteBranch(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", "plugins/cookbook", "url");
+			assertEquals(subUrl, "../plugins/cookbook");
+		}
+
+		child.close();
+		dest.close();
+	}
+
 	@Test
 	public void absoluteRemoteURL() throws Exception {
 		Repository child =
@@ -217,6 +320,7 @@ public class RepoCommandTest extends RepositoryTestCase {
 					.setInputStream(new ByteArrayInputStream(xmlContent.toString().getBytes(StandardCharsets.UTF_8)))
 					.setRemoteReader(repos)
 					.setURI(baseUrl)
+					.setTargetURI("gerrit")
 					.setRecordRemoteBranch(true)
 					.setRecordSubmoduleLabels(true)
 					.call();
@@ -997,4 +1101,27 @@ public class RepoCommandTest extends RepositoryTestCase {
 			start = newStart;
 		}
 	}
+
+	void testRelative(String a, String b, String want) {
+		String got = RepoCommand.relativize(URI.create(a), URI.create(b)).toString();
+
+		if (!got.equals(want)) {
+			fail(String.format("relative('%s', '%s') = '%s', want '%s'", a, b, got, want));
+		}
+	}
+
+	@Test
+	public void relative() {
+		testRelative("a/b/", "a/", "../");
+		// Normalization:
+		testRelative("a/p/..//b/", "a/", "../");
+		testRelative("a/b", "a/", "");
+		testRelative("a/", "a/b/", "b/");
+		testRelative("a/", "a/b", "b");
+		testRelative("/a/b/c", "/b/c", "../../b/c");
+		testRelative("/abc", "bcd", "bcd");
+		testRelative("abc", "/bcd", "/bcd");
+		testRelative("http://a", "a/b", "a/b");
+		testRelative("http://base.com/a/", "http://child.com/a/b", "http://child.com/a/b");
+	}
 }

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

@@ -49,11 +49,13 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.StringJoiner;
 
 import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.api.Git;
@@ -106,7 +108,8 @@ import org.eclipse.jgit.util.FileUtils;
  */
 public class RepoCommand extends GitCommand<RevCommit> {
 	private String manifestPath;
-	private String uri;
+	private String baseUri;
+	private URI targetUri;
 	private String groupsParam;
 	private String branch;
 	private String targetBranch = Constants.HEAD;
@@ -274,7 +277,23 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	 * @return this command
 	 */
 	public RepoCommand setURI(String uri) {
-		this.uri = uri;
+		this.baseUri = uri;
+		return this;
+	}
+
+	/**
+	 * Set the URI of the superproject (this repository), so the .gitmodules file can specify the
+	 * submodule URLs relative to the superproject.
+	 *
+	 * @param uri the URI of the repository holding the superproject.
+	 * @return this command
+	 */
+	public RepoCommand setTargetURI(String uri) {
+		// The repo name is interpreted as a directory, for example
+		// Gerrit (http://gerrit.googlesource.com/gerrit) has a
+		// .gitmodules referencing ../plugins/hooks, which is
+		// on http://gerrit.googlesource.com/plugins/hooks,
+		this.targetUri = URI.create(uri + "/"); //$NON-NLS-1$
 		return this;
 	}
 
@@ -452,9 +471,8 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	public RevCommit call() throws GitAPIException {
 		try {
 			checkCallable();
-			if (uri == null || uri.length() == 0) {
+			if (baseUri == null) {
-				throw new IllegalArgumentException(
+				baseUri = ""; //$NON-NLS-1$
-					JGitText.get().uriNotConfigured);
 			}
 			if (inputStream == null) {
 				if (manifestPath == null || manifestPath.length() == 0)
@@ -478,7 +496,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 				git = new Git(repo);
 
 			ManifestParser parser = new ManifestParser(
-					includedReader, manifestPath, branch, uri, groupsParam, repo);
+					includedReader, manifestPath, branch, baseUri, groupsParam, repo);
 			try {
 				parser.read(inputStream);
 				for (RepoProject proj : parser.getFilteredProjects()) {
@@ -550,8 +568,13 @@ public class RepoCommand extends GitCommand<RevCommit> {
 						rec.append("\n"); //$NON-NLS-1$
 						attributes.append(rec.toString());
 					}
+
+					URI submodUrl = URI.create(nameUri);
+					if (targetUri != null) {
+						submodUrl = relativize(targetUri, submodUrl);
+					}
 					cfg.setString("submodule", path, "path", path); //$NON-NLS-1$ //$NON-NLS-2$
-					cfg.setString("submodule", path, "url", nameUri); //$NON-NLS-1$ //$NON-NLS-2$
+					cfg.setString("submodule", path, "url", submodUrl.toString()); //$NON-NLS-1$ //$NON-NLS-2$
 
 					// create gitlink
 					DirCacheEntry dcEntry = new DirCacheEntry(path);
@@ -672,6 +695,66 @@ public class RepoCommand extends GitCommand<RevCommit> {
 		}
 	}
 
+	/*
+	 * Assume we are document "a/b/index.html", what should we put in a href to get to "a/" ?
+	 * Returns the child if either base or child is not a bare path. This provides a missing feature in
+	 * java.net.URI (see http://bugs.java.com/view_bug.do?bug_id=6226081).
+	 */
+	static URI relativize(URI current, URI target) {
+		// We only handle bare paths for now.
+		if (!target.toString().equals(target.getPath())) {
+			return target;
+		}
+		if (!current.toString().equals(current.getPath())) {
+			return target;
+		}
+
+		String cur = current.normalize().getPath();
+		String dest = target.normalize().getPath();
+
+		// TODO(hanwen): maybe (absolute, relative) should throw an exception.
+		if (cur.startsWith("/") != dest.startsWith("/")) { //$NON-NLS-1$//$NON-NLS-2$
+			return target;
+		}
+
+		while (cur.startsWith("/")) { //$NON-NLS-1$
+			cur = cur.substring(1);
+		}
+		while (dest.startsWith("/")) { //$NON-NLS-1$
+			dest = dest.substring(1);
+		}
+
+		if (!cur.endsWith("/")) { //$NON-NLS-1$
+			// The current file doesn't matter.
+			cur = cur.substring(0, cur.lastIndexOf('/'));
+		}
+		String destFile = ""; //$NON-NLS-1$
+		if (!dest.endsWith("/")) { //$NON-NLS-1$
+			// We always have to provide the destination file.
+			destFile = dest.substring(dest.lastIndexOf('/') + 1, dest.length());
+			dest = dest.substring(0, dest.lastIndexOf('/'));
+		}
+
+		String[] cs = cur.split("/"); //$NON-NLS-1$
+		String[] ds = dest.split("/"); //$NON-NLS-1$
+
+		int common = 0;
+		while (common < cs.length && common < ds.length && cs[common].equals(ds[common])) {
+			common++;
+		}
+
+		StringJoiner j = new StringJoiner("/"); //$NON-NLS-1$
+		for (int i = common; i < cs.length; i++) {
+			j.add(".."); //$NON-NLS-1$
+		}
+		for (int i = common; i < ds.length; i++) {
+			j.add(ds[i]);
+		}
+
+		j.add(destFile);
+		return URI.create(j.toString());
+	}
+
 	private static String findRef(String ref, Repository repo)
 			throws IOException {
 		if (!ObjectId.isId(ref)) {