opendev/gerrit
Code Issues Proposed changes

Revert "Expand submodule subscription support"

Browse Source 
This reverts commit 333676bd1b
Too many problems were identified during a secondary review.

Change-Id: I6112e6594827a537ac57ccd283b55674506eef20
This commit is contained in:
Shawn Pearce
2012-12-07 11:17:31 -08:00
committed by Gerrit Code Review
parent af17ea96dd
commit d5714d73d4
8 changed files with 47 additions and 210 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
Documentation/user-submodules.txt
View File

@@ -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 @@ commit having the updated gitlinks.
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 @@ at path 'a' by executing the following command when being inside
'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 @@ Creating a New Subscription
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 @@ After adding the git submodule to a super project, one should edit
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 @@ is merged in branch 'dev-of-a' of 'a' project, Gerrit automatically
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
----------------------

12
gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
View File

@@ -42,8 +42,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.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 @@ public class Daemon extends SiteProgram {
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

29
gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddress.java
View File

@@ -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 {
}

30
gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressModule.java
View File

@@ -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();
}

46
gerrit-server/src/main/java/com/google/gerrit/server/config/SshdListenAddressProvider.java
View File

@@ -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;
}
}

32
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
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 @@ public class SubmoduleOp {
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 class SubmoduleOp {
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 @@ public class SubmoduleOp {
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 @@ public class SubmoduleOp {
}
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 @@ public class SubmoduleOp {
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());

48
gerrit-server/src/test/java/com/google/gerrit/server/git/SubmoduleOpTest.java
View File

@@ -71,7 +71,6 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
private SubmoduleSubscriptionAccess subscriptions;
private ReviewDb schema;
private Provider<String> urlProvider;
private Provider<String> sshProvider;
private GitRepositoryManager repoManager;
private GitReferenceUpdated replication;
@@ -85,19 +84,18 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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 @@ public class SubmoduleOpTest extends LocalDiskRepositoryTestCase {
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();
}

8
gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java
View File

@@ -27,8 +27,6 @@ import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
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 @@ public class WebAppInitializer extends GuiceServletContextListener {
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);
}