Merge "Update replication plugin" into stable-2.10
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4ad7d21..3368db9 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -706,14 +706,24 @@
==== [[cache_options]]Cache Options
-[[cache.diff_intraline.maxIdleWorkers]]cache.diff_intraline.maxIdleWorkers::
+[[cache.diff.timeout]]cache.diff.timeout::
+
-Number of idle worker threads to maintain for the intraline difference
-computations. There is no upper bound on how many concurrent requests
-can occur at once, if additional threads are started to handle a peak
-load, only this many will remain idle afterwards.
+Maximum number of milliseconds to wait for diff data before giving up and
+falling back on a simpler diff algorithm that will not be able to break down
+modified regions into smaller ones. This is a work around for an infinite loop
+bug in the default difference algorithm implementation.
+
-Default is 1.5x number of available CPUs.
+Values should use common unit suffixes to express their setting:
++
+* ms, milliseconds
+* s, sec, second, seconds
+* m, min, minute, minutes
+* h, hr, hour, hours
+
++
+If a unit suffix is not specified, `milliseconds` is assumed.
++
+Default is 5 seconds.
[[cache.diff_intraline.timeout]]cache.diff_intraline.timeout::
+
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthServiceProvider.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthServiceProvider.java
index 8375e31..9be2630 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthServiceProvider.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthServiceProvider.java
@@ -23,29 +23,20 @@
public interface OAuthServiceProvider {
/**
- * Retrieve the request token.
- *
- * @return request token
- */
- OAuthToken getRequestToken();
-
- /**
* Returns the URL where you should redirect your users to authenticate
* your application.
*
- * @param requestToken the request token you need to authorize
- * @return the URL where you should redirect your users
+ * @return the OAuth service URL to redirect your users for authentication
*/
- String getAuthorizationUrl(OAuthToken requestToken);
+ String getAuthorizationUrl();
/**
* Retrieve the access token
*
- * @param requestToken request token (obtained previously)
* @param verifier verifier code
* @return access token
*/
- OAuthToken getAccessToken(OAuthToken requestToken, OAuthVerifier verifier);
+ OAuthToken getAccessToken(OAuthVerifier verifier);
/**
* After establishing of secure communication channel, this method supossed to
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java
index 23a7bec..388ce36 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/auth/oauth/OAuthUserInfo.java
@@ -20,15 +20,18 @@
private final String userName;
private final String emailAddress;
private final String displayName;
+ private final String claimedIdentity;
public OAuthUserInfo(String externalId,
String userName,
String emailAddress,
- String displayName) {
+ String displayName,
+ String claimedIdentity) {
this.externalId = externalId;
this.userName = userName;
this.emailAddress = emailAddress;
this.displayName = displayName;
+ this.claimedIdentity = claimedIdentity;
}
public String getExternalId() {
@@ -46,4 +49,8 @@
public String getDisplayName() {
return displayName;
}
+
+ public String getClaimedIdentity() {
+ return claimedIdentity;
+ }
}
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
index d3dc963..41c83eb 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/OnlineReindexer.java
@@ -79,7 +79,9 @@
ChangeBatchIndexer.Result result = batchIndexer.indexAll(
index, projectCache.all(), -1, -1, null, null);
if (!result.success()) {
- log.error("Online reindex of schema version {} failed", version(index));
+ log.error("Online reindex of schema version {} failed. Successfully"
+ + " indexed {} changes, failed to index {} changes",
+ version(index), result.doneCount(), result.failedCount());
return;
}
diff --git a/gerrit-oauth/BUCK b/gerrit-oauth/BUCK
index 4641e81..fa5a8e2 100644
--- a/gerrit-oauth/BUCK
+++ b/gerrit-oauth/BUCK
@@ -11,9 +11,11 @@
'//gerrit-common:annotations',
'//gerrit-extension-api:api',
'//gerrit-httpd:httpd',
+ '//gerrit-reviewdb:server',
'//gerrit-server:server',
'//lib:gson',
'//lib:guava',
+ '//lib:gwtorm',
'//lib/commons:codec',
'//lib/guice:guice',
'//lib/guice:guice-servlet',
diff --git a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
index d625e02..8ffbbe6 100644
--- a/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
+++ b/gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java
@@ -23,9 +23,11 @@
import com.google.gerrit.extensions.auth.oauth.OAuthVerifier;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.httpd.WebSession;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthResult;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.servlet.SessionScoped;
@@ -87,8 +89,7 @@
}
log.debug("Login-Retrieve-User " + this);
- token = oauth.getAccessToken(null,
- new OAuthVerifier(request.getParameter("code")));
+ token = oauth.getAccessToken(new OAuthVerifier(request.getParameter("code")));
user = oauth.getUserInfo(token);
@@ -103,7 +104,7 @@
} else {
log.debug("Login-PHASE1 " + this);
redirectUrl = request.getRequestURI();
- response.sendRedirect(oauth.getAuthorizationUrl(null) +
+ response.sendRedirect(oauth.getAuthorizationUrl() +
"&state=" + state);
return false;
}
@@ -113,11 +114,48 @@
throws IOException {
com.google.gerrit.server.account.AuthRequest areq =
new com.google.gerrit.server.account.AuthRequest(user.getExternalId());
- areq.setUserName(user.getUserName());
- areq.setEmailAddress(user.getEmailAddress());
- areq.setDisplayName(user.getDisplayName());
AuthResult arsp;
try {
+ String claimedIdentifier = user.getClaimedIdentity();
+ Account.Id actualId = accountManager.lookup(user.getExternalId());
+ if (!Strings.isNullOrEmpty(claimedIdentifier)) {
+ Account.Id claimedId = accountManager.lookup(claimedIdentifier);
+ if (claimedId != null && actualId != null) {
+ if (claimedId.equals(actualId)) {
+ // Both link to the same account, that's what we expected.
+ log.debug("OAuth2: claimed identity equals current id");
+ } else {
+ // This is (for now) a fatal error. There are two records
+ // for what might be the same user.
+ //
+ log.error("OAuth accounts disagree over user identity:\n"
+ + " Claimed ID: " + claimedId + " is " + claimedIdentifier
+ + "\n" + " Delgate ID: " + actualId + " is "
+ + user.getExternalId());
+ rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
+ return;
+ }
+ } else if (claimedId != null && actualId == null) {
+ // Claimed account already exists: link to it.
+ //
+ log.info("OAuth2: linking claimed identity to {}",
+ claimedId.toString());
+ try {
+ accountManager.link(claimedId, areq);
+ } catch (OrmException e) {
+ log.error("Cannot link: " + user.getExternalId()
+ + " to user identity:\n"
+ + " Claimed ID: " + claimedId + " is " + claimedIdentifier);
+ rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
+ return;
+ }
+ }
+ } else {
+ log.debug("OAuth2: claimed identity is empty");
+ }
+ areq.setUserName(user.getUserName());
+ areq.setEmailAddress(user.getEmailAddress());
+ areq.setDisplayName(user.getDisplayName());
arsp = accountManager.authenticate(areq);
} catch (AccountException e) {
log.error("Unable to authenticate user \"" + user + "\"", e);
diff --git a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
index 1681bc2..44b58c3 100644
--- a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
+++ b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -157,6 +157,7 @@
final AuthRequest aReq;
try {
aReq = manager.authenticate(state.discovered, state.retTo.toString());
+ log.debug("OpenID: openid-realm={}", state.contextUrl);
aReq.setRealm(state.contextUrl);
if (requestRegistration(aReq)) {
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 e28b5ba..1c667e3 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
@@ -63,7 +63,7 @@
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
import com.google.gerrit.server.mail.SmtpEmailSender;
-import com.google.gerrit.server.patch.IntraLineWorkerPool;
+import com.google.gerrit.server.patch.DiffExecutorModule;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
import com.google.gerrit.server.plugins.PluginRestApiModule;
import com.google.gerrit.server.schema.DataSourceProvider;
@@ -320,7 +320,7 @@
modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule());
modules.add(new MergeabilityChecksExecutorModule());
- modules.add(new IntraLineWorkerPool.Module());
+ modules.add(new DiffExecutorModule());
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultCacheFactory.Module());
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
index c0cf8f9..161efc1 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java
@@ -70,6 +70,7 @@
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.NoteDbModule;
+import com.google.gerrit.server.patch.DiffExecutorModule;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.CommentLinkInfo;
@@ -191,6 +192,7 @@
private Injector createSysInjector() {
List<Module> modules = Lists.newArrayList();
+ modules.add(new DiffExecutorModule());
modules.add(PatchListCacheImpl.module());
AbstractModule changeIndexModule;
switch (IndexModule.getIndexType(dbInjector)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
index 4b8f0b4..938d940 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DefaultRealm.java
@@ -40,8 +40,7 @@
@Override
public boolean allowsEdit(final Account.FieldName field) {
- if (authConfig.getAuthType() == AuthType.HTTP
- || authConfig.getAuthType() == AuthType.OAUTH) {
+ if (authConfig.getAuthType() == AuthType.HTTP) {
switch (field) {
case USER_NAME:
return false;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index b35d7e4..709a8ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -52,6 +52,7 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.Merger;
+import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
import org.eclipse.jgit.merge.ThreeWayMerger;
import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.revwalk.FooterLine;
@@ -89,6 +90,12 @@
return cfg.getBoolean("core", null, "useRecursiveMerge", true);
}
+ public static ThreeWayMergeStrategy getMergeStrategy(Config cfg) {
+ return useRecursiveMerge(cfg)
+ ? MergeStrategy.RECURSIVE
+ : MergeStrategy.RESOLVE;
+ }
+
public static interface Factory {
MergeUtil create(ProjectState project);
MergeUtil create(ProjectState project, boolean useContentMerge);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
index 5ee240b..be87ea0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
@@ -32,7 +32,9 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.MergeabilityChecker;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.MultiProgressMonitor;
import com.google.gerrit.server.git.MultiProgressMonitor.Task;
import com.google.gerrit.server.patch.PatchListLoader;
@@ -43,6 +45,7 @@
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffFormatter;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
@@ -50,6 +53,7 @@
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevTree;
@@ -112,6 +116,7 @@
private final ListeningExecutorService executor;
private final ChangeIndexer.Factory indexerFactory;
private final MergeabilityChecker mergeabilityChecker;
+ private final ThreeWayMergeStrategy mergeStrategy;
@Inject
ChangeBatchIndexer(SchemaFactory<ReviewDb> schemaFactory,
@@ -119,6 +124,7 @@
GitRepositoryManager repoManager,
@IndexExecutor ListeningExecutorService executor,
ChangeIndexer.Factory indexerFactory,
+ @GerritServerConfig Config config,
@Nullable MergeabilityChecker mergeabilityChecker) {
this.schemaFactory = schemaFactory;
this.changeDataFactory = changeDataFactory;
@@ -126,6 +132,7 @@
this.executor = executor;
this.indexerFactory = indexerFactory;
this.mergeabilityChecker = mergeabilityChecker;
+ this.mergeStrategy = MergeUtil.getMergeStrategy(config);
}
public Result indexAll(ChangeIndex index, Iterable<Project.NameKey> projects,
@@ -150,9 +157,7 @@
final AtomicBoolean ok = new AtomicBoolean(true);
for (final Project.NameKey project : projects) {
- if (!updateMergeable(project)) {
- ok.set(false);
- }
+ updateMergeable(project);
final ListenableFuture<?> future = executor.submit(reindexProject(
indexerFactory.create(index), project, doneTask, failedTask,
verboseWriter));
@@ -212,7 +217,7 @@
if (mergeabilityChecker != null) {
try {
mergeabilityChecker.newCheck().addProject(project).run();
- } catch (IOException e) {
+ } catch (Exception e) {
log.error("Error in mergeability checker", e);
return false;
}
@@ -239,8 +244,13 @@
byId.put(r.getObjectId(), changeDataFactory.create(db, c));
}
}
- new ProjectIndexer(indexer, byId, repo, done, failed, verboseWriter)
- .call();
+ new ProjectIndexer(indexer,
+ mergeStrategy,
+ byId,
+ repo,
+ done,
+ failed,
+ verboseWriter).call();
} catch (RepositoryNotFoundException rnfe) {
log.error(rnfe.getMessage());
} finally {
@@ -261,6 +271,7 @@
public static class ProjectIndexer implements Callable<Void> {
private final ChangeIndexer indexer;
+ private final ThreeWayMergeStrategy mergeStrategy;
private final Multimap<ObjectId, ChangeData> byId;
private final ProgressMonitor done;
private final ProgressMonitor failed;
@@ -268,16 +279,15 @@
private final Repository repo;
private RevWalk walk;
- public ProjectIndexer(ChangeIndexer indexer,
- Multimap<ObjectId, ChangeData> changesByCommitId, Repository repo) {
- this(indexer, changesByCommitId, repo,
- NullProgressMonitor.INSTANCE, NullProgressMonitor.INSTANCE, null);
- }
-
- ProjectIndexer(ChangeIndexer indexer,
- Multimap<ObjectId, ChangeData> changesByCommitId, Repository repo,
- ProgressMonitor done, ProgressMonitor failed, PrintWriter verboseWriter) {
+ private ProjectIndexer(ChangeIndexer indexer,
+ ThreeWayMergeStrategy mergeStrategy,
+ Multimap<ObjectId, ChangeData> changesByCommitId,
+ Repository repo,
+ ProgressMonitor done,
+ ProgressMonitor failed,
+ PrintWriter verboseWriter) {
this.indexer = indexer;
+ this.mergeStrategy = mergeStrategy;
this.byId = changesByCommitId;
this.repo = repo;
this.done = done;
@@ -376,7 +386,7 @@
walk.parseBody(a);
return walk.parseTree(a.getTree());
case 2:
- return PatchListLoader.automerge(repo, walk, b);
+ return PatchListLoader.automerge(repo, walk, b, mergeStrategy);
default:
return null;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutor.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutor.java
new file mode 100644
index 0000000..564ca58
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutor.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2015 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.patch;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+
+import java.lang.annotation.Retention;
+import java.util.concurrent.ExecutorService;
+
+/**
+ * Marker on {@link ExecutorService} used by
+ * {@link IntraLineLoader} and {@link PatchListLoader}.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface DiffExecutor {
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutorModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutorModule.java
new file mode 100644
index 0000000..9eaea3a
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/DiffExecutorModule.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2015 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.patch;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+/** Module providing the {@link DiffExecutor}. */
+public class DiffExecutorModule extends AbstractModule {
+
+ @Override
+ protected void configure() {
+ }
+
+ @Provides
+ @Singleton
+ @DiffExecutor
+ public ExecutorService createDiffExecutor() {
+ return Executors.newCachedThreadPool(new ThreadFactoryBuilder()
+ .setNameFormat("Diff-%d").setDaemon(true).build());
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
index 4961bef..009b492 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineLoader.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.patch;
+import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -29,7 +30,12 @@
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
import java.util.regex.Pattern;
class IntraLineLoader extends CacheLoader<IntraLineDiffKey, IntraLineDiff> {
@@ -41,12 +47,13 @@
private static final Pattern CONTROL_BLOCK_START_RE = Pattern
.compile("[{:][ \\t]*$");
- private final IntraLineWorkerPool workerPool;
+ private final ExecutorService diffExecutor;
private final long timeoutMillis;
@Inject
- IntraLineLoader(IntraLineWorkerPool pool, @GerritServerConfig Config cfg) {
- workerPool = pool;
+ IntraLineLoader(@DiffExecutor ExecutorService diffExecutor,
+ @GerritServerConfig Config cfg) {
+ this.diffExecutor = diffExecutor;
timeoutMillis =
ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.INTRA_NAME,
"timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS),
@@ -54,27 +61,30 @@
}
@Override
- public IntraLineDiff load(IntraLineDiffKey key) throws Exception {
- IntraLineWorkerPool.Worker w = workerPool.acquire();
- IntraLineWorkerPool.Worker.Result r = w.computeWithTimeout(key, timeoutMillis);
-
- if (r == IntraLineWorkerPool.Worker.Result.TIMEOUT) {
- // Don't keep this thread. We have to murder it unsafely, which
- // means its unable to be reused in the future. Return back a
- // null result, indicating the cache cannot load this key.
- //
+ public IntraLineDiff load(final IntraLineDiffKey key) throws Exception {
+ Future<IntraLineDiff> result = diffExecutor.submit(new Callable<IntraLineDiff>() {
+ @Override
+ public IntraLineDiff call() throws Exception {
+ return IntraLineLoader.compute(key);
+ }
+ });
+ try {
+ return result.get(timeoutMillis, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException | TimeoutException e) {
+ log.warn(timeoutMillis + " ms timeout reached for IntraLineDiff"
+ + " in project " + key.getProject().get()
+ + " on commit " + key.getCommit().name()
+ + " for path " + key.getPath()
+ + " comparing " + key.getBlobA().name()
+ + ".." + key.getBlobB().name());
+ result.cancel(true);
return new IntraLineDiff(IntraLineDiff.Status.TIMEOUT);
- }
- workerPool.release(w);
-
- if (r.error != null) {
+ } catch (ExecutionException e) {
// If there was an error computing the result, carry it
// up to the caller so the cache knows this key is invalid.
- //
- throw r.error;
+ Throwables.propagateIfInstanceOf(e.getCause(), Exception.class);
+ throw new Exception(e.getMessage(), e.getCause());
}
-
- return r.diff;
}
static IntraLineDiff compute(IntraLineDiffKey key) throws Exception {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineWorkerPool.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineWorkerPool.java
deleted file mode 100644
index 49ae950..0000000
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/IntraLineWorkerPool.java
+++ /dev/null
@@ -1,182 +0,0 @@
-// Copyright (C) 2009 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.patch;
-
-import static com.google.gerrit.server.patch.IntraLineLoader.log;
-
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.inject.AbstractModule;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-import org.eclipse.jgit.lib.Config;
-
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
-
-@Singleton
-public class IntraLineWorkerPool {
- public static class Module extends AbstractModule {
- @Override
- protected void configure() {
- bind(IntraLineWorkerPool.class);
- }
- }
-
- private final BlockingQueue<Worker> workerPool;
-
- @Inject
- public IntraLineWorkerPool(@GerritServerConfig Config cfg) {
- int workers = cfg.getInt(
- "cache", PatchListCacheImpl.INTRA_NAME, "maxIdleWorkers",
- Runtime.getRuntime().availableProcessors() * 3 / 2);
- workerPool = new ArrayBlockingQueue<>(workers, true /* fair */);
- }
-
- Worker acquire() {
- Worker w = workerPool.poll();
- if (w == null) {
- // If no worker is immediately available, start a new one.
- // Maximum parallelism is controlled by the web server.
- w = new Worker();
- w.start();
- }
- return w;
- }
-
- void release(Worker w) {
- if (!workerPool.offer(w)) {
- // If the idle worker pool is full, terminate the worker.
- w.shutdownGracefully();
- }
- }
-
- static class Worker extends Thread {
- private static final AtomicInteger count = new AtomicInteger(1);
-
- private final ArrayBlockingQueue<Input> input;
- private final ArrayBlockingQueue<Result> result;
-
- Worker() {
- input = new ArrayBlockingQueue<>(1);
- result = new ArrayBlockingQueue<>(1);
-
- setName("IntraLineDiff-" + count.getAndIncrement());
- setDaemon(true);
- }
-
- Result computeWithTimeout(IntraLineDiffKey key, long timeoutMillis)
- throws Exception {
- if (!input.offer(new Input(key))) {
- log.error("Cannot enqueue task to thread " + getName());
- return Result.TIMEOUT;
- }
-
- Result r = result.poll(timeoutMillis, TimeUnit.MILLISECONDS);
- if (r != null) {
- return r;
- } else {
- log.warn(timeoutMillis + " ms timeout reached for IntraLineDiff"
- + " in project " + key.getProject().get()
- + " on commit " + key.getCommit().name()
- + " for path " + key.getPath()
- + " comparing " + key.getBlobA().name()
- + ".." + key.getBlobB().name()
- + ". Killing " + getName());
- forcefullyKillThreadInAnUglyWay();
- return Result.TIMEOUT;
- }
- }
-
- @SuppressWarnings("deprecation")
- private void forcefullyKillThreadInAnUglyWay() {
- try {
- stop();
- } catch (Throwable error) {
- // Ignore any reason the thread won't stop.
- log.error("Cannot stop runaway thread " + getName(), error);
- }
- }
-
- private void shutdownGracefully() {
- if (!input.offer(Input.END_THREAD)) {
- log.error("Cannot gracefully stop thread " + getName());
- }
- }
-
- @Override
- public void run() {
- try {
- for (;;) {
- Input in;
- try {
- in = input.take();
- } catch (InterruptedException e) {
- log.error("Unexpected interrupt on " + getName());
- continue;
- }
-
- if (in == Input.END_THREAD) {
- return;
- }
-
- Result r;
- try {
- r = new Result(IntraLineLoader.compute(in.key));
- } catch (Exception error) {
- r = new Result(error);
- }
-
- if (!result.offer(r)) {
- log.error("Cannot return result from " + getName());
- }
- }
- } catch (ThreadDeath iHaveBeenShot) {
- // Handle thread death by gracefully returning to the caller,
- // allowing the thread to be destroyed.
- }
- }
-
- private static class Input {
- static final Input END_THREAD = new Input(null);
-
- final IntraLineDiffKey key;
-
- Input(IntraLineDiffKey key) {
- this.key = key;
- }
- }
-
- static class Result {
- static final Result TIMEOUT = new Result((IntraLineDiff) null);
-
- final IntraLineDiff diff;
- final Exception error;
-
- Result(IntraLineDiff diff) {
- this.diff = diff;
- this.error = null;
- }
-
- Result(Exception error) {
- this.diff = null;
- this.error = error;
- }
- }
- }
-}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index 7b7c731..6c769f7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -35,7 +35,7 @@
/** Provides a cached list of {@link PatchListEntry}. */
@Singleton
public class PatchListCacheImpl implements PatchListCache {
- private static final String FILE_NAME = "diff";
+ static final String FILE_NAME = "diff";
static final String INTRA_NAME = "diff_intraline";
public static Module module() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
index 4eede00..d9e7969 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
@@ -16,12 +16,16 @@
package com.google.gerrit.server.patch;
import com.google.common.base.Function;
+import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.common.collect.FluentIterable;
import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.ConfigUtil;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeUtil;
import com.google.inject.Inject;
import org.eclipse.jgit.diff.DiffEntry;
@@ -35,6 +39,7 @@
import org.eclipse.jgit.dircache.DirCache;
import org.eclipse.jgit.dircache.DirCacheBuilder;
import org.eclipse.jgit.dircache.DirCacheEntry;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.ObjectId;
@@ -45,8 +50,8 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.MergeFormatter;
import org.eclipse.jgit.merge.MergeResult;
-import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ResolveMerger;
+import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
import org.eclipse.jgit.patch.FileHeader;
import org.eclipse.jgit.patch.FileHeader.PatchType;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -66,17 +71,36 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
static final Logger log = LoggerFactory.getLogger(PatchListLoader.class);
private final GitRepositoryManager repoManager;
private final PatchListCache patchListCache;
+ private final ThreeWayMergeStrategy mergeStrategy;
+ private final ExecutorService diffExecutor;
+ private final long timeoutMillis;
+
@Inject
- PatchListLoader(GitRepositoryManager mgr, PatchListCache plc) {
+ PatchListLoader(GitRepositoryManager mgr,
+ PatchListCache plc,
+ @GerritServerConfig Config cfg,
+ @DiffExecutor ExecutorService de) {
repoManager = mgr;
patchListCache = plc;
+ mergeStrategy = MergeUtil.getMergeStrategy(cfg);
+ diffExecutor = de;
+ timeoutMillis =
+ ConfigUtil.getTimeUnit(cfg, "cache", PatchListCacheImpl.FILE_NAME,
+ "timeout", TimeUnit.MILLISECONDS.convert(5, TimeUnit.SECONDS),
+ TimeUnit.MILLISECONDS);
}
@Override
@@ -158,7 +182,7 @@
DiffEntry diffEntry = diffEntries.get(i);
if (paths == null || paths.contains(diffEntry.getNewPath())
|| paths.contains(diffEntry.getOldPath())) {
- FileHeader fh = df.toFileHeader(diffEntry);
+ FileHeader fh = toFileHeader(key, df, diffEntry);
entries.add(newEntry(aTree, fh));
}
}
@@ -169,6 +193,44 @@
}
}
+ private FileHeader toFileHeader(PatchListKey key,
+ final DiffFormatter diffFormatter, final DiffEntry diffEntry)
+ throws IOException {
+
+ Future<FileHeader> result = diffExecutor.submit(new Callable<FileHeader>() {
+ @Override
+ public FileHeader call() throws IOException {
+ return diffFormatter.toFileHeader(diffEntry);
+ }
+ });
+
+ try {
+ return result.get(timeoutMillis, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException | TimeoutException e) {
+ log.warn(timeoutMillis + " ms timeout reached for Diff loader"
+ + " in project " + key.projectKey.get()
+ + " on commit " + key.getNewId().name()
+ + " on path " + diffEntry.getNewPath()
+ + " comparing " + diffEntry.getOldId().name()
+ + ".." + diffEntry.getNewId().name());
+ result.cancel(true);
+ return toFileHeaderWithoutMyersDiff(diffFormatter, diffEntry);
+ } catch (ExecutionException e) {
+ // If there was an error computing the result, carry it
+ // up to the caller so the cache knows this key is invalid.
+ Throwables.propagateIfInstanceOf(e.getCause(), IOException.class);
+ throw new IOException(e.getMessage(), e.getCause());
+ }
+ }
+
+ private FileHeader toFileHeaderWithoutMyersDiff(DiffFormatter diffFormatter,
+ DiffEntry diffEntry) throws IOException {
+ HistogramDiff histogramDiff = new HistogramDiff();
+ histogramDiff.setFallbackAlgorithm(null);
+ diffFormatter.setDiffAlgorithm(histogramDiff);
+ return diffFormatter.toFileHeader(diffEntry);
+ }
+
private PatchListEntry newCommitMessage(final RawTextComparator cmp,
final Repository db, final ObjectReader reader,
final RevCommit aCommit, final RevCommit bCommit) throws IOException {
@@ -224,7 +286,7 @@
}
}
- private static RevObject aFor(final PatchListKey key,
+ private RevObject aFor(final PatchListKey key,
final Repository repo, final RevWalk rw, final RevCommit b)
throws IOException {
if (key.getOldId() != null) {
@@ -240,20 +302,20 @@
return r;
}
case 2:
- return automerge(repo, rw, b);
+ return automerge(repo, rw, b, mergeStrategy);
default:
// TODO(sop) handle an octopus merge.
return null;
}
}
- public static RevTree automerge(Repository repo, RevWalk rw, RevCommit b)
- throws IOException {
- return automerge(repo, rw, b, true);
+ public static RevTree automerge(Repository repo, RevWalk rw, RevCommit b,
+ ThreeWayMergeStrategy mergeStrategy) throws IOException {
+ return automerge(repo, rw, b, mergeStrategy, true);
}
public static RevTree automerge(Repository repo, RevWalk rw, RevCommit b,
- boolean save) throws IOException {
+ ThreeWayMergeStrategy mergeStrategy, boolean save) throws IOException {
String hash = b.name();
String refName = RefNames.REFS_CACHE_AUTOMERGE
+ hash.substring(0, 2)
@@ -264,8 +326,7 @@
return rw.parseTree(ref.getObjectId());
}
- ObjectId treeId;
- ResolveMerger m = (ResolveMerger) MergeStrategy.RESOLVE.newMerger(repo, true);
+ ResolveMerger m = (ResolveMerger) mergeStrategy.newMerger(repo, true);
final ObjectInserter ins = repo.newObjectInserter();
try {
DirCache dc = DirCache.newInCore();
@@ -297,6 +358,7 @@
return null;
}
+ ObjectId treeId;
if (couldMerge) {
treeId = m.getResultTreeId();
@@ -381,17 +443,18 @@
treeId = dc.writeTree(ins);
}
ins.flush();
+
+ if (save) {
+ RefUpdate update = repo.updateRef(refName);
+ update.setNewObjectId(treeId);
+ update.disableRefLog();
+ update.forceUpdate();
+ }
+
+ return rw.lookupTree(treeId);
} finally {
ins.release();
}
-
- if (save) {
- RefUpdate update = repo.updateRef(refName);
- update.setNewObjectId(treeId);
- update.disableRefLog();
- update.forceUpdate();
- }
- return rw.parseTree(treeId);
}
private static ObjectId emptyTree(final Repository repo) throws IOException {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
index 6b2f7c7..d96e861 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
@@ -18,6 +18,7 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.net.InetAddresses;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.DisabledChangeHooks;
import com.google.gerrit.reviewdb.client.AuthType;
@@ -49,6 +50,7 @@
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
import com.google.gerrit.server.mail.SmtpEmailSender;
+import com.google.gerrit.server.patch.DiffExecutor;
import com.google.gerrit.server.schema.Current;
import com.google.gerrit.server.schema.DataSourceType;
import com.google.gerrit.server.schema.SchemaCreator;
@@ -75,6 +77,7 @@
import java.lang.reflect.InvocationTargetException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
+import java.util.concurrent.ExecutorService;
public class InMemoryModule extends FactoryModule {
public static Config newDefaultConfig() {
@@ -157,6 +160,18 @@
return CanonicalWebUrlProvider.class;
}
});
+ //Replacement of DiffExecutorModule to not use thread pool in the tests
+ install(new AbstractModule() {
+ @Override
+ protected void configure() {
+ }
+ @Provides
+ @Singleton
+ @DiffExecutor
+ public ExecutorService createDiffExecutor() {
+ return MoreExecutors.sameThreadExecutor();
+ }
+ });
install(new DefaultCacheFactory.Module());
install(new SmtpEmailSender.Module());
install(new SignedTokenEmailTokenVerifier.Module());
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 e9bd296..f59401c 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
@@ -46,7 +46,7 @@
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
import com.google.gerrit.server.mail.SmtpEmailSender;
-import com.google.gerrit.server.patch.IntraLineWorkerPool;
+import com.google.gerrit.server.patch.DiffExecutorModule;
import com.google.gerrit.server.plugins.PluginGuiceEnvironment;
import com.google.gerrit.server.plugins.PluginRestApiModule;
import com.google.gerrit.server.schema.DataSourceModule;
@@ -280,7 +280,7 @@
modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule());
modules.add(new MergeabilityChecksExecutorModule());
- modules.add(new IntraLineWorkerPool.Module());
+ modules.add(new DiffExecutorModule());
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultCacheFactory.Module());
diff --git a/lib/jgit/BUCK b/lib/jgit/BUCK
index 2d0af1b..a658fc3 100644
--- a/lib/jgit/BUCK
+++ b/lib/jgit/BUCK
@@ -1,13 +1,15 @@
include_defs('//lib/maven.defs')
-VERS = '3.6.2.201501210735-r'
+REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
+VERS = '3.7.0.201502260915-r.58-g65c379e'
maven_jar(
name = 'jgit',
id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS,
- bin_sha1 = '47d59dffb5f02470ccfb6c1a5a31b6040a1636e5',
- src_sha1 = '52e133360f2c38046de262c827dfec8ec5b7c885',
+ bin_sha1 = '8fc9620ec499169facad3355f7417eb6a8aff511',
+ src_sha1 = '40bd9ae8af8e0b03eb4e43f44f5feda8b7325221',
license = 'jgit',
+ repository = REPO,
unsign = True,
deps = [':ewah'],
exclude = [
@@ -20,8 +22,9 @@
maven_jar(
name = 'jgit-servlet',
id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS,
- sha1 = 'f7788bbd0c0414e856c84ddf353e6f4c62fe1d28',
+ sha1 = 'cecc2b9c0b94455348c3a0c63eb83f72cc595757',
license = 'jgit',
+ repository = REPO,
deps = [':jgit'],
unsign = True,
exclude = [
@@ -33,8 +36,9 @@
maven_jar(
name = 'jgit-archive',
id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS,
- sha1 = 'ca9da919275dad5ac2844529f4cdccdd919bab5f',
+ sha1 = '7ccc7c78bf47566045ea7a3c08508ba18e4684ca',
license = 'jgit',
+ repository = REPO,
deps = [':jgit',
'//lib/commons:compress',
'//lib:tukaani-xz',
@@ -49,8 +53,9 @@
maven_jar(
name = 'junit',
id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS,
- sha1 = 'b7418e19efbeb9490b359c8a921cf32bfc57ebbd',
+ sha1 = '87d64d722447dc3971ace30d2a72593c72a4d05f',
license = 'DO_NOT_DISTRIBUTE',
+ repository = REPO,
unsign = True,
deps = [':jgit'],
)
diff --git a/lib/maven.defs b/lib/maven.defs
index 5f4006f..23708ca 100644
--- a/lib/maven.defs
+++ b/lib/maven.defs
@@ -40,7 +40,8 @@
sha1 = '', bin_sha1 = '', src_sha1 = '',
repository = MAVEN_CENTRAL,
attach_source = True,
- visibility = ['PUBLIC']):
+ visibility = ['PUBLIC'],
+ local_license = False):
from os import path
parts = id.split(':')
@@ -89,7 +90,10 @@
cmd = ' '.join(cmd),
out = binjar,
)
- license = ['//lib:LICENSE-' + license]
+ license = ':LICENSE-' + license
+ if not local_license:
+ license = '//lib' + license
+ license = [license]
if src_sha1 or attach_source:
cmd = ['$(exe //tools:download_file)', '-o', '$OUT', '-u', srcurl]
