Page MenuHomePhabricator
Files F13993031

D8599.diff
No OneTemporary
Actions

D8599.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -70,6 +70,7 @@
'rsrc/css/application/feed/feed.css' => '0d17c209',
'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad',
'rsrc/css/application/flag/flag.css' => '5337623f',
+ 'rsrc/css/application/harbormaster/harbormaster.css' => 'cec833b7',
'rsrc/css/application/herald/herald-test.css' => '2b7d0f54',
'rsrc/css/application/herald/herald.css' => '59d48f01',
'rsrc/css/application/maniphest/batch-editor.css' => '8f380ebc',
@@ -521,6 +522,7 @@
'diviner-shared-css' => '38813222',
'font-source-sans-pro' => '225851dd',
'global-drag-and-drop-css' => '697324ad',
+ 'harbormaster-css' => 'cec833b7',
'herald-css' => '59d48f01',
'herald-rule-editor' => '4173dbd8',
'herald-test-css' => '2b7d0f54',
diff --git a/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php b/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php
--- a/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php
+++ b/src/applications/harbormaster/controller/HarbormasterPlanOrderController.php
@@ -1,8 +1,5 @@
<?php
-/**
- * @group search
- */
final class HarbormasterPlanOrderController extends HarbormasterController {
private $id;
@@ -46,36 +43,8 @@
$reordered_steps[] = $step;
}
- // We must ensure that steps with artifacts become invalid if they are
- // placed before the steps that produce them.
- foreach ($reordered_steps as $step) {
- $implementation = $step->getStepImplementation();
- $settings = $implementation->getSettings();
- foreach ($implementation->getSettingDefinitions() as $name => $opt) {
- switch ($opt['type']) {
- case BuildStepImplementation::SETTING_TYPE_ARTIFACT:
- $value = $settings[$name];
- $filter = $opt['artifact_type'];
- $available_artifacts =
- BuildStepImplementation::getAvailableArtifacts(
- $plan,
- $reordered_steps,
- $step,
- $filter);
- $artifact_found = false;
- foreach ($available_artifacts as $key => $type) {
- if ($key === $value) {
- $artifact_found = true;
- }
- }
- if (!$artifact_found) {
- $step->setDetail($name, null);
- }
- break;
- }
- $step->save();
- }
- }
+ // NOTE: Reordering steps may invalidate artifacts. This is fine; the UI
+ // will show that there are ordering issues.
// Force the page to re-render.
return id(new AphrontRedirectResponse());
diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php
--- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php
+++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php
@@ -50,7 +50,16 @@
$crumbs = $this->buildApplicationCrumbs();
$crumbs->addTextCrumb(pht("Plan %d", $id));
- $step_list = $this->buildStepList($plan);
+ list($step_list, $has_any_conflicts) = $this->buildStepList($plan);
+
+ if ($has_any_conflicts) {
+ $box->setFormErrors(
+ array(
+ pht(
+ 'This build plan has conflicts in one or more build steps. '.
+ 'Examine the step list and resolve the listed errors.'),
+ ));
+ }
return $this->buildApplicationPage(
array(
@@ -91,6 +100,8 @@
'listID' => $list_id,
'orderURI' => '/harbormaster/plan/order/'.$plan->getID().'/',
));
+
+ $has_any_conflicts = false;
foreach ($steps as $step) {
$implementation = null;
try {
@@ -121,27 +132,14 @@
->setObjectName("Step ".$i++)
->setHeader($implementation->getName());
- if (!$implementation->validateSettings()) {
- $item
- ->setBarColor('red')
- ->addAttribute(pht('This step is not configured correctly.'));
- } else {
- $item->addAttribute($implementation->getDescription());
- }
+ $item->addAttribute($implementation->getDescription());
+
+ $step_id = $step->getID();
+ $edit_uri = $this->getApplicationURI("step/edit/{$step_id}/");
+ $delete_uri = $this->getApplicationURI("step/delete/{$step_id}/");
if ($can_edit) {
- $edit_uri = $this->getApplicationURI("step/edit/".$step->getID()."/");
- $item
- ->setHref($edit_uri)
- ->addAction(
- id(new PHUIListItemView())
- ->setIcon('delete')
- ->addSigil('harbormaster-build-step-delete')
- ->setWorkflow(true)
- ->setRenderNameAsTooltip(true)
- ->setName(pht("Delete"))
- ->setHref(
- $this->getApplicationURI("step/delete/".$step->getID()."/")));
+ $item->setHref($edit_uri);
$item->setGrippable(true);
$item->addSigil('build-step');
$item->setMetadata(
@@ -150,10 +148,60 @@
));
}
+ $item
+ ->setHref($edit_uri)
+ ->addAction(
+ id(new PHUIListItemView())
+ ->setIcon('delete')
+ ->addSigil('harbormaster-build-step-delete')
+ ->setWorkflow(true)
+ ->setDisabled(!$can_edit)
+ ->setHref(
+ $this->getApplicationURI("step/delete/".$step->getID()."/")));
+
+ $inputs = $step->getStepImplementation()->getArtifactInputs();
+ $outputs = $step->getStepImplementation()->getArtifactOutputs();
+
+ $has_conflicts = false;
+ if ($inputs || $outputs) {
+ $available_artifacts = BuildStepImplementation::loadAvailableArtifacts(
+ $plan,
+ $step,
+ null);
+
+ list($inputs_ui, $has_conflicts) = $this->buildArtifactList(
+ $inputs,
+ 'in',
+ pht('Input Artifacts'),
+ $available_artifacts);
+
+ list($outputs_ui) = $this->buildArtifactList(
+ $outputs,
+ 'out',
+ pht('Output Artifacts'),
+ array());
+
+ $item->appendChild(
+ phutil_tag(
+ 'div',
+ array(
+ 'class' => 'harbormaster-artifact-io',
+ ),
+ array(
+ $inputs_ui,
+ $outputs_ui,
+ )));
+ }
+
+ if ($has_conflicts) {
+ $has_any_conflicts = true;
+ $item->setBarColor('red');
+ }
+
$step_list->addItem($item);
}
- return $step_list;
+ return array($step_list, $has_any_conflicts);
}
private function buildActionList(HarbormasterBuildPlan $plan) {
@@ -233,4 +281,102 @@
}
+ private function buildArtifactList(
+ array $artifacts,
+ $kind,
+ $name,
+ array $available_artifacts) {
+ $has_conflicts = false;
+
+ if (!$artifacts) {
+ return array(null, $has_conflicts);
+ }
+
+
+ $this->requireResource('harbormaster-css');
+
+ $header = phutil_tag(
+ 'div',
+ array(
+ 'class' => 'harbormaster-artifact-summary-header',
+ ),
+ $name);
+
+ $is_input = ($kind == 'in');
+
+ $list = new PHUIStatusListView();
+ foreach ($artifacts as $artifact) {
+ $error = null;
+
+ $key = idx($artifact, 'key');
+ if (!strlen($key)) {
+ $bound = phutil_tag('em', array(), pht('(null)'));
+ if ($is_input) {
+ // This is an unbound input. For now, all inputs are always required.
+ $icon = 'warning-red';
+ $icon_label = pht('Required Input');
+ $has_conflicts = true;
+ $error = pht('This input is required, but not configured.');
+ } else {
+ // This is an unnamed output. Outputs do not necessarily need to be
+ // named.
+ $icon = 'open';
+ $icon_label = pht('Unused Output');
+ }
+ } else {
+ $bound = phutil_tag('strong', array(), $key);
+ if ($is_input) {
+ if (isset($available_artifacts[$key])) {
+ if ($available_artifacts[$key] == idx($artifact, 'type')) {
+ $icon = 'accept-green';
+ $icon_label = pht('Valid Input');
+ } else {
+ $icon = 'warning-red';
+ $icon_label = pht('Bad Input Type');
+ $has_conflicts = true;
+ $error = pht(
+ 'This input is bound to the wrong artifact type. It is bound '.
+ 'to a "%s" artifact, but should be bound to a "%s" artifact.',
+ $available_artifacts[$key],
+ idx($artifact, 'type'));
+ }
+ } else {
+ $icon = 'question-red';
+ $icon_label = pht('Unknown Input');
+ $has_conflicts = true;
+ $error = pht(
+ 'This input is bound to an artifact ("%s") which does not exist '.
+ 'at this stage in the build process.',
+ $key);
+ }
+ } else {
+ $icon = 'down-green';
+ $icon_label = pht('Valid Output');
+ }
+ }
+
+ if ($error) {
+ $note = array(
+ phutil_tag('strong', array(), pht('ERROR:')),
+ ' ',
+ $error);
+ } else {
+ $note = $bound;
+ }
+
+ $list->addItem(
+ id(new PHUIStatusItemView())
+ ->setIcon($icon, $icon_label)
+ ->setTarget($artifact['name'])
+ ->setNote($note));
+ }
+
+ $ui = array(
+ $header,
+ $list,
+ );
+
+ return array($ui, $has_conflicts);
+ }
+
}
diff --git a/src/applications/harbormaster/step/BuildStepImplementation.php b/src/applications/harbormaster/step/BuildStepImplementation.php
--- a/src/applications/harbormaster/step/BuildStepImplementation.php
+++ b/src/applications/harbormaster/step/BuildStepImplementation.php
@@ -50,6 +50,10 @@
return $this->settings;
}
+ public function getSetting($key, $default = null) {
+ return idx($this->settings, $key, $default);
+ }
+
/**
* Validate the current settings of this build step.
*/
@@ -103,7 +107,11 @@
*
* @return array The mappings of artifact names to their types.
*/
- public function getArtifactMappings() {
+ public function getArtifactInputs() {
+ return array();
+ }
+
+ public function getArtifactOutputs() {
return array();
}
@@ -141,9 +149,10 @@
$previous_implementations[] = $build_step->getStepImplementation();
}
- $artifact_arrays = mpull($previous_implementations, 'getArtifactMappings');
+ $artifact_arrays = mpull($previous_implementations, 'getArtifactOutputs');
$artifacts = array();
foreach ($artifact_arrays as $array) {
+ $array = ipull($array, 'type', 'key');
foreach ($array as $name => $type) {
if ($type !== $artifact_type && $artifact_type !== null) {
continue;
diff --git a/src/applications/harbormaster/step/CommandBuildStepImplementation.php b/src/applications/harbormaster/step/CommandBuildStepImplementation.php
--- a/src/applications/harbormaster/step/CommandBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/CommandBuildStepImplementation.php
@@ -90,6 +90,16 @@
return true;
}
+ public function getArtifactInputs() {
+ return array(
+ array(
+ 'name' => pht('Run on Host'),
+ 'key' => $this->getSetting('hostartifact'),
+ 'type' => HarbormasterBuildArtifact::TYPE_HOST,
+ ),
+ );
+ }
+
public function getSettingDefinitions() {
return array(
'command' => array(
diff --git a/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php
--- a/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php
@@ -51,13 +51,6 @@
$artifact->save();
}
- public function getArtifactMappings() {
- $settings = $this->getSettings();
-
- return array(
- $settings['name'] => HarbormasterBuildArtifact::TYPE_HOST);
- }
-
public function validateSettings() {
$settings = $this->getSettings();
@@ -71,6 +64,16 @@
return true;
}
+ public function getArtifactOutputs() {
+ return array(
+ array(
+ 'name' => pht('Leased Host'),
+ 'key' => $this->getSetting('name'),
+ 'type' => HarbormasterBuildArtifact::TYPE_HOST,
+ ),
+ );
+ }
+
public function getSettingDefinitions() {
return array(
'name' => array(
diff --git a/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php b/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php
--- a/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/PublishFragmentBuildStepImplementation.php
@@ -73,6 +73,16 @@
return true;
}
+ public function getArtifactInputs() {
+ return array(
+ array(
+ 'name' => pht('Publishes File'),
+ 'key' => $this->getSetting('artifact'),
+ 'type' => HarbormasterBuildArtifact::TYPE_FILE,
+ ),
+ );
+ }
+
public function getSettingDefinitions() {
return array(
'path' => array(
diff --git a/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php b/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php
--- a/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/UploadArtifactBuildStepImplementation.php
@@ -51,13 +51,6 @@
$artifact->save();
}
- public function getArtifactMappings() {
- $settings = $this->getSettings();
-
- return array(
- $settings['name'] => HarbormasterBuildArtifact::TYPE_FILE);
- }
-
public function validateSettings() {
$settings = $this->getSettings();
@@ -77,6 +70,26 @@
return true;
}
+ public function getArtifactInputs() {
+ return array(
+ array(
+ 'name' => pht('Upload From Host'),
+ 'key' => $this->getSetting('hostartifact'),
+ 'type' => HarbormasterBuildArtifact::TYPE_HOST,
+ ),
+ );
+ }
+
+ public function getArtifactOutputs() {
+ return array(
+ array(
+ 'name' => pht('Uploaded File'),
+ 'key' => $this->getSetting('name'),
+ 'type' => HarbormasterBuildArtifact::TYPE_FILE,
+ ),
+ );
+ }
+
public function getSettingDefinitions() {
return array(
'path' => array(
diff --git a/webroot/rsrc/css/application/harbormaster/harbormaster.css b/webroot/rsrc/css/application/harbormaster/harbormaster.css
new file mode 100644
--- /dev/null
+++ b/webroot/rsrc/css/application/harbormaster/harbormaster.css
@@ -0,0 +1,19 @@
+/**
+ * @provides harbormaster-css
+ */
+
+.harbormaster-artifact-io {
+ margin: 0 0 0 8px;
+ padding: 4px 8px;
+ border-width: 1px 0 0 1px;
+ border-style: solid;
+ box-shadow: inset 2px 2px 1px rgba(0, 0, 0, 0.075);
+ background: {$lightbluebackground};
+ border-color: {$lightblueborder};
+}
+
+.harbormaster-artifact-summary-header {
+ font-weight: bold;
+ margin-bottom: 2px;
+ color: {$darkbluetext};
+}

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 23, 8:13 PM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6745017
Default Alt Text
D8599.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility