diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -360,11 +360,13 @@ * @return void */ private function updateBuildable(HarbormasterBuildable $buildable) { + $viewer = $this->getViewer(); + $lock_key = 'harbormaster.buildable:'.$buildable->getID(); $lock = PhabricatorGlobalLock::newLock($lock_key)->lock(15); $buildable = id(new HarbormasterBuildableQuery()) - ->setViewer($this->getViewer()) + ->setViewer($viewer) ->withIDs(array($buildable->getID())) ->needBuilds(true) ->executeOne(); @@ -389,12 +391,62 @@ $new_status = HarbormasterBuildable::STATUS_BUILDING; } - if ($buildable->getBuildableStatus() != $new_status) { + $old_status = $buildable->getBuildableStatus(); + $did_update = ($old_status != $new_status); + if ($did_update) { $buildable->setBuildableStatus($new_status); $buildable->save(); } $lock->unlock(); + + // 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. + + // NOTE: We only post transactions for automatic buildables, not for + // manual ones: manual builds are test builds, whoever is doing tests + // can look at the results themselves, and other users generally don't + // care about the outcome. + + if ($did_update && !$buildable->getIsManualBuildable()) { + + $object = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($buildable->getBuildablePHID())) + ->executeOne(); + + if ($object instanceof PhabricatorApplicationTransactionInterface) { + $template = $object->getApplicationTransactionTemplate(); + if ($template) { + $template + ->setTransactionType(PhabricatorTransactions::TYPE_BUILDABLE) + ->setMetadataValue( + 'harbormaster:buildablePHID', + $buildable->getPHID()) + ->setOldValue($old_status) + ->setNewValue($new_status); + + $harbormaster_phid = id(new PhabricatorApplicationHarbormaster()) + ->getPHID(); + + $daemon_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_DAEMON, + array()); + + $editor = $object->getApplicationTransactionEditor() + ->setActor($viewer) + ->setActingAsPHID($harbormaster_phid) + ->setContentSource($daemon_source) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $editor->applyTransactions( + $object->getApplicationTransactionObject(), + array($template)); + } + } + } + } } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -292,19 +292,32 @@ return $this->save(); } - protected function didWriteData() { - parent::didWriteData(); + public function save() { + if ($this->getID()) { + return parent::save(); + } + + // NOTE: When mail is sent from CLI scripts that run tasks in-process, we + // may re-enter this method from within scheduleTask(). The implementation + // is intended to avoid anything awkward if we end up reentering this + // method. - if (!$this->getWorkerTaskID()) { + $this->openTransaction(); + // Save to generate a task ID. + parent::save(); + + // Queue a task to send this mail. $mailer_task = PhabricatorWorker::scheduleTask( 'PhabricatorMetaMTAWorker', $this->getID()); + // Save again to update the task ID. $this->setWorkerTaskID($mailer_task->getID()); - $this->save(); - } - } + $result = parent::save(); + $this->saveTransaction(); + return $result; + } public function buildDefaultMailer() { return PhabricatorEnv::newObjectFromConfig('metamta.mail-adapter'); diff --git a/src/applications/transactions/constants/PhabricatorTransactions.php b/src/applications/transactions/constants/PhabricatorTransactions.php --- a/src/applications/transactions/constants/PhabricatorTransactions.php +++ b/src/applications/transactions/constants/PhabricatorTransactions.php @@ -9,6 +9,7 @@ const TYPE_JOIN_POLICY = 'core:join-policy'; const TYPE_EDGE = 'core:edge'; const TYPE_CUSTOMFIELD = 'core:customfield'; + const TYPE_BUILDABLE = 'harbormaster:buildable'; const COLOR_RED = 'red'; const COLOR_ORANGE = 'orange'; diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -155,6 +155,10 @@ $types[] = PhabricatorTransactions::TYPE_CUSTOMFIELD; } + if ($this->object instanceof HarbormasterBuildableInterface) { + $types[] = PhabricatorTransactions::TYPE_BUILDABLE; + } + return $types; } @@ -222,6 +226,7 @@ case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY: case PhabricatorTransactions::TYPE_JOIN_POLICY: + case PhabricatorTransactions::TYPE_BUILDABLE: return $xaction->getNewValue(); case PhabricatorTransactions::TYPE_EDGE: return $this->getEdgeTransactionNewValue($xaction); @@ -311,6 +316,8 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_BUILDABLE: + return; case PhabricatorTransactions::TYPE_VIEW_POLICY: $object->setViewPolicy($xaction->getNewValue()); break; @@ -321,6 +328,7 @@ $field = $this->getCustomFieldForTransaction($object, $xaction); return $field->applyApplicationTransactionInternalEffects($xaction); } + return $this->applyCustomInternalTransaction($object, $xaction); } @@ -328,6 +336,8 @@ PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_BUILDABLE: + return; case PhabricatorTransactions::TYPE_SUBSCRIBERS: $subeditor = id(new PhabricatorSubscriptionsEditor()) ->setObject($object) diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -52,6 +52,8 @@ public function shouldGenerateOldValue() { switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_BUILDABLE: + return false; case PhabricatorTransactions::TYPE_CUSTOMFIELD: return false; } @@ -234,6 +236,12 @@ $phids[] = array($new); } break; + case PhabricatorTransactions::TYPE_BUILDABLE: + $phid = $this->getMetadataValue('harbormaster:buildablePHID'); + if ($phid) { + $phids[] = array($phid); + } + break; } return array_mergev($phids); @@ -325,12 +333,24 @@ return 'lock'; case PhabricatorTransactions::TYPE_EDGE: return 'link'; + case PhabricatorTransactions::TYPE_BUILDABLE: + return 'wrench'; } return 'edit'; } public function getColor() { + switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_BUILDABLE: + switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_PASSED: + return 'green'; + case HarbormasterBuildable::STATUS_FAILED: + return 'red'; + } + break; + } return null; } @@ -379,6 +399,17 @@ } public function shouldHideForMail(array $xactions) { + switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_BUILDABLE: + switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_PASSED: + // For now, just never send mail when builds pass. We might let + // you customize this later, but in most cases this is probably + // completely uninteresting. + return true; + } + } + return $this->shouldHide(); } @@ -537,6 +568,24 @@ $this->renderHandleLink($author_phid)); } + case PhabricatorTransactions::TYPE_BUILDABLE: + switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_PASSED: + return pht( + '%s completed building %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID'))); + case HarbormasterBuildable::STATUS_FAILED: + return pht( + '%s failed to build %s!', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID'))); + default: + return null; + } + default: return pht( '%s edited this %s.', @@ -596,6 +645,25 @@ $this->renderHandleLink($author_phid), $this->renderHandleLink($object_phid)); } + case PhabricatorTransactions::TYPE_BUILDABLE: + switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_PASSED: + return pht( + '%s completed building %s for %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID')), + $this->renderHandleLink($object_phid)); + case HarbormasterBuildable::STATUS_FAILED: + return pht( + '%s failed to build %s for %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID')), + $this->renderHandleLink($object_phid)); + default: + return null; + } } @@ -649,6 +717,15 @@ return pht('Changed Policy'); case PhabricatorTransactions::TYPE_SUBSCRIBERS: return pht('Changed Subscribers'); + case PhabricatorTransactions::TYPE_BUILDABLE: + switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_PASSED: + return pht('Build Passed'); + case HarbormasterBuildable::STATUS_FAILED: + return pht('Build Failed'); + default: + return pht('Build Status'); + } default: return pht('Updated'); }