Page MenuHomePhabricator

D14222.id.diff
No OneTemporary

D14222.id.diff

diff --git a/resources/sql/autopatches/20151002.harbormaster.bparam.1.sql b/resources/sql/autopatches/20151002.harbormaster.bparam.1.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20151002.harbormaster.bparam.1.sql
@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_build
+ ADD buildParameters LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL;
+
+UPDATE {$NAMESPACE}_harbormaster.harbormaster_build
+ SET buildParameters = '{}' WHERE buildParameters = '';
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
@@ -972,6 +972,7 @@
'HarbormasterBuildPlanTransaction' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php',
'HarbormasterBuildPlanTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanTransactionQuery.php',
'HarbormasterBuildQuery' => 'applications/harbormaster/query/HarbormasterBuildQuery.php',
+ 'HarbormasterBuildRequest' => 'applications/harbormaster/engine/HarbormasterBuildRequest.php',
'HarbormasterBuildStep' => 'applications/harbormaster/storage/configuration/HarbormasterBuildStep.php',
'HarbormasterBuildStepCoreCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php',
'HarbormasterBuildStepCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCustomField.php',
@@ -4757,6 +4758,7 @@
'HarbormasterBuildPlanTransaction' => 'PhabricatorApplicationTransaction',
'HarbormasterBuildPlanTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'HarbormasterBuildQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+ 'HarbormasterBuildRequest' => 'Phobject',
'HarbormasterBuildStep' => array(
'HarbormasterDAO',
'PhabricatorApplicationTransactionInterface',
diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
--- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
@@ -10,7 +10,7 @@
protected $changesets;
private $haveHunks;
- private $buildPlanPHIDs = array();
+ private $buildRequests = array();
public function getAdapterApplicationClass() {
return 'PhabricatorDifferentialApplication';
@@ -139,12 +139,13 @@
return $this->getObject()->getPHID();
}
- public function getQueuedHarbormasterBuildPlanPHIDs() {
- return $this->buildPlanPHIDs;
+ public function getQueuedHarbormasterBuildRequests() {
+ return $this->buildRequests;
}
- public function queueHarbormasterBuildPlanPHID($phid) {
- $this->buildPlanPHIDs[] = $phid;
+ public function queueHarbormasterBuildRequest(
+ HarbormasterBuildRequest $request) {
+ $this->buildRequests[] = $request;
}
}
diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php
--- a/src/applications/diffusion/herald/HeraldCommitAdapter.php
+++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php
@@ -17,7 +17,7 @@
protected $affectedPackages;
protected $auditNeededPackages;
- private $buildPlanPHIDs = array();
+ private $buildRequests = array();
public function getAdapterApplicationClass() {
return 'PhabricatorDiffusionApplication';
@@ -308,12 +308,13 @@
return $this->getObject()->getRepository()->getPHID();
}
- public function getQueuedHarbormasterBuildPlanPHIDs() {
- return $this->buildPlanPHIDs;
+ public function getQueuedHarbormasterBuildRequests() {
+ return $this->buildRequests;
}
- public function queueHarbormasterBuildPlanPHID($phid) {
- $this->buildPlanPHIDs[] = $phid;
+ public function queueHarbormasterBuildRequest(
+ HarbormasterBuildRequest $request) {
+ $this->buildRequests[] = $request;
}
}
diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php
--- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php
+++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php
@@ -62,7 +62,7 @@
if (!$errors) {
$buildable->save();
- $buildable->applyPlan($plan);
+ $buildable->applyPlan($plan, array());
$buildable_uri = '/B'.$buildable->getID();
return id(new AphrontRedirectResponse())->setURI($buildable_uri);
diff --git a/src/applications/harbormaster/engine/HarbormasterBuildRequest.php b/src/applications/harbormaster/engine/HarbormasterBuildRequest.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/engine/HarbormasterBuildRequest.php
@@ -0,0 +1,37 @@
+<?php
+
+/**
+ * Structure used to ask Harbormaster to start a build.
+ *
+ * Requests to start builds sometimes originate several layers away from where
+ * they are processed. For example, Herald rules which start builds pass the
+ * requests through the adapter and then through the editor before they reach
+ * Harbormaster.
+ *
+ * This class is just a thin wrapper around these requests so we can make them
+ * more complex later without needing to rewrite any APIs.
+ */
+final class HarbormasterBuildRequest extends Phobject {
+
+ private $buildPlanPHID;
+ private $buildParameters = array();
+
+ public function setBuildPlanPHID($build_plan_phid) {
+ $this->buildPlanPHID = $build_plan_phid;
+ return $this;
+ }
+
+ public function getBuildPlanPHID() {
+ return $this->buildPlanPHID;
+ }
+
+ public function setBuildParameters(array $build_parameters) {
+ $this->buildParameters = $build_parameters;
+ return $this;
+ }
+
+ public function getBuildParameters() {
+ return $this->buildParameters;
+ }
+
+}
diff --git a/src/applications/harbormaster/engine/HarbormasterTargetEngine.php b/src/applications/harbormaster/engine/HarbormasterTargetEngine.php
--- a/src/applications/harbormaster/engine/HarbormasterTargetEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterTargetEngine.php
@@ -206,7 +206,7 @@
// resource and "own" it, so we don't try to handle this, but may need
// to be more careful here if use of autotargets expands.
- $build = $buildable->applyPlan($plan);
+ $build = $buildable->applyPlan($plan, array());
PhabricatorWorker::setRunAllTasksInProcess(false);
} catch (Exception $ex) {
PhabricatorWorker::setRunAllTasksInProcess(false);
diff --git a/src/applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php b/src/applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php
--- a/src/applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php
+++ b/src/applications/harbormaster/herald/HarbormasterBuildableAdapterInterface.php
@@ -4,8 +4,9 @@
public function getHarbormasterBuildablePHID();
public function getHarbormasterContainerPHID();
- public function getQueuedHarbormasterBuildPlanPHIDs();
- public function queueHarbormasterBuildPlanPHID($phid);
+ public function getQueuedHarbormasterBuildRequests();
+ public function queueHarbormasterBuildRequest(
+ HarbormasterBuildRequest $request);
}
diff --git a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
--- a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
+++ b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php
@@ -31,7 +31,9 @@
$phids = array_fuse(array_keys($targets));
foreach ($phids as $phid) {
- $adapter->queueHarbormasterBuildPlanPHID($phid);
+ $request = id(new HarbormasterBuildRequest())
+ ->setBuildPlanPHID($phid);
+ $adapter->queueHarbormasterBuildRequest($request);
}
$this->logEffect(self::DO_BUILD, $phids);
diff --git a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php
--- a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php
+++ b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php
@@ -89,7 +89,7 @@
PhabricatorEnv::getProductionURI('/B'.$buildable->getID()));
PhabricatorWorker::setRunAllTasksInProcess(true);
- $buildable->applyPlan($plan);
+ $buildable->applyPlan($plan, array());
$console->writeOut("%s\n", pht('Done.'));
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
@@ -194,7 +194,7 @@
* @return string String with variables replaced safely into it.
*/
protected function mergeVariables($function, $pattern, array $variables) {
- $regexp = '/\\$\\{(?P<name>[a-z\\.]+)\\}/';
+ $regexp = '@\\$\\{(?P<name>[a-z\\./-]+)\\}@';
$matches = null;
preg_match_all($regexp, $pattern, $matches);
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
@@ -96,15 +96,21 @@
}
/**
- * Looks up the plan PHIDs and applies the plans to the specified
- * object identified by it's PHID.
+ * Start builds for a given buildable.
+ *
+ * @param phid PHID of the object to build.
+ * @param phid Container PHID for the buildable.
+ * @param list<HarbormasterBuildRequest> List of builds to perform.
+ * @return void
*/
public static function applyBuildPlans(
$phid,
$container_phid,
- array $plan_phids) {
+ array $requests) {
- if (!$plan_phids) {
+ assert_instances_of($requests, 'HarbormasterBuildRequest');
+
+ if (!$requests) {
return;
}
@@ -116,31 +122,49 @@
return;
}
+ $viewer = PhabricatorUser::getOmnipotentUser();
+
$buildable = self::createOrLoadExisting(
- PhabricatorUser::getOmnipotentUser(),
+ $viewer,
$phid,
$container_phid);
+ $plan_phids = mpull($requests, 'getBuildPlanPHID');
$plans = id(new HarbormasterBuildPlanQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->setViewer($viewer)
->withPHIDs($plan_phids)
->execute();
- foreach ($plans as $plan) {
+ $plans = mpull($plans, null, 'getPHID');
+
+ foreach ($requests as $request) {
+ $plan_phid = $request->getBuildPlanPHID();
+ $plan = idx($plans, $plan_phid);
+
+ if (!$plan) {
+ throw new Exception(
+ pht(
+ 'Failed to load build plan ("%s").',
+ $plan_phid));
+ }
+
if ($plan->isDisabled()) {
// TODO: This should be communicated more clearly -- maybe we should
// create the build but set the status to "disabled" or "derelict".
continue;
}
- $buildable->applyPlan($plan);
+ $parameters = $request->getBuildParameters();
+ $buildable->applyPlan($plan, $parameters);
}
}
- public function applyPlan(HarbormasterBuildPlan $plan) {
+ public function applyPlan(HarbormasterBuildPlan $plan, array $parameters) {
+
$viewer = PhabricatorUser::getOmnipotentUser();
$build = HarbormasterBuild::initializeNewBuild($viewer)
->setBuildablePHID($this->getPHID())
->setBuildPlanPHID($plan->getPHID())
+ ->setBuildParameters($parameters)
->setBuildStatus(HarbormasterBuild::STATUS_PENDING);
$auto_key = $plan->getPlanAutoKey();
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
@@ -9,6 +9,7 @@
protected $buildPlanPHID;
protected $buildStatus;
protected $buildGeneration;
+ protected $buildParameters = array();
protected $planAutoKey;
private $buildable = self::ATTACHABLE;
@@ -156,6 +157,9 @@
protected function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
+ self::CONFIG_SERIALIZATION => array(
+ 'buildParameters' => self::SERIALIZATION_JSON,
+ ),
self::CONFIG_COLUMN_SCHEMA => array(
'buildStatus' => 'text32',
'buildGeneration' => 'uint32',
@@ -258,6 +262,10 @@
'build.id' => null,
);
+ foreach ($this->getBuildParameters() as $key => $value) {
+ $results['build/'.$key] = $value;
+ }
+
$buildable = $this->getBuildable();
$object = $buildable->getBuildableObject();
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -2837,7 +2837,7 @@
HarbormasterBuildable::applyBuildPlans(
$adapter->getHarbormasterBuildablePHID(),
$adapter->getHarbormasterContainerPHID(),
- $adapter->getQueuedHarbormasterBuildPlanPHIDs());
+ $adapter->getQueuedHarbormasterBuildRequests());
}
return array_merge(

File Metadata

Mime Type
text/plain
Expires
Mon, Oct 21, 6:07 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6739498
Default Alt Text
D14222.id.diff (13 KB)

Event Timeline