Fix relative 'fetch' locations.
The Android manifest
(https://android.googlesource.com/platform/manifest/+/master/default.xml)
uses this.
Change-Id: I27720dedc5afef143a8bfc3c8f7daa9438792c46
diff --git a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index f6a45d4..447252a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -41,6 +41,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import org.apache.http.client.utils.URIBuilder;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.InvalidRemoteException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
@@ -70,7 +71,7 @@
private static final String SECTION_NAME = "superproject";
private final GitRepositoryManager repoManager;
- private final String canonicalWebUrl;
+ private final URI canonicalWebUrl;
private final PluginConfigFactory cfgFactory;
private final String pluginName;
private final AllProjectsName allProjectsName;
@@ -93,7 +94,12 @@
this.serverIdent = serverIdent;
this.allProjectsName = allProjectsName;
this.repoManager = repoManager;
- this.canonicalWebUrl = canonicalWebUrl;
+ try {
+ this.canonicalWebUrl = new URI(canonicalWebUrl);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
+
this.cfgFactory = cfgFactory;
this.projectCache = projectCache;
}
@@ -112,6 +118,7 @@
private static class ConfigEntry {
Project.NameKey srcRepoKey;
String srcRef;
+ URI srcRepoUrl;
String xmlPath;
Project.NameKey destRepoKey;
boolean recordSubmoduleLabels;
@@ -179,23 +186,27 @@
}
for (String subsect : cfg.getSubsections(SECTION_NAME)) {
try {
- ConfigEntry e = newConfigEntry(cfg, subsect);
- if (destinations.contains(e.srcRepoKey.get()) || sources.contains(e.destRepoKey.get())) {
+ ConfigEntry configEntry = newConfigEntry(cfg, subsect);
+ if (destinations.contains(configEntry.srcRepoKey.get())
+ || sources.contains(configEntry.destRepoKey.get())) {
// Don't want cyclic dependencies.
throw new ConfigInvalidException(
- String.format("repo in entry %s cannot be both source and destination", e));
+ String.format("repo in entry %s cannot be both source and destination", configEntry));
}
- if (e.destBranch.equals("*")) {
- if (wildcardDestinations.contains(e.destRepoKey.get())) {
+ if (configEntry.destBranch.equals("*")) {
+ if (wildcardDestinations.contains(configEntry.destRepoKey.get())) {
throw new ConfigInvalidException(
- String.format("repo %s already has a wildcard destination branch.", e.destRepoKey));
+ String.format(
+ "repo %s already has a wildcard destination branch.", configEntry.destRepoKey));
}
- wildcardDestinations.add(e.destRepoKey.get());
+ wildcardDestinations.add(configEntry.destRepoKey.get());
}
- sources.add(e.srcRepoKey.get());
- destinations.add(e.destRepoKey.get());
- newConf.add(e);
+
+ sources.add(configEntry.srcRepoKey.get());
+ destinations.add(configEntry.destRepoKey.get());
+
+ newConf.add(configEntry);
} catch (ConfigInvalidException e) {
log.error("ConfigInvalidException: " + e.toString());
@@ -205,7 +216,7 @@
return newConf;
}
- private static ConfigEntry newConfigEntry(Config cfg, String name) throws ConfigInvalidException {
+ private ConfigEntry newConfigEntry(Config cfg, String name) throws ConfigInvalidException {
String[] parts = name.split(":");
if (parts.length != 2) {
throw new ConfigInvalidException(
@@ -262,6 +273,16 @@
e.destBranch = destRef.substring(REFS_HEADS.length());
e.recordSubmoduleLabels = cfg.getBoolean(SECTION_NAME, name, "recordSubmoduleLabels", false);
+
+
+ try {
+ String newPath = canonicalWebUrl.getPath() + "/" + e.srcRepoKey.toString();
+ e.srcRepoUrl =
+ new URIBuilder(canonicalWebUrl).setPath(newPath).build().normalize();
+ } catch (URISyntaxException exception) {
+ throw new ConfigInvalidException("could not build src URL", exception);
+ }
+
return e;
}
@@ -378,9 +399,7 @@
cmd.setInputStream(manifestStream);
cmd.setRecommendShallow(true);
cmd.setRemoteReader(reader);
-
- // Is this the right URL?
- cmd.setURI(canonicalWebUrl);
+ cmd.setURI(c.srcRepoUrl.toString());
// Must setup a included file reader; the default is to read the file from the filesystem
// otherwise, which would leak data from the serving machine.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
index e4f0533..9a6cbe3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.supermanifest;
import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.fail;
import com.google.gerrit.acceptance.GitUtil;
@@ -25,8 +26,11 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import java.net.URI;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@TestPlugin(
@@ -36,10 +40,10 @@
public class SuperManifestIT extends LightweightPluginDaemonTest {
Project.NameKey[] testRepoKeys;
- void setupTestRepos() throws Exception {
+ void setupTestRepos(String prefix) throws Exception {
testRepoKeys = new Project.NameKey[2];
for (int i = 0; i < 2; i++) {
- testRepoKeys[i] = createProject("project" + i);
+ testRepoKeys[i] = createProject(prefix + i);
TestRepository<InMemoryRepository> repo = cloneProject(testRepoKeys[i], admin);
@@ -63,7 +67,7 @@
@Test
public void basicFunctionalityWorks() throws Exception {
- setupTestRepos();
+ setupTestRepos("project");
// Make sure the manifest exists so the configuration loads successfully.
Project.NameKey manifestKey = createProject("manifest");
@@ -83,13 +87,13 @@
+ " srcPath = default.xml\n");
String remoteXml = " <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
- String originXml = " <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+ String defaultXml = " <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
// XML change will trigger commit to superproject.
String xml =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<manifest>\n"
+ remoteXml
- + originXml
+ + defaultXml
+ " <project name=\""
+ testRepoKeys[0].get()
+ "\" path=\"project1\" />\n"
@@ -113,7 +117,7 @@
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<manifest>\n"
+ remoteXml
- + originXml
+ + defaultXml
+ " <project name=\""
+ testRepoKeys[0].get()
+ "\" path=\"project1\" />\n"
@@ -146,7 +150,7 @@
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ "<manifest>\n"
+ remoteXml
- + originXml
+ + defaultXml
+ " <project name=\""
+ testRepoKeys[1].get()
+ "\" path=\"project3\" />\n"
@@ -163,7 +167,7 @@
@Test
public void wildcardDestBranchWorks() throws Exception {
- setupTestRepos();
+ setupTestRepos("project");
// Make sure the manifest exists so the configuration loads successfully.
Project.NameKey manifestKey = createProject("manifest");
@@ -237,7 +241,7 @@
@Test
public void manifestIncludesOtherManifest() throws Exception {
- setupTestRepos();
+ setupTestRepos("project");
String remoteXml = " <remote name=\"origin\" fetch=\"" + canonicalWebUrl.get() + "\" />\n";
String originXml = " <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
@@ -294,5 +298,65 @@
assertThat(branch.file("project1").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
}
+ @Test
+ public void relativeFetch() throws Exception {
+ // Test the setup that Android uses, where the "fetch" field is relative to the location of the
+ // manifest repo.
+ setupTestRepos("platform/project");
+
+ // The test framework adds more cruft to the prefix.
+ String realPrefix = testRepoKeys[0].get().split("/")[0];
+
+ Project.NameKey manifestKey = createProject(realPrefix + "/manifest");
+ TestRepository<InMemoryRepository> manifestRepo = cloneProject(manifestKey, admin);
+
+ Project.NameKey superKey = createProject("superproject");
+
+ TestRepository<InMemoryRepository> superRepo = cloneProject(superKey, admin);
+
+ pushConfig(
+ "[superproject \""
+ + superKey.get()
+ + ":refs/heads/destbranch\"]\n"
+ + " srcRepo = "
+ + manifestKey.get()
+ + "\n"
+ + " srcRef = refs/heads/srcbranch\n"
+ + " srcPath = default.xml\n");
+
+ String url = canonicalWebUrl.get();
+ String remoteXml = " <remote name=\"origin\" fetch=\"..\" review=\"" + url + "\" />\n";
+ String defaultXml = " <default remote=\"origin\" revision=\"refs/heads/master\" />\n";
+
+ String xml =
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ + "<manifest>\n"
+ + remoteXml
+ + defaultXml
+ + " <project name=\""
+ + testRepoKeys[0].get()
+ + "\" path=\"path1\" />\n"
+ + "</manifest>\n";
+ pushFactory
+ .create(db, admin.getIdent(), manifestRepo, "Subject", "default.xml", xml)
+ .to("refs/heads/srcbranch")
+ .assertOkStatus();
+
+ BranchApi branch = gApi.projects().name(superKey.get()).branch("refs/heads/destbranch");
+ assertThat(branch.file("path1").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
+
+ Config base = new Config();
+ BlobBasedConfig cfg =
+ new BlobBasedConfig(base, branch.file(".gitmodules").asString().getBytes(UTF_8));
+
+ String subUrl = cfg.getString("submodule", "path1", "url");
+
+ // URL is valid.
+ URI.create(subUrl);
+
+ // URL is clean.
+ assertThat(subUrl).doesNotContain("../");
+ }
+
// TODO - should add tests for all the error handling in configuration parsing?
}