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 @@ +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( + " ** %s ** %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();