When publishing buildables in Differential, ignore autobuilds (local lint and unit)

Summary:
Depends on D19280. Ref T13110. Although Harbormaster cares about all builds, Differential does not practically care about local lint and unit results in determining build status.

In Differential, orient publishing around "remote builds" instead of "builds".

This does not yet change any of the draft logic, it just makes the timeline story use newer logic.

Test Plan: Used `bin/harbormaster publish` (with some guard-clause removal) to publish some buildables to revisions without anything crashing.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13110

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

src/__phutil_library_map__.php

@@ -530,6 +530,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionAffectedFilesHeraldField' => 'applications/differential/herald/DifferentialRevisionAffectedFilesHeraldField.php',
     'DifferentialRevisionAuthorHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorHeraldField.php',
     'DifferentialRevisionAuthorProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorProjectsHeraldField.php',
+    'DifferentialRevisionBuildableTransaction' => 'applications/differential/xaction/DifferentialRevisionBuildableTransaction.php',
     'DifferentialRevisionCloseDetailsController' => 'applications/differential/controller/DifferentialRevisionCloseDetailsController.php',
     'DifferentialRevisionCloseTransaction' => 'applications/differential/xaction/DifferentialRevisionCloseTransaction.php',
     'DifferentialRevisionClosedStatusDatasource' => 'applications/differential/typeahead/DifferentialRevisionClosedStatusDatasource.php',
@@ -5780,6 +5781,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionAffectedFilesHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionAuthorProjectsHeraldField' => 'DifferentialRevisionHeraldField',
+    'DifferentialRevisionBuildableTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionCloseDetailsController' => 'DifferentialController',
     'DifferentialRevisionCloseTransaction' => 'DifferentialRevisionActionTransaction',
     'DifferentialRevisionClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource',

src/applications/differential/harbormaster/DifferentialBuildableEngine.php

@@ -1,4 +1,57 @@
 <?php
 
 final class DifferentialBuildableEngine
-  extends HarbormasterBuildableEngine {}
+  extends HarbormasterBuildableEngine {
+
+  protected function getPublishableObject() {
+    $object = $this->getObject();
+
+    if ($object instanceof DifferentialDiff) {
+      return $object->getRevision();
+    }
+
+    return $object;
+  }
+
+  public function publishBuildable(
+    HarbormasterBuildable $old,
+    HarbormasterBuildable $new) {
+
+    // If we're publishing to a diff that is not actually attached to a
+    // revision, we have nothing to publish to, so just bail out.
+    $revision = $this->getPublishableObject();
+    if (!$revision) {
+      return;
+    }
+
+    // Don't publish manual buildables.
+    if ($new->getIsManualBuildable()) {
+      return;
+    }
+
+    // Don't publish anything if the buildable is still building. Differential
+    // treats more buildables as "building" than Harbormaster does, but the
+    // Differential definition is a superset of the Harbormaster definition.
+    if ($new->isBuilding()) {
+      return;
+    }
+
+    $viewer = $this->getViewer();
+
+    $old_status = $revision->getBuildableStatus($new->getPHID());
+    $new_status = $revision->newBuildableStatus($viewer, $new->getPHID());
+    if ($old_status === $new_status) {
+      return;
+    }
+
+    $buildable_type = DifferentialRevisionBuildableTransaction::TRANSACTIONTYPE;
+
+    $xaction = $this->newTransaction()
+      ->setMetadataValue('harbormaster:buildablePHID', $new->getPHID())
+      ->setTransactionType($buildable_type)
+      ->setNewValue($new_status);
+
+    $this->applyTransactions(array($xaction));
+  }
+
+}

src/applications/differential/storage/DifferentialDiff.php

@@ -509,10 +509,6 @@ final class DifferentialDiff
     return null;
   }
 
-  public function getHarbormasterPublishablePHID() {
-    return $this->getHarbormasterContainerPHID();
-  }
-
   public function getBuildVariables() {
     $results = array();
 

src/applications/differential/storage/DifferentialRevision.php

@@ -62,6 +62,7 @@ final class DifferentialRevision extends DifferentialDAO
   const PROPERTY_HAS_BROADCAST = 'draft.broadcast';
   const PROPERTY_LINES_ADDED = 'lines.added';
   const PROPERTY_LINES_REMOVED = 'lines.removed';
+  const PROPERTY_BUILDABLES = 'buildables';
 
   public static function initializeNewRevision(PhabricatorUser $actor) {
     $app = id(new PhabricatorApplicationQuery())
@@ -740,6 +741,78 @@ final class DifferentialRevision extends DifferentialDAO
     return $this->getProperty(self::PROPERTY_LINES_REMOVED);
   }
 
+
+  public function getBuildableStatus($phid) {
+    $buildables = $this->getProperty(self::PROPERTY_BUILDABLES);
+    if (!is_array($buildables)) {
+      $buildables = array();
+    }
+
+    $buildable = idx($buildables, $phid);
+    if (!is_array($buildable)) {
+      $buildable = array();
+    }
+
+    return idx($buildable, 'status');
+  }
+
+  public function setBuildableStatus($phid, $status) {
+    $buildables = $this->getProperty(self::PROPERTY_BUILDABLES);
+    if (!is_array($buildables)) {
+      $buildables = array();
+    }
+
+    $buildable = idx($buildables, $phid);
+    if (!is_array($buildable)) {
+      $buildable = array();
+    }
+
+    $buildable['status'] = $status;
+
+    $buildables[$phid] = $buildable;
+
+    return $this->setProperty(self::PROPERTY_BUILDABLES, $buildables);
+  }
+
+  public function newBuildableStatus(PhabricatorUser $viewer, $phid) {
+    // For Differential, we're ignoring autobuilds (local lint and unit)
+    // when computing build status. Differential only cares about remote
+    // builds when making publishing and undrafting decisions.
+
+    $builds = id(new HarbormasterBuildQuery())
+      ->setViewer($viewer)
+      ->withBuildablePHIDs(array($phid))
+      ->withAutobuilds(false)
+      ->withBuildStatuses(
+        array(
+          HarbormasterBuildStatus::STATUS_INACTIVE,
+          HarbormasterBuildStatus::STATUS_PENDING,
+          HarbormasterBuildStatus::STATUS_BUILDING,
+          HarbormasterBuildStatus::STATUS_FAILED,
+          HarbormasterBuildStatus::STATUS_ABORTED,
+          HarbormasterBuildStatus::STATUS_ERROR,
+          HarbormasterBuildStatus::STATUS_PAUSED,
+          HarbormasterBuildStatus::STATUS_DEADLOCKED,
+        ))
+      ->execute();
+
+    // If we have nothing but passing builds, the buildable passes.
+    if (!$builds) {
+      return HarbormasterBuildableStatus::STATUS_PASSED;
+    }
+
+    // If we have any completed, non-passing builds, the buildable fails.
+    foreach ($builds as $build) {
+      $status = $build->getBuildStatusObject();
+      if ($status->isComplete()) {
+        return HarbormasterBuildableStatus::STATUS_FAILED;
+      }
+    }
+
+    // Otherwise, we're still waiting for the build to pass or fail.
+    return null;
+  }
+
   public function loadActiveBuilds(PhabricatorUser $viewer) {
     $diff = $this->getActiveDiff();
 
@@ -788,10 +861,6 @@ final class DifferentialRevision extends DifferentialDAO
     return $this->getPHID();
   }
 
-  public function getHarbormasterPublishablePHID() {
-    return $this->getPHID();
-  }
-
   public function getBuildVariables() {
     return array();
   }

src/applications/differential/xaction/DifferentialRevisionBuildableTransaction.php

@@ -0,0 +1,93 @@
+<?php
+
+final class DifferentialRevisionBuildableTransaction
+  extends DifferentialRevisionTransactionType {
+
+  // NOTE: This uses an older constant for compatibility. We should perhaps
+  // migrate these at some point.
+  const TRANSACTIONTYPE = 'harbormaster:buildable';
+
+  public function generateNewValue($object, $value) {
+    return $value;
+  }
+
+  public function generateOldValue($object) {
+    return $object->getBuildableStatus($this->getBuildablePHID());
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $object->setBuildableStatus($this->getBuildablePHID(), $value);
+  }
+
+  public function getIcon() {
+    return $this->newBuildableStatus()->getIcon();
+  }
+
+  public function getColor() {
+    return $this->newBuildableStatus()->getColor();
+  }
+
+  public function getActionName() {
+    return $this->newBuildableStatus()->getActionName();
+  }
+
+  public function shouldHideForFeed() {
+    return !$this->newBuildableStatus()->isFailed();
+  }
+
+  public function shouldHideForMail() {
+    return !$this->newBuildableStatus()->isFailed();
+  }
+
+  public function getTitle() {
+    $new = $this->getNewValue();
+    $buildable_phid = $this->getBuildablePHID();
+
+    switch ($new) {
+      case HarbormasterBuildableStatus::STATUS_PASSED:
+        return pht(
+          '%s completed remote builds in %s.',
+          $this->renderAuthor(),
+          $this->renderHandle($buildable_phid));
+      case HarbormasterBuildableStatus::STATUS_FAILED:
+        return pht(
+          '%s failed remote builds in %s!',
+          $this->renderAuthor(),
+          $this->renderHandle($buildable_phid));
+    }
+
+    return null;
+  }
+
+  public function getTitleForFeed() {
+    $new = $this->getNewValue();
+    $buildable_phid = $this->getBuildablePHID();
+
+    switch ($new) {
+      case HarbormasterBuildableStatus::STATUS_PASSED:
+        return pht(
+          '%s completed remote builds in %s for %s.',
+          $this->renderAuthor(),
+          $this->renderHandle($buildable_phid),
+          $this->renderObject());
+      case HarbormasterBuildableStatus::STATUS_FAILED:
+        return pht(
+          '%s failed remote builds in %s for %s!',
+          $this->renderAuthor(),
+          $this->renderHandle($buildable_phid),
+          $this->renderObject());
+    }
+
+    return null;
+  }
+
+  private function newBuildableStatus() {
+    $new = $this->getNewValue();
+    return HarbormasterBuildableStatus::newBuildableStatusObject($new);
+  }
+
+  private function getBuildablePHID() {
+    return $this->getMetadataValue('harbormaster:buildablePHID');
+  }
+
+}

src/applications/harbormaster/engine/HarbormasterBuildableEngine.php

@@ -45,6 +45,10 @@ abstract class HarbormasterBuildableEngine
     return $this->object;
   }
 
+  protected function getPublishableObject() {
+    return $this->getObject();
+  }
+
   public function publishBuildable(
     HarbormasterBuildable $old,
     HarbormasterBuildable $new) {
@@ -60,7 +64,7 @@ abstract class HarbormasterBuildableEngine
   }
 
   final protected function newEditor() {
-    $publishable = $this->getObject();
+    $publishable = $this->getPublishableObject();
 
     $viewer = $this->getViewer();
 
@@ -83,13 +87,13 @@ abstract class HarbormasterBuildableEngine
   }
 
   final protected function newTransaction() {
-    $publishable = $this->getObject();
+    $publishable = $this->getPublishableObject();
 
     return $publishable->getApplicationTransactionTemplate();
   }
 
   final protected function applyTransactions(array $xactions) {
-    $publishable = $this->getObject();
+    $publishable = $this->getPublishableObject();
     $editor = $this->newEditor();
 
     $editor->applyTransactions(

src/applications/harbormaster/interface/HarbormasterBuildableInterface.php

@@ -18,21 +18,6 @@ interface HarbormasterBuildableInterface {
   public function getHarbormasterBuildablePHID();
   public function getHarbormasterContainerPHID();
 
-
-  /**
-   * Get the object PHID which build status should be published to.
-   *
-   * In some cases (like commits), this is the object itself. In other cases,
-   * it is a different object: for example, diffs publish builds to revisions.
-   *
-   * This method can return `null` to disable publishing.
-   *
-   * @return phid|null Build status updates will be published to this object's
-   *  transaction timeline.
-   */
-  public function getHarbormasterPublishablePHID();
-
-
   public function getBuildVariables();
   public function getAvailableBuildVariables();
 

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -333,10 +333,6 @@ final class HarbormasterBuildable
     return $this->getContainerPHID();
   }
 
-  public function getHarbormasterPublishablePHID() {
-    return $this->getBuildableObject()->getHarbormasterPublishablePHID();
-  }
-
   public function getBuildVariables() {
     return array();
   }

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -517,10 +517,6 @@ final class PhabricatorRepositoryCommit
     return $this->getRepository()->getPHID();
   }
 
-  public function getHarbormasterPublishablePHID() {
-    return $this->getPHID();
-  }
-
   public function getBuildVariables() {
     $results = array();