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 @@ -2537,7 +2537,6 @@ 'SubscriptionListDialogBuilder' => 'applications/subscriptions/view/SubscriptionListDialogBuilder.php', 'SubscriptionListStringBuilder' => 'applications/subscriptions/view/SubscriptionListStringBuilder.php', 'UploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/UploadArtifactBuildStepImplementation.php', - 'VariableBuildStepImplementation' => 'applications/harbormaster/step/VariableBuildStepImplementation.php', 'WaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php', ), 'function' => @@ -2659,7 +2658,7 @@ 'CelerityResourceGraph' => 'AbstractDirectedGraph', 'CelerityResourceTransformerTestCase' => 'PhabricatorTestCase', 'CelerityResourcesOnDisk' => 'CelerityPhysicalResources', - 'CommandBuildStepImplementation' => 'VariableBuildStepImplementation', + 'CommandBuildStepImplementation' => 'BuildStepImplementation', 'ConduitAPIMethod' => array( 0 => 'Phobject', @@ -3339,7 +3338,7 @@ 'HarbormasterCapabilityManagePlans' => 'PhabricatorPolicyCapability', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterDAO' => 'PhabricatorLiskDAO', - 'HarbormasterHTTPRequestBuildStepImplementation' => 'VariableBuildStepImplementation', + 'HarbormasterHTTPRequestBuildStepImplementation' => 'BuildStepImplementation', 'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow', 'HarbormasterObject' => 'HarbormasterDAO', @@ -5384,7 +5383,7 @@ 'PonderVoteSaveController' => 'PonderController', 'ProjectCapabilityCreateProjects' => 'PhabricatorPolicyCapability', 'ProjectRemarkupRule' => 'PhabricatorRemarkupRuleObject', - 'PublishFragmentBuildStepImplementation' => 'VariableBuildStepImplementation', + 'PublishFragmentBuildStepImplementation' => 'BuildStepImplementation', 'QueryFormattingTestCase' => 'PhabricatorTestCase', 'ReleephAuthorFieldSpecification' => 'ReleephFieldSpecification', 'ReleephBranch' => @@ -5491,8 +5490,7 @@ 'SleepBuildStepImplementation' => 'BuildStepImplementation', 'SlowvoteEmbedView' => 'AphrontView', 'SlowvoteRemarkupRule' => 'PhabricatorRemarkupRuleObject', - 'UploadArtifactBuildStepImplementation' => 'VariableBuildStepImplementation', - 'VariableBuildStepImplementation' => 'BuildStepImplementation', + 'UploadArtifactBuildStepImplementation' => 'BuildStepImplementation', 'WaitForPreviousBuildStepImplementation' => 'BuildStepImplementation', ), )); diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -24,13 +24,7 @@ return new Aphront404Response(); } - $plan = id(new HarbormasterBuildPlanQuery()) - ->setViewer($viewer) - ->withPHIDs(array($step->getBuildPlanPHID())) - ->executeOne(); - if (!$plan) { - return new Aphront404Response(); - } + $plan = $step->getBuildPlan(); $implementation = $step->getStepImplementation(); $implementation->validateSettingDefinitions(); @@ -52,9 +46,8 @@ } } - if (count($errors) === 0) { + if (!$errors) { $step->save(); - return id(new AphrontRedirectResponse()) ->setURI($this->getApplicationURI('plan/'.$plan->getID().'/')); } @@ -63,11 +56,6 @@ $form = id(new AphrontFormView()) ->setUser($viewer); - $instructions = $implementation->getSettingRemarkupInstructions(); - if ($instructions !== null) { - $form->appendRemarkupInstructions($instructions); - } - // We need to render out all of the fields for the settings that // the implementation has. foreach ($implementation->getSettingDefinitions() as $name => $opt) { @@ -121,7 +109,7 @@ $form->appendChild( id(new AphrontFormSubmitControl()) - ->setValue(pht('Save Step Configuration')) + ->setValue(pht('Save Build Step')) ->addCancelButton( $this->getApplicationURI('plan/'.$plan->getID().'/'))); @@ -137,10 +125,13 @@ $this->getApplicationURI("plan/{$id}/")); $crumbs->addTextCrumb(pht('Edit Step')); + $variables = $this->renderBuildVariablesTable(); + return $this->buildApplicationPage( array( $crumbs, $box, + $variables, ), array( 'title' => $implementation->getName(), @@ -173,4 +164,31 @@ } } + private function renderBuildVariablesTable() { + $viewer = $this->getRequest()->getUser(); + + $variables = HarbormasterBuild::getAvailableBuildVariables(); + ksort($variables); + + $rows = array(); + $rows[] = pht( + 'The following variables can be used in most fields. To reference '. + 'a variable, use `${name}` in a field.'); + $rows[] = pht('| Variable | Description |'); + $rows[] = '|---|---|'; + foreach ($variables as $name => $description) { + $rows[] = '| `'.$name.'` | '.$description.' |'; + } + $rows = implode("\n", $rows); + + $form = id(new AphrontFormView()) + ->setUser($viewer) + ->appendRemarkupInstructions($rows); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Build Variables')) + ->appendChild($form); + } + + } 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 @@ -91,13 +91,6 @@ } /** - * Return relevant setting instructions as Remarkup. - */ - public function getSettingRemarkupInstructions() { - return null; - } - - /** * Return the name of artifacts produced by this command. * * Something like: @@ -160,4 +153,39 @@ } return $artifacts; } + + /** + * Convert a user-provided string with variables in it, like: + * + * ls ${dirname} + * + * ...into a string with variables merged into it safely: + * + * ls 'dir with spaces' + * + * @param string Name of a `vxsprintf` function, like @{function:vcsprintf}. + * @param string User-provided pattern string containing `${variables}`. + * @param dict List of available replacement variables. + * @return string String with variables replaced safely into it. + */ + protected function mergeVariables($function, $pattern, array $variables) { + $regexp = '/\\$\\{(?P[a-z\\.]+)\\}/'; + + $matches = null; + preg_match_all($regexp, $pattern, $matches); + + $argv = array(); + foreach ($matches['name'] as $name) { + if (!array_key_exists($name, $variables)) { + throw new Exception(pht("No such variable '%s'!", $name)); + } + $argv[] = $variables[$name]; + } + + $pattern = str_replace('%', '%%', $pattern); + $pattern = preg_replace($regexp, '%s', $pattern); + + return call_user_func($function, $pattern, $argv); + } + } 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 @@ -1,7 +1,7 @@ [a-z\\.]+)\\}/'; - - $matches = null; - preg_match_all($regexp, $pattern, $matches); - - $argv = array(); - foreach ($matches['name'] as $name) { - if (!array_key_exists($name, $variables)) { - throw new Exception(pht("No such variable '%s'!", $name)); - } - $argv[] = $variables[$name]; - } - - $pattern = str_replace('%', '%%', $pattern); - $pattern = preg_replace($regexp, '%s', $pattern); - - return call_user_func($function, $pattern, $argv); - } - - public function getSettingRemarkupInstructions() { - $variables = HarbormasterBuild::getAvailableBuildVariables(); - $text = ''; - $text .= pht('The following variables are available: ')."\n"; - $text .= "\n"; - foreach ($variables as $name => $desc) { - $text .= ' - `'.$name.'`: '.$desc."\n"; - } - $text .= "\n"; - $text .= "Use `\${name}` to merge a variable into a setting."; - return $text; - } - -}