Page MenuHomePhabricator

D19281.diff
No OneTemporary

D19281.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -530,6 +530,7 @@
'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 @@
'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
--- a/src/applications/differential/harbormaster/DifferentialBuildableEngine.php
+++ b/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));
+ }
+
+}
diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php
--- a/src/applications/differential/storage/DifferentialDiff.php
+++ b/src/applications/differential/storage/DifferentialDiff.php
@@ -509,10 +509,6 @@
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
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -62,6 +62,7 @@
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 @@
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 @@
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
--- /dev/null
+++ b/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');
+ }
+
+}
diff --git a/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php
--- a/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterBuildableEngine.php
@@ -45,6 +45,10 @@
return $this->object;
}
+ protected function getPublishableObject() {
+ return $this->getObject();
+ }
+
public function publishBuildable(
HarbormasterBuildable $old,
HarbormasterBuildable $new) {
@@ -60,7 +64,7 @@
}
final protected function newEditor() {
- $publishable = $this->getObject();
+ $publishable = $this->getPublishableObject();
$viewer = $this->getViewer();
@@ -83,13 +87,13 @@
}
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
--- a/src/applications/harbormaster/interface/HarbormasterBuildableInterface.php
+++ b/src/applications/harbormaster/interface/HarbormasterBuildableInterface.php
@@ -18,21 +18,6 @@
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
--- a/src/applications/harbormaster/storage/HarbormasterBuildable.php
+++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php
@@ -333,10 +333,6 @@
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
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -517,10 +517,6 @@
return $this->getRepository()->getPHID();
}
- public function getHarbormasterPublishablePHID() {
- return $this->getPHID();
- }
-
public function getBuildVariables() {
$results = array();

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 2:52 PM (3 d, 12 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7715594
Default Alt Text
D19281.diff (13 KB)

Event Timeline