From fe5437e96b91222e30d9105e7bab490fd68c2d52 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Tue, 28 Mar 2017 14:00:38 +0200 Subject: [PATCH] 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 Signed-off-by: David Pursehouse --- .../eclipse/jgit/gitrepo/RepoCommandTest.java | 127 ++++++++++++++++++ .../org/eclipse/jgit/gitrepo/RepoCommand.java | 97 ++++++++++++- 2 files changed, 217 insertions(+), 7 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 9cf4569d6..24b5ad7da 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 @@ -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("\n") + .append("") + .append("") + .append("") + .append("") + .append(""); + 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("\n") + .append("") + .append("") + .append("") + .append("") + .append(""); + 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"); + } } 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 e105dc063..3e8d524a4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java +++ 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 { 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 { * @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 { public RevCommit call() throws GitAPIException { try { checkCallable(); - if (uri == null || uri.length() == 0) { - throw new IllegalArgumentException( - JGitText.get().uriNotConfigured); + if (baseUri == null) { + baseUri = ""; //$NON-NLS-1$ } if (inputStream == null) { if (manifestPath == null || manifestPath.length() == 0) @@ -478,7 +496,7 @@ public class RepoCommand extends GitCommand { 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 { 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 { } } + /* + * 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)) {