Merge branch 'stable-3.4' into stable-3.5

* stable-3.4:
  Update jgit to c6b0ee04e49c96e0beec4154196c416abcf2bcc9
  config-mail.txt: note about necessary restart
  Reintroduce the Change-Id footer in change screen
  Limit the number of changes that can be submitted together
  Fix HTTP 404 when browsing tags on Gitweb
  GitwebServlet: Retrieve git path from FileRepository
  Make delegate() method public

Release-Notes: skip
Change-Id: I3015c3541824bb8feda28bce50c0cd896a504881

9 files changed, 276 insertions, 16 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0ce5dae270..180960dd96 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1511,6 +1511,13 @@ section:link:#change.mergeabilityComputationBehavior[change.mergeabilityComputat
 
 Default is true.
 
+[[change.maxSubmittableAtOnce]]change.maxSubmittableAtOnce::
++
+Maximum number of changes that can be chained together in the same repository
+to be submitted at once.
++
+Default is 32767.
+
 [[change.move]]change.move::
 +
 Whether the link:rest-api-changes.html#move-change[Move Change] REST
diff --git a/Documentation/config-mail.txt b/Documentation/config-mail.txt
index 85006dc64d..029877ba1a 100644
--- a/Documentation/config-mail.txt
+++ b/Documentation/config-mail.txt
@@ -20,6 +20,11 @@ files will have no effect on the behavior of Gerrit.  However, copying an
 example template to an equivalently named file without the `.example` extension
 and modifying it will allow an administrator to customize the template.
 
+[NOTE]
+The content of the templates at `'$site_path'/etc/mail/.*\.soy` are cached at
+startup by Gerrit. If they are modified Gerrit needs to be restarted before the
+changes takes effect.
+
 == Supported Mail Templates
 
 Each mail that Gerrit sends out is controlled by at least one template.  These
diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
index 897d96f1b5..0875317542 100644
--- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
+++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
@@ -43,12 +43,13 @@ import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GitwebCgiConfig;
 import com.google.gerrit.server.config.GitwebConfig;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.DelegateRepository;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.LocalDiskRepositoryManager;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
@@ -85,6 +86,7 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Repository;
 
@@ -101,7 +103,7 @@ class GitwebServlet extends HttpServlet {
   private final Set<String> deniedActions;
   private final Path gitwebCgi;
   private final URI gitwebUrl;
-  private final LocalDiskRepositoryManager repoManager;
+  private final GitRepositoryManager repoManager;
   private final ProjectCache projectCache;
   private final PermissionBackend permissionBackend;
   private final Provider<AnonymousUser> anonymousUserProvider;
@@ -119,12 +121,10 @@ class GitwebServlet extends HttpServlet {
       SshInfo sshInfo,
       Provider<AnonymousUser> anonymousUserProvider,
       GitwebConfig gitwebConfig,
-      GitwebCgiConfig gitwebCgiConfig)
+      GitwebCgiConfig gitwebCgiConfig,
+      AllProjectsName allProjects)
       throws IOException {
-    if (!(repoManager instanceof LocalDiskRepositoryManager)) {
-      throw new ProvisionException("Gitweb can only be used with LocalDiskRepositoryManager");
-    }
-    this.repoManager = (LocalDiskRepositoryManager) repoManager;
+    this.repoManager = repoManager;
     this.projectCache = projectCache;
     this.permissionBackend = permissionBackend;
     this.anonymousUserProvider = anonymousUserProvider;
@@ -132,6 +132,9 @@ class GitwebServlet extends HttpServlet {
     this.gitwebCgi = gitwebCgiConfig.getGitwebCgi();
     this.deniedActions = new HashSet<>();
 
+    // ensure that Gitweb works on supported repository type by checking All-Projects project
+    getProjectRoot(allProjects);
+
     final String url = gitwebConfig.getUrl();
     if ((url != null) && (!url.equals("gitweb"))) {
       URI uri = null;
@@ -537,7 +540,8 @@ class GitwebServlet extends HttpServlet {
     }
   }
 
-  private String[] makeEnv(HttpServletRequest req, ProjectState projectState) {
+  private String[] makeEnv(HttpServletRequest req, ProjectState projectState)
+      throws RepositoryNotFoundException, IOException {
     final EnvList env = new EnvList(_env);
     final int contentLength = Math.max(0, req.getContentLength());
 
@@ -579,7 +583,7 @@ class GitwebServlet extends HttpServlet {
     env.set("GERRIT_CONTEXT_PATH", req.getContextPath() + "/");
     env.set("GERRIT_PROJECT_NAME", nameKey.get());
 
-    env.set("GITWEB_PROJECTROOT", repoManager.getBasePath(nameKey).toAbsolutePath().toString());
+    env.set("GITWEB_PROJECTROOT", getProjectRoot(nameKey));
 
     if (projectState.statePermitsRead()
         && permissionBackend
@@ -636,6 +640,25 @@ class GitwebServlet extends HttpServlet {
     return env.getEnvArray();
   }
 
+  private String getProjectRoot(Project.NameKey nameKey)
+      throws RepositoryNotFoundException, IOException {
+    try (Repository repo = repoManager.openRepository(nameKey)) {
+      return getProjectRoot(repo);
+    }
+  }
+
+  private String getProjectRoot(Repository repo) {
+    if (repo instanceof DelegateRepository) {
+      return getProjectRoot(((DelegateRepository) repo).delegate());
+    }
+
+    if (repo instanceof FileRepository) {
+      return repo.getDirectory().getAbsolutePath();
+    }
+
+    throw new ProvisionException("Gitweb can only be used with FileRepository");
+  }
+
   private void copyContentToCGI(HttpServletRequest req, OutputStream dst) throws IOException {
     final int contentLength = req.getContentLength();
     final InputStream src = req.getInputStream();
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index c477bb5a0b..99bd62da0f 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -144,7 +144,7 @@ public class GitwebConfig {
         type.setProject("?p=${project}.git;a=summary");
         type.setRevision("?p=${project}.git;a=commit;h=${commit}");
         type.setBranch("?p=${project}.git;a=shortlog;h=${branch}");
-        type.setTag("?p=${project}.git;a=tag;h=${tag}");
+        type.setTag("?p=${project}.git;a=shortlog;h=${tag}");
         type.setRootTree("?p=${project}.git;a=tree;hb=${commit}");
         type.setFile("?p=${project}.git;hb=${commit};f=${file}");
         type.setFileHistory("?p=${project}.git;a=history;hb=${branch};f=${file}");
diff --git a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
index 89ba1facac..85efcd2dcc 100644
--- a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
+++ b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
@@ -30,6 +30,7 @@ import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
@@ -48,6 +49,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevSort;
@@ -59,6 +61,8 @@ import org.eclipse.jgit.revwalk.RevSort;
 public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
+  public static final int MAX_SUBMITTABLE_CHANGES_AT_ONCE_DEFAULT = 1024;
+
   public static class LocalMergeSuperSetComputationModule extends AbstractModule {
     @Override
     protected void configure() {
@@ -83,15 +87,20 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
   private final Map<QueryKey, ImmutableList<ChangeData>> queryCache;
   private final Map<BranchNameKey, Optional<RevCommit>> heads;
   private final ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory;
+  private final int maxSubmittableChangesAtOnce;
 
   @Inject
   LocalMergeSuperSetComputation(
       Provider<InternalChangeQuery> queryProvider,
-      ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory) {
+      ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory,
+      @GerritServerConfig Config gerritConfig) {
     this.queryProvider = queryProvider;
     this.queryCache = new HashMap<>();
     this.heads = new HashMap<>();
     this.changeIsVisibleToPredicateFactory = changeIsVisibleToPredicateFactory;
+    this.maxSubmittableChangesAtOnce =
+        gerritConfig.getInt(
+            "change", "maxSubmittableAtOnce", MAX_SUBMITTABLE_CHANGES_AT_ONCE_DEFAULT);
   }
 
   @Override
@@ -130,9 +139,15 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
       }
 
       Set<String> visibleHashes =
-          walkChangesByHashes(visibleCommits, Collections.emptySet(), or, branchNameKey);
+          walkChangesByHashes(
+              visibleCommits,
+              Collections.emptySet(),
+              or,
+              branchNameKey,
+              maxSubmittableChangesAtOnce);
       Set<String> nonVisibleHashes =
-          walkChangesByHashes(nonVisibleCommits, visibleHashes, or, branchNameKey);
+          walkChangesByHashes(
+              nonVisibleCommits, visibleHashes, or, branchNameKey, maxSubmittableChangesAtOnce);
 
       ChangeSet partialSet =
           byCommitsOnBranchNotMerged(or, branchNameKey, visibleHashes, nonVisibleHashes, user);
@@ -216,7 +231,11 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
 
   @UsedAt(UsedAt.Project.GOOGLE)
   public Set<String> walkChangesByHashes(
-      Collection<RevCommit> sourceCommits, Set<String> ignoreHashes, OpenRepo or, BranchNameKey b)
+      Collection<RevCommit> sourceCommits,
+      Set<String> ignoreHashes,
+      OpenRepo or,
+      BranchNameKey b,
+      int limit)
       throws IOException {
     Set<String> destHashes = new HashSet<>();
     or.rw.reset();
@@ -226,7 +245,11 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
       if (ignoreHashes.contains(name)) {
         continue;
       }
-      destHashes.add(name);
+      if (destHashes.size() < limit) {
+        destHashes.add(name);
+      } else {
+        break;
+      }
       or.rw.markStart(c);
     }
     for (RevCommit c : or.rw) {
@@ -234,7 +257,11 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
       if (ignoreHashes.contains(name)) {
         continue;
       }
-      destHashes.add(name);
+      if (destHashes.size() < limit) {
+        destHashes.add(name);
+      } else {
+        break;
+      }
     }
 
     return destHashes;
diff --git a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
index a97fb499ef..a2a6caaf03 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
@@ -20,7 +20,9 @@ import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.N
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.config.GerritConfig;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.entities.Project;
@@ -184,6 +186,26 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
   }
 
   @Test
+  @Sandboxed
+  @GerritConfig(name = "change.maxSubmittableAtOnce", value = "2")
+  public void submittedTogetherWithMaxChangesLimit() throws Exception {
+    String targetRef = "refs/for/master";
+
+    commitBuilder().add("a.txt", "1").message("subject: 1").create();
+    pushHead(testRepo, targetRef, false);
+
+    RevCommit c2_1 = commitBuilder().add("b.txt", "2").message("subject: 2").create();
+    String id2 = getChangeId(c2_1);
+    pushHead(testRepo, targetRef, false);
+
+    RevCommit c3_1 = commitBuilder().add("b.txt", "3").message("subject: 3").create();
+    String id3 = getChangeId(c3_1);
+    pushHead(testRepo, targetRef, false);
+
+    assertSubmittedTogether(id3, id3, id2);
+  }
+
+  @Test
   public void respectTopicsOnAncestors() throws Exception {
     RevCommit initialHead = projectOperations.project(project).getHead("master");
 
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index b635d3f6da..41d0931195 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -197,6 +197,13 @@ import {
 } from '../../../utils/attention-set-util';
 import {listen} from '../../../services/shortcuts/shortcuts-service';
 
+const CHANGE_ID_ERROR = {
+  MISMATCH: 'mismatch',
+  MISSING: 'missing',
+};
+const CHANGE_ID_REGEX_PATTERN =
+  /^(Change-Id:\s|Link:.*\/id\/)(I[0-9a-f]{8,40})/gm;
+
 const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
 
 const REVIEWERS_REGEX = /^(R|CC)=/gm;
@@ -389,6 +396,13 @@ export class GrChangeView extends base {
   @property({type: Number})
   _lineHeight?: number;
 
+  @property({
+    type: String,
+    computed:
+      '_computeChangeIdCommitMessageError(_latestCommitMessage, _change)',
+  })
+  _changeIdCommitMessageError?: string;
+
   @property({type: Object})
   _patchRange?: ChangeViewPatchRange;
 
@@ -1480,6 +1494,53 @@ export class GrChangeView extends base {
     return GerritNav.getUrlForChange(change);
   }
 
+  _computeChangeIdClass(displayChangeId: string) {
+    return displayChangeId === CHANGE_ID_ERROR.MISMATCH ? 'warning' : '';
+  }
+
+  _computeTitleAttributeWarning(displayChangeId: string) {
+    if (displayChangeId === CHANGE_ID_ERROR.MISMATCH) {
+      return 'Change-Id mismatch';
+    } else if (displayChangeId === CHANGE_ID_ERROR.MISSING) {
+      return 'No Change-Id in commit message';
+    }
+    return undefined;
+  }
+
+  _computeChangeIdCommitMessageError(
+    commitMessage?: string,
+    change?: ChangeInfo
+  ) {
+    if (change === undefined) {
+      return undefined;
+    }
+
+    if (!commitMessage) {
+      return CHANGE_ID_ERROR.MISSING;
+    }
+
+    // Find the last match in the commit message:
+    let changeId;
+    let changeIdArr;
+
+    while ((changeIdArr = CHANGE_ID_REGEX_PATTERN.exec(commitMessage))) {
+      changeId = changeIdArr[2];
+    }
+
+    if (changeId) {
+      // A change-id is detected in the commit message.
+
+      if (changeId === change.change_id) {
+        // The change-id found matches the real change-id.
+        return null;
+      }
+      // The change-id found does not match the change-id.
+      return CHANGE_ID_ERROR.MISMATCH;
+    }
+    // There is no change-id in the commit message.
+    return CHANGE_ID_ERROR.MISSING;
+  }
+
   _computeReplyButtonLabel(
     drafts?: {[path: string]: UIDraft[]},
     canStartReview?: boolean
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index b97d3ab405..9386778e2f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -93,6 +93,11 @@ export const htmlTemplate = html`
       background-color: var(--view-background-color);
       box-shadow: var(--elevation-level-1);
     }
+    .changeId {
+      color: var(--deemphasized-text-color);
+      font-family: var(--font-family);
+      margin-top: var(--spacing-l);
+    }
     .changeMetadata {
       /* Limit meta section to half of the screen at max */
       max-width: 50%;
@@ -437,6 +442,19 @@ export const htmlTemplate = html`
                     remove-zero-width-space=""
                   ></gr-linked-text>
                 </gr-editable-content>
+                <div
+                  class="changeId"
+                  hidden$="[[!_changeIdCommitMessageError]]"
+                >
+                  <hr />
+                  Change-Id:
+                  <span
+                    class$="[[_computeChangeIdClass(_changeIdCommitMessageError)]]"
+                    title$="[[_computeTitleAttributeWarning(_changeIdCommitMessageError)]]"
+                  >
+                    [[_change.change_id]]
+                  </span>
+                </div>
               </div>
               <h3 class="assistive-tech-only">Comments and Checks Summary</h3>
               <gr-change-summary
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 6cbe59c838..5ef23b8783 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1518,6 +1518,103 @@ suite('gr-change-view tests', () => {
     element._handleCommitMessageSave(mockEvent('\n\n\n\n\n\n\n\n'));
     assert.equal(putStub.lastCall.args[1], '\n\n\n\n\n\n\n\n');
   });
+  test('_computeChangeIdCommitMessageError', () => {
+    let commitMessage = 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483';
+    let change: ChangeInfo = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      null
+    );
+
+    change = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      'mismatch'
+    );
+
+    commitMessage = 'This is the greatest change.';
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      'missing'
+    );
+  });
+
+  test('multiple change Ids in commit message picks last', () => {
+    const commitMessage = [
+      'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484',
+      'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
+    ].join('\n');
+    let change: ChangeInfo = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      null
+    );
+    change = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      'mismatch'
+    );
+  });
+
+  test('does not count change Id that starts mid line', () => {
+    const commitMessage = [
+      'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484',
+      'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
+    ].join(' and ');
+    let change: ChangeInfo = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      null
+    );
+    change = {
+      ...createChangeViewChange(),
+      change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
+    };
+    assert.equal(
+      element._computeChangeIdCommitMessageError(commitMessage, change),
+      'mismatch'
+    );
+  });
+
+  test('_computeTitleAttributeWarning', () => {
+    let changeIdCommitMessageError = 'missing';
+    assert.equal(
+      element._computeTitleAttributeWarning(changeIdCommitMessageError),
+      'No Change-Id in commit message'
+    );
+
+    changeIdCommitMessageError = 'mismatch';
+    assert.equal(
+      element._computeTitleAttributeWarning(changeIdCommitMessageError),
+      'Change-Id mismatch'
+    );
+  });
+
+  test('_computeChangeIdClass', () => {
+    let changeIdCommitMessageError = 'missing';
+    assert.equal(element._computeChangeIdClass(changeIdCommitMessageError), '');
+
+    changeIdCommitMessageError = 'mismatch';
+    assert.equal(
+      element._computeChangeIdClass(changeIdCommitMessageError),
+      'warning'
+    );
+  });
 
   test('topic is coalesced to null', async () => {
     sinon.stub(element, '_changeChanged');