Page MenuHomePhabricator
Files F15287615

D19376.diff
No OneTemporary
Actions

D19376.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
@@ -1263,6 +1263,7 @@
'FundInitiativeTransactionType' => 'applications/fund/xaction/FundInitiativeTransactionType.php',
'FundInitiativeViewController' => 'applications/fund/controller/FundInitiativeViewController.php',
'FundSchemaSpec' => 'applications/fund/storage/FundSchemaSpec.php',
+ 'HarbormasterAbortOlderBuildsBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterAbortOlderBuildsBuildStepImplementation.php',
'HarbormasterArcLintBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterArcLintBuildStepImplementation.php',
'HarbormasterArcUnitBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterArcUnitBuildStepImplementation.php',
'HarbormasterArtifact' => 'applications/harbormaster/artifact/HarbormasterArtifact.php',
@@ -1358,6 +1359,7 @@
'HarbormasterCircleCIBuildableInterface' => 'applications/harbormaster/interface/HarbormasterCircleCIBuildableInterface.php',
'HarbormasterCircleCIHookController' => 'applications/harbormaster/controller/HarbormasterCircleCIHookController.php',
'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php',
+ 'HarbormasterControlBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterControlBuildStepGroup.php',
'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php',
'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php',
'HarbormasterCreatePlansCapability' => 'applications/harbormaster/capability/HarbormasterCreatePlansCapability.php',
@@ -6636,6 +6638,7 @@
'FundInitiativeTransactionType' => 'PhabricatorModularTransactionType',
'FundInitiativeViewController' => 'FundController',
'FundSchemaSpec' => 'PhabricatorConfigSchemaSpec',
+ 'HarbormasterAbortOlderBuildsBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterArcLintBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterArcUnitBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterArtifact' => 'Phobject',
@@ -6771,6 +6774,7 @@
'HarbormasterCircleCIBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterCircleCIHookController' => 'HarbormasterController',
'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod',
+ 'HarbormasterControlBuildStepGroup' => 'HarbormasterBuildStepGroup',
'HarbormasterController' => 'PhabricatorController',
'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
'HarbormasterCreatePlansCapability' => 'PhabricatorPolicyCapability',
diff --git a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
--- a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
+++ b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
@@ -98,6 +98,19 @@
);
}
+ public static function getIncompleteStatusConstants() {
+ $map = self::getBuildStatusSpecMap();
+
+ $constants = array();
+ foreach ($map as $constant => $spec) {
+ if (!$spec['isComplete']) {
+ $constants[] = $constant;
+ }
+ }
+
+ return $constants;
+ }
+
public static function getCompletedStatusConstants() {
return array(
self::STATUS_PASSED,
diff --git a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
--- a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
+++ b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php
@@ -51,18 +51,7 @@
}
if ($request->isDialogFormPost() && $can_issue) {
- $editor = id(new HarbormasterBuildTransactionEditor())
- ->setActor($viewer)
- ->setContentSourceFromRequest($request)
- ->setContinueOnNoEffect(true)
- ->setContinueOnMissingFields(true);
-
- $xaction = id(new HarbormasterBuildTransaction())
- ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
- ->setNewValue($action);
-
- $editor->applyTransactions($build, array($xaction));
-
+ $build->sendMessage($viewer, $action);
return id(new AphrontRedirectResponse())->setURI($return_uri);
}
diff --git a/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php b/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php
--- a/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php
+++ b/src/applications/harbormaster/query/HarbormasterBuildStepQuery.php
@@ -22,48 +22,39 @@
return $this;
}
+ public function newResultObject() {
+ return new HarbormasterBuildStep();
+ }
+
protected function loadPage() {
- $table = new HarbormasterBuildStep();
- $conn_r = $table->establishConnection('r');
-
- $data = queryfx_all(
- $conn_r,
- 'SELECT * FROM %T %Q %Q %Q',
- $table->getTableName(),
- $this->buildWhereClause($conn_r),
- $this->buildOrderClause($conn_r),
- $this->buildLimitClause($conn_r));
-
- return $table->loadAllFromArray($data);
+ return $this->loadStandardPage($this->newResultObject());
}
- protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
- $where = array();
+ protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+ $where = parent::buildWhereClauseParts($conn);
- if ($this->ids) {
+ if ($this->ids !== null) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'id IN (%Ld)',
$this->ids);
}
- if ($this->phids) {
+ if ($this->phids !== null) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'phid in (%Ls)',
$this->phids);
}
- if ($this->buildPlanPHIDs) {
+ if ($this->buildPlanPHIDs !== null) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'buildPlanPHID in (%Ls)',
$this->buildPlanPHIDs);
}
- $where[] = $this->buildPagingClause($conn_r);
-
- return $this->formatWhereClause($where);
+ return $where;
}
protected function willFilterPage(array $page) {
diff --git a/src/applications/harbormaster/step/HarbormasterAbortOlderBuildsBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterAbortOlderBuildsBuildStepImplementation.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/step/HarbormasterAbortOlderBuildsBuildStepImplementation.php
@@ -0,0 +1,135 @@
+<?php
+
+final class HarbormasterAbortOlderBuildsBuildStepImplementation
+ extends HarbormasterBuildStepImplementation {
+
+ public function getName() {
+ return pht('Abort Older Builds');
+ }
+
+ public function getGenericDescription() {
+ return pht(
+ 'When building a revision, abort copies of this build plan which are '.
+ 'currently running against older diffs.');
+ }
+
+ public function getBuildStepGroupKey() {
+ return HarbormasterControlBuildStepGroup::GROUPKEY;
+ }
+
+ public function getEditInstructions() {
+ return pht(<<<EOTEXT
+When run against a revision, this build step will abort any older copies of
+the same build plan which are currently running against older diffs.
+
+There are some nuances to the behavior:
+
+ - if this build step is triggered manually, it won't abort anything;
+ - this build step won't abort manual builds;
+ - this build step won't abort anything if the diff it is building isn't
+ the active diff when it runs.
+
+Build results on outdated diffs often aren't very important, so this may
+reduce build queue load without any substantial cost.
+EOTEXT
+ );
+ }
+
+ public function willStartBuild(
+ PhabricatorUser $viewer,
+ HarbormasterBuildable $buildable,
+ HarbormasterBuild $build,
+ HarbormasterBuildPlan $plan,
+ HarbormasterBuildStep $step) {
+
+ if ($buildable->getIsManualBuildable()) {
+ // Don't abort anything if this is a manual buildable.
+ return;
+ }
+
+ $object_phid = $buildable->getBuildablePHID();
+ if (phid_get_type($object_phid) !== DifferentialDiffPHIDType::TYPECONST) {
+ // If this buildable isn't building a diff, bail out. For example, we
+ // might be building a commit. In this case, this step has no effect.
+ return;
+ }
+
+ $diff = id(new DifferentialDiffQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($object_phid))
+ ->executeOne();
+ if (!$diff) {
+ return;
+ }
+
+ $revision_id = $diff->getRevisionID();
+
+ $revision = id(new DifferentialRevisionQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($revision_id))
+ ->executeOne();
+ if (!$revision) {
+ return;
+ }
+
+ $active_phid = $revision->getActiveDiffPHID();
+ if ($active_phid !== $object_phid) {
+ // If we aren't building the active diff, bail out.
+ return;
+ }
+
+ $diffs = id(new DifferentialDiffQuery())
+ ->setViewer($viewer)
+ ->withRevisionIDs(array($revision_id))
+ ->execute();
+ $abort_diff_phids = array();
+ foreach ($diffs as $diff) {
+ if ($diff->getPHID() !== $active_phid) {
+ $abort_diff_phids[] = $diff->getPHID();
+ }
+ }
+
+ if (!$abort_diff_phids) {
+ return;
+ }
+
+ // We're fetching buildables even if they have "passed" or "failed"
+ // because they may still have ongoing builds. At the time of writing
+ // only "failed" buildables may still be ongoing, but it seems likely that
+ // "passed" buildables may be ongoing in the future.
+
+ $abort_buildables = id(new HarbormasterBuildableQuery())
+ ->setViewer($viewer)
+ ->withBuildablePHIDs($abort_diff_phids)
+ ->withManualBuildables(false)
+ ->execute();
+ if (!$abort_buildables) {
+ return;
+ }
+
+ $statuses = HarbormasterBuildStatus::getIncompleteStatusConstants();
+
+ $abort_builds = id(new HarbormasterBuildQuery())
+ ->setViewer($viewer)
+ ->withBuildablePHIDs(mpull($abort_buildables, 'getPHID'))
+ ->withBuildPlanPHIDs(array($plan->getPHID()))
+ ->withBuildStatuses($statuses)
+ ->execute();
+ if (!$abort_builds) {
+ return;
+ }
+
+ foreach ($abort_builds as $abort_build) {
+ $abort_build->sendMessage(
+ $viewer,
+ HarbormasterBuildCommand::COMMAND_ABORT);
+ }
+ }
+
+ public function execute(
+ HarbormasterBuild $build,
+ HarbormasterBuildTarget $build_target) {
+ return;
+ }
+
+}
diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
--- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
@@ -308,6 +308,15 @@
'enabled in configuration.'));
}
+ public function willStartBuild(
+ PhabricatorUser $viewer,
+ HarbormasterBuildable $buildable,
+ HarbormasterBuild $build,
+ HarbormasterBuildPlan $plan,
+ HarbormasterBuildStep $step) {
+ return;
+ }
+
/* -( Automatic Targets )-------------------------------------------------- */
diff --git a/src/applications/harbormaster/stepgroup/HarbormasterControlBuildStepGroup.php b/src/applications/harbormaster/stepgroup/HarbormasterControlBuildStepGroup.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/stepgroup/HarbormasterControlBuildStepGroup.php
@@ -0,0 +1,20 @@
+<?php
+
+final class HarbormasterControlBuildStepGroup
+ extends HarbormasterBuildStepGroup {
+
+ const GROUPKEY = 'harbormaster.control';
+
+ public function getGroupName() {
+ return pht('Flow Control');
+ }
+
+ public function getGroupOrder() {
+ return 5000;
+ }
+
+ public function shouldShowIfEmpty() {
+ return false;
+ }
+
+}
diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php
--- a/src/applications/harbormaster/storage/HarbormasterBuildable.php
+++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php
@@ -141,6 +141,15 @@
$build->save();
+ $steps = id(new HarbormasterBuildStepQuery())
+ ->setViewer($viewer)
+ ->withBuildPlanPHIDs(array($plan->getPHID()))
+ ->execute();
+
+ foreach ($steps as $step) {
+ $step->willStartBuild($viewer, $this, $build, $plan);
+ }
+
PhabricatorWorker::scheduleTask(
'HarbormasterBuildWorker',
array(
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php
@@ -356,6 +356,35 @@
}
}
+ public function sendMessage(PhabricatorUser $viewer, $command) {
+ // TODO: This should not be an editor transaction, but there are plans to
+ // merge BuildCommand into BuildMessage which should moot this. As this
+ // exists today, it can race against BuildEngine.
+
+ // This is a bogus content source, but this whole flow should be obsolete
+ // soon.
+ $content_source = PhabricatorContentSource::newForSource(
+ PhabricatorConsoleContentSource::SOURCECONST);
+
+ $editor = id(new HarbormasterBuildTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSource($content_source)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
+
+ $viewer_phid = $viewer->getPHID();
+ if (!$viewer_phid) {
+ $acting_phid = id(new PhabricatorHarbormasterApplication())->getPHID();
+ $editor->setActingAsPHID($acting_phid);
+ }
+
+ $xaction = id(new HarbormasterBuildTransaction())
+ ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
+ ->setNewValue($command);
+
+ $editor->applyTransactions($this, array($xaction));
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php
--- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php
+++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php
@@ -100,6 +100,19 @@
return ($this->getStepAutoKey() !== null);
}
+ public function willStartBuild(
+ PhabricatorUser $viewer,
+ HarbormasterBuildable $buildable,
+ HarbormasterBuild $build,
+ HarbormasterBuildPlan $plan) {
+ return $this->getStepImplementation()->willStartBuild(
+ $viewer,
+ $buildable,
+ $build,
+ $plan,
+ $this);
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 5, 7:56 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7226868
Default Alt Text
D19376.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility