Support for parallel builds

Staging ref is reset to branch head after new build command.
Staging ref is rebuilt after successful build.
Staging ref is not rebuilt after failed build.
Staging ref rebuilt doesn't include integrating changes.
Lock added to prevent parallel execution of build approve commands.

Change-Id: I5707cf4d8b1c165dff78d6cc2b9356dfb31ad6c8
Reviewed-by: Daniel Smith <[email protected]>
Reviewed-by: Edward Welbourne <[email protected]>
Reviewed-by: Lars Knoll <[email protected]>

Author: jujokini

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCherryPickPatch.java

@@ -122,6 +122,7 @@ public CodeReviewCommit cherryPickPatch(
           RevCommit commit =
               QtUtil.merge(committerIdent, git, oi, revWalk, commitToCherryPick, baseCommit, true);
           cherryPickCommit = revWalk.parseCommit(commit);
+          logger.atInfo().log("qtcodereview: %s merged as %s", commitToCherryPick, cherryPickCommit);
         }
       } else {
         String commitMessage =
@@ -138,14 +139,14 @@ public CodeReviewCommit cherryPickPatch(
                 0,
                 true, // ignoreIdenticalTree
                 false); // allowConflicts
+        boolean patchSetNotChanged = cherryPickCommit.equals(commitToCherryPick);
+        if (!patchSetNotChanged) {
+          logger.atInfo().log(
+              "qtcodereview: %s cherrypicked as %s", commitToCherryPick, cherryPickCommit);
+          oi.flush();
+        }
       }
 
-      boolean patchSetNotChanged = cherryPickCommit.equals(commitToCherryPick);
-      if (!patchSetNotChanged) {
-        logger.atInfo().log(
-            "qtcodereview: %s cherrypicked as %s", commitToCherryPick, cherryPickCommit);
-        oi.flush();
-      }
       Timestamp commitTimestamp = new Timestamp(committerIdent.getWhen().getTime());
       BatchUpdate bu = batchUpdateFactory.create(project, identifiedUser, commitTimestamp);
       bu.addOp(

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandBuildApprove.java

@@ -40,6 +40,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.concurrent.locks.ReentrantLock;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.ObjectId;
@@ -82,6 +83,8 @@ class QtCommandBuildApprove extends SshCommand {
 
   @Inject private QtChangeUpdateOp.Factory qtUpdateFactory;
 
+  private final ReentrantLock buildApproveLock = new ReentrantLock();
+
   @Option(
       name = "--project",
       aliases = {"-p"},
@@ -139,6 +142,15 @@ class QtCommandBuildApprove extends SshCommand {
 
   @Override
   protected void run() throws UnloggedFailure {
+    buildApproveLock.lock();  // block processing of parallel requests
+    try {
+      runBuildApprove();
+    } finally {
+      buildApproveLock.unlock();
+    }
+  }
+
+  private void runBuildApprove() throws UnloggedFailure {
     logger.atInfo().log(
         "qtcodereview: staging-approve -p %s -i %s -r %s -m %s -b %s",
         project, buildBranch, result, message, destBranch);
@@ -229,8 +241,10 @@ private void approveBuildChanges()
         QtUtil.mergeBranches(user.asIdentifiedUser(), git, buildBranchKey, destBranchKey);
 
     if (result != Result.FAST_FORWARD) {
-      message =
-          "Branch update failed, changed back to NEW. Either the destination branch was changed externally, or this is an issue in the Qt plugin.";
+      message = "Unable to merge this integration because another integration parallel to this one "
+              + "successfully merged first and created a conflict in one of the tested changes.\n"
+              + "Please review, resolve conflicts if necessary, and restage.";
+      logger.atInfo().log(message);
       rejectBuildChanges();
       return;
     }
@@ -241,6 +255,10 @@ private void approveBuildChanges()
     logger.atInfo().log(
         "qtcodereview: staging-approve build %s merged into branch %s", buildBranch, destBranchKey);
 
+    // need to rebuild the staging ref to include recently merged changes
+    qtUtil.rebuildStagingBranch(
+        git, user.asIdentifiedUser(), projectKey, stagingBranchKey, destBranchShortKey);
+
     ObjectId newId = git.resolve(destBranchKey.branch());
     // send ref updated event only if there are changes to build
     if (!newId.equals(oldId)) {
@@ -262,10 +280,6 @@ private void rejectBuildChanges()
         ChangeMessagesUtil.TAG_REVERT,
         false);
 
-    // need to rebuild the staging ref because the reject changes need to be removed from there
-    qtUtil.rebuildStagingBranch(
-        git, user.asIdentifiedUser(), projectKey, stagingBranchKey, destBranchShortKey);
-
     logger.atInfo().log(
         "qtcodereview: staging-approve build %s rejected for branch %s",
         buildBranch, destBranchKey);

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtCommandNewBuild.java

@@ -162,6 +162,9 @@ protected void run() throws UnloggedFailure {
         }
       }
 
+      // reset staging ref back to branch head
+      result = QtUtil.createStagingBranch(git, destBranchShortKey);
+
       logger.atInfo().log(
           "qtcodereview: staging-new-build build %s for %s created", build, destBranchShortKey);
 

src/main/java/com/googlesource/gerrit/plugins/qtcodereview/QtUtil.java

@@ -361,44 +361,15 @@ private ObjectId pickChangesToStagingRef(
     return newId;
   }
 
-  // Step backwards from staging head and return 1st commit in integrating status
-  private ObjectId findIntegrationHead(
-      Repository git,
-      ObjectId stagingHead,
-      ObjectId branchHead,
-      List<ChangeData> integratingChanges)
-      throws MissingObjectException, IOException {
-
-    if (stagingHead.equals(branchHead)) return branchHead;
-
-    RevWalk revWalk = new RevWalk(git);
-    RevCommit commit = revWalk.parseCommit(stagingHead);
-    int count = 0;
-    do {
-      count++;
-      String changeId = getChangeId(commit);
-      ChangeData change = findChangeFromList(changeId, integratingChanges);
-      if (change != null) return commit;
-
-      if (commit.getParentCount() > 0) {
-        // It can always be trusted that parent in index 0 is the correct one
-        commit = revWalk.parseCommit(commit.getParent(0));
-      } else commit = null;
-
-    } while (commit != null && !commit.equals(branchHead) && count < 100);
-
-    return branchHead;
-  }
-
   // Step backwards from staging head and find commit that can be reused
   private ObjectId findReusableStagingHead(
       Repository git,
       ObjectId stagingHead,
-      ObjectId integrationHead,
+      ObjectId branchHead,
       List<ChangeData> stagedChanges)
       throws MissingObjectException, IOException {
 
-    if (stagingHead.equals(integrationHead)) return integrationHead;
+    if (stagingHead.equals(branchHead)) return branchHead;
 
     ObjectId reusableHead = null;
     RevWalk revWalk = new RevWalk(git);
@@ -417,9 +388,9 @@ private ObjectId findReusableStagingHead(
         commit = revWalk.parseCommit(commit.getParent(0));
       } else commit = null;
 
-    } while (commit != null && !commit.equals(integrationHead) && count < 100);
+    } while (commit != null && !commit.equals(branchHead) && count < 100);
 
-    if (reusableHead == null) reusableHead = integrationHead;
+    if (reusableHead == null) reusableHead = branchHead;
     return reusableHead;
   }
 
@@ -431,23 +402,18 @@ public void rebuildStagingBranch(
       final BranchNameKey destBranchShortKey)
       throws MergeConflictException {
     InternalChangeQuery query = null;
-    List<ChangeData> changes_integrating = null;
     List<ChangeData> changes_staged = null;
     List<ChangeData> changes_to_cherrypick = null;
     ObjectId oldStageRef = null;
     ObjectId branchRef = null;
     ObjectId newStageRef = null;
-    ObjectId integratingRef = null;
     String stagingBranchName = null;
 
     try {
       stagingBranchName = stagingBranchKey.branch();
       oldStageRef = git.resolve(stagingBranchName);
       branchRef = git.resolve(destBranchShortKey.branch());
 
-      query = queryProvider.get();
-      changes_integrating = query.byBranchStatus(destBranchShortKey, Change.Status.INTEGRATING);
-
       query = queryProvider.get();
       changes_staged = query.byBranchStatus(destBranchShortKey, Change.Status.STAGED);
     } catch (IOException e) {
@@ -466,9 +432,7 @@ public void rebuildStagingBranch(
         throw new NoSuchRefException("Cannot create staging ref: " + stagingBranchName);
       logger.atInfo().log(
           "qtcodereview: rebuild staging ref reset to %s with result %s", branchRef, result);
-      integratingRef = findIntegrationHead(git, oldStageRef, branchRef, changes_integrating);
-      logger.atInfo().log("qtcodereview: rebuild staging integration ref is %s", integratingRef);
-      newStageRef = findReusableStagingHead(git, oldStageRef, integratingRef, changes_staged);
+      newStageRef = findReusableStagingHead(git, oldStageRef, branchRef, changes_staged);
       logger.atInfo().log("qtcodereview: rebuild staging reused staging ref is %s", newStageRef);
       changes_to_cherrypick = arrangeOrderLikeInRef(git, oldStageRef, newStageRef, changes_staged);
     } catch (NoSuchRefException | IOException e) {
@@ -481,7 +445,7 @@ public void rebuildStagingBranch(
       newStageRef = pickChangesToStagingRef(git, projectKey, changes_to_cherrypick, newStageRef);
     } catch (Exception e) {
       logger.atInfo().log("qtcodereview: rebuild staging ref %s merge conflict", stagingBranchKey);
-      newStageRef = integratingRef;
+      newStageRef = branchRef;
       String message =
           "Merge conflict in staging branch. Status changed back to new. Please stage again.";
       QtChangeUpdateOp op =
@@ -612,7 +576,9 @@ public static RevCommit merge(
     mergeCommit.setCommitter(committerIdent);
     mergeCommit.setMessage(message);
 
-    return revWalk.parseCommit(objInserter.insert(mergeCommit));
+    ObjectId commit = objInserter.insert(mergeCommit);
+    objInserter.flush();
+    return revWalk.parseCommit(commit);
   }
 
   public static RefUpdate.Result mergeBranches(
@@ -648,7 +614,6 @@ private static RefUpdate.Result mergeObjectToBranch(
           user.newCommitterIdent(new Timestamp(System.currentTimeMillis()), TimeZone.getDefault());
 
       RevCommit mergeCommit = merge(committer, git, objInserter, revWalk, toMerge, mergeTip, false);
-      objInserter.flush();
       logger.atInfo().log("qtcodereview: merge commit for %s added to %s", srcId, destination);
 
       RefUpdate refUpdate = git.updateRef(destination.branch());

src/test/java/com/googlesource/gerrit/plugins/qtcodereview/QtCodeReviewIT.java

@@ -209,7 +209,13 @@ protected PushOneCommit.Result createUserChange(
     return result;
   }
 
-  protected void assertCherryPick(RevCommit head, RevCommit source, RevCommit base) {
+  protected void assertCherryPick(RevCommit head, RevCommit source, RevCommit base) throws Exception {
+    // Fetch all commit data
+    Repository repo = repoManager.openRepository(project);
+    RevWalk revWalk = new RevWalk(repo);
+    source = revWalk.parseCommit(source);
+    head = revWalk.parseCommit(head);
+
     assertThat(head).isNotEqualTo(source);
     assertThat(head.getName()).isNotEqualTo(source.getName());
     assertThat(head.getShortMessage()).isEqualTo(source.getShortMessage());