Start buildables in "PREPARING", move them to "BUILDING" after builds queue

Summary:
Depends on D19064. Ref T13054. See that task for additional discussion.

When buildables are created by `arc` and have lint/unit messages, they can currently pass or fail before Herald triggers actual builds. This puts them in a pre-build state where they can't complete until Herald says it's okay.

On its own, this change intentionally strands `arc diff --only` diffs in the "PREPARING" stage forever.

Test Plan:
  - Ran a build with `bin/harbormaster`, saw it build normally.
  - Ran a build with web UI, saw it build normally.
  - Ran a build with `arc diff`, saw it build normally.
  - Ran a build with `arc diff --only`, saw it hang in "PREPARING" forever.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19065

src/applications/harbormaster/constants/HarbormasterBuildableStatus.php

@@ -2,6 +2,7 @@
 
 final class HarbormasterBuildableStatus extends Phobject {
 
+  const STATUS_PREPARING = 'preparing';
   const STATUS_BUILDING = 'building';
   const STATUS_PASSED = 'passed';
   const STATUS_FAILED = 'failed';
@@ -42,12 +43,21 @@ final class HarbormasterBuildableStatus extends Phobject {
     return $this->getProperty('color');
   }
 
+  public function isPreparing() {
+    return ($this->key === self::STATUS_PREPARING);
+  }
+
   public static function getOptionMap() {
     return ipull(self::getSpecifications(), 'name');
   }
 
   private static function getSpecifications() {
     return array(
+      self::STATUS_PREPARING => array(
+        'name' => pht('Preparing'),
+        'color' => 'blue',
+        'icon' => 'fa-hourglass-o',
+      ),
       self::STATUS_BUILDING => array(
         'name' => pht('Building'),
         'color' => 'blue',

src/applications/harbormaster/controller/HarbormasterPlanRunController.php

@@ -59,6 +59,12 @@ final class HarbormasterPlanRunController extends HarbormasterPlanController {
 
       if (!$errors) {
         $buildable->save();
+
+        $buildable->sendMessage(
+          $viewer,
+          HarbormasterMessageType::BUILDABLE_BUILD,
+          false);
+
         $buildable->applyPlan($plan, array(), $viewer->getPHID());
 
         $buildable_uri = '/B'.$buildable->getID();

src/applications/harbormaster/engine/HarbormasterBuildEngine.php

@@ -428,7 +428,7 @@ final class HarbormasterBuildEngine extends Phobject {
    * @param   HarbormasterBuild The buildable to update.
    * @return  void
    */
-  private function updateBuildable(HarbormasterBuildable $buildable) {
+   public function updateBuildable(HarbormasterBuildable $buildable) {
     $viewer = $this->getViewer();
 
     $lock_key = 'harbormaster.buildable:'.$buildable->getID();
@@ -440,35 +440,79 @@ final class HarbormasterBuildEngine extends Phobject {
       ->needBuilds(true)
       ->executeOne();
 
-    $all_pass = true;
-    $any_fail = false;
-    foreach ($buildable->getBuilds() as $build) {
-      if (!$build->isPassed()) {
-        $all_pass = false;
+    $messages = id(new HarbormasterBuildMessageQuery())
+      ->setViewer($viewer)
+      ->withReceiverPHIDs(array($buildable->getPHID()))
+      ->withConsumed(false)
+      ->execute();
+
+    $done_preparing = false;
+    foreach ($messages as $message) {
+      switch ($message->getType()) {
+        case HarbormasterMessageType::BUILDABLE_BUILD:
+          $done_preparing = true;
+          break;
+        default:
+          break;
       }
 
-      if ($build->isComplete() && !$build->isPassed()) {
-        $any_fail = true;
+      $message
+        ->setIsConsumed(true)
+        ->save();
+    }
+
+    // If we received a "build" command, all builds are scheduled and we can
+    // move out of "preparing" into "building".
+
+    if ($done_preparing) {
+      if ($buildable->isPreparing()) {
+        $buildable
+          ->setBuildableStatus(HarbormasterBuildableStatus::STATUS_BUILDING)
+          ->save();
       }
     }
 
-    if ($any_fail) {
-      $new_status = HarbormasterBuildableStatus::STATUS_FAILED;
-    } else if ($all_pass) {
-      $new_status = HarbormasterBuildableStatus::STATUS_PASSED;
-    } else {
-      $new_status = HarbormasterBuildableStatus::STATUS_BUILDING;
-    }
+    // Don't update the buildable status if we're still preparing builds: more
+    // builds may still be scheduled shortly, so even if every build we know
+    // about so far has passed, that doesn't mean the buildable has actually
+    // passed everything it needs to.
 
-    $old_status = $buildable->getBuildableStatus();
-    $did_update = ($old_status != $new_status);
-    if ($did_update) {
-      $buildable->setBuildableStatus($new_status);
-      $buildable->save();
+    if (!$buildable->isPreparing()) {
+      $all_pass = true;
+      $any_fail = false;
+      foreach ($buildable->getBuilds() as $build) {
+        if (!$build->isPassed()) {
+          $all_pass = false;
+        }
+
+        if ($build->isComplete() && !$build->isPassed()) {
+          $any_fail = true;
+        }
+      }
+
+      if ($any_fail) {
+        $new_status = HarbormasterBuildableStatus::STATUS_FAILED;
+      } else if ($all_pass) {
+        $new_status = HarbormasterBuildableStatus::STATUS_PASSED;
+      } else {
+        $new_status = HarbormasterBuildableStatus::STATUS_BUILDING;
+      }
+
+      $old_status = $buildable->getBuildableStatus();
+      $did_update = ($old_status != $new_status);
+      if ($did_update) {
+        $buildable->setBuildableStatus($new_status);
+        $buildable->save();
+      }
     }
 
     $lock->unlock();
 
+    // Don't publish anything if we're still preparing builds.
+    if ($buildable->isPreparing()) {
+      return;
+    }
+
     // If we changed the buildable status, try to post a transaction to the
     // object about it. We can safely do this outside of the locked region.
 

src/applications/harbormaster/engine/HarbormasterMessageType.php

@@ -6,6 +6,8 @@ final class HarbormasterMessageType extends Phobject {
   const MESSAGE_FAIL = 'fail';
   const MESSAGE_WORK = 'work';
 
+  const BUILDABLE_BUILD = 'build';
+
   public static function getAllMessages() {
     return array_keys(self::getMessageSpecifications());
   }

src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php

@@ -83,6 +83,11 @@ final class HarbormasterManagementBuildWorkflow
       ->setContainerPHID($buildable->getHarbormasterContainerPHID())
       ->save();
 
+    $buildable->sendMessage(
+      $viewer,
+      HarbormasterMessageType::BUILDABLE_BUILD,
+      false);
+
     $console->writeOut(
       "%s\n",
       pht(

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -18,7 +18,7 @@ final class HarbormasterBuildable extends HarbormasterDAO
   public static function initializeNewBuildable(PhabricatorUser $actor) {
     return id(new HarbormasterBuildable())
       ->setIsManualBuildable(0)
-      ->setBuildableStatus(HarbormasterBuildableStatus::STATUS_BUILDING);
+      ->setBuildableStatus(HarbormasterBuildableStatus::STATUS_PREPARING);
   }
 
   public function getMonogram() {
@@ -227,6 +227,38 @@ final class HarbormasterBuildable extends HarbormasterDAO
     return $this->getBuildableStatusObject()->getColor();
   }
 
+  public function isPreparing() {
+    return $this->getBuildableStatusObject()->isPreparing();
+  }
+
+
+/* -(  Messages  )----------------------------------------------------------- */
+
+
+  public function sendMessage(
+    PhabricatorUser $viewer,
+    $message_type,
+    $queue_update) {
+
+    $message = HarbormasterBuildMessage::initializeNewMessage($viewer)
+      ->setReceiverPHID($this->getPHID())
+      ->setType($message_type)
+      ->save();
+
+    if ($queue_update) {
+      PhabricatorWorker::scheduleTask(
+        'HarbormasterBuildWorker',
+        array(
+          'buildablePHID' => $this->getPHID(),
+        ),
+        array(
+          'objectPHID' => $this->getPHID(),
+        ));
+    }
+
+    return $message;
+  }
+
 
 /* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
 

src/applications/harbormaster/worker/HarbormasterBuildWorker.php

@@ -17,12 +17,21 @@ final class HarbormasterBuildWorker extends HarbormasterWorker {
 
   protected function doWork() {
     $viewer = $this->getViewer();
-    $build = $this->loadBuild();
 
-    id(new HarbormasterBuildEngine())
-      ->setViewer($viewer)
-      ->setBuild($build)
-      ->continueBuild();
+    $engine = id(new HarbormasterBuildEngine())
+      ->setViewer($viewer);
+
+    $data = $this->getTaskData();
+    $build_id = idx($data, 'buildID');
+
+    if ($build_id) {
+      $build = $this->loadBuild();
+      $engine->setBuild($build);
+      $engine->continueBuild();
+    } else {
+      $buildable = $this->loadBuildable();
+      $engine->updateBuildable($buildable);
+    }
   }
 
   private function loadBuild() {
@@ -42,4 +51,21 @@ final class HarbormasterBuildWorker extends HarbormasterWorker {
     return $build;
   }
 
+  private function loadBuildable() {
+    $data = $this->getTaskData();
+    $phid = idx($data, 'buildablePHID');
+
+    $viewer = $this->getViewer();
+    $buildable = id(new HarbormasterBuildableQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($phid))
+      ->executeOne();
+    if (!$buildable) {
+      throw new PhabricatorWorkerPermanentFailureException(
+        pht('Invalid buildable PHID "%s".', $phid));
+    }
+
+    return $buildable;
+  }
+
 }

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -3262,10 +3262,32 @@ abstract class PhabricatorApplicationTransactionEditor
     $this->setHeraldTranscript($xscript);
 
     if ($adapter instanceof HarbormasterBuildableAdapterInterface) {
+      $buildable_phid = $adapter->getHarbormasterBuildablePHID();
+
       HarbormasterBuildable::applyBuildPlans(
-        $adapter->getHarbormasterBuildablePHID(),
+        $buildable_phid,
         $adapter->getHarbormasterContainerPHID(),
         $adapter->getQueuedHarbormasterBuildRequests());
+
+      // Whether we queued any builds or not, any automatic buildable for this
+      // object is now done preparing builds and can transition into a
+      // completed status.
+      $buildables = id(new HarbormasterBuildableQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withManualBuildables(false)
+        ->withBuildablePHIDs(array($buildable_phid))
+        ->execute();
+      foreach ($buildables as $buildable) {
+        // If this buildable has already moved beyond preparation, we don't
+        // need to nudge it again.
+        if (!$buildable->isPreparing()) {
+          continue;
+        }
+        $buildable->sendMessage(
+          $this->getActor(),
+          HarbormasterMessageType::BUILDABLE_BUILD,
+          true);
+      }
     }
 
     $this->mustEncrypt = $adapter->getMustEncryptReasons();