diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 010232a64c..80279c40fc 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/differential/harbormaster/DifferentialBuildableEngine.php b/src/applications/differential/harbormaster/DifferentialBuildableEngine.php index 9ba44d9b81..df38885edf 100644 --- a/src/applications/differential/harbormaster/DifferentialBuildableEngine.php +++ b/src/applications/differential/harbormaster/DifferentialBuildableEngine.php @@ -1,4 +1,57 @@ 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)); + } + +} diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index 9a352af48c..728ec4b538 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/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(); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 45deef4616..446d180018 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/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(); } diff --git a/src/applications/differential/xaction/DifferentialRevisionBuildableTransaction.php b/src/applications/differential/xaction/DifferentialRevisionBuildableTransaction.php new file mode 100644 index 0000000000..3c4eaff729 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionBuildableTransaction.php @@ -0,0 +1,93 @@ +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'); + } + +} diff --git a/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php index 900839bb10..41d31673d1 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php +++ b/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( diff --git a/src/applications/harbormaster/interface/HarbormasterBuildableInterface.php b/src/applications/harbormaster/interface/HarbormasterBuildableInterface.php index 9994baba21..72ddf6ac99 100644 --- a/src/applications/harbormaster/interface/HarbormasterBuildableInterface.php +++ b/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(); diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php index b7d7281922..7d4f2e7c83 100644 --- a/src/applications/harbormaster/storage/HarbormasterBuildable.php +++ b/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(); } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 1bc0930e67..f6e6e22c3a 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/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();