@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Update buildable containerPHIDs in a proper way via BuildWorker rather than via sneaky uncoordinated write

Summary:
Depends on D19065. Ref T13054. Instead of just updating `containerPHID` and hoping for the best, queue a proper BuildWorker to process a "your container has changed, update it" message.

We also need to remove a (superfluous) `withContainerPHIDs()` when loading active diffs for a revision.

Test Plan:
- Without daemons, created a revision and saw builds stick in "preparing" with no container PHID, but also stay in draft mode.
- With daemons, saw builds actually build and get the right container PHID.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13054

Differential Revision: https://secure.phabricator.com/D19066

+36 -16
+15 -14
src/applications/differential/editor/DifferentialTransactionEditor.php
··· 365 365 $diff->setRevisionID($object->getID()); 366 366 $diff->save(); 367 367 368 - // Update Harbormaster to set the containerPHID correctly for any 369 - // existing buildables. We may otherwise have buildables stuck with 370 - // the old (`null`) container. 368 + // If there are any outstanding buildables for this diff, tell 369 + // Harbormaster that their containers need to be updated. This is 370 + // common, because `arc` creates buildables so it can upload lint 371 + // and unit results. 371 372 372 - // TODO: This is a bit iffy, maybe we can find a cleaner approach? 373 - // In particular, this could (rarely) be overwritten by Harbormaster 374 - // workers. 375 - $table = new HarbormasterBuildable(); 376 - $conn_w = $table->establishConnection('w'); 377 - queryfx( 378 - $conn_w, 379 - 'UPDATE %T SET containerPHID = %s WHERE buildablePHID = %s', 380 - $table->getTableName(), 381 - $object->getPHID(), 382 - $diff->getPHID()); 373 + $buildables = id(new HarbormasterBuildableQuery()) 374 + ->setViewer(PhabricatorUser::getOmnipotentUser()) 375 + ->withManualBuildables(false) 376 + ->withBuildablePHIDs(array($diff->getPHID())) 377 + ->execute(); 378 + foreach ($buildables as $buildable) { 379 + $buildable->sendMessage( 380 + $this->getActor(), 381 + HarbormasterMessageType::BUILDABLE_CONTAINER, 382 + true); 383 + } 383 384 384 385 return; 385 386 }
+2 -1
src/applications/differential/storage/DifferentialRevision.php
··· 743 743 public function loadActiveBuilds(PhabricatorUser $viewer) { 744 744 $diff = $this->getActiveDiff(); 745 745 746 + // NOTE: We can't use `withContainerPHIDs()` here because the container 747 + // update in Harbormaster is not synchronous. 746 748 $buildables = id(new HarbormasterBuildableQuery()) 747 749 ->setViewer($viewer) 748 - ->withContainerPHIDs(array($this->getPHID())) 749 750 ->withBuildablePHIDs(array($diff->getPHID())) 750 751 ->withManualBuildables(false) 751 752 ->execute();
+18 -1
src/applications/harbormaster/engine/HarbormasterBuildEngine.php
··· 447 447 ->execute(); 448 448 449 449 $done_preparing = false; 450 + $update_container = false; 450 451 foreach ($messages as $message) { 451 452 switch ($message->getType()) { 452 453 case HarbormasterMessageType::BUILDABLE_BUILD: 453 454 $done_preparing = true; 455 + break; 456 + case HarbormasterMessageType::BUILDABLE_CONTAINER: 457 + $update_container = true; 454 458 break; 455 459 default: 456 460 break; ··· 463 467 464 468 // If we received a "build" command, all builds are scheduled and we can 465 469 // move out of "preparing" into "building". 466 - 467 470 if ($done_preparing) { 468 471 if ($buildable->isPreparing()) { 469 472 $buildable 470 473 ->setBuildableStatus(HarbormasterBuildableStatus::STATUS_BUILDING) 474 + ->save(); 475 + } 476 + } 477 + 478 + // If we've been informed that the container for the buildable has 479 + // changed, update it. 480 + if ($update_container) { 481 + $object = id(new PhabricatorObjectQuery()) 482 + ->setViewer($viewer) 483 + ->withPHIDs(array($buildable->getBuildablePHID())) 484 + ->executeOne(); 485 + if ($object) { 486 + $buildable 487 + ->setContainerPHID($object->getHarbormasterContainerPHID()) 471 488 ->save(); 472 489 } 473 490 }
+1
src/applications/harbormaster/engine/HarbormasterMessageType.php
··· 7 7 const MESSAGE_WORK = 'work'; 8 8 9 9 const BUILDABLE_BUILD = 'build'; 10 + const BUILDABLE_CONTAINER = 'container'; 10 11 11 12 public static function getAllMessages() { 12 13 return array_keys(self::getMessageSpecifications());