From 373794473a8a7cf6f6efe8f07b8363d58a9a44a7 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 3 Feb 2015 09:03:55 -0800 Subject: [PATCH 01/11] RestApiServlet: Flush pending padding as well This issue was found by scan.coverity.com (CID 19911) which is a static code analysis tool, free for open source code. Originally it was classified as a resource leak, but this is a miss classification as the auto closing of the OutputStream will flush any pending padding. Change-Id: I4dc2d1cd9f52740490fda7c37e98b115fa59ec3a --- .../com/google/gerrit/httpd/restapi/RestApiServlet.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index b7528b5de4..1279339215 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -740,10 +740,11 @@ public class RestApiServlet extends HttpServlet { b64 = new BinaryResult() { @Override public void writeTo(OutputStream out) throws IOException { - OutputStream e = BaseEncoding.base64().encodingStream( - new OutputStreamWriter(out, ISO_8859_1)); - src.writeTo(e); - e.flush(); + try (OutputStreamWriter w = new OutputStreamWriter(out, ISO_8859_1); + OutputStream e = BaseEncoding.base64().encodingStream(w)) { + src.writeTo(e); + e.flush(); + } } }; } From a92bcf416c11fa69761bcc91cb5915621e16ce89 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 4 Feb 2015 17:12:37 -0800 Subject: [PATCH 02/11] RestApiServlet: Leave OutputStream open when flushing base64 padding Some Java servlet containers fail if the response's OutputStream is closed twice by the application. This appears to contradict standard behavior in Java where most streams gracefully ignore extra close. Unfortunately the container is required to power gerrit-review and as such Gerrit needs to try to tolerate its behavior. Wrap the supplied OutputStream delegating all calls except for close(). No-op the close() method so the Java 7 try-with-resources block does not automatically close the servlet OutputStream, leaving this for the caller's finally block. Change-Id: I84bd3c8031580f805d5d4ef5d70f09b89e170450 --- .../google/gerrit/httpd/restapi/RestApiServlet.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 1279339215..45144d7179 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -103,6 +103,7 @@ import org.slf4j.LoggerFactory; import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.EOFException; +import java.io.FilterOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -740,10 +741,15 @@ public class RestApiServlet extends HttpServlet { b64 = new BinaryResult() { @Override public void writeTo(OutputStream out) throws IOException { - try (OutputStreamWriter w = new OutputStreamWriter(out, ISO_8859_1); + try (OutputStreamWriter w = new OutputStreamWriter( + new FilterOutputStream(out) { + @Override + public void close() { + // Do not close out, but only w and e. + } + }, ISO_8859_1); OutputStream e = BaseEncoding.base64().encodingStream(w)) { src.writeTo(e); - e.flush(); } } }; From 3b6c86cb621c694f6b67075be4ce3453eae6b9a6 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 25 Apr 2015 12:33:28 +0200 Subject: [PATCH 03/11] Hybrid OpenID/OAuth: Check for session validity during logout GitHub-Bug: https://github.com/davido/gerrit-oauth-provider/issues/9 Change-Id: I17aaed508ef61959a3fc5634d76eb5386305f9a0 --- .../httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java index 8ca71ff858..8fad0ad3c9 100644 --- a/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java +++ b/gerrit-openid/src/main/java/com/google/gerrit/httpd/auth/openid/OAuthOverOpenIDLogoutServlet.java @@ -52,6 +52,8 @@ class OAuthOverOpenIDLogoutServlet extends HttpLogoutServlet { protected void doLogout(HttpServletRequest req, HttpServletResponse rsp) throws IOException { super.doLogout(req, rsp); - oauthSession.get().logout(); + if (req.getSession(false) != null) { + oauthSession.get().logout(); + } } } From 18c4ccd2c335691eff1202a4facd5dde3adcf05f Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sun, 26 Apr 2015 18:52:45 +0200 Subject: [PATCH 04/11] Bump JGit version to 3.7.1.201504261725-r This version fixed JGit regression, causing severe (>10x) performance degradation on huge repositories (>2GB) on git push and CPU consumption explosion during replication: [1]. [1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=465509 Bug: Issue 3300 Change-Id: I6b1fa985fa3738801d3fa27d690a1c02c1afc1db --- lib/jgit/BUCK | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/jgit/BUCK b/lib/jgit/BUCK index a658fc36e7..ff7388868e 100644 --- a/lib/jgit/BUCK +++ b/lib/jgit/BUCK @@ -1,13 +1,13 @@ include_defs('//lib/maven.defs') -REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL. -VERS = '3.7.0.201502260915-r.58-g65c379e' +REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL. +VERS = '3.7.1.201504261725-r' maven_jar( name = 'jgit', id = 'org.eclipse.jgit:org.eclipse.jgit:' + VERS, - bin_sha1 = '8fc9620ec499169facad3355f7417eb6a8aff511', - src_sha1 = '40bd9ae8af8e0b03eb4e43f44f5feda8b7325221', + bin_sha1 = '28bae05826c1a34381826fb1d9ea5fd0ec47cc67', + src_sha1 = 'ef46620a8981d49eef2b27090ada4ca80a2b9766', license = 'jgit', repository = REPO, unsign = True, @@ -22,7 +22,7 @@ maven_jar( maven_jar( name = 'jgit-servlet', id = 'org.eclipse.jgit:org.eclipse.jgit.http.server:' + VERS, - sha1 = 'cecc2b9c0b94455348c3a0c63eb83f72cc595757', + sha1 = '874137fe9417c23cd0225ee6b86a13366b58b16e', license = 'jgit', repository = REPO, deps = [':jgit'], @@ -36,7 +36,7 @@ maven_jar( maven_jar( name = 'jgit-archive', id = 'org.eclipse.jgit:org.eclipse.jgit.archive:' + VERS, - sha1 = '7ccc7c78bf47566045ea7a3c08508ba18e4684ca', + sha1 = '8aab2d5d8deeaa8345d6df456de215b3d973c4b2', license = 'jgit', repository = REPO, deps = [':jgit', @@ -53,7 +53,7 @@ maven_jar( maven_jar( name = 'junit', id = 'org.eclipse.jgit:org.eclipse.jgit.junit:' + VERS, - sha1 = '87d64d722447dc3971ace30d2a72593c72a4d05f', + sha1 = '7ab2c3f5f40d07c84e7305b0fd60a8006257639c', license = 'DO_NOT_DISTRIBUTE', repository = REPO, unsign = True, From 25d42dd5f4e5aebae819c53ed4874a9915639187 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Thu, 23 Apr 2015 09:50:17 -0400 Subject: [PATCH 05/11] Fix slave cloning issue by binding Scanning rather than Searching /index Move ChangeCache implementation binding from GerritGlobalModule to new ChangeCacheImplModule. This is so that GerritGlobalModule no longer assumes a SearchingChangeCacheImpl. This was preventing slave gerrit servers from binding a ScanningChangeCacheImpl instead, as they do not rely on searching (an index); scanning, rather, relies on the git repo. Add new ChangeCacheImplModule usage to Daemon, so the latter can tell the former about gerrit server running as a slave or not. ChangeCacheImplModule can then install either the ScanningChangeCacheImpl or SearchingChangeCacheImpl module, which binds the corresponding ChangeCache implementation. Adapt InMemoryModule and WebAppInitializer accordingly. Fix ScanningChangeCacheImpl logger initialization. The Daemon non-slave case is already covered by existing IT testing. The slave case (involving a new branch in ChangeCacheImplModule) has to be tested manually; below bug/issue has a scenario. Now, ScanningChangeCacheImpl is already covered by IT. And, that slave case is exercised each time a slave gerrit server runs. InMemoryModule is exercised by existing unit testing; WebAppInitializer is used by web.xml. In both cases, the existing behavior was kept in the code. Bug: Issue 3323 Change-Id: Idfa255a5ef754979ca8289ae178e022c1e94be97 --- .../java/com/google/gerrit/pgm/Daemon.java | 2 + .../server/config/GerritGlobalModule.java | 3 -- .../server/git/ChangeCacheImplModule.java | 38 +++++++++++++++++++ .../server/git/ScanningChangeCacheImpl.java | 2 +- .../gerrit/testutil/InMemoryModule.java | 2 + .../gerrit/httpd/WebAppInitializer.java | 2 + 6 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCacheImplModule.java 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 ae29ff76f8..2b9af2f9a4 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 @@ -53,6 +53,7 @@ import com.google.gerrit.server.config.MasterNodeStartup; import com.google.gerrit.server.config.RestCacheAdminModule; import com.google.gerrit.server.contact.ContactStoreModule; import com.google.gerrit.server.contact.HttpContactStoreConnection; +import com.google.gerrit.server.git.ChangeCacheImplModule; import com.google.gerrit.server.git.GarbageCollectionModule; import com.google.gerrit.server.git.ReceiveCommitsExecutorModule; import com.google.gerrit.server.git.WorkQueue; @@ -321,6 +322,7 @@ public class Daemon extends SiteProgram { modules.add(new DiffExecutorModule()); modules.add(new MimeUtil2Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); + modules.add(new ChangeCacheImplModule(slave)); modules.add(new InternalAccountDirectory.Module()); modules.add(new DefaultCacheFactory.Module()); modules.add(new SmtpEmailSender.Module()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 91312b3c11..7bdccaa78c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -79,7 +79,6 @@ import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.NotesBranchUtil; import com.google.gerrit.server.git.ReceivePackInitializer; -import com.google.gerrit.server.git.SearchingChangeCacheImpl; import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.validators.CommitValidationListener; @@ -162,7 +161,6 @@ public class GerritGlobalModule extends FactoryModule { install(authModule); install(AccountByEmailCacheImpl.module()); install(AccountCacheImpl.module()); - install(SearchingChangeCacheImpl.module()); install(ChangeKindCacheImpl.module()); install(ConflictsCacheImpl.module()); install(GroupCacheImpl.module()); @@ -265,7 +263,6 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), ProjectDeletedListener.class); DynamicSet.setOf(binder(), HeadUpdatedListener.class); DynamicSet.setOf(binder(), UsageDataPublishedListener.class); - DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(SearchingChangeCacheImpl.class); DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ReindexAfterUpdate.class); DynamicSet.bind(binder(), GitReferenceUpdatedListener.class) .to(ProjectConfigEntry.UpdateChecker.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCacheImplModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCacheImplModule.java new file mode 100644 index 0000000000..90109a972d --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCacheImplModule.java @@ -0,0 +1,38 @@ +// 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.git; + +import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.inject.AbstractModule; + +public class ChangeCacheImplModule extends AbstractModule { + private final boolean slave; + + public ChangeCacheImplModule(boolean slave) { + this.slave = slave; + } + + @Override + protected void configure() { + if (slave) { + install(ScanningChangeCacheImpl.module()); + } else { + install(SearchingChangeCacheImpl.module()); + DynamicSet.bind(binder(), GitReferenceUpdatedListener.class) + .to(SearchingChangeCacheImpl.class); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java index 2c781335df..65808fcda2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java @@ -48,7 +48,7 @@ import java.util.concurrent.ExecutionException; @Singleton public class ScanningChangeCacheImpl implements ChangeCache { private static final Logger log = - LoggerFactory.getLogger(SearchingChangeCacheImpl.class); + LoggerFactory.getLogger(ScanningChangeCacheImpl.class); public static Module module() { return new CacheModule() { 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 dcfadd45ba..76af6f169c 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 @@ -41,6 +41,7 @@ import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFootersProvider; +import com.google.gerrit.server.git.ChangeCacheImplModule; import com.google.gerrit.server.git.EmailReviewCommentsExecutor; import com.google.gerrit.server.git.GarbageCollection; import com.google.gerrit.server.git.GitRepositoryManager; @@ -126,6 +127,7 @@ public class InMemoryModule extends FactoryModule { } }); install(cfgInjector.getInstance(GerritGlobalModule.class)); + install(new ChangeCacheImplModule(false)); factory(GarbageCollection.Factory.class); bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST); 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 99e545049d..4e2365cb87 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 @@ -39,6 +39,7 @@ import com.google.gerrit.server.config.RestCacheAdminModule; import com.google.gerrit.server.config.SitePath; import com.google.gerrit.server.contact.ContactStoreModule; import com.google.gerrit.server.contact.HttpContactStoreConnection; +import com.google.gerrit.server.git.ChangeCacheImplModule; import com.google.gerrit.server.git.GarbageCollectionModule; import com.google.gerrit.server.git.LocalDiskRepositoryManager; import com.google.gerrit.server.git.ReceiveCommitsExecutorModule; @@ -287,6 +288,7 @@ public class WebAppInitializer extends GuiceServletContextListener modules.add(new DiffExecutorModule()); modules.add(new MimeUtil2Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); + modules.add(new ChangeCacheImplModule(false)); modules.add(new InternalAccountDirectory.Module()); modules.add(new DefaultCacheFactory.Module()); modules.add(new SmtpEmailSender.Module()); From c869c232bedd916dc50f0d65b384348e177deccf Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 4 Mar 2015 14:52:04 +0900 Subject: [PATCH 06/11] Add support to consume CodeMirror from org.webjars on Maven Central CodeMirror is available as a JAR file on Maven Central under the org.webjars group ID. Add support in the download wrapper to consume CodeMirror from this location and extract from the JAR file rather than a ZIP file. The support to download as a ZIP file from the Gerrit bucket is kept for backward compatibility in case we want to use our own build of the library at any point in the future. Change-Id: Ie21b72b57fe1a90894c4aadd7ef4bdc91d055cd5 --- lib/codemirror/BUCK | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/codemirror/BUCK b/lib/codemirror/BUCK index dc163c2239..cfb43ff172 100644 --- a/lib/codemirror/BUCK +++ b/lib/codemirror/BUCK @@ -2,12 +2,19 @@ include_defs('//lib/maven.defs') include_defs('//lib/codemirror/cm.defs') include_defs('//lib/codemirror/closure.defs') +REPO = GERRIT VERSION = 'd0a2ddaa04' SHA1 = '1df573141fcceec039d0260d2d66a5b15d663f9a' -URL = GERRIT + 'net/codemirror/codemirror-%s.zip' % VERSION -ZIP = 'codemirror-%s.zip' % VERSION -TOP = 'codemirror-%s' % VERSION +if REPO == MAVEN_CENTRAL: + URL = REPO + 'org/webjars/codemirror/%s/codemirror-%s.jar' % (VERSION, VERSION) + TOP = 'META-INF/resources/webjars/codemirror/%s' % VERSION + ZIP = 'codemirror-%s.jar' % VERSION +else: + URL = REPO + 'net/codemirror/codemirror-%s.zip' % VERSION + TOP = 'codemirror-%s' % VERSION + ZIP = 'codemirror-%s.zip' % VERSION + CLOSURE_VERSION = 'v20141120' From 4ba5b80596277b5b9662db2df1b2eeaa121175c2 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 4 Mar 2015 14:57:22 +0900 Subject: [PATCH 07/11] Update CodeMirror to 5.0 and consume from Maven Central Change-Id: Iac85d62a70b6d831909c0b7fd73f6bb3b2e0a2da --- lib/codemirror/BUCK | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/codemirror/BUCK b/lib/codemirror/BUCK index cfb43ff172..4c235e4b6f 100644 --- a/lib/codemirror/BUCK +++ b/lib/codemirror/BUCK @@ -2,9 +2,9 @@ include_defs('//lib/maven.defs') include_defs('//lib/codemirror/cm.defs') include_defs('//lib/codemirror/closure.defs') -REPO = GERRIT -VERSION = 'd0a2ddaa04' -SHA1 = '1df573141fcceec039d0260d2d66a5b15d663f9a' +REPO = MAVEN_CENTRAL +VERSION = '5.0' +SHA1 = '24982be364be130fd7b2930c41f7203b63dbd86c' if REPO == MAVEN_CENTRAL: URL = REPO + 'org/webjars/codemirror/%s/codemirror-%s.jar' % (VERSION, VERSION) From bbd54fa065c18025a16a55e40aed2a14bd79b7c2 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 28 Apr 2015 14:54:27 +0900 Subject: [PATCH 08/11] H2: Don't show stack trace when failing to build BloomFilter Only show the exception message. Change-Id: I210ed24e5baaf4f7b6f62f10f5c0fa448e9079d8 --- .../java/com/google/gerrit/server/cache/h2/H2CacheImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java index 1aaebf5959..123bb9afd4 100644 --- a/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java +++ b/gerrit-cache-h2/src/main/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java @@ -388,7 +388,7 @@ public class H2CacheImpl extends AbstractLoadingCache implements return b; } } catch (SQLException e) { - log.warn("Cannot build BloomFilter for " + url, e); + log.warn("Cannot build BloomFilter for " + url + ": " + e.getMessage()); c = close(c); return null; } finally { From cb76c31af8da051966d02f079ffdb599db01798d Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 29 Apr 2015 08:58:37 +0200 Subject: [PATCH 09/11] Log IOExceptions on update of project configuration Without logging these exceptions it's hard to guess why the update of the project configuration is failing. Bug: issue 3342 Change-Id: I587e2bbee50f20833f9b820f31e5a12a378f59fb Signed-off-by: Edwin Kempin --- .../main/java/com/google/gerrit/server/project/PutConfig.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java index ca15287728..8c1fda718f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java @@ -191,6 +191,8 @@ public class PutConfig implements RestModifyView { throw new ResourceConflictException("Cannot update " + projectName + ": " + e.getCause().getMessage()); } else { + log.warn(String.format("Failed to update config of project %s.", + projectName), e); throw new ResourceConflictException("Cannot update " + projectName); } } From e4e4646a9d81ee7d10b601d0073c45e351954997 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 29 Apr 2015 14:30:22 +0200 Subject: [PATCH 10/11] Always show 'Not Current' as state when looking at old patch set E.g. for merged changes it is confusing for users to see the status as 'Merged' when they look at an old patch set. This may give the wrong impression that the old patch set that is currently viewed was merged. Bug: issue 3191 Change-Id: Ie7fde2f92e49cf798827d0b573dd6376d5c68a26 Signed-off-by: Edwin Kempin --- .../java/com/google/gerrit/client/change/ChangeScreen.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java index db7b485850..5922e7a694 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java @@ -1044,13 +1044,12 @@ public class ChangeScreen extends Screen { changeInfo = info; lastDisplayedUpdate = info.updated(); RevisionInfo revisionInfo = info.revision(revision); - boolean current = info.status().isOpen() - && revision.equals(info.current_revision()) + boolean current = revision.equals(info.current_revision()) && !revisionInfo.is_edit(); if (revisionInfo.is_edit()) { statusText.setInnerText(Util.C.changeEdit()); - } else if (!current && info.status() == Change.Status.NEW) { + } else if (!current) { statusText.setInnerText(Util.C.notCurrent()); labels.setVisible(false); } else { @@ -1104,7 +1103,7 @@ public class ChangeScreen extends Screen { } history.set(commentLinkProcessor, replyAction, changeId, info); - if (current) { + if (current && info.status().isOpen()) { quickApprove.set(info, revision, replyAction); loadSubmitType(info.status(), isSubmittable(info)); } else { From b3f31d8b6f4aaaabdbb29816680be897845c5f90 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 29 Apr 2015 14:24:23 +0200 Subject: [PATCH 11/11] Correct documentation of required capabilities for listing plugins Change-Id: Ib1a38e6514be179af46bf513cb2918140cdd6f3e Signed-off-by: Edwin Kempin --- Documentation/cmd-plugin-ls.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/cmd-plugin-ls.txt b/Documentation/cmd-plugin-ls.txt index 905c9aba2c..234ce8758b 100644 --- a/Documentation/cmd-plugin-ls.txt +++ b/Documentation/cmd-plugin-ls.txt @@ -14,9 +14,12 @@ plugin ls - List the installed plugins. List the installed plugins and show their version and status. == ACCESS -* Caller must be a member of the privileged 'Administrators' group. +* The caller must be a member of a group that is granted the + link:access-control.html#capability_viewPlugins[View Plugins] + capability or the link:access-control.html#capability_administrateServer[ + Administrate Server] capability. * link:config-gerrit.html#plugins.allowRemoteAdmin[plugins.allowRemoteAdmin] -must be enabled in `$site_path/etc/gerrit.config`. + must be enabled in `$site_path/etc/gerrit.config`. == SCRIPTING This command is intended to be used in scripts.