Inject GerritRemoteReader instance
When a manifest contains a project in a second host, the manifest parser
generates a correct .gitmodules but the GerritRemoteReader ignores the
host and tries to read the sha1 of the submodule from <this
host>/<project>. In some cases this produces an inconsistent
superproject: the .gitmodules points to a repo but the SHA1 in the
gitlink doesn't exist there.
Cross-host is a feature of the backend, the default implementation
should not count on it.
Make the GerritRemoteReader injectable, so providers can use a
cross-host implementation if supported in their platform.
Change-Id: I0475348373dd3c6c750ced864728c07c82b831b5
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestModule.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestModule.java
index 33aa1dd..b1666ac 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestModule.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestModule.java
@@ -33,6 +33,8 @@
DynamicSet.bind(binder(), LifecycleListener.class)
.to(SuperManifestRefUpdatedListener.class)
.in(SINGLETON);
+ bind(SuperManifestRefUpdatedListener.GerritRemoteReader.class)
+ .to(SuperManifestRefUpdatedListener.GerritRemoteReaderImpl.class);
post(BRANCH_KIND, "update_manifest").to(SuperManifestRefUpdatedListener.class).in(SINGLETON);
}
}
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index 43602ac..6a7b8d7 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -82,7 +82,7 @@
RestModifyView<BranchResource, BranchInput> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final GitRepositoryManager repoManager;
+ private final Provider<GerritRemoteReader> remoteReaderProvider;
private final URI canonicalWebUrl;
private final PluginConfigFactory cfgFactory;
private final String pluginName;
@@ -105,14 +105,14 @@
PluginConfigFactory cfgFactory,
ProjectCache projectCache,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
- GitRepositoryManager repoManager,
+ Provider<GerritRemoteReader> remoteReaderProvider,
Provider<IdentifiedUser> identifiedUser,
PermissionBackend permissionBackend) {
this.pluginName = pluginName;
this.serverIdent = serverIdent;
this.allProjectsName = allProjectsName;
- this.repoManager = repoManager;
+ this.remoteReaderProvider = remoteReaderProvider;
try {
this.canonicalWebUrl = new URI(canonicalWebUrl);
} catch (URISyntaxException e) {
@@ -327,7 +327,7 @@
throw new ConfigInvalidException(
String.format("invalid toolType: %s", c.getToolType().name()));
}
- try (GerritRemoteReader reader = new GerritRemoteReader()) {
+ try (GerritRemoteReader reader = remoteReaderProvider.get()) {
subModuleUpdater.update(reader, c, refName);
}
}
@@ -350,12 +350,33 @@
return trimmed.toArray(new StackTraceElement[trimmed.size()]);
}
- // GerritRemoteReader is for injecting Gerrit's Git implementation into JGit.
- class GerritRemoteReader implements RepoCommand.RemoteReader, Closeable {
- private final Map<String, Repository> repos;
+ interface GerritRemoteReader extends RepoCommand.RemoteReader, Closeable {
+ /**
+ * @param name repository name relative to current host (e.g. "submodule" for
+ * "gerrit.googlesource.com/submodule")
+ * @return the open repository. The reader keeps it cached, so the caller MUST not close it.
+ * @throws IOException error opening the repo.
+ */
+ Repository openRepository(String name) throws IOException;
+ }
- GerritRemoteReader() {
+ // GerritRemoteReader is for injecting Gerrit's Git implementation into JGit.
+ static class GerritRemoteReaderImpl implements GerritRemoteReader {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final Map<String, Repository> repos;
+ private final GitRepositoryManager repoManager;
+ private final URI canonicalWebUrl;
+
+ @Inject
+ GerritRemoteReaderImpl(
+ GitRepositoryManager repoManager, @CanonicalWebUrl String canonicalWebUrl) {
this.repos = new HashMap<>();
+ this.repoManager = repoManager;
+ try {
+ this.canonicalWebUrl = new URI(canonicalWebUrl);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
}
@Override
@@ -383,7 +404,8 @@
Repository repo = openRepository(repoName);
Ref ref = repo.findRef(refName);
if (ref == null || ref.getObjectId() == null) {
- warn("in repo %s: cannot resolve ref %s", uriStr, refName);
+ logger.atWarning().log(
+ "%s: in repo %s: cannot resolve ref %s", canonicalWebUrl, uriStr, refName);
return null;
}
@@ -391,7 +413,8 @@
ObjectId id = ref.getPeeledObjectId();
return id != null ? id : ref.getObjectId();
} catch (RepositoryNotFoundException e) {
- warn("failed to open repository %s: %s", repoName, e);
+ logger.atWarning().log(
+ "%s: failed to open repository %s: %s", canonicalWebUrl, repoName, e);
return null;
} catch (IOException io) {
RefNotFoundException e =
@@ -418,6 +441,7 @@
tw.getFileMode(0));
}
+ @Override
public Repository openRepository(String name) throws IOException {
name = urlToRepoKey(canonicalWebUrl, name);
if (repos.containsKey(name)) {