Page MenuHomePhabricator
Files F13225488

D20236.diff
No OneTemporary
Actions

D20236.diff
View Options

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
@@ -42,6 +42,7 @@
'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php',
'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php',
'ArcanistBrowseWorkflow' => 'workflow/ArcanistBrowseWorkflow.php',
+ 'ArcanistBuildPlanRef' => 'ref/ArcanistBuildPlanRef.php',
'ArcanistBuildRef' => 'ref/ArcanistBuildRef.php',
'ArcanistBundle' => 'parser/ArcanistBundle.php',
'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php',
@@ -464,6 +465,7 @@
'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow',
'ArcanistBrowseWorkflow' => 'ArcanistWorkflow',
+ 'ArcanistBuildPlanRef' => 'Phobject',
'ArcanistBuildRef' => 'Phobject',
'ArcanistBundle' => 'Phobject',
'ArcanistBundleTestCase' => 'PhutilTestCase',
diff --git a/src/ref/ArcanistBuildPlanRef.php b/src/ref/ArcanistBuildPlanRef.php
new file mode 100644
--- /dev/null
+++ b/src/ref/ArcanistBuildPlanRef.php
@@ -0,0 +1,25 @@
+<?php
+
+final class ArcanistBuildPlanRef
+ extends Phobject {
+
+ private $parameters;
+
+ public static function newFromConduit(array $data) {
+ $ref = new self();
+ $ref->parameters = $data;
+ return $ref;
+ }
+
+ public function getPHID() {
+ return $this->parameters['phid'];
+ }
+
+ public function getBehavior($behavior_key, $default = null) {
+ return idxv(
+ $this->parameters,
+ array('fields', 'behaviors', $behavior_key, 'value'),
+ $default);
+ }
+
+}
diff --git a/src/ref/ArcanistBuildRef.php b/src/ref/ArcanistBuildRef.php
--- a/src/ref/ArcanistBuildRef.php
+++ b/src/ref/ArcanistBuildRef.php
@@ -69,6 +69,10 @@
return $this->parameters['id'];
}
+ public function getPHID() {
+ return $this->parameters['phid'];
+ }
+
public function getName() {
if (isset($this->parameters['fields']['name'])) {
return $this->parameters['fields']['name'];
@@ -96,11 +100,32 @@
return pht('Build %d', $this->getID());
}
+ public function getBuildPlanPHID() {
+ return idxv($this->parameters, array('fields', 'buildPlanPHID'));
+ }
+
+ public function isComplete() {
+ switch ($this->getStatus()) {
+ case 'passed':
+ case 'failed':
+ case 'aborted':
+ case 'error':
+ case 'deadlocked':
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ public function isPassed() {
+ return ($this->getStatus() === 'passed');
+ }
+
public function getStatusSortVector() {
$status = $this->getStatus();
// For now, just sort passed builds first.
- if ($this->getStatus() == 'passed') {
+ if ($this->isPassed()) {
$status_class = 1;
} else {
$status_class = 2;
diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php
--- a/src/workflow/ArcanistLandWorkflow.php
+++ b/src/workflow/ArcanistLandWorkflow.php
@@ -1369,6 +1369,19 @@
* before landing if it does.
*/
private function checkForBuildables($diff_phid) {
+ // Try to use the more modern check which respects the "Warn on Land"
+ // behavioral flag on build plans if we can. This newer check won't work
+ // unless the server is running code from March 2019 or newer since the
+ // API methods we need won't exist yet. We'll fall back to the older check
+ // if this one doesn't work out.
+ try {
+ $this->checkForBuildablesWithPlanBehaviors($diff_phid);
+ } catch (ArcanistUserAbortException $abort_ex) {
+ throw $abort_ex;
+ } catch (Exception $ex) {
+ // Continue with the older approach, below.
+ }
+
// NOTE: Since Harbormaster is still beta and this stuff all got added
// recently, just bail if we can't find a buildable. This is just an
// advisory check intended to prevent human error.
@@ -1449,6 +1462,166 @@
}
}
+ private function checkForBuildablesWithPlanBehaviors($diff_phid) {
+ // TODO: These queries should page through all results instead of fetching
+ // only the first page, but we don't have good primitives to support that
+ // in "master" yet.
+
+ $this->writeInfo(
+ pht('BUILDS'),
+ pht('Checking build status...'));
+
+ $raw_buildables = $this->getConduit()->callMethodSynchronous(
+ 'harbormaster.buildable.search',
+ array(
+ 'constraints' => array(
+ 'objectPHIDs' => array(
+ $diff_phid,
+ ),
+ 'manual' => false,
+ ),
+ ));
+
+ if (!$raw_buildables['data']) {
+ return;
+ }
+
+ $buildables = $raw_buildables['data'];
+ $buildable_phids = ipull($buildables, 'phid');
+
+ $raw_builds = $this->getConduit()->callMethodSynchronous(
+ 'harbormaster.build.search',
+ array(
+ 'constraints' => array(
+ 'buildables' => $buildable_phids,
+ ),
+ ));
+
+ if (!$raw_builds['data']) {
+ return;
+ }
+
+ $builds = array();
+ foreach ($raw_builds['data'] as $raw_build) {
+ $build_ref = ArcanistBuildRef::newFromConduit($raw_build);
+ $build_phid = $build_ref->getPHID();
+ $builds[$build_phid] = $build_ref;
+ }
+
+ $plan_phids = mpull($builds, 'getBuildPlanPHID');
+ $plan_phids = array_values($plan_phids);
+
+ $raw_plans = $this->getConduit()->callMethodSynchronous(
+ 'harbormaster.buildplan.search',
+ array(
+ 'constraints' => array(
+ 'phids' => $plan_phids,
+ ),
+ ));
+
+ $plans = array();
+ foreach ($raw_plans['data'] as $raw_plan) {
+ $plan_ref = ArcanistBuildPlanRef::newFromConduit($raw_plan);
+ $plan_phid = $plan_ref->getPHID();
+ $plans[$plan_phid] = $plan_ref;
+ }
+
+ $ongoing_builds = array();
+ $failed_builds = array();
+
+ $builds = msort($builds, 'getStatusSortVector');
+ foreach ($builds as $build_ref) {
+ $plan = idx($plans, $build_ref->getBuildPlanPHID());
+ if (!$plan) {
+ continue;
+ }
+
+ $plan_behavior = $plan->getBehavior('arc-land', 'always');
+ $if_building = ($plan_behavior == 'building');
+ $if_complete = ($plan_behavior == 'complete');
+ $if_never = ($plan_behavior == 'never');
+
+ // If the build plan "Never" warns when landing, skip it.
+ if ($if_never) {
+ continue;
+ }
+
+ // If the build plan warns when landing "If Complete" but the build is
+ // not complete, skip it.
+ if ($if_complete && !$build_ref->isComplete()) {
+ continue;
+ }
+
+ // If the build plan warns when landing "If Building" but the build is
+ // complete, skip it.
+ if ($if_building && $build_ref->isComplete()) {
+ continue;
+ }
+
+ // Ignore passing builds.
+ if ($build_ref->isPassed()) {
+ continue;
+ }
+
+ if (!$build_ref->isComplete()) {
+ $ongoing_builds[] = $build_ref;
+ } else {
+ $failed_builds[] = $build_ref;
+ }
+ }
+
+ if (!$ongoing_builds && !$failed_builds) {
+ return;
+ }
+
+ if ($failed_builds) {
+ $this->writeWarn(
+ pht('BUILD FAILURES'),
+ pht(
+ 'Harbormaster failed to build the active diff for this revision:'));
+ $prompt = pht('Land revision anyway, despite build failures?');
+ } else if ($ongoing_builds) {
+ $this->writeWarn(
+ pht('ONGOING BUILDS'),
+ pht(
+ 'Harbormaster is still building the active diff for this revision:'));
+ $prompt = pht('Land revision anyway, despite ongoing build?');
+ }
+
+ $show_builds = array_merge($failed_builds, $ongoing_builds);
+ echo "\n";
+ foreach ($show_builds as $build_ref) {
+ $ansi_color = $build_ref->getStatusANSIColor();
+ $status_name = $build_ref->getStatusName();
+ $object_name = $build_ref->getObjectName();
+ $build_name = $build_ref->getName();
+
+ echo tsprintf(
+ " **<bg:".$ansi_color."> %s </bg>** %s: %s\n",
+ $status_name,
+ $object_name,
+ $build_name);
+ }
+
+ echo tsprintf(
+ "\n%s\n\n",
+ pht('You can review build details here:'));
+
+ foreach ($buildables as $buildable) {
+ $buildable_uri = id(new PhutilURI($this->getConduitURI()))
+ ->setPath(sprintf('/B%d', $buildable['id']));
+
+ echo tsprintf(
+ " **%s**: __%s__\n",
+ pht('Buildable %d', $buildable['id']),
+ $buildable_uri);
+ }
+
+ if (!phutil_console_confirm($prompt)) {
+ throw new ArcanistUserAbortException();
+ }
+ }
+
public function buildEngineMessage(ArcanistLandEngine $engine) {
// TODO: This is oh-so-gross.
$this->findRevision();

File Metadata

Mime Type
text/plain
Expires
Mon, May 20, 3:27 PM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6290128
Default Alt Text
D20236.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility