1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 06:42:41 +01:00

Implement "Warn When Landing" behavior for Build Plans in Arcanist

Summary:
Ref T13258. This makes "arc land" respect the new "Warn When Landing" behavior.

This will only work if you have very up-to-date APIs. Just fall back to the older code if the new API calls fail.

Test Plan: Ran `arc land` on a revision with builds in various states and with the different "Warn When Landing" behaviors. Saw appropriate warnings.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13258

Differential Revision: https://secure.phabricator.com/D20236
This commit is contained in:
epriestley 2019-03-01 06:11:45 -08:00
parent 96fde137a1
commit f6b8480adc
4 changed files with 226 additions and 1 deletions

View file

@ -42,6 +42,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow',
'ArcanistBrowseWorkflow' => 'ArcanistWorkflow',
'ArcanistBuildPlanRef' => 'Phobject',
'ArcanistBuildRef' => 'Phobject',
'ArcanistBundle' => 'Phobject',
'ArcanistBundleTestCase' => 'PhutilTestCase',

View file

@ -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);
}
}

View file

@ -69,6 +69,10 @@ final class ArcanistBuildRef
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 @@ final class ArcanistBuildRef
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;

View file

@ -1369,6 +1369,19 @@ EOTEXT
* 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 @@ EOTEXT
}
}
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();