Revert "Expand submodule subscription support"
This reverts commit 333676bd1b6b24459c86def586ae9769b1fd2b33
Too many problems were identified during a secondary review.
Change-Id: I6112e6594827a537ac57ccd283b55674506eef20
diff --git a/Documentation/user-submodules.txt b/Documentation/user-submodules.txt
index f965c27..cfaf3e9 100644
--- a/Documentation/user-submodules.txt
+++ b/Documentation/user-submodules.txt
@@ -1,5 +1,5 @@
-Gerrit Code Review - Superproject subscription to submodules updates
-====================================================================
+Gerrit Code Review - Superprojects subscribed to submodules updates
+===================================================================
Description
-----------
@@ -25,7 +25,7 @@
Git Submodules Overview
-----------------------
-Submodules are a git feature that allows an external repository to be
+It is a git feature that allows an external repository to be
attached inside a repository at a specific path. The objective here
is to provide a brief overview, further details can be found
in the official git submodule command documentation.
@@ -37,7 +37,7 @@
'super':
=====
git submodule add ssh://server/a a
-=====
+====
Still considering the above example, after its execution notice that
inside the local repository 'super' the 'a' folder is considered a
@@ -63,11 +63,11 @@
Defining the Submodule Branch
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-This is required because submodule subscription is actually the
+This is required because Submodule subscription is actually the
subscription of a submodule project and one of its branches for
a branch of a super project.
-Since Gerrit manages subscriptions in the branch scope, we could have
+Since it manages subscriptions in the branch scope, we could have
a scenario having a project called 'super' having a branch 'integration'
subscribed to a project called 'a' in branch 'integration', and also
having the same 'super' project but in branch 'dev' subscribed to the 'a'
@@ -77,23 +77,22 @@
the .gitmodules file to add a branch field to each submodule
section which is supposed to be subscribed.
-As the branch field is a Gerrit specific field it will not be filled
-automatically by the git submodule command, so one needs to edit it
-manually. Its value should indicate the branch of a submodule project
-that when updated will trigger automatic update of its registered
-gitlink.
+The branch field is not filled by the git submodule command. Its value
+should indicate the branch of a submodule project that when updated
+will trigger automatic update of its registered gitlink.
The branch value could be '.' if the submodule project branch
has the same name as the destination branch of the commit having
gitlinks/.gitmodules file.
-If the intention is to make use of the Gerrit feature described
-here, one should always be sure to update the .gitmodules file after
-adding submodules to a super project.
+The branch field of a submodule section is a custom git submodule
+feature for Gerrit use. One should always be sure to fill it in
+editing .gitmodules file after adding submodules to a super project,
+if it is the intention to make use of the Gerrit feature introduced here.
-If a git submodule is added but the branch field is not added to the
-.gitmodules file Gerrit will not create a subscribtion for the
-submodule and there will be no automatic updates to the superproject.
+Any git submodules which are added and not have the branch field
+available in the .gitmodules file will not be subscribed by Gerrit
+to automatically update the superproject.
Detecting and Subscribing Submodules
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -119,22 +118,17 @@
creates a new commit on branch 'dev' of 'super' updating the gitlink
to point to the just merged commit.
-Limitations in subscription
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Canonical Web Url
+~~~~~~~~~~~~~~~~~
-Gerrit will only automatically update superprojects where the
-submodules are hosted on the same Gerrit instance as the superproject.
-
-Gerrit determines this by checking the hostname of the submodule
-specified in the .gitmodules file and comparing it with the Gerrit
-instance's hostname. By default the hostname from the canonical web
-url is used, but if SSH is turned on and the SSH server name is
-different from the canonical hostname it will be used instead.
+Gerrit will automatically update only the superprojects that added
+the submodules of urls of the running server (the one described in
+the canonical web url value in Gerrit configuration file).
The Gerrit instance administrator group should always certify to
provide the canonical web url value in its configuration file. Users
-should certify to use the correct hostname of the running Gerrit
-instance to add/subscribe submodules.
+should certify to use the url value of the running Gerrit instance to
+add/subscribe submodules.
Removing Subscriptions
----------------------
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 3af7b29..1cbaff2 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -42,8 +42,6 @@
import com.google.gerrit.server.config.AuthConfigModule;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
-import com.google.gerrit.server.config.SshdListenAddressModule;
-import com.google.gerrit.server.config.SshdListenAddressProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.MasterNodeStartup;
@@ -297,16 +295,6 @@
modules.add(new SmtpEmailSender.Module());
modules.add(new SignedTokenEmailTokenVerifier.Module());
modules.add(new PluginModule());
-
- if (sshd) {
- modules.add(new SshdListenAddressModule() {
- @Override
- protected Class<? extends Provider<String>> provider() {
- return SshdListenAddressProvider.class;
- }
- });
- }
-
if (httpd) {
modules.add(new CanonicalWebUrlModule() {
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddress.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddress.java
deleted file mode 100644
index 94c26b9..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddress.java
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (C) 2012 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.config;
-
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import com.google.inject.BindingAnnotation;
-
-import java.lang.annotation.Retention;
-
-/**
- * Marker on a {@link String} holding the SSH listen address for this server.
- */
-@Retention(RUNTIME)
-@BindingAnnotation
-public @interface SshdListenAddress {
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressModule.java
deleted file mode 100644
index 044fefb..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressModule.java
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (C) 2012 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.config;
-
-import com.google.inject.AbstractModule;
-import com.google.inject.Provider;
-
-/** Supports binding the {@link SshdListenAddress} annotation. */
-public abstract class SshdListenAddressModule extends AbstractModule {
- @Override
- protected void configure() {
- final Class<? extends Provider<String>> provider = provider();
- bind(String.class).annotatedWith(SshdListenAddress.class)
- .toProvider(provider);
- }
-
- protected abstract Class<? extends Provider<String>> provider();
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressProvider.java
deleted file mode 100644
index 2e56b3d..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressProvider.java
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright (C) 2012 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.config;
-
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-
-import org.eclipse.jgit.lib.Config;
-
-/** Provides {@link SshdListenAddressl} from {@code sshd.listenAddress}. */
-public class SshdListenAddressProvider implements Provider<String> {
- private final String sshAddress;
-
- @Inject
- public SshdListenAddressProvider(@GerritServerConfig final Config config) {
- String sshdListenAddress = config.getString("sshd", null, "listenAddress");
- String sshdAdvertisedAddress = config.getString("sshd", null, "advertisedAddress");
-
- /*
- * If advertised address is specified it should take precedence over the
- * "normal" listening address.
- */
- if (sshdAdvertisedAddress != null && ! sshdAdvertisedAddress.isEmpty()) {
- sshAddress = sshdAdvertisedAddress;
- } else {
- sshAddress = sshdListenAddress;
- }
- }
-
- @Override
- public String get() {
- return sshAddress;
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
index 3976315..7129b86 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
@@ -21,7 +21,6 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.SshdListenAddress;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.util.SubmoduleSectionParser;
import com.google.gwtorm.server.OrmException;
@@ -79,7 +78,6 @@
private RevCommit mergeTip;
private RevWalk rw;
private final Provider<String> urlProvider;
- private final Provider<String> sshProvider;
private ReviewDb schema;
private Repository db;
private Project destProject;
@@ -95,7 +93,6 @@
public SubmoduleOp(@Assisted final Branch.NameKey destBranch,
@Assisted RevCommit mergeTip, @Assisted RevWalk rw,
@CanonicalWebUrl @Nullable final Provider<String> urlProvider,
- @SshdListenAddress @Nullable final Provider<String> sshProvider,
final SchemaFactory<ReviewDb> sf, @Assisted Repository db,
@Assisted Project destProject, @Assisted List<Change> submitted,
@Assisted final Map<Change.Id, CodeReviewCommit> commits,
@@ -105,7 +102,6 @@
this.mergeTip = mergeTip;
this.rw = rw;
this.urlProvider = urlProvider;
- this.sshProvider = sshProvider;
this.schemaFactory = sf;
this.db = db;
this.destProject = destProject;
@@ -141,32 +137,6 @@
}
try {
- String thisServer = null;
- String sshHostname = null;
- String webHostname = new URI(urlProvider.get()).getHost();
-
- // If SSH is disabled on the command line there will be no
- // sshProvider to ask. Translate this to the gerrit.config
- // option "off" inside the [sshd] section.
- if (sshProvider == null) {
- sshHostname = "off";
- } else {
- // Make sure we only remove the port part and not a substring
- // of an IPv6 address.
- sshHostname = sshProvider.get().split(":\\d+$")[0];
- }
- // Use the web host name (canonical web url) for the following
- // conditions:
- // - If the ssh hostname is the same as the web host
- // - If we listen on all addresses
- // - If sshd is turned off
- if (webHostname.equals(sshHostname) || sshHostname.equals("*") ||
- sshHostname.equals("off")) {
- thisServer = webHostname;
- } else {
- thisServer = sshHostname;
- }
-
final TreeWalk tw = TreeWalk.forPath(db, GIT_MODULES, mergeTip.getTree());
if (tw != null
&& (FileMode.REGULAR_FILE.equals(tw.getRawMode(0)) || FileMode.EXECUTABLE_FILE
@@ -175,6 +145,8 @@
BlobBasedConfig bbc =
new BlobBasedConfig(null, db, mergeTip, GIT_MODULES);
+ final String thisServer = new URI(urlProvider.get()).getHost();
+
final Branch.NameKey target =
new Branch.NameKey(new Project.NameKey(destProject.getName()),
destBranch.get());
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
index e917e12..67858aa 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
@@ -71,7 +71,6 @@
private SubmoduleSubscriptionAccess subscriptions;
private ReviewDb schema;
private Provider<String> urlProvider;
- private Provider<String> sshProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated replication;
@@ -85,19 +84,18 @@
schema = createStrictMock(ReviewDb.class);
subscriptions = createStrictMock(SubmoduleSubscriptionAccess.class);
urlProvider = createStrictMock(Provider.class);
- sshProvider = createStrictMock(Provider.class);
repoManager = createStrictMock(GitRepositoryManager.class);
replication = createStrictMock(GitReferenceUpdated.class);
}
private void doReplay() {
- replay(schemaFactory, schema, subscriptions, urlProvider, sshProvider,
- repoManager, replication);
+ replay(schemaFactory, schema, subscriptions, urlProvider, repoManager,
+ replication);
}
private void doVerify() {
- verify(schemaFactory, schema, subscriptions, urlProvider, sshProvider,
- repoManager, replication);
+ verify(schemaFactory, schema, subscriptions, urlProvider, repoManager,
+ replication);
}
/**
@@ -119,8 +117,7 @@
final Branch.NameKey branchNameKey =
new Branch.NameKey(new Project.NameKey("test-project"), "test-branch");
- expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
- expect(sshProvider.get()).andReturn("*:29418");
+ expect(urlProvider.get()).andReturn("http://localhost:8080");
expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
final ResultSet<SubmoduleSubscription> emptySubscriptions =
@@ -133,9 +130,9 @@
doReplay();
final SubmoduleOp submoduleOp =
- new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb),
- urlProvider, sshProvider, schemaFactory, realDb, null,
- new ArrayList<Change>(), null, null, null, null);
+ new SubmoduleOp(branchNameKey, mergeTip, new RevWalk(realDb), urlProvider,
+ schemaFactory, realDb, null, new ArrayList<Change>(), null, null,
+ null, null);
submoduleOp.update();
@@ -629,8 +626,7 @@
new Branch.NameKey(new Project.NameKey("target-project"),
sourceBranchNameKey.get());
- expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
- expect(sshProvider.get()).andReturn("*:29418");
+ expect(urlProvider.get()).andReturn("http://localhost:8080");
expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
final ResultSet<SubmoduleSubscription> subscribers =
@@ -661,9 +657,10 @@
final SubmoduleOp submoduleOp =
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
- sourceRepository), urlProvider, sshProvider, schemaFactory,
- sourceRepository, new Project(sourceBranchNameKey.getParentKey()),
- submitted, mergedCommits, myIdent, repoManager, replication);
+ sourceRepository), urlProvider, schemaFactory, sourceRepository,
+ new Project(sourceBranchNameKey.getParentKey()), submitted,
+ mergedCommits, myIdent, repoManager, replication);
+
submoduleOp.update();
doVerify();
@@ -730,8 +727,7 @@
new Branch.NameKey(new Project.NameKey("target-project"),
sourceBranchNameKey.get());
- expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
- expect(sshProvider.get()).andReturn("*:29418");
+ expect(urlProvider.get()).andReturn("http://localhost:8080");
expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
final ResultSet<SubmoduleSubscription> subscribers =
@@ -764,9 +760,10 @@
final SubmoduleOp submoduleOp =
new SubmoduleOp(sourceBranchNameKey, sourceMergeTip, new RevWalk(
- sourceRepository), urlProvider, sshProvider, schemaFactory,
- sourceRepository, new Project(sourceBranchNameKey.getParentKey()),
- submitted, mergedCommits, myIdent, repoManager, replication);
+ sourceRepository), urlProvider, schemaFactory, sourceRepository,
+ new Project(sourceBranchNameKey.getParentKey()), submitted,
+ mergedCommits, myIdent, repoManager, replication);
+
submoduleOp.update();
doVerify();
@@ -867,8 +864,7 @@
final RevCommit mergeTip = git.commit().setMessage("test").call();
- expect(urlProvider.get()).andReturn("http://localhost:8080").atLeastOnce();
- expect(sshProvider.get()).andReturn("*:29418");
+ expect(urlProvider.get()).andReturn("http://localhost:8080").times(2);
expect(schema.submoduleSubscriptions()).andReturn(subscriptions);
expect(subscriptions.bySuperProject(mergedBranch)).andReturn(
@@ -919,9 +915,9 @@
final SubmoduleOp submoduleOp =
new SubmoduleOp(mergedBranch, mergeTip, new RevWalk(realDb),
- urlProvider, sshProvider, schemaFactory, realDb, new Project(
- mergedBranch.getParentKey()), new ArrayList<Change>(), null,
- null, repoManager, null);
+ urlProvider, schemaFactory, realDb, new Project(mergedBranch
+ .getParentKey()), new ArrayList<Change>(), null, null,
+ repoManager, null);
submoduleOp.update();
}
diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
index 07d38c8..6dab0d3 100644
--- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
+++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
@@ -27,8 +27,6 @@
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.AuthConfigModule;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
-import com.google.gerrit.server.config.SshdListenAddressModule;
-import com.google.gerrit.server.config.SshdListenAddressProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerConfigModule;
@@ -240,12 +238,6 @@
return HttpCanonicalWebUrlProvider.class;
}
});
- modules.add(new SshdListenAddressModule() {
- @Override
- protected Class<? extends Provider<String>> provider() {
- return SshdListenAddressProvider.class;
- }
- });
modules.add(new MasterNodeStartup());
return cfgInjector.createChildInjector(modules);
}
