Don't reuse ALREADY_MERGED for an identical tree

ALREADY_MERGED now means the change is already reachable from the
branch tip. This is different from the case where we catch
MergeIdenticalTreeException, which indicates the change was marked as
merged despite not actually appearing in the commit history.

Distinguish between these cases by using a newer, more descriptive
CommitMergeStatus. Add a test for this case, fixing an
IllegalStateException found in the process.

Change-Id: I842e579d12ce745b2bc84dfd93a9f5b7213c71a8

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

@@ -30,6 +30,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.change.Submit.TestSubmitInput;
+import com.google.gerrit.server.git.strategy.CommitMergeStatus;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
@@ -268,6 +269,31 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     assertNew(change3.getChangeId());
   }
 
+  @Test
+  @TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
+  public void submitIdenticalTree() throws Exception {
+    RevCommit initialHead = getRemoteHead();
+
+    PushOneCommit.Result change1 = createChange("Change 1", "a.txt", "a");
+
+    testRepo.reset(initialHead);
+    PushOneCommit.Result change2 = createChange("Change 2", "a.txt", "a");
+
+    submit(change1.getChangeId());
+    RevCommit oldHead = getRemoteHead();
+    assertThat(oldHead.getShortMessage()).isEqualTo("Change 1");
+
+    // Don't check merge result, since ref isn't updated.
+    submit(change2.getChangeId(), new SubmitInput(), null, null, false);
+
+    assertThat(getRemoteHead()).isEqualTo(oldHead);
+
+    ChangeInfo info2 = get(change2.getChangeId());
+    assertThat(info2.status).isEqualTo(ChangeStatus.MERGED);
+    assertThat(Iterables.getLast(info2.messages).message)
+        .isEqualTo(CommitMergeStatus.SKIPPED_IDENTICAL_TREE.getMessage());
+  }
+
   @Test
   public void repairChangeStateAfterFailure() throws Exception {
     RevCommit initialHead = getRemoteHead();

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.git.strategy;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.git.strategy.CommitMergeStatus.SKIPPED_IDENTICAL_TREE;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.extensions.restapi.MergeConflictException;
@@ -126,7 +127,7 @@ public class CherryPick extends SubmitStrategy {
         toMerge.setStatusCode(CommitMergeStatus.PATH_CONFLICT);
         return;
       } catch (MergeIdenticalTreeException mie) {
-        toMerge.setStatusCode(CommitMergeStatus.ALREADY_MERGED);
+        toMerge.setStatusCode(SKIPPED_IDENTICAL_TREE);
         return;
       }
       // Initial copy doesn't have new patch set ID since change hasn't been
@@ -146,6 +147,10 @@ public class CherryPick extends SubmitStrategy {
     @Override
     public PatchSet updateChangeImpl(ChangeContext ctx) throws OrmException,
          NoSuchChangeException, IOException {
+      if (newCommit == null
+          && toMerge.getStatusCode() == SKIPPED_IDENTICAL_TREE) {
+        return null;
+      }
       checkState(newCommit != null,
           "no new commit produced by CherryPick of %s, expected to fail fast",
           toMerge.change().getId());

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

@@ -36,6 +36,9 @@ public enum CommitMergeStatus {
           + "\n"
           + "Please rebase the change locally and upload the rebased commit for review."),
 
+  SKIPPED_IDENTICAL_TREE(
+      "Marking change merged without cherry-picking to branch, as the resulting commit would be empty."),
+
   MISSING_DEPENDENCY(""),
 
   MANUAL_RECURSIVE_MERGE("The change requires a local merge to resolve.\n"

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

@@ -106,6 +106,7 @@ public class SubmitStrategyListener extends BatchUpdate.Listener {
         case CLEAN_MERGE:
         case CLEAN_REBASE:
         case CLEAN_PICK:
+        case SKIPPED_IDENTICAL_TREE:
           break; // Merge strategy accepted this change.
 
         case ALREADY_MERGED:

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

@@ -274,6 +274,8 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
           || s == CommitMergeStatus.CLEAN_PICK) {
         msg = message(ctx, commit.getPatchsetId(),
             txt + " as " + commit.name() + getByAccountName());
+      } else if (s == CommitMergeStatus.SKIPPED_IDENTICAL_TREE) {
+        msg = message(ctx, commit.getPatchsetId(), txt);
       } else if (s == CommitMergeStatus.ALREADY_MERGED) {
         msg = null;
       } else {