Merge branch 'stable-2.8'

* stable-2.8:
  Fix Merge Always and Merge If Necessary strategies
  Fix formatting problem in buck documentation
  Buck: Expand instructions related to setting up the path

Conflicts:
	Documentation/dev-buck.txt
	gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
	gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
	gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java

Change-Id: I56fd7f427d9d4bd5c1ec5c37ab5e744065cfd3b9

7 files changed, 188 insertions, 12 deletions

diff --git a/Documentation/dev-buck.txt b/Documentation/dev-buck.txt
index b48671eede..db367329f1 100644
--- a/Documentation/dev-buck.txt
+++ b/Documentation/dev-buck.txt
@@ -15,14 +15,21 @@ Clone the git and build it:
   ant
 ----
 
-Make sure you have a `bin/` directory in your home directory and that
-it is included in your path:
+If you don't have a `bin/` directory in your home directory, create one:
 
 ----
   mkdir ~/bin
+----
+
+Add the `~/bin` folder to the path:
+
+----
   PATH=~/bin:$PATH
 ----
 
+Note that the buck executable needs to be available in all shell sessions,
+so also make sure it is appended to the path globally.
+
 Add a symbolic link in `~/bin` to the buck executable:
 
 ----
@@ -142,8 +149,9 @@ Deploy {extension,plugin,gwt}-api to the remote maven repository:
 ----
 
 The type of the repo is induced from the Gerrit version name, i.e.
-* 2.9-SNAPSHOT: snapshot repo
-* 2.9: release repo
+
+* `2.9-SNAPSHOT`: snapshot repo
+* `2.9`: release repo
 
 Deploying to the remote repository still depends on Maven, and the credentials
 for the repository need to be
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index 797a20ecc3..13901c32e5 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.PushOneCommit;
@@ -62,6 +63,9 @@ import org.junit.Test;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Collections;
+import java.util.List;
 
 public abstract class AbstractSubmit extends AbstractDaemonTest {
 
@@ -153,6 +157,22 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     submit(changeId, HttpStatus.SC_CONFLICT);
   }
 
+  protected void submitStatusOnly(String changeId)
+      throws IOException, OrmException {
+    approve(changeId);
+    Change c = db.changes().byKey(new Change.Key(changeId)).toList().get(0);
+    c.setStatus(Change.Status.SUBMITTED);
+    db.changes().update(Collections.singleton(c));
+    db.patchSetApprovals().insert(Collections.singleton(
+        new PatchSetApproval(
+            new PatchSetApproval.Key(
+                c.currentPatchSetId(),
+                admin.id,
+                PatchSetApproval.LabelId.SUBMIT),
+            (short) 1,
+            new Timestamp(System.currentTimeMillis()))));
+  }
+
   private void submit(String changeId, int expectedStatus) throws IOException {
     approve(changeId);
     SubmitInput subm = new SubmitInput();
@@ -236,6 +256,22 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     }
   }
 
+  protected List<RevCommit> getRemoteLog() throws IOException {
+    Repository repo = repoManager.openRepository(project);
+    try {
+      RevWalk rw = new RevWalk(repo);
+      try {
+        rw.markStart(rw.parseCommit(
+            repo.getRef("refs/heads/master").getObjectId()));
+        return Lists.newArrayList(rw);
+      } finally {
+        rw.release();
+      }
+    } finally {
+      repo.close();
+    }
+  }
+
   private RevCommit getHead(Repository repo, String name) throws IOException {
     try {
       RevWalk rw = new RevWalk(repo);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index d65c3ef7b7..d137e62f1a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -16,9 +16,11 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.gerrit.acceptance.GitUtil.checkout;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
 
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.reviewdb.client.Project.SubmitType;
+import com.google.gwtorm.server.OrmException;
 
 import com.jcraft.jsch.JSchException;
 
@@ -28,6 +30,7 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.List;
 
 public class SubmitByCherryPickIT extends AbstractSubmit {
 
@@ -154,4 +157,42 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
     assertCurrentRevision(change3.getChangeId(), 1, change3.getCommitId());
     assertSubmitter(change3.getChangeId(), 1);
   }
+
+  @Test
+  public void submitMultipleChanges()
+      throws JSchException, IOException, GitAPIException, OrmException {
+    Git git = createProject();
+    RevCommit initialHead = getRemoteHead();
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change2 = createChange(git, "Change 2", "b", "b");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change3 = createChange(git, "Change 3", "c", "c");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change4 = createChange(git, "Change 4", "d", "d");
+
+    submitStatusOnly(change2.getChangeId());
+    submitStatusOnly(change3.getChangeId());
+    submit(change4.getChangeId());
+
+    List<RevCommit> log = getRemoteLog();
+    assertEquals(
+        change4.getCommit().getShortMessage(),
+        log.get(0).getShortMessage());
+    assertSame(log.get(1), log.get(0).getParent(0));
+
+    assertEquals(
+        change3.getCommit().getShortMessage(),
+        log.get(1).getShortMessage());
+    assertSame(log.get(2), log.get(1).getParent(0));
+
+    assertEquals(
+        change2.getCommit().getShortMessage(),
+        log.get(2).getShortMessage());
+    assertSame(log.get(3), log.get(2).getParent(0));
+
+    assertEquals(initialHead.getId(), log.get(3).getId());
+  }
 }
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
index 7107890ebb..2a01d84df8 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
@@ -14,15 +14,23 @@
 
 package com.google.gerrit.acceptance.rest.change;
 
+import static com.google.gerrit.acceptance.GitUtil.checkout;
 import static org.junit.Assert.assertEquals;
 
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.reviewdb.client.Project.SubmitType;
+import com.google.gwtorm.server.OrmException;
+
+import com.jcraft.jsch.JSchException;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
+import java.io.IOException;
+import java.util.List;
+
 public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
 
   @Override
@@ -42,4 +50,42 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
     assertEquals(change.getCommitId(), head.getParent(1));
     assertSubmitter(change.getChangeId(), 1);
   }
+
+  @Test
+  public void submitMultipleChanges()
+      throws JSchException, IOException, GitAPIException, OrmException {
+    Git git = createProject();
+    RevCommit initialHead = getRemoteHead();
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change2 = createChange(git, "Change 2", "b", "b");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change3 = createChange(git, "Change 3", "c", "c");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change4 = createChange(git, "Change 4", "d", "d");
+
+    submitStatusOnly(change2.getChangeId());
+    submitStatusOnly(change3.getChangeId());
+    submit(change4.getChangeId());
+
+    List<RevCommit> log = getRemoteLog();
+    RevCommit tip = log.get(0);
+    assertEquals(
+        change4.getCommit().getShortMessage(),
+        tip.getParent(1).getShortMessage());
+
+    tip = tip.getParent(0);
+    assertEquals(
+        change3.getCommit().getShortMessage(),
+        tip.getParent(1).getShortMessage());
+
+    tip = tip.getParent(0);
+    assertEquals(
+        change2.getCommit().getShortMessage(),
+        tip.getParent(1).getShortMessage());
+
+    assertEquals(initialHead.getId(), tip.getParent(0).getId());
+  }
 }
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
index d74a5ca984..486c529ed4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
@@ -1,14 +1,22 @@
 package com.google.gerrit.acceptance.rest.change;
 
+import static com.google.gerrit.acceptance.GitUtil.checkout;
 import static org.junit.Assert.assertEquals;
 
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.reviewdb.client.Project.SubmitType;
+import com.google.gwtorm.server.OrmException;
+
+import com.jcraft.jsch.JSchException;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
+import java.io.IOException;
+import java.util.List;
+
 public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
   @Override
@@ -27,4 +35,42 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertEquals(oldHead, head.getParent(0));
     assertSubmitter(change.getChangeId(), 1);
   }
+
+  @Test
+  public void submitMultipleChanges()
+      throws JSchException, IOException, GitAPIException, OrmException {
+    Git git = createProject();
+    RevCommit initialHead = getRemoteHead();
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change2 = createChange(git, "Change 2", "b", "b");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change3 = createChange(git, "Change 3", "c", "c");
+
+    checkout(git, initialHead.getId().getName());
+    PushOneCommit.Result change4 = createChange(git, "Change 4", "d", "d");
+
+    submitStatusOnly(change2.getChangeId());
+    submitStatusOnly(change3.getChangeId());
+    submit(change4.getChangeId());
+
+    List<RevCommit> log = getRemoteLog();
+    RevCommit tip = log.get(0);
+    assertEquals(
+        change4.getCommit().getShortMessage(),
+        tip.getParent(1).getShortMessage());
+
+    tip = tip.getParent(0);
+    assertEquals(
+        change3.getCommit().getShortMessage(),
+        tip.getParent(1).getShortMessage());
+
+    tip = tip.getParent(0);
+    assertEquals(
+        change2.getCommit().getShortMessage(),
+        tip.getShortMessage());
+
+    assertEquals(initialHead.getId(), tip.getParent(0).getId());
+  }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java
index 2448c66195..dc0d72192e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java
@@ -36,20 +36,19 @@ public class MergeAlways extends SubmitStrategy {
       // create the branch.
       mergeTip = toMerge.remove(0);
     }
-    CodeReviewCommit newMergeTip = mergeTip;
     while (!toMerge.isEmpty()) {
-      newMergeTip =
+      mergeTip =
           args.mergeUtil.mergeOneCommit(args.myIdent, args.repo, args.rw,
               args.inserter, args.canMergeFlag, args.destBranch, mergeTip,
               toMerge.remove(0));
     }
 
     final PatchSetApproval submitApproval =
-        args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, newMergeTip,
+        args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip,
             args.alreadyAccepted);
     setRefLogIdent(submitApproval);
 
-    return newMergeTip;
+    return mergeTip;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java
index 1cdc1b2eb2..601c6f8ccc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java
@@ -37,22 +37,22 @@ public class MergeIfNecessary extends SubmitStrategy {
       mergeTip = toMerge.remove(0);
     }
 
-    CodeReviewCommit newMergeTip =
+    mergeTip =
         args.mergeUtil.getFirstFastForward(mergeTip, args.rw, toMerge);
 
     // For every other commit do a pair-wise merge.
     while (!toMerge.isEmpty()) {
-      newMergeTip =
+      mergeTip =
           args.mergeUtil.mergeOneCommit(args.myIdent, args.repo, args.rw,
               args.inserter, args.canMergeFlag, args.destBranch, mergeTip,
               toMerge.remove(0));
     }
 
     final PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges(
-        args.rw, args.canMergeFlag, newMergeTip, args.alreadyAccepted);
+        args.rw, args.canMergeFlag, mergeTip, args.alreadyAccepted);
     setRefLogIdent(submitApproval);
 
-    return newMergeTip;
+    return mergeTip;
   }
 
   @Override