diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -365,21 +365,22 @@ $diff->setRevisionID($object->getID()); $diff->save(); - // Update Harbormaster to set the containerPHID correctly for any - // existing buildables. We may otherwise have buildables stuck with - // the old (`null`) container. - - // TODO: This is a bit iffy, maybe we can find a cleaner approach? - // In particular, this could (rarely) be overwritten by Harbormaster - // workers. - $table = new HarbormasterBuildable(); - $conn_w = $table->establishConnection('w'); - queryfx( - $conn_w, - 'UPDATE %T SET containerPHID = %s WHERE buildablePHID = %s', - $table->getTableName(), - $object->getPHID(), - $diff->getPHID()); + // If there are any outstanding buildables for this diff, tell + // Harbormaster that their containers need to be updated. This is + // common, because `arc` creates buildables so it can upload lint + // and unit results. + + $buildables = id(new HarbormasterBuildableQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withManualBuildables(false) + ->withBuildablePHIDs(array($diff->getPHID())) + ->execute(); + foreach ($buildables as $buildable) { + $buildable->sendMessage( + $this->getActor(), + HarbormasterMessageType::BUILDABLE_CONTAINER, + true); + } return; } 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 @@ -743,9 +743,10 @@ public function loadActiveBuilds(PhabricatorUser $viewer) { $diff = $this->getActiveDiff(); + // NOTE: We can't use `withContainerPHIDs()` here because the container + // update in Harbormaster is not synchronous. $buildables = id(new HarbormasterBuildableQuery()) ->setViewer($viewer) - ->withContainerPHIDs(array($this->getPHID())) ->withBuildablePHIDs(array($diff->getPHID())) ->withManualBuildables(false) ->execute(); 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 @@ -447,11 +447,15 @@ ->execute(); $done_preparing = false; + $update_container = false; foreach ($messages as $message) { switch ($message->getType()) { case HarbormasterMessageType::BUILDABLE_BUILD: $done_preparing = true; break; + case HarbormasterMessageType::BUILDABLE_CONTAINER: + $update_container = true; + break; default: break; } @@ -463,7 +467,6 @@ // 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 @@ -472,6 +475,20 @@ } } + // If we've been informed that the container for the buildable has + // changed, update it. + if ($update_container) { + $object = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withPHIDs(array($buildable->getBuildablePHID())) + ->executeOne(); + if ($object) { + $buildable + ->setContainerPHID($object->getHarbormasterContainerPHID()) + ->save(); + } + } + // 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 diff --git a/src/applications/harbormaster/engine/HarbormasterMessageType.php b/src/applications/harbormaster/engine/HarbormasterMessageType.php --- a/src/applications/harbormaster/engine/HarbormasterMessageType.php +++ b/src/applications/harbormaster/engine/HarbormasterMessageType.php @@ -7,6 +7,7 @@ const MESSAGE_WORK = 'work'; const BUILDABLE_BUILD = 'build'; + const BUILDABLE_CONTAINER = 'container'; public static function getAllMessages() { return array_keys(self::getMessageSpecifications());