Page MenuHomePhabricator

D19066.diff
No OneTemporary

D19066.diff

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());

File Metadata

Mime Type
text/plain
Expires
Thu, May 30, 5:19 AM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6294034
Default Alt Text
D19066.diff (4 KB)

Event Timeline