Merge "Merge branch 'stable-2.12'"

Documentation/cmd-ls-groups.txt

@@ -10,7 +10,6 @@ gerrit ls-groups - List groups visible to caller
   [--user <NAME> | -u <NAME>]
   [--owned]
   [--visible-to-all]
-  [--type {internal | system}]
   [-q <GROUP>]
   [--verbose | -v]
 --
@@ -67,15 +66,6 @@ This option can't be used together with the '--project' option.
 	(groups that are explicitly marked as visible to all registered
 	users).
 
---type::
-	Display only groups of the specified type. If not specified,
-	groups of all types are displayed. Supported types:
-+
---
-`internal`:: Any group defined within Gerrit.
-`system`:: Any system defined and managed group.
---
-
 -q::
 	Group that should be inspected. The `-q` option can be specified
 	multiple times to define several groups to be inspected. If

Documentation/config-gerrit.txt

@@ -1098,7 +1098,7 @@ how the replacement is displayed to the user.
 ----
 [commentlink "changeid"]
   match = (I[0-9a-f]{8,40})
-  link = "#q,$1"
+  link = "#/q/$1"
 
 [commentlink "bugzilla"]
   match = "(bug\\s+#?)(\\d+)"
@@ -2405,10 +2405,14 @@ Defaults to no limit.
 +
 Maximum number of leaf terms to allow in a query. Too-large queries may
 perform poorly, so setting this option causes query parsing to fail fast
-before attempting to send them to the secondary index. Set to 0 for no
+before attempting to send them to the secondary index. Should this limit
-limit.
+be reached, database is used instead of index as applicable.
 +
-Defaults to 500.
+When the index type is `LUCENE`, also sets the maximum number of clauses
+permitted per BooleanQuery. This is so that all enforced query limits
+are the same.
++
+Defaults to 1024.
 
 ==== Lucene configuration
 
@@ -2417,14 +2421,6 @@ Open and closed changes are indexed in separate indexes named
 
 The following settings are only used when the index type is `LUCENE`.
 
-[[index.defaultMaxClauseCount]]index.defaultMaxClauseCount::
-+
-Only used when the type is `LUCENE`.
-+
-Sets the maximum number of clauses permitted per BooleanQuery.
-+
-Defaults to 1024.
-
 [[index.name.ramBufferSize]]index.name.ramBufferSize::
 +
 Determines the amount of RAM that may be used for buffering added documents
@@ -2468,7 +2464,6 @@ Sample Lucene index configuration:
 ----
 [index]
   type = LUCENE
-  defaultMaxClauseCount = 2048
 
 [index "changes_open"]
   ramBufferSize = 60 m

Documentation/rest-api-accounts.txt

@@ -496,6 +496,9 @@ For the development mode email addresses are directly added without
 confirmation. A Gerrit administrator may add an email address without
 confirmation by setting `no_confirmation` in the
 link:#email-input[EmailInput].
+If link:config-gerrit.html#sendemail.allowrcpt[sendemail.allowrcpt] is
+configured, the added email address must belong to a domain that is
+allowed, unless `no_confirmation` is set.
 
 In the request body additional data for the email address can be
 provided as link:#email-input[EmailInput].

ReleaseNotes/ReleaseNotes-2.11.6.txt

@@ -0,0 +1,130 @@
+Release notes for Gerrit 2.11.6
+===============================
+
+Gerrit 2.11.6 is now available:
+
+link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.6.war[
+https://gerrit-releases.storage.googleapis.com/gerrit-2.11.6.war]
+
+There are no schema changes from link:ReleaseNotes-2.11.5.html[2.11.5].
+
+Bug Fixes
+---------
+
+General
+~~~~~~~
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3742[Issue 3742]:
+Use merge strategy for mergeability testing on 'Rebase if Necessary' strategy.
++
+When pushing several interdependent commits to a project with the
+'Rebase if Necessary' strategy, all the commits except the first one were
+marked as 'Cannot merge'.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3762[Issue 3762]:
+Fix server error when querying changes with the `query` ssh command.
+
+* Fix server error when listing annotated/signed tag that has no tagger info.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3698[Issue 3698]:
+Fix creation of the administrator user on databases with pre-allocated
+auto-increment column values.
++
+When using a database configuration where auto-increment column values are
+pre-allocated, it was possible that the 'Administrators' group was created
+with an ID other than `1`. In this case, the created admin user was not added
+to the correct group, and did not have the correct admin permissions.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3018[Issue 3018]:
+Fix query for changes using a label with a group operator.
++
+The `group` operator was being ignored when searching for changes with labels
+because the search index does not contain group information.
+
+* Fix online reindexing of changes that don't already exist in the index.
++
+Changes are now always reloaded from the database during online reindex.
+
+* Fix reviewer suggestion for accounts containing upper case letters.
++
+When an email for an account contained upper-case letter(s), this account
+couldn't be added as a reviewer by selecting it from the suggested list of
+accounts.
+
+Authentication
+~~~~~~~~~~~~~~
+
+* Fix handling of lowercase HTTP username.
++
+When `auth.userNameToLowerCase` is set to true the HTTP-provided username
+should be converted to lowercase as it is done on all the other authentication
+mechanisms.
+
+* Don't create new account when claimed OAuth identity is unknown.
++
+The Claimed Identity feature was enabled to support old Google OpenID accounts,
+that cannot be activated anymore. In some corner cases, when for example the URL
+is not from the production Gerrit site, for example on a staging instance, the
+OpenID identity may deviate from the original one. In case of mismatch, the lookup
+of the user for the claimed identity would fail, causing a new account to be
+created.
+
+UI
+~~
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3714[Issue 3714]:
+Improve visibility of comments on dark themes.
+
+* Fix highlighting of search results and trailing whitespaces in intraline
+diff chunks.
+
+Plugins
+~~~~~~~
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3768[Issue 3768]:
+Fix usage of `EqualsFilePredicate` in plugins.
+
+* Suggest to upgrade installed plugins per default during site initialization
+to new Gerrit version.
++
+The default was 'No' which resulted in some sites not upgrading core
+plugins and running the wrong versions.
+
+* Fix reading of plugin documentation.
++
+Under some circumstances it was possible to fail with an IO error.
+
+* Replication
+
+** Recursively include parent groups of groups specified in `authGroup`.
++
+An `authGroup` could be included in other groups and should be granted the
+same permission as its parents.
+
+** Put back erroneously removed documentation of `remote.NAME.timeout`.
+
+** Add logging of cancelled replication events.
+
+* API
+
+** Allow to use `CurrentSchemaVersion`.
+
+** Allow to use `InternalChangeQuery.query()`.
+
+** Allow to use `JdbcUtil.port()`.
+
+** Allow to use GWTORM `Key` classes.
+
+Documentation Updates
+---------------------
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=412[Issue 412]:
+Update documentation of `commentlink.match` regular expression to clarify
+that the expression is applied to the rendered HTML.
+
+* Remove warning about unstable change edit REST API endpoints.
++
+These endpoints should be considered stable since version 2.11.
+
+* Document that `ldap.groupBase` and `ldap.accountBase` are repeatable.
+

ReleaseNotes/ReleaseNotes-2.11.7.txt

@@ -0,0 +1,44 @@
+Release notes for Gerrit 2.11.7
+===============================
+
+Gerrit 2.11.7 is now available:
+
+link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war[
+https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war]
+
+There are no schema changes from link:ReleaseNotes-2.11.6.html[2.11.6].
+
+Bug Fixes
+---------
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3882[Issue 3882]:
+Fix 'No user on email thread' exception when label with group parameter is
+used in watched projects predicate.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3877[Issue 3877]:
+Include files in output when using `gerrit query` with combination of
+search operators.
++
+A regression introduced in version 2.11.6 caused files to be omitted
+in the output.
+
+* Include comments in output when using `gerrit query` with the
+`--current-patch-set` option.
++
+Comments were added at the change level but were not added at the
+patch set level.
+
+* Honor the `sendemail.allowrcpt` setting when adding new email address.
++
+When adding a new email address via the UI or REST API, it was possible for
+the user to add an address that does not belong to a domain allowed by the
+`sendemail.allowrcpt` configuration. However, when sending the verification
+email, the recipient address was (correctly) dropped, and the email had no
+recipients. This resulted in an error from the SMTP server and an 'Internal
+server error' message to the user.
+
+* Remove unnecessary log messages.
++
+The messages 'Assuming empty group membership' and 'Skipping delivery of
+email' do not add any value and were filling up the error log.
+

ReleaseNotes/ReleaseNotes-2.12.txt

@@ -376,6 +376,16 @@ menu on the change screen.
 Plugins can refer to groups so that when they are renamed, the project
 config will also be updated in this section.
 
+* API
+
+** Allow to use `CurrentSchemaVersion`.
+
+** Allow to use `InternalChangeQuery.query()`.
+
+** Allow to use `JdbcUtil.port()`.
+
+** Allow to use GWTORM `Key` classes.
+
 
 Other
 ~~~~~
@@ -480,6 +490,65 @@ Users who are unable to upgrade the plugin may instead change the
 trigger's branch configuration to type `Path` with a pattern like
 `refs/*/master` instead of `Plain` and `master`.
 
+* link:https://code.google.com/p/gerrit/issues/detail?id=3714[Issue 3714]:
+Improve visibility of comments on dark themes.
+
+* Fix highlighting of search results and trailing whitespaces in intraline
+diff chunks.
+
+* Fix server error when listing annotated/signed tag that has no tagger info.
+
+* Don't create new account when claimed OAuth identity is unknown.
++
+The Claimed Identity feature was enabled to support old Google OpenID accounts,
+that cannot be activated anymore. In some corner cases, when for example the URL
+is not from the production Gerrit site, for example on a staging instance, the
+OpenID identity may deviate from the original one. In case of mismatch, the lookup
+of the user for the claimed identity would fail, causing a new account to be
+created.
+
+* Suggest to upgrade installed plugins per default during site initialization
+to new Gerrit version.
++
+The default was 'No' which resulted in some sites not upgrading core
+plugins and running the wrong versions.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3698[Issue 3698]:
+Fix creation of the administrator user on databases with pre-allocated
+auto-increment column values.
++
+When using a database configuration where auto-increment column values are
+pre-allocated, it was possible that the 'Administrators' group was created
+with an ID other than `1`. In this case, the created admin user was not added
+to the correct group, and did not have the correct admin permissions.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3018[Issue 3018]:
+Fix query for changes using a label with a group operator.
++
+The `group` operator was being ignored when searching for changes with labels
+because the search index does not contain group information.
+
+* Fix online reindexing of changes that don't already exist in the index.
++
+Changes are now always reloaded from the database during online reindex.
+
+* Fix reading of plugin documentation.
++
+Under some circumstances it was possible to fail with an IO error.
+
+Documentation Updates
+---------------------
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=412[Issue 412]:
+Update documentation of `commentlink.match` regular expression to clarify
+that the expression is applied to the rendered HTML.
+
+* Remove warning about unstable change edit REST API endpoints.
++
+These endpoints should be considered stable since version 2.11.
+
+* Document that `ldap.groupBase` and `ldap.accountBase` are repeatable.
+
 Upgrades
 --------
 

ReleaseNotes/index.txt

@@ -14,6 +14,8 @@ Version 2.12.x
 [[2_11]]
 Version 2.11.x
 --------------
+* link:ReleaseNotes-2.11.7.html[2.11.7]
+* link:ReleaseNotes-2.11.6.html[2.11.6]
 * link:ReleaseNotes-2.11.5.html[2.11.5]
 * link:ReleaseNotes-2.11.4.html[2.11.4]
 * link:ReleaseNotes-2.11.3.html[2.11.3]

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -159,7 +159,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertChangeMessages(change,
         ImmutableList.of("Uploaded patch set 1.",
             "Uploaded patch set 2.",
-            "Patch set 3: Published edit on patch set 2."));
+            "Patch Set 3: Published edit on patch set 2."));
   }
 
   @Test
@@ -179,7 +179,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertChangeMessages(change,
         ImmutableList.of("Uploaded patch set 1.",
             "Uploaded patch set 2.",
-            "Patch set 3: Published edit on patch set 2."));
+            "Patch Set 3: Published edit on patch set 2."));
   }
 
   @Test
@@ -357,7 +357,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertChangeMessages(change,
         ImmutableList.of("Uploaded patch set 1.",
             "Uploaded patch set 2.",
-            "Patch set 3: Commit message was updated."));
+            "Patch Set 3: Commit message was updated."));
   }
 
   @Test
@@ -384,7 +384,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertChangeMessages(change,
         ImmutableList.of("Uploaded patch set 1.",
             "Uploaded patch set 2.",
-            "Patch set 3: Commit message was updated."));
+            "Patch Set 3: Commit message was updated."));
   }
 
   @Test

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java

@@ -55,6 +55,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
   private TestAccount user1;
   private TestAccount user2;
   private TestAccount user3;
+  private TestAccount user4;
 
   @Before
   public void setUp() throws Exception {
@@ -65,6 +66,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
     user1 = user("user1", "First1 Last1", group1);
     user2 = user("user2", "First2 Last2", group2);
     user3 = user("user3", "First3 Last3", group1, group2);
+    user4 = user("jdoe", "John Doe", "JDOE");
   }
 
   @Test
@@ -205,14 +207,18 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
     reviewers = suggestReviewers(changeId, user1.username, 2);
     assertThat(reviewers).hasSize(1);
 
-    reviewers = suggestReviewers(changeId, "example.com", 6);
+    reviewers = suggestReviewers(changeId, "example.com", 7);
-    assertThat(reviewers).hasSize(5);
+    assertThat(reviewers).hasSize(6);
 
     reviewers = suggestReviewers(changeId, user1.email, 2);
     assertThat(reviewers).hasSize(1);
 
     reviewers = suggestReviewers(changeId, user1.username + " example", 2);
     assertThat(reviewers).hasSize(1);
+
+    reviewers = suggestReviewers(changeId, user4.email.toLowerCase(), 2);
+    assertThat(reviewers).hasSize(1);
+    assertThat(reviewers.get(0).account.email).isEqualTo(user4.email);
   }
 
   @Test
@@ -254,9 +260,8 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
     return GroupDescriptions.toAccountGroup(d);
   }
 
-  private TestAccount user(String name, String fullName, AccountGroup... groups)
+  private TestAccount user(String name, String fullName, String emailName,
-      throws Exception {
+      AccountGroup... groups) throws Exception {
-    name = name(name);
     String[] groupNames = FluentIterable.from(Arrays.asList(groups))
         .transform(new Function<AccountGroup, String>() {
           @Override
@@ -264,6 +269,12 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
             return in.getName();
           }
         }).toArray(String.class);
-    return accounts.create(name, name + "@example.com", fullName, groupNames);
+    return accounts.create(name(name), name(emailName) + "@example.com",
+        fullName, groupNames);
+  }
+
+  private TestAccount user(String name, String fullName, AccountGroup... groups)
+      throws Exception {
+    return user(name, fullName, name, groups);
   }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java

@@ -0,0 +1,320 @@
+// Copyright (C) 2016 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.acceptance.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.server.data.ChangeAttribute;
+import com.google.gson.Gson;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+@NoHttpd
+public class QueryIT extends AbstractDaemonTest {
+
+  private static Gson gson = new Gson();
+
+  @Test
+  public void testBasicQueryJSON() throws Exception {
+    String changeId1 = createChange().getChangeId();
+    String changeId2 = createChange().getChangeId();
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery("1234");
+    assertThat(changes.size()).isEqualTo(0);
+
+    changes = executeSuccessfulQuery(changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId1);
+
+    changes = executeSuccessfulQuery(changeId1 + " OR " + changeId2);
+    assertThat(changes.size()).isEqualTo(2);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId2);
+    assertThat(changes.get(1).project).isEqualTo(project.toString());
+    assertThat(changes.get(1).id).isEqualTo(changeId1);
+
+    changes =
+        executeSuccessfulQuery("--start=1 " + changeId1 + " OR " + changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId1);
+  }
+
+  @Test
+  public void testAllApprovalsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNull();
+
+    changes = executeSuccessfulQuery("--all-approvals " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testAllReviewersOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes().id(changeId).addReviewer(in);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).allReviewers).isNull();
+
+    changes = executeSuccessfulQuery("--all-reviewers " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).allReviewers).isNotNull();
+    assertThat(changes.get(0).allReviewers.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommitMessageOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--commit-message " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).commitMessage).isNotNull();
+    assertThat(changes.get(0).commitMessage).contains(PushOneCommit.SUBJECT);
+  }
+
+  @Test
+  public void testCurrentPatchSetOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    amendChange(changeId);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNull();
+
+    changes = executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.number).isEqualTo("2");
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes = executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.approvals).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.approvals.size()).isEqualTo(1);
+
+  }
+
+  @Test
+  public void testPatchSetsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    amendChange(changeId);
+    amendChange(changeId);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNull();
+
+    changes = executeSuccessfulQuery("--patch-sets " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNotNull();
+    assertThat(changes.get(0).patchSets.size()).isEqualTo(3);
+  }
+
+  @Test
+  public void shouldFailWithFilesWithoutPatchSetsOrCurrentPatchSetsOption()
+      throws Exception {
+    String changeId = createChange().getChangeId();
+    sshSession.exec("gerrit query --files " + changeId);
+    assertThat(sshSession.hasError()).isTrue();
+    assertThat(sshSession.getError()).contains(
+        "needs --patch-sets or --current-patch-set");
+  }
+
+  @Test
+  public void testFileOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--current-patch-set --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.files).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.files.size()).isEqualTo(2);
+
+    changes = executeSuccessfulQuery("--patch-sets --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes =
+        executeSuccessfulQuery("--patch-sets --files --all-approvals "
+            + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).comments).isNull();
+
+    changes = executeSuccessfulQuery("--comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).comments).isNotNull();
+    assertThat(changes.get(0).comments.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionsInCurrentPatchSetJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    ReviewInput review = new ReviewInput();
+    ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
+    comment.path = PushOneCommit.FILE_NAME;
+    comment.side = Side.REVISION;
+    comment.message = "comment 1";
+    review.comments = new HashMap<>();
+    review.comments.put(comment.path, Lists.newArrayList(comment));
+    gApi.changes().id(changeId).current().review(review);
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.comments).isNull();
+
+    changes =
+        executeSuccessfulQuery("--current-patch-set --comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.comments).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.comments.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionInPatchSetsJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    ReviewInput review = new ReviewInput();
+    ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
+    comment.path = PushOneCommit.FILE_NAME;
+    comment.side = Side.REVISION;
+    comment.message = "comment 1";
+    review.comments = new HashMap<>();
+    review.comments.put(comment.path, Lists.newArrayList(comment));
+    gApi.changes().id(changeId).current().review(review);
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--patch-sets " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNull();
+
+    changes = executeSuccessfulQuery("--patch-sets --comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+
+    changes =
+        executeSuccessfulQuery("--patch-sets --comments --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes =
+        executeSuccessfulQuery("--patch-sets --comments --files --all-approvals "
+            + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testDependenciesOptionJSON() throws Exception {
+    String changeId1 = createChange().getChangeId();
+    String changeId2 = createChange().getChangeId();
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery("--dependencies " + changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery(changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery("--dependencies " + changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNotNull();
+    assertThat(changes.get(0).dependsOn.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testSubmitRecordsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).submitRecords).isNull();
+
+    changes = executeSuccessfulQuery("--submit-records " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).submitRecords).isNotNull();
+    assertThat(changes.get(0).submitRecords.size()).isEqualTo(1);
+  }
+
+  private List<ChangeAttribute> executeSuccessfulQuery(String params)
+      throws Exception {
+    String rawResponse =
+        sshSession.exec("gerrit query --format=JSON " + params);
+    assert_().withFailureMessage(sshSession.getError())
+        .that(sshSession.hasError()).isFalse();
+    return getChanges(rawResponse);
+  }
+
+  private static List<ChangeAttribute> getChanges(String rawResponse) {
+    String[] lines = rawResponse.split("\\n");
+    List<ChangeAttribute> changes = new ArrayList<>(lines.length - 1);
+    for (int i = 0; i < lines.length - 1; i++) {
+      changes.add(gson.fromJson(lines[i], ChangeAttribute.class));
+    }
+    return changes;
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java

@@ -109,12 +109,12 @@ public class ProjectTagsScreen extends PaginatedProjectScreen {
 
     TagsTable() {
       table.setWidth("");
-      table.setText(0, 0, Util.C.columnTagName());
+      table.setText(0, 1, Util.C.columnTagName());
-      table.setText(0, 1, Util.C.columnBranchRevision());
+      table.setText(0, 2, Util.C.columnBranchRevision());
 
       FlexCellFormatter fmt = table.getFlexCellFormatter();
-      fmt.addStyleName(0, 0, Gerrit.RESOURCES.css().dataHeader());
       fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().dataHeader());
+      fmt.addStyleName(0, 2, Gerrit.RESOURCES.css().dataHeader());
     }
 
     void display(List<TagInfo> tags) {
@@ -135,18 +135,18 @@ public class ProjectTagsScreen extends PaginatedProjectScreen {
     }
 
     void populate(int row, TagInfo k) {
-      table.setWidget(row, 0, new InlineHTML(highlight(k.getShortName(), match)));
+      table.setWidget(row, 1, new InlineHTML(highlight(k.getShortName(), match)));
 
       if (k.revision() != null) {
-        table.setText(row, 1, k.revision());
+        table.setText(row, 2, k.revision());
       } else {
-        table.setText(row, 1, "");
+        table.setText(row, 2, "");
       }
 
       FlexCellFormatter fmt = table.getFlexCellFormatter();
       String dataCellStyle = Gerrit.RESOURCES.css().dataCell();
-      fmt.addStyleName(row, 0, dataCellStyle);
       fmt.addStyleName(row, 1, dataCellStyle);
+      fmt.addStyleName(row, 2, dataCellStyle);
 
       setRowItem(row, k);
     }

gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java

@@ -74,6 +74,7 @@ public class ModeInfo extends JavaScriptObject {
       Modes.I.sql(),
       Modes.I.stex(),
       Modes.I.swift(),
+      Modes.I.tcl(),
       Modes.I.velocity(),
       Modes.I.verilog(),
       Modes.I.vhdl(),

gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/DirectoryDocServlet.java

@@ -0,0 +1,35 @@
+// Copyright (C) 2016 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.httpd.raw;
+
+import com.google.common.cache.Cache;
+
+import java.nio.file.Path;
+
+class DirectoryDocServlet extends ResourceServlet {
+  private static final long serialVersionUID = 1L;
+
+  private final Path doc;
+
+  DirectoryDocServlet(Cache<Path, Resource> cache, Path unpackedWar) {
+    super(cache, true);
+    this.doc = unpackedWar.resolve("Documentation");
+  }
+
+  @Override
+  protected Path getResourcePath(String pathInfo) {
+    return doc.resolve(pathInfo);
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java

@@ -120,6 +120,8 @@ public class StaticModule extends ServletModule {
     Paths p = getPaths();
     if (p.warFs != null) {
       return new WarDocServlet(cache, p.warFs);
+    } else if (p.unpackedWar != null && !p.isDev()) {
+      return new DirectoryDocServlet(cache, p.unpackedWar);
     } else {
       return new HttpServlet() {
         private static final long serialVersionUID = 1L;

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java

@@ -55,7 +55,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
   private final Provider<IdentifiedUser> currentUser;
   private final ProjectControl.Factory projectControlFactory;
   private final AgreementInfoFactory.Factory agreementInfoFactory;
-  private final ChangeQueryBuilder queryBuilder;
+  private final Provider<ChangeQueryBuilder> queryBuilder;
   private final SetDiffPreferences setDiff;
 
   @Inject
@@ -63,7 +63,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
       final Provider<IdentifiedUser> identifiedUser,
       final ProjectControl.Factory projectControlFactory,
       final AgreementInfoFactory.Factory agreementInfoFactory,
-      final ChangeQueryBuilder queryBuilder,
+      final Provider<ChangeQueryBuilder> queryBuilder,
       SetDiffPreferences setDiff) {
     super(schema, identifiedUser);
     this.currentUser = identifiedUser;
@@ -147,7 +147,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
 
         if (filter != null) {
           try {
-            queryBuilder.parse(filter);
+            queryBuilder.get().parse(filter);
           } catch (QueryParseException badFilter) {
             throw new InvalidQueryException(badFilter.getMessage(), filter);
           }

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -230,7 +230,7 @@ public class LuceneChangeIndex implements ChangeIndex {
             CUSTOM_CHAR_MAPPING);
     queryBuilder = new QueryBuilder(analyzer);
 
-    BooleanQuery.setMaxClauseCount(cfg.getInt("index", "defaultMaxClauseCount",
+    BooleanQuery.setMaxClauseCount(cfg.getInt("index", "maxTerms",
         BooleanQuery.getMaxClauseCount()));
 
     GerritIndexWriterConfig openConfig =

gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java

@@ -103,7 +103,8 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
 
   public Response<EmailInfo> apply(IdentifiedUser user, EmailInput input)
       throws AuthException, BadRequestException, ResourceConflictException,
-      ResourceNotFoundException, OrmException, EmailException {
+      ResourceNotFoundException, OrmException, EmailException,
+      MethodNotAllowedException {
     if (input.email != null && !email.equals(input.email)) {
       throw new BadRequestException("email address must match URL");
     }
@@ -126,7 +127,11 @@ public class CreateEmail implements RestModifyView<AccountResource, EmailInput>
       }
     } else {
       try {
-        registerNewEmailFactory.create(email).send();
+        RegisterNewEmailSender sender = registerNewEmailFactory.create(email);
+        if (!sender.isAllowed()) {
+          throw new MethodNotAllowedException("Not allowed to add email address " + email);
+        }
+        sender.send();
         info.pendingConfirmation = true;
       } catch (EmailException | RuntimeException e) {
         log.error("Cannot send email verification message to " + email, e);

gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java

@@ -225,8 +225,6 @@ import javax.security.auth.login.LoginException;
         try {
           account = findAccount(schema, ctx, username, false);
         } catch (AccountException e) {
-          LdapRealm.log.warn("Account " + username +
-              " not found, assuming empty group membership");
           return Collections.emptySet();
         }
       }
@@ -248,8 +246,6 @@ import javax.security.auth.login.LoginException;
         try {
           account = findAccount(schema, ctx, username, true);
         } catch (AccountException e) {
-          LdapRealm.log.warn("Account " + username +
-              " not found, assuming empty group membership");
           return Collections.emptySet();
         }
       }

gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java

@@ -17,12 +17,12 @@ package com.google.gerrit.server.change;
 import static com.google.gerrit.server.PatchLineCommentsUtil.PLC_ORDER;
 
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 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.IdentifiedUser;
 import com.google.gerrit.server.git.SendEmailExecutor;
 import com.google.gerrit.server.mail.CommentSender;
 import com.google.gerrit.server.notedb.ChangeNotes;
@@ -32,7 +32,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
-import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
 import com.google.inject.assistedinject.Assisted;
@@ -51,7 +50,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
         NotifyHandling notify,
         ChangeNotes notes,
         PatchSet patchSet,
-        Account.Id authorId,
+        IdentifiedUser user,
         ChangeMessage message,
         List<PatchLineComment> comments);
   }
@@ -65,7 +64,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
   private final NotifyHandling notify;
   private final ChangeNotes notes;
   private final PatchSet patchSet;
-  private final Account.Id authorId;
+  private final IdentifiedUser user;
   private final ChangeMessage message;
   private List<PatchLineComment> comments;
   private ReviewDb db;
@@ -80,7 +79,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
       @Assisted NotifyHandling notify,
       @Assisted ChangeNotes notes,
       @Assisted PatchSet patchSet,
-      @Assisted Account.Id authorId,
+      @Assisted IdentifiedUser user,
       @Assisted ChangeMessage message,
       @Assisted List<PatchLineComment> comments) {
     this.sendEmailsExecutor = executor;
@@ -91,7 +90,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
     this.notify = notify;
     this.notes = notes;
     this.patchSet = patchSet;
-    this.authorId = authorId;
+    this.user = user;
     this.message = message;
     this.comments = PLC_ORDER.sortedCopy(comments);
   }
@@ -107,7 +106,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
 
       CommentSender cm = commentSenderFactory.create(notes.getProjectName(),
           notes.getChangeId());
-      cm.setFrom(authorId);
+      cm.setFrom(user.getAccountId());
       cm.setPatchSet(patchSet,
           patchSetInfoFactory.get(notes.getProjectName(), patchSet));
       cm.setChangeMessage(message);
@@ -132,7 +131,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
 
   @Override
   public CurrentUser getUser() {
-    throw new OutOfScopeException("No user on email thread");
+    return user.getRealUser();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -384,7 +384,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
             in.notify,
             notes,
             ps,
-            user.getAccountId(),
+            user,
             message,
             comments).sendAsync();
       }

gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java

@@ -172,9 +172,9 @@ public class ReviewerSuggestionCache {
       doc.add(new TextField(NAME, a.getFullName(), Store.YES));
     }
     if (a.getPreferredEmail() != null) {
+      doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES));
       doc.add(new StringField(EMAIL, a.getPreferredEmail().toLowerCase(),
           Store.YES));
-      doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES));
     }
     AccountExternalIdAccess extIdAccess = db.get().accountExternalIds();
     String username = AccountState.getUserName(

gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java

@@ -247,7 +247,7 @@ public class ChangeEditUtil {
     PatchSetInserter inserter =
         patchSetInserterFactory.create(ctl, psId, squashed);
 
-    StringBuilder message = new StringBuilder("Patch set ")
+    StringBuilder message = new StringBuilder("Patch Set ")
       .append(inserter.getPatchSetId().get())
       .append(": ");
 

gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java

@@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.mail.MergedSender;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -48,6 +49,7 @@ public class EmailMerge implements Runnable, RequestContext {
   private final MergedSender.Factory mergedSenderFactory;
   private final SchemaFactory<ReviewDb> schemaFactory;
   private final ThreadLocalRequestContext requestContext;
+  private final IdentifiedUser.GenericFactory identifiedUserFactory;
 
   private final Project.NameKey project;
   private final Change.Id changeId;
@@ -59,6 +61,7 @@ public class EmailMerge implements Runnable, RequestContext {
       MergedSender.Factory mergedSenderFactory,
       SchemaFactory<ReviewDb> schemaFactory,
       ThreadLocalRequestContext requestContext,
+      IdentifiedUser.GenericFactory identifiedUserFactory,
       @Assisted Project.NameKey project,
       @Assisted Change.Id changeId,
       @Assisted @Nullable Account.Id submitter) {
@@ -66,6 +69,7 @@ public class EmailMerge implements Runnable, RequestContext {
     this.mergedSenderFactory = mergedSenderFactory;
     this.schemaFactory = schemaFactory;
     this.requestContext = requestContext;
+    this.identifiedUserFactory = identifiedUserFactory;
     this.project = project;
     this.changeId = changeId;
     this.submitter = submitter;
@@ -102,6 +106,10 @@ public class EmailMerge implements Runnable, RequestContext {
 
   @Override
   public CurrentUser getUser() {
+    if (submitter != null) {
+      return identifiedUserFactory.create(
+          getReviewDbProvider(), submitter).getRealUser();
+    }
     throw new OutOfScopeException("No user on email thread");
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -798,7 +798,7 @@ public class ReceiveCommits {
         } catch (RestApiException err) {
           reject(replace.inputCommand, "internal server error");
           log.error(String.format(
-              "Cannot add patch set to %d of %s",
+              "Cannot add patch set to change %d in project %s",
               e.getKey().get(), project.getName()), err);
         }
       } else if (replace.inputCommand.getResult() == NOT_ATTEMPTED) {
@@ -2169,6 +2169,9 @@ public class ReceiveCommits {
             try (RequestState state = requestState(caller)) {
               return insertPatchSet(state);
             }
+          } catch (OrmException | IOException  e) {
+            log.error("Failed to insert patch set", e);
+            throw e;
           } finally {
             synchronizedIncrement(replaceProgress);
           }

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java

@@ -216,8 +216,10 @@ public class RebaseIfNecessary extends SubmitStrategy {
   static boolean dryRun(SubmitDryRun.Arguments args,
       CodeReviewCommit mergeTip, CodeReviewCommit toMerge)
       throws IntegrationException {
+    // Test for merge instead of cherry pick to avoid false negatives
+    // on commit chains.
     return !args.mergeUtil.hasMissingDependencies(args.mergeSorter, toMerge)
-        && args.mergeUtil.canCherryPick(args.mergeSorter, args.repo, mergeTip,
+        && args.mergeUtil.canMerge(args.mergeSorter, args.repo, mergeTip,
-            args.rw, toMerge);
+             toMerge);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java

@@ -29,7 +29,7 @@ import org.eclipse.jgit.lib.Config;
  */
 @AutoValue
 public abstract class IndexConfig {
-  private static final int DEFAULT_MAX_TERMS = 500;
+  private static final int DEFAULT_MAX_TERMS = 1024;
 
   public static IndexConfig createDefault() {
     return create(0, 0, DEFAULT_MAX_TERMS);
@@ -47,7 +47,7 @@ public abstract class IndexConfig {
     return new AutoValue_IndexConfig(
         checkLimit(maxLimit, "maxLimit", Integer.MAX_VALUE),
         checkLimit(maxPages, "maxPages", Integer.MAX_VALUE),
-        checkLimit(maxTerms, "maxTerms", Integer.MAX_VALUE));
+        checkLimit(maxTerms, "maxTerms", DEFAULT_MAX_TERMS));
   }
 
   private static int checkLimit(int limit, String name, int defaultValue) {

gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java

@@ -29,6 +29,7 @@ import com.google.gerrit.server.validators.ValidationException;
 import com.google.gwtorm.server.OrmException;
 
 import org.apache.commons.lang.StringUtils;
+import org.apache.commons.validator.routines.EmailValidator;
 import org.apache.velocity.Template;
 import org.apache.velocity.VelocityContext;
 import org.apache.velocity.context.InternalContextAdapterImpl;
@@ -336,7 +337,6 @@ public abstract class OutgoingEmail {
   protected boolean shouldSendMessage() {
     if (body.length() == 0) {
       // If we have no message body, don't send.
-      log.warn("Skipping delivery of email with no body");
       return false;
     }
 
@@ -344,7 +344,6 @@ public abstract class OutgoingEmail {
       // If we have nobody to send this message to, then all of our
       // selection filters previously for this type of message were
       // unable to match a destination. Don't bother sending it.
-      log.warn("Skipping delivery of email with no recipients");
       return false;
     }
 
@@ -394,8 +393,11 @@ public abstract class OutgoingEmail {
   /** Schedule delivery of this message to the given account. */
   protected void add(final RecipientType rt, final Address addr) {
     if (addr != null && addr.email != null && addr.email.length() > 0) {
-      if (args.emailSender.canEmail(addr.email)) {
+      if (!EmailValidator.getInstance().isValid(addr.email)) {
-        if (smtpRcptTo.add(addr)) {
+        log.warn("Not emailing " + addr.email + " (invalid email address)");
+      } else if (!args.emailSender.canEmail(addr.email)) {
+        log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");
+      } else if (smtpRcptTo.add(addr)) {
         switch (rt) {
           case TO:
             ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr);
@@ -407,9 +409,6 @@ public abstract class OutgoingEmail {
             break;
         }
       }
-      } else {
-        log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)");
-      }
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java

@@ -49,11 +49,6 @@ public class RegisterNewEmailSender extends OutgoingEmail {
     add(RecipientType.TO, new Address(addr));
   }
 
-  @Override
-  protected boolean shouldSendMessage() {
-    return true;
-  }
-
   @Override
   protected void format() throws EmailException {
     appendText(velocifyFile("RegisterNewEmail.vm"));
@@ -70,4 +65,8 @@ public class RegisterNewEmailSender extends OutgoingEmail {
     }
     return emailToken;
   }
+
+  public boolean isAllowed() {
+    return args.emailSender.canEmail(addr);
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java

@@ -249,16 +249,19 @@ public class SmtpEmailSender implements EmailSender {
       client.enableSSL(sslVerify);
     }
 
-    try {
     client.setConnectTimeout(connectTimeout);
+    try {
       client.connect(smtpHost, smtpPort);
-      if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) {
+      int replyCode = client.getReplyCode();
-        throw new EmailException("SMTP server rejected connection");
+      String replyString = client.getReplyString();
+      if (!SMTPReply.isPositiveCompletion(replyCode)) {
+        throw new EmailException(
+            String.format("SMTP server rejected connection: %d: %s",
+                replyCode, replyString));
       }
       if (!client.login()) {
-        String e = client.getReplyString();
         throw new EmailException(
-            "SMTP server rejected HELO/EHLO greeting: " + e);
+            "SMTP server rejected HELO/EHLO greeting: " + replyString);
       }
 
       if (smtpEncryption == Encryption.TLS) {
@@ -266,34 +269,26 @@ public class SmtpEmailSender implements EmailSender {
           throw new EmailException("SMTP server does not support TLS");
         }
         if (!client.login()) {
-          String e = client.getReplyString();
+          throw new EmailException("SMTP server rejected login: " + replyString);
-          throw new EmailException("SMTP server rejected login: " + e);
         }
       }
 
       if (smtpUser != null && !client.auth(smtpUser, smtpPass)) {
-        String e = client.getReplyString();
+        throw new EmailException("SMTP server rejected auth: " + replyString);
-        throw new EmailException("SMTP server rejected auth: " + e);
-      }
-    } catch (IOException e) {
-      if (client.isConnected()) {
-        try {
-          client.disconnect();
-        } catch (IOException e2) {
-          //Ignored
-        }
-      }
-      throw new EmailException(e.getMessage(), e);
-    } catch (EmailException e) {
-      if (client.isConnected()) {
-        try {
-          client.disconnect();
-        } catch (IOException e2) {
-          // Ignored
-        }
-      }
-      throw e;
       }
       return client;
+    } catch (IOException | EmailException e) {
+      if (client.isConnected()) {
+        try {
+          client.disconnect();
+        } catch (IOException e2) {
+          //Ignored
+        }
+      }
+      if (e instanceof EmailException) {
+        throw (EmailException) e;
+      }
+      throw new EmailException(e.getMessage(), e);
+    }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.plugins;
 
-import static com.google.gerrit.common.FileUtil.lastModified;
-
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.Nullable;
@@ -171,6 +169,6 @@ public abstract class Plugin {
   protected abstract boolean canReload();
 
   boolean isModified(Path jar) {
-    return snapshot.lastModified() != lastModified(jar);
+    return snapshot.isModified(jar.toFile());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java

@@ -564,7 +564,9 @@ public class PluginGuiceEnvironment {
       return false;
     }
     Class<?> type = key.getTypeLiteral().getRawType();
-    if (LifecycleListener.class.isAssignableFrom(type)) {
+    if (LifecycleListener.class.isAssignableFrom(type)
+        // This is needed for secondary index to work from plugin listeners
+        && !is("com.google.gerrit.server.index.IndexCollection", type)) {
       return false;
     }
     if (StartPluginListener.class.isAssignableFrom(type)) {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java

@@ -293,6 +293,10 @@ public class OutputStreamQuery {
           eventFactory.addPatchSetFileNames(c.currentPatchSet,
               d.change(), d.currentPatchSet());
         }
+        if (includeComments) {
+          eventFactory.addPatchSetComments(c.currentPatchSet,
+              d.publishedComments());
+        }
       }
     }
 
@@ -301,7 +305,7 @@ public class OutputStreamQuery {
       if (includePatchSets) {
         eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
             includeApprovals ? d.approvals().asMap() : null,
-            labelTypes);
+            includeFiles, d.notes(), labelTypes);
         for (PatchSetAttribute attribute : c.patchSets) {
           eventFactory.addPatchSetComments(
               attribute, d.publishedComments());

gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties

@@ -47,6 +47,7 @@ soy = text/x-soy
 st = text/x-stsrc
 stex = text/x-stex
 swift = text/x-swift
+tcl = text/x-tcl
 v = text/x-verilog
 vert = x-shader/x-vertex
 vh = text/x-verilog