From 9c8791c8a5787a3e245075cd327672743cf65a51 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 12 Mar 2014 16:43:31 +0900 Subject: [PATCH 01/16] Don't add "Patch File" download link for merge commits The patch download does not work for commits with more than one parent. Bug: Issue 2538 Change-Id: I631500ab2b30c213f62d517cd492a1aca52f1211 --- .../java/com/google/gerrit/client/change/DownloadBox.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java index 7c57620ad4..dbc1319e23 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java @@ -108,7 +108,9 @@ class DownloadBox extends VerticalPanel { insertCommand(commandName, copyLabel); } } - insertPatch(); + if (change.revision(revision).commit().parents().length() == 1) { + insertPatch(); + } insertCommand(null, scheme); } From fc5a2b8ef45579945003517fe6c8d8180963e69b Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 11 Mar 2014 22:47:07 +0100 Subject: [PATCH 02/16] ChangeScreen2: Respect comment visibility strategy Bug: Issue 2456 Change-Id: I71f9ae2664a8f8c7f4eb8e019696764982781f58 --- .../gerrit/client/change/ChangeScreen2.java | 2 +- .../google/gerrit/client/change/History.java | 59 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java index 8069068d19..db3696f0ab 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java @@ -747,7 +747,7 @@ public class ChangeScreen2 extends Screen { related.set(info, revision); reviewers.set(info); quickApprove.set(info, revision); - history.set(commentLinkProcessor, changeId, info); + history.set(commentLinkProcessor, changeId, info, expandAll, collapseAll); if (Gerrit.isSignedIn()) { initEditMessageAction(info, revision); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java index e043e3c1ee..e167927016 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/History.java @@ -14,6 +14,10 @@ package com.google.gerrit.client.change; +import static com.google.gerrit.reviewdb.client.AccountGeneralPreferences.CommentVisibilityStrategy.COLLAPSE_ALL; +import static com.google.gerrit.reviewdb.client.AccountGeneralPreferences.CommentVisibilityStrategy.EXPAND_ALL; + +import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.account.AccountInfo; import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.changes.ChangeInfo; @@ -22,10 +26,12 @@ import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.rpc.NativeMap; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.ui.CommentLinkProcessor; +import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.CommentVisibilityStrategy; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.JsArray; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.Widget; @@ -39,6 +45,10 @@ import java.util.Map; import java.util.Set; class History extends FlowPanel { + // This basically means 7 days in milliseconds and could be rewritten as + // TimeUnit.DAYS.toMillis(7) + // in plain Java, but unfortunately it isn't available in GWT + private final static long AGE = 7 * 24 * 60 * 60 * 1000L; private CommentLinkProcessor clp; private Change.Id changeId; @@ -46,7 +56,8 @@ class History extends FlowPanel { private final Map> byAuthor = new HashMap>(); - void set(CommentLinkProcessor clp, Change.Id id, ChangeInfo info) { + void set(CommentLinkProcessor clp, Change.Id id, ChangeInfo info, + Button expandAll, Button collapseAll) { this.clp = clp; this.changeId = id; @@ -60,6 +71,7 @@ class History extends FlowPanel { add(ui); } } + initCommentVisibilityStrategy(expandAll, collapseAll); } CommentLinkProcessor getCommentLinkProcessor() { @@ -149,6 +161,51 @@ class History extends FlowPanel { return match; } + private void initCommentVisibilityStrategy(Button expandAll, Button collapseAll) { + CommentVisibilityStrategy commentVisibilityStrategy = + CommentVisibilityStrategy.EXPAND_RECENT; + if (Gerrit.isSignedIn()) { + commentVisibilityStrategy = Gerrit.getUserAccount() + .getGeneralPreferences().getCommentVisibilityStrategy(); + } + + Timestamp aged = new Timestamp(System.currentTimeMillis() - AGE); + + int n = getWidgetCount(); + for (int i = 0; i < n; i++) { + Message msg = (Message) getWidget(i); + + boolean isRecent = (i == n - 1) + ? true + : msg.getMessageInfo().date().after(aged); + + boolean isOpen = false; + switch (commentVisibilityStrategy) { + case COLLAPSE_ALL: + break; + case EXPAND_ALL: + isOpen = true; + break; + case EXPAND_MOST_RECENT: + isOpen = i == n - 1; + break; + case EXPAND_RECENT: + default: + isOpen = isRecent; + break; + } + msg.setOpen(isOpen); + } + + if (commentVisibilityStrategy == COLLAPSE_ALL) { + expandAll.setVisible(true); + collapseAll.setVisible(false); + } else if (commentVisibilityStrategy == EXPAND_ALL) { + expandAll.setVisible(false); + collapseAll.setVisible(true); + } + } + private static final class AuthorRevision { final int author; final int revision; From f56a8c687fef7396af2cca1cfef1b19e5ab0c773 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 25 Mar 2014 18:33:55 -0700 Subject: [PATCH 03/16] SideBySide2: Fix syntax highlighting for shell files Change-Id: I7b209eaeeb131d5e9c7a6d8417acca91bcb699cc (cherry picked from commit 866c8e92a402a28427097e17285f6c179c98933c) --- gerrit-gwtui/src/main/java/net/codemirror/mode/mode_map | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gerrit-gwtui/src/main/java/net/codemirror/mode/mode_map b/gerrit-gwtui/src/main/java/net/codemirror/mode/mode_map index 2d076cd0d7..e186b8ae0a 100644 --- a/gerrit-gwtui/src/main/java/net/codemirror/mode/mode_map +++ b/gerrit-gwtui/src/main/java/net/codemirror/mode/mode_map @@ -52,6 +52,7 @@ text/x-ruby shell: text/x-sh +application/x-shellscript sql: text/x-sql @@ -67,5 +68,6 @@ text/xml application/xml application/x-javascript = application/javascript +application/x-shellscript = text/x-sh text/x-h = text/x-c++hdr text/x-java-source = text/x-java From 3daa739e13447989d741a52d5ffe1ab94a984b9d Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Thu, 20 Mar 2014 16:48:55 -0400 Subject: [PATCH 04/16] Emit ref-updated event when editing project access via web UI Bug: Issue 2564 Change-Id: I9426f6969a234b081f72197ed0cb061512bc6e09 --- .../httpd/rpc/project/ChangeProjectAccess.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java index 0dd3d166ae..ececdcc267 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ChangeProjectAccess.java @@ -14,11 +14,15 @@ package com.google.gerrit.httpd.rpc.project; +import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.ProjectAccess; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.project.NoSuchProjectException; @@ -29,6 +33,7 @@ import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.revwalk.RevCommit; import java.io.IOException; import java.util.List; @@ -41,6 +46,8 @@ class ChangeProjectAccess extends ProjectAccessHandler { @Nullable @Assisted String message); } + private final ChangeHooks hooks; + private final IdentifiedUser user; private final ProjectAccessFactory.Factory projectAccessFactory; private final ProjectCache projectCache; @@ -49,6 +56,7 @@ class ChangeProjectAccess extends ProjectAccessHandler { final ProjectControl.Factory projectControlFactory, final ProjectCache projectCache, final GroupBackend groupBackend, final MetaDataUpdate.User metaDataUpdateFactory, + ChangeHooks hooks, IdentifiedUser user, @Assisted final Project.NameKey projectName, @Nullable @Assisted final ObjectId base, @@ -58,12 +66,19 @@ class ChangeProjectAccess extends ProjectAccessHandler { projectName, base, sectionList, message, true); this.projectAccessFactory = projectAccessFactory; this.projectCache = projectCache; + this.hooks = hooks; + this.user = user; } @Override protected ProjectAccess updateProjectConfig(ProjectConfig config, MetaDataUpdate md) throws IOException, NoSuchProjectException, ConfigInvalidException { - config.commit(md); + RevCommit commit = config.commit(md); + + hooks.doRefUpdatedHook( + new Branch.NameKey(config.getProject().getNameKey(), GitRepositoryManager.REF_CONFIG), + base, commit.getId(), user.getAccount()); + projectCache.evict(config.getProject()); return projectAccessFactory.create(projectName).call(); } From af058e6dd384eaa6f968dfe72925004e58719b1d Mon Sep 17 00:00:00 2001 From: Bruce Zu Date: Fri, 21 Mar 2014 10:03:05 +0800 Subject: [PATCH 05/16] Disable commitWithin when running Reindex Disable the 'commitWithin' from within Reindex by overriding the configuration with '-1'. Treat negative values as the original behavior, auto-flushing but not auto-committing, which is the least safe but the most efficient for reindexing the entire site. Change-Id: Ifdba797bee871d2a3d8928810a6304bacb850c8c --- Documentation/config-gerrit.txt | 18 +++-- .../gerrit/lucene/LuceneChangeIndex.java | 12 ++-- .../com/google/gerrit/lucene/SubIndex.java | 68 +++++++++---------- .../java/com/google/gerrit/pgm/Reindex.java | 10 +++ .../gerrit/server/config/ConfigUtil.java | 4 ++ 5 files changed, 64 insertions(+), 48 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index a34fa848f7..c5ef758ee0 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -1894,18 +1894,16 @@ Determines the period at which changes are automatically committed to stable store on disk. This is a costly operation and may block additional index writes, so lower with caution. + -If zero or negative, changes are committed after every write. This is -very costly but may be useful if offline reindexing is infeasible, or -for development servers. +If zero, changes are committed after every write. This is very costly +but may be useful if offline reindexing is infeasible, or for development +servers. + -Values can be specified using standard time unit abbreviations (`ms`, -`sec`, `min`, etc.). +Values can be specified using standard time unit abbreviations (`ms`, `sec`, +`min`, etc.). + -This setting also applies when running the reindex program. If it is -configured to commit on every write, this will cause reindex to take -an unnecessarily long time to complete on sites that have a lot of -changes. It is recommended to temporarily set a higher value while -running reindex. +If negative, `commitWithin` is disabled. Changes are flushed to disk when +the in-memory buffer fills, but only committed and guaranteed to be synced +to disk when the process finishes. Defaults to 300000 ms (5 minutes). diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 5ce591fffb..a434ac9b0f 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -137,7 +137,7 @@ public class LuceneChangeIndex implements ChangeIndex { static class GerritIndexWriterConfig { private final IndexWriterConfig luceneConfig; - private final long commitWithinMs; + private long commitWithinMs; private GerritIndexWriterConfig(Version version, Config cfg, String name) { luceneConfig = new IndexWriterConfig(version, @@ -150,9 +150,13 @@ public class LuceneChangeIndex implements ChangeIndex { luceneConfig.setMaxBufferedDocs(cfg.getInt( "index", name, "maxBufferedDocs", IndexWriterConfig.DEFAULT_MAX_BUFFERED_DOCS)); - commitWithinMs = ConfigUtil.getTimeUnit( - cfg, "index", name, "commitWithin", - MILLISECONDS.convert(5, MINUTES), MILLISECONDS); + try { + commitWithinMs = + ConfigUtil.getTimeUnit(cfg, "index", name, "commitWithin", + MILLISECONDS.convert(5, MINUTES), MILLISECONDS); + } catch (IllegalArgumentException e) { + commitWithinMs = cfg.getLong("index", name, "commitWithin", 0); + } } IndexWriterConfig getLuceneConfig() { diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java index 34eb2de88a..d9f7fd900e 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java @@ -23,6 +23,7 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.gerrit.lucene.LuceneChangeIndex.GerritIndexWriterConfig; import org.apache.lucene.document.Document; +import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.Term; import org.apache.lucene.index.TrackingIndexWriter; import org.apache.lucene.search.ControlledRealTimeReopenThread; @@ -40,7 +41,6 @@ import java.io.IOException; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -55,7 +55,6 @@ class SubIndex { private final SearcherManager searcherManager; private final ControlledRealTimeReopenThread reopenThread; private final ConcurrentMap refreshListeners; - private final ScheduledExecutorService commitExecutor; SubIndex(File file, GerritIndexWriterConfig writerConfig) throws IOException { this(FSDirectory.open(file), file.getName(), writerConfig); @@ -64,44 +63,45 @@ class SubIndex { SubIndex(Directory dir, final String dirName, GerritIndexWriterConfig writerConfig) throws IOException { this.dir = dir; - - final AutoCommitWriter delegateWriter; + IndexWriter delegateWriter; long commitPeriod = writerConfig.getCommitWithinMs(); - if (commitPeriod <= 0) { - commitExecutor = null; + + if (commitPeriod < 0) { + delegateWriter = new IndexWriter(dir, writerConfig.getLuceneConfig()); + } else if (commitPeriod == 0) { delegateWriter = new AutoCommitWriter(dir, writerConfig.getLuceneConfig(), true); } else { - commitExecutor = new ScheduledThreadPoolExecutor(1, - new ThreadFactoryBuilder() - .setNameFormat("Commit-%d " + dirName) - .setDaemon(true) - .build()); - delegateWriter = + final AutoCommitWriter autoCommitWriter = new AutoCommitWriter(dir, writerConfig.getLuceneConfig(), false); - commitExecutor.scheduleAtFixedRate(new Runnable() { - @Override - public void run() { - try { - if (delegateWriter.hasUncommittedChanges()) { - delegateWriter.manualFlush(); - delegateWriter.commit(); - } - } catch (IOException e) { - log.error("Error committing Lucene index " + dirName, e); - } catch (OutOfMemoryError e) { - log.error("Error committing Lucene index " + dirName, e); - try { - delegateWriter.close(); - } catch (IOException e2) { - log.error("SEVERE: Error closing Lucene index " - + dirName + " after OOM; index may be corrupted.", e); - } - } - } - }, commitPeriod, commitPeriod, MILLISECONDS); - } + delegateWriter = autoCommitWriter; + new ScheduledThreadPoolExecutor(1, new ThreadFactoryBuilder() + .setNameFormat("Commit-%d " + dirName) + .setDaemon(true) + .build()) + .scheduleAtFixedRate(new Runnable() { + @Override + public void run() { + try { + if (autoCommitWriter.hasUncommittedChanges()) { + autoCommitWriter.manualFlush(); + autoCommitWriter.commit(); + } + } catch (IOException e) { + log.error("Error committing Lucene index " + dirName, e); + } catch (OutOfMemoryError e) { + log.error("Error committing Lucene index " + dirName, e); + try { + autoCommitWriter.close(); + } catch (IOException e2) { + log.error("SEVERE: Error closing Lucene index " + dirName + + " after OOM; index may be corrupted.", e); + } + } + } + }, commitPeriod, commitPeriod, MILLISECONDS); + } writer = new TrackingIndexWriter(delegateWriter); searcherManager = new SearcherManager( writer.getIndexWriter(), true, new SearcherFactory()); 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 e39dd3b5c6..b67ac878a0 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 @@ -95,6 +95,7 @@ public class Reindex extends SiteProgram { throw die("index.type must be configured (or not SQL)"); } limitThreads(); + disableLuceneAutomaticCommit(); if (version == null) { version = ChangeSchemas.getLatest().getVersion(); } @@ -162,6 +163,15 @@ public class Reindex extends SiteProgram { return dbInjector.createChildInjector(modules); } + private void disableLuceneAutomaticCommit() { + Config cfg = + dbInjector.getInstance(Key.get(Config.class, GerritServerConfig.class)); + if (IndexModule.getIndexType(dbInjector) == IndexType.LUCENE) { + cfg.setLong("index", "changes_open", "commitWithin", -1); + cfg.setLong("index", "changes_closed", "commitWithin", -1); + } + } + private class ReviewDbModule extends LifecycleModule { @Override protected void configure() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java index 95c1500a1d..e081b02afd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java @@ -207,6 +207,10 @@ public class ConfigUtil { return defaultValue; } + if (s.startsWith("-")/* negative */) { + throw notTimeUnit(section, subsection, setting, valueString); + } + try { return getTimeUnit(s, defaultValue, wantUnit); } catch (IllegalArgumentException notTime) { From 59e21083116ae159836067f1b65f3be26aef84e7 Mon Sep 17 00:00:00 2001 From: Lewis Diamond Date: Tue, 18 Mar 2014 10:14:37 -0400 Subject: [PATCH 06/16] By default don't allow admins to create new branches by push When pushing changes it is easy to make a typo in the refspec and in this case new branches should not be created. If administrators want to create branches by push they should explicitly assign themselves the needed access rights. This was broken by [1]. [1] https://gerrit-review.googlesource.com/#/c/54048/ Change-Id: I768bfd6d7cb193b0ec9f51c3fa768087ceb91acd Bug: Issue 2557 --- .../java/com/google/gerrit/server/project/RefControl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index 589b36fd0f..6b1073d05d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -237,18 +237,21 @@ public class RefControl { return false; } boolean owner; + boolean admin; switch (getCurrentUser().getAccessPath()) { case REST_API: case JSON_RPC: owner = isOwner(); + admin = getCurrentUser().getCapabilities().canAdministrateServer(); break; default: owner = false; + admin = false; } if (object instanceof RevCommit) { - return getCurrentUser().getCapabilities().canAdministrateServer() + return admin || (owner && !isBlocked(Permission.CREATE)) || (canPerform(Permission.CREATE) && (!existsOnServer && canUpdate() || projectControl .canReadCommit(rw, (RevCommit) object))); From 3374a0bd634047287b5cf050094fec529d7303fb Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Tue, 18 Feb 2014 11:05:07 +0100 Subject: [PATCH 07/16] Update the mysql documentation concerning charsets This commit changes the mysql setup documentation because there is no need to use latin1 encoding if you are using another engine than the MyISAM engine. Bug: Issue 1273 Change-Id: I432957381385c3e0390bb0db245139954300ecb8 (cherry picked from commit 13d07ecc7cd98cfd928a2b06d8755aea1fddde61) --- Documentation/database-setup.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/database-setup.txt b/Documentation/database-setup.txt index 3800473b7e..2016ac3363 100644 --- a/Documentation/database-setup.txt +++ b/Documentation/database-setup.txt @@ -56,11 +56,18 @@ rights on it: CREATE USER 'gerrit2'@'localhost' IDENTIFIED BY 'secret'; CREATE DATABASE reviewdb; - ALTER DATABASE reviewdb charset=latin1; GRANT ALL ON reviewdb.* TO 'gerrit2'@'localhost'; FLUSH PRIVILEGES; ---- +If you are using the MyISAM engine, you need to set latin1 as charset: + +---- + mysql + + ALTER DATABASE reviewdb charset=latin1; +---- + Visit MySQL's link:http://dev.mysql.com/doc/[documentation] for further information regarding using MySQL. From 0337f85377953031c5e5236c41b2ccbb00f2d43e Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Mar 2014 12:29:38 -0700 Subject: [PATCH 08/16] Bump GERRIT_VERSION to 2.8.4 Change-Id: I3d1300a0d87e9ca59d971cc20f162d9f9eb8237f --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index e6ab10bb7c..b647a45d16 100644 --- a/VERSION +++ b/VERSION @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = '2.8.3' +GERRIT_VERSION = '2.8.4' From b547ca9d7b18f50d4c589b076f8be2a89d50efaa Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 26 Mar 2014 17:54:39 -0700 Subject: [PATCH 09/16] Serialize GWT dbg and opt compiles The GWT compiler swamps a system when it builds, using multiple threads to build each permutation in parallel. It also requires a lot of RAM. Buck is not aware of the cost of the genrule and currently schedules both to build at the same time. Make ui_opt depend on ui_dbg so Buck is forced to serialize these build steps, reducing the load on average systems while possibly extending build time on very powerful (e.g. 16 core) systems. Change-Id: Icc1856c9c4e7919c28dc533e792fa25aadcdf801 --- gerrit-gwtui/BUCK | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gerrit-gwtui/BUCK b/gerrit-gwtui/BUCK index 1ae823e24d..6969bad8cc 100644 --- a/gerrit-gwtui/BUCK +++ b/gerrit-gwtui/BUCK @@ -32,7 +32,7 @@ gwt_application( '-XdisableClassMetadata', '-XdisableCastChecking', ], - deps = APP_DEPS, + deps = APP_DEPS + [':ui_dbg'], ) gwt_application( From 9a13f852f04b956f1d8b51526732d187258ef58b Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Mar 2014 14:45:02 -0700 Subject: [PATCH 10/16] Helper script to update API version in plugin archetype pom files Running the script in the root of the Gerrit folder: ./tools/version NEW_VERSION will replace the value in the first occurrence of the tag in the plugin archetype pom files with the value specified by NEW_VERSION. Change-Id: I9f529852af22a1eef7f30e2fcaea8acea28057d9 --- tools/version.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100755 tools/version.py diff --git a/tools/version.py b/tools/version.py new file mode 100755 index 0000000000..60489c8751 --- /dev/null +++ b/tools/version.py @@ -0,0 +1,49 @@ +#!/usr/bin/python +# Copyright (C) 2014 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. + +from __future__ import print_function +from optparse import OptionParser +import os.path +import re +import sys + +parser = OptionParser() +opts, args = parser.parse_args() + +if not len(args): + parser.error('not enough arguments') +elif len(args) > 1: + parser.error('too many arguments') + +new_version = args[0] +pattern = re.compile(r'(\s*)[-.\w]+') + +for project in ['gerrit-plugin-archetype', 'gerrit-plugin-gwt-archetype', + 'gerrit-plugin-gwtui', 'gerrit-plugin-js-archetype']: + pom = os.path.join(project, 'pom.xml') + try: + outxml = "" + found = False + for line in open(pom, "r"): + m = pattern.match(line) + if m and not found: + outxml += "%s%s\n" % (m.group(1), new_version) + found = True + else: + outxml += line + with open(pom, "w") as outfile: + outfile.write(outxml) + except IOError as err: + print('error updating %s: %s' % (pom, err), file=sys.stderr) From c06a36c1bf1f2a855a796203152ad94eab723e1a Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Mar 2014 12:34:10 -0700 Subject: [PATCH 11/16] Bump version to 2.8.4 in plugin API and archetypes Change-Id: Icb5153289195b6ea8539f21ef89e0a33c34b37de --- gerrit-plugin-archetype/pom.xml | 2 +- gerrit-plugin-gwt-archetype/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-plugin-js-archetype/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gerrit-plugin-archetype/pom.xml b/gerrit-plugin-archetype/pom.xml index ae35afd3ea..80afa568f9 100644 --- a/gerrit-plugin-archetype/pom.xml +++ b/gerrit-plugin-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-archetype - 2.8.3 + 2.8.4 Gerrit Code Review - Plugin Archetype diff --git a/gerrit-plugin-gwt-archetype/pom.xml b/gerrit-plugin-gwt-archetype/pom.xml index 251278b73a..7a49b5638a 100644 --- a/gerrit-plugin-gwt-archetype/pom.xml +++ b/gerrit-plugin-gwt-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-gwt-archetype - 2.8.3 + 2.8.4 Gerrit Code Review - Web Ui GWT Plugin Archetype diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index d6cb626fa6..5e3b9ea57b 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -21,7 +21,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-gwtui - 2.8.3 + 2.8.4 Gerrit Code Review - Plugin GWT UI diff --git a/gerrit-plugin-js-archetype/pom.xml b/gerrit-plugin-js-archetype/pom.xml index 055150d5e5..14a62860f5 100644 --- a/gerrit-plugin-js-archetype/pom.xml +++ b/gerrit-plugin-js-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-js-archetype - 2.8.3 + 2.8.4 Gerrit Code Review - Web UI JavaScript Plugin Archetype From 48c1f20ef7c97032d48d1c7b74296db3685fad63 Mon Sep 17 00:00:00 2001 From: Dariusz Luksza Date: Tue, 25 Mar 2014 14:30:55 +0100 Subject: [PATCH 12/16] Make skip bar more user friendly It is odd to put block expansion link only on number of skipped lines. This change puts link on whole sentence "... skipped XX common lines ...". Change-Id: I5c48a26fa7fa8758e35386994b80ef741e3ffa50 Signed-off-by: Dariusz Luksza --- .../main/java/com/google/gerrit/client/diff/SkipBar.java | 6 ++++-- .../main/java/com/google/gerrit/client/diff/SkipBar.ui.xml | 2 -- .../com/google/gerrit/client/patches/PatchMessages.java | 1 + .../google/gerrit/client/patches/PatchMessages.properties | 1 + .../gerrit/client/patches/PatchMessages_en.properties | 1 + 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java index cc8ac2e70e..9da142efe8 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java @@ -97,11 +97,12 @@ class SkipBar extends Composite { void setMarker(TextMarker marker, int length) { this.marker = marker; numSkipLines = length; - skipNum.setText(Integer.toString(length)); if (checkAndUpdateArrows()) { upArrow.setHTML(PatchUtil.M.expandBefore(NUM_ROWS_TO_EXPAND)); downArrow.setHTML(PatchUtil.M.expandAfter(NUM_ROWS_TO_EXPAND)); } + skipNum.setText(PatchUtil.M.patchSkipRegion(Integer + .toString(length))); } static void link(SkipBar barA, SkipBar barB) { @@ -111,7 +112,8 @@ class SkipBar extends Composite { private void updateSkipNum() { numSkipLines -= NUM_ROWS_TO_EXPAND; - skipNum.setText(String.valueOf(numSkipLines)); + skipNum.setText(PatchUtil.M.patchSkipRegion(Integer + .toString(numSkipLines))); checkAndUpdateArrows(); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml index f5c6719d39..0ff23a74c0 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml @@ -44,9 +44,7 @@ limitations under the License.
- ... skipped - common lines ...
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.java index 18a358a934..4046fecc47 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.java @@ -22,4 +22,5 @@ public interface PatchMessages extends Messages { String expandBefore(int cnt); String expandAfter(int cnt); String draftSaved(Date when); + String patchSkipRegion(String lineNumber); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.properties index 2d01e24714..076ab5f333 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.properties @@ -1,3 +1,4 @@ expandBefore = +{0}⇧ expandAfter = +{0}⇩ draftSaved = Draft saved at {0,time,short} +patchSkipRegion = ... skipped {0} common lines ... diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages_en.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages_en.properties index 2f3c20a69a..e24333e78e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages_en.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages_en.properties @@ -1,2 +1,3 @@ expandBefore = +{0}⇧ expandAfter = +{0}⇩ +patchSkipRegion = ... skipped {0} common lines ... From a8861d04f9caa178c80c2ac1f0c47c9895677a56 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Mar 2014 22:22:40 -0700 Subject: [PATCH 13/16] Use consistent grammatical tense in command descriptions Change-Id: Iee7f99ec9f133b1e7cfb73a29e90cc2f28ac75ca --- .../com/google/gerrit/sshd/commands/ListMembersCommand.java | 2 +- .../java/com/google/gerrit/sshd/commands/SetMembersCommand.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java index b7dd380e22..7d34b1aa28 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListMembersCommand.java @@ -39,7 +39,7 @@ import javax.inject.Inject; /** * Implements a command that allows the user to see the members of a group. */ -@CommandMetaData(name = "ls-members", description = "Lists the members of a given group") +@CommandMetaData(name = "ls-members", description = "List the members of a given group") public class ListMembersCommand extends BaseCommand { @Inject ListMembersCommandImpl impl; diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java index 48c37b8edd..cc9e6edd4b 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java @@ -43,7 +43,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.util.List; -@CommandMetaData(name = "set-members", description = "Modifies members of specific group or number of groups") +@CommandMetaData(name = "set-members", description = "Modify members of specific group or number of groups") public class SetMembersCommand extends SshCommand { @Option(name = "--add", aliases = {"-a"}, metaVar = "USER", usage = "users that should be added as group member") From a7e343131777750d11c12d06354e52aaae9badc5 Mon Sep 17 00:00:00 2001 From: Bruce Zu Date: Tue, 1 Apr 2014 17:35:41 +0800 Subject: [PATCH 14/16] Fix: The email notification of review comments gets stuck. Sometimes it is found that one thread goes stuck when waiting for an answer from the SMTP server. Fixed. Enable user to -config the timeout value of opening a socket connected to a remote SMTP server. -config the worker-thread pool size of executor used for sending out review comments notification when it is not enough to dedicate only one thread. -config the default size of the background execution thread pool when one thread is not enough to handle miscellaneous tasks including sending out every kind email notification. Change-Id: Id8177b374f7049cfac617c50e66b2c83ae71641b --- Documentation/config-gerrit.txt | 30 +++++++++++++++++++ .../server/change/EmailReviewComments.java | 11 +++---- .../git/EmailReviewCommentsExecutor.java | 30 +++++++++++++++++++ .../git/ReceiveCommitsExecutorModule.java | 9 ++++++ .../google/gerrit/server/git/WorkQueue.java | 8 +++-- .../gerrit/server/mail/SmtpEmailSender.java | 11 +++++++ 6 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index c5ef758ee0..038d946e05 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2412,6 +2412,16 @@ only the default internal rules will be used. + Default is true, to execute project specific rules. +[[execution]]Section execution +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +[[execution.defaultThreadPoolSize]]execution.defaultThreadPoolSize:: ++ +The default size of the background execution thread pool in +which miscellaneous tasks are handled. ++ +Default is 1. + [[sendemail]]Section sendemail ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -2422,6 +2432,26 @@ and all other properties of section sendemail are ignored. + By default, true, allowing notifications to be sent. +[[sendemail.connectTimeout]]sendemail.connectTimeout:: ++ +The connection timeout of opening a socket connected to a +remote SMTP server. ++ +Values can be specified using standard time unit abbreviations +('ms', 'sec', 'min', etc.). +If no unit is specified, milliseconds is assumed. ++ +Default is 0. A timeout of zero is interpreted as an infinite +timeout. The connection will then block until established or +an error occurs. + +[[sendemail.threadPoolSize]]sendemail.threadPoolSize:: ++ +Maximum size of thread pool in which the review comments +notifications are sent out asynchronously. ++ +By default, 1. + [[sendemail.from]]sendemail.from:: + Designates what name and address Gerrit will place in the From diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java index 11a9eddc7c..eea2f4827c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java @@ -23,7 +23,8 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.PostReview.NotifyHandling; -import com.google.gerrit.server.git.WorkQueue; +import com.google.gerrit.server.git.EmailReviewCommentsExecutor; +import com.google.gerrit.server.git.WorkQueue.Executor; import com.google.gerrit.server.mail.CommentSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.util.RequestContext; @@ -55,7 +56,7 @@ class EmailReviewComments implements Runnable, RequestContext { List comments); } - private final WorkQueue workQueue; + private final Executor sendEmailsExecutor; private final PatchSetInfoFactory patchSetInfoFactory; private final CommentSender.Factory commentSenderFactory; private final SchemaFactory schemaFactory; @@ -71,7 +72,7 @@ class EmailReviewComments implements Runnable, RequestContext { @Inject EmailReviewComments ( - WorkQueue workQueue, + @EmailReviewCommentsExecutor final Executor executor, PatchSetInfoFactory patchSetInfoFactory, CommentSender.Factory commentSenderFactory, SchemaFactory schemaFactory, @@ -82,7 +83,7 @@ class EmailReviewComments implements Runnable, RequestContext { @Assisted Account.Id authorId, @Assisted ChangeMessage message, @Assisted List comments) { - this.workQueue = workQueue; + this.sendEmailsExecutor = executor; this.patchSetInfoFactory = patchSetInfoFactory; this.commentSenderFactory = commentSenderFactory; this.schemaFactory = schemaFactory; @@ -96,7 +97,7 @@ class EmailReviewComments implements Runnable, RequestContext { } void sendAsync() { - workQueue.getDefaultQueue().submit(this); + sendEmailsExecutor.submit(this); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java new file mode 100644 index 0000000000..1581ec48d1 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailReviewCommentsExecutor.java @@ -0,0 +1,30 @@ +// Copyright (C) 2014 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 static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.inject.BindingAnnotation; + +import java.lang.annotation.Retention; + +/** + * Marker on the global {@link WorkQueue.Executor} used by + * {@link EmailReviewComments}. + */ +@Retention(RUNTIME) +@BindingAnnotation +public @interface EmailReviewCommentsExecutor { +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java index 1cbd227c4c..81ce05d8b4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsExecutorModule.java @@ -45,6 +45,15 @@ public class ReceiveCommitsExecutorModule extends AbstractModule { return queues.createQueue(poolSize, "ReceiveCommits"); } + @Provides + @Singleton + @EmailReviewCommentsExecutor + public WorkQueue.Executor createEmailReviewCommentsExecutor( + @GerritServerConfig Config config, WorkQueue queues) { + int poolSize = config.getInt("sendemail", null, "threadPoolSize", 1); + return queues.createQueue(poolSize, "EmailReviewComments"); + } + @Provides @Singleton @ChangeUpdateExecutor diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java index 90c5335378..d7a2cf4d4c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/WorkQueue.java @@ -18,10 +18,12 @@ import com.google.common.collect.Lists; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.reviewdb.client.Project.NameKey; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.util.IdGenerator; import com.google.inject.Inject; import com.google.inject.Singleton; +import org.eclipse.jgit.lib.Config; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,19 +85,21 @@ public class WorkQueue { }; private Executor defaultQueue; + private int defaultQueueSize; private final IdGenerator idGenerator; private final CopyOnWriteArrayList queues; @Inject - WorkQueue(final IdGenerator idGenerator) { + WorkQueue(final IdGenerator idGenerator, @GerritServerConfig final Config cfg) { this.idGenerator = idGenerator; this.queues = new CopyOnWriteArrayList(); + defaultQueueSize = cfg.getInt("execution", "defaultThreadPoolSize", 1); } /** Get the default work queue, for miscellaneous tasks. */ public synchronized Executor getDefaultQueue() { if (defaultQueue == null) { - defaultQueue = createQueue(1, "WorkQueue"); + defaultQueue = createQueue(defaultQueueSize, "WorkQueue"); } return defaultQueue; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java index 89fc6b98cd..48e2d035d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.mail; +import com.google.common.primitives.Ints; import com.google.gerrit.common.Version; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.server.config.ConfigUtil; @@ -39,10 +40,14 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; /** Sends email via a nearby SMTP server. */ @Singleton public class SmtpEmailSender implements EmailSender { + /** The socket's connect timeout (0 = infinite timeout) */ + private static final int DEFAULT_CONNECT_TIMEOUT = 0; + public static class Module extends AbstractModule { @Override protected void configure() { @@ -55,6 +60,7 @@ public class SmtpEmailSender implements EmailSender { } private final boolean enabled; + private final int connectTimeout; private String smtpHost; private int smtpPort; @@ -69,6 +75,10 @@ public class SmtpEmailSender implements EmailSender { @Inject SmtpEmailSender(@GerritServerConfig final Config cfg) { enabled = cfg.getBoolean("sendemail", null, "enable", true); + connectTimeout = + Ints.checkedCast(ConfigUtil.getTimeUnit(cfg, "sendemail", null, + "connectTimeout", DEFAULT_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS)); + smtpHost = cfg.getString("sendemail", null, "smtpserver"); if (smtpHost == null) { @@ -239,6 +249,7 @@ public class SmtpEmailSender implements EmailSender { } try { + client.setConnectTimeout(connectTimeout); client.connect(smtpHost, smtpPort); if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) { throw new EmailException("SMTP server rejected connection"); From 483f12b8dc16fee4adfd53ab206bc56c96b9876e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Ar=C3=A8s?= Date: Mon, 31 Mar 2014 09:25:22 -0400 Subject: [PATCH 15/16] Fix ChangeListener auto-registered implementations Add missing @ExtensionPoint in ChangeListener so implementors can use @Listen to register. Change-Id: Ia7fd16a7afdffbfc278880317fd10297f8b9e19d (cherry picked from commit eff0592a70ce7fb3dee91abd5262dd462339591e) --- .../src/main/java/com/google/gerrit/common/ChangeListener.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeListener.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeListener.java index 65a9857e9d..b34305ec34 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeListener.java @@ -14,9 +14,10 @@ package com.google.gerrit.common; +import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.server.events.ChangeEvent; - +@ExtensionPoint public interface ChangeListener { public void onChangeEvent(ChangeEvent event); } From 701218b8d045a0a6dc1a92a857ec7498db063479 Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Wed, 26 Mar 2014 15:52:14 -0400 Subject: [PATCH 16/16] Emit ref-updated event when editing project access via web UI 3daa739e13 took care of firing the ref-updated event for updates via the web UI access pane, but not changes via the web UI general pane, which uses the REST 'PUT /projects/:project-name/config' endpoint. This commit takes care of that case, as well as the REST 'PUT /projects/:project-name/description' endpoint. Note: there's still no ref-updated event for the case of changing a project from "hidden" state back to read-only or "active" as hooks are suppressed for hidden projects deeper in the code. Bug: Issue 2571 Change-Id: I0a260011c0968193ccb5900a18935e654d25f042 --- .../gerrit/server/project/PutConfig.java | 19 ++++++++++++++++++- .../gerrit/server/project/PutDescription.java | 15 ++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) 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 cb25af31c9..4d17627163 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 @@ -14,17 +14,22 @@ package com.google.gerrit.server.project; +import com.google.common.base.Objects; import com.google.common.base.Strings; +import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestView; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project.InheritableBoolean; import com.google.gerrit.reviewdb.client.Project.SubmitType; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.TransferConfig; @@ -34,6 +39,7 @@ import com.google.inject.Provider; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.lib.ObjectId; import java.io.IOException; @@ -56,6 +62,7 @@ public class PutConfig implements RestModifyView { private final TransferConfig config; private final DynamicMap> views; private final Provider currentUser; + private final ChangeHooks hooks; @Inject PutConfig(MetaDataUpdate.User metaDataUpdateFactory, @@ -64,6 +71,7 @@ public class PutConfig implements RestModifyView { ProjectState.Factory projectStateFactory, TransferConfig config, DynamicMap> views, + ChangeHooks hooks, Provider currentUser) { this.metaDataUpdateFactory = metaDataUpdateFactory; this.projectCache = projectCache; @@ -71,6 +79,7 @@ public class PutConfig implements RestModifyView { this.projectStateFactory = projectStateFactory; this.config = config; this.views = views; + this.hooks = hooks; this.currentUser = currentUser; } @@ -128,7 +137,15 @@ public class PutConfig implements RestModifyView { md.setMessage("Modified project settings\n"); try { - projectConfig.commit(md); + ObjectId baseRev = projectConfig.getRevision(); + ObjectId commitRev = projectConfig.commit(md); + // Only fire hook if project was actually changed. + if (!Objects.equal(baseRev, commitRev)) { + IdentifiedUser user = (IdentifiedUser) currentUser.get(); + hooks.doRefUpdatedHook( + new Branch.NameKey(projectName, GitRepositoryManager.REF_CONFIG), + baseRev, commitRev, user.getAccount()); + }; (new PerRequestProjectControlCache(projectCache, self.get())) .evict(projectConfig.getProject()); } catch (IOException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java index f7dd3cb4ec..e794678e87 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutDescription.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.project; import com.google.common.base.Objects; import com.google.common.base.Strings; +import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.DefaultInput; @@ -23,6 +24,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.GitRepositoryManager; @@ -33,6 +35,7 @@ import com.google.inject.Inject; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.lib.ObjectId; import java.io.IOException; @@ -46,13 +49,16 @@ class PutDescription implements RestModifyView { private final ProjectCache cache; private final MetaDataUpdate.Server updateFactory; private final GitRepositoryManager gitMgr; + private final ChangeHooks hooks; @Inject PutDescription(ProjectCache cache, MetaDataUpdate.Server updateFactory, + ChangeHooks hooks, GitRepositoryManager gitMgr) { this.cache = cache; this.updateFactory = updateFactory; + this.hooks = hooks; this.gitMgr = gitMgr; } @@ -85,7 +91,14 @@ class PutDescription implements RestModifyView { } md.setAuthor(user); md.setMessage(msg); - config.commit(md); + ObjectId baseRev = config.getRevision(); + ObjectId commitRev = config.commit(md); + // Only fire hook if project was actually changed. + if (!Objects.equal(baseRev, commitRev)) { + hooks.doRefUpdatedHook( + new Branch.NameKey(resource.getNameKey(), GitRepositoryManager.REF_CONFIG), + baseRev, commitRev, user.getAccount()); + } cache.evict(ctl.getProject()); gitMgr.setProjectDescription( resource.getNameKey(),