From c64b822bee621a14323e4615177c7cd36db9379a Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 26 Feb 2016 10:29:17 -0800 Subject: [PATCH] Remove obsolete, confusing Harbormaster builds steps Summary: Fixes T10458. These steps are obsolete and have not worked since the last updates to Drydock. They may eventually return in some form, but get rid of them for now since they're confusing. Test Plan: - Created a build plan with these steps. - Removed these steps. - Verified the build plan showed that the steps were invalid, and that I could delete them. - Deleted them. - Added new steps, no obsolete steps were available for selection. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10458 Differential Revision: https://secure.phabricator.com/D15352 --- src/__phutil_library_map__.php | 4 - .../HarbormasterPlanViewController.php | 22 +-- .../HarbormasterStepViewController.php | 21 ++- .../HarbormasterBuildStepCoreCustomField.php | 7 +- .../engine/HarbormasterBuildGraph.php | 7 +- ...ormasterCommandBuildStepImplementation.php | 149 ------------------ ...masterLeaseHostBuildStepImplementation.php | 74 --------- 7 files changed, 42 insertions(+), 242 deletions(-) delete mode 100644 src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php delete mode 100644 src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a4cea24c74..8528aa9a19 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1094,7 +1094,6 @@ phutil_register_library_map(array( 'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php', 'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php', 'HarbormasterBuiltinBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuiltinBuildStepGroup.php', - 'HarbormasterCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php', 'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php', 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', @@ -1108,7 +1107,6 @@ phutil_register_library_map(array( 'HarbormasterFileArtifact' => 'applications/harbormaster/artifact/HarbormasterFileArtifact.php', 'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php', 'HarbormasterHostArtifact' => 'applications/harbormaster/artifact/HarbormasterHostArtifact.php', - 'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php', 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php', 'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php', 'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php', @@ -5259,7 +5257,6 @@ phutil_register_library_map(array( 'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildableViewController' => 'HarbormasterController', 'HarbormasterBuiltinBuildStepGroup' => 'HarbormasterBuildStepGroup', - 'HarbormasterCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', @@ -5273,7 +5270,6 @@ phutil_register_library_map(array( 'HarbormasterFileArtifact' => 'HarbormasterArtifact', 'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterHostArtifact' => 'HarbormasterDrydockLeaseArtifact', - 'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterLintMessagesController' => 'HarbormasterController', 'HarbormasterLintPropertyView' => 'AphrontView', diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index ed1381006b..f4212c38b3 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -105,31 +105,31 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { $i++; } + $step_id = $step->getID(); + $view_uri = $this->getApplicationURI("step/view/{$step_id}/"); + + $item = id(new PHUIObjectItemView()) + ->setObjectName(pht('Step %d.%d', $depth, $i)) + ->setHeader($step->getName()) + ->setHref($view_uri); + + $step_list->addItem($item); + $implementation = null; try { $implementation = $step->getStepImplementation(); } catch (Exception $ex) { // We can't initialize the implementation. This might be because // it's been renamed or no longer exists. - $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Step %d.%d', $depth, $i)) - ->setHeader(pht('Unknown Implementation')) + $item ->setStatusIcon('fa-warning red') ->addAttribute(pht( 'This step has an invalid implementation (%s).', $step->getClassName())); - $step_list->addItem($item); continue; } - $item = id(new PHUIObjectItemView()) - ->setObjectName(pht('Step %d.%d', $depth, $i)) - ->setHeader($step->getName()); $item->addAttribute($implementation->getDescription()); - - $step_id = $step->getID(); - - $view_uri = $this->getApplicationURI("step/view/{$step_id}/"); $item->setHref($view_uri); $depends = $step->getStepImplementation()->getDependencies($step); diff --git a/src/applications/harbormaster/controller/HarbormasterStepViewController.php b/src/applications/harbormaster/controller/HarbormasterStepViewController.php index d8d1d3f0b0..8f8abf6ea9 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepViewController.php @@ -18,8 +18,6 @@ final class HarbormasterStepViewController extends HarbormasterController { $plan_id = $plan->getID(); $plan_uri = $this->getApplicationURI("plan/{$plan_id}/"); - $implementation = $step->getStepImplementation(); - $field_list = PhabricatorCustomField::getObjectFields( $step, PhabricatorCustomField::ROLE_VIEW); @@ -65,6 +63,25 @@ final class HarbormasterStepViewController extends HarbormasterController { ->setUser($viewer) ->setObject($step); + try { + $implementation = $step->getStepImplementation(); + } catch (Exception $ex) { + $implementation = null; + } + + if ($implementation) { + $type = $implementation->getName(); + } else { + $type = phutil_tag( + 'em', + array(), + pht( + 'Invalid Implementation ("%s")!', + $step->getClassName())); + } + + $view->addProperty(pht('Step Type'), $type); + $view->addProperty( pht('Created'), phabricator_datetime($step->getDateCreated(), $viewer)); diff --git a/src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php b/src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php index 296669d2b5..dc0c759da6 100644 --- a/src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php +++ b/src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php @@ -9,7 +9,12 @@ final class HarbormasterBuildStepCoreCustomField } public function createFields($object) { - $impl = $object->getStepImplementation(); + try { + $impl = $object->getStepImplementation(); + } catch (Exception $ex) { + return array(); + } + $specs = $impl->getFieldSpecifications(); if ($impl->supportsWaitForMessage()) { diff --git a/src/applications/harbormaster/engine/HarbormasterBuildGraph.php b/src/applications/harbormaster/engine/HarbormasterBuildGraph.php index 757f112d05..dc9e4b7691 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildGraph.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildGraph.php @@ -48,7 +48,12 @@ final class HarbormasterBuildGraph extends AbstractDirectedGraph { $map = array(); foreach ($nodes as $node) { $step = $this->stepMap[$node]; - $deps = $step->getStepImplementation()->getDependencies($step); + + try { + $deps = $step->getStepImplementation()->getDependencies($step); + } catch (Exception $ex) { + $deps = array(); + } $map[$node] = array(); foreach ($deps as $dep) { diff --git a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php deleted file mode 100644 index a99c56d0a3..0000000000 --- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php +++ /dev/null @@ -1,149 +0,0 @@ -formatSettingForDescription('command'), - $this->formatSettingForDescription('hostartifact')); - } - - public function escapeCommand($pattern, array $args) { - array_unshift($args, $pattern); - - $mode = PhutilCommandString::MODE_DEFAULT; - if ($this->platform == 'windows') { - $mode = PhutilCommandString::MODE_POWERSHELL; - } - - return id(new PhutilCommandString($args)) - ->setEscapingMode($mode); - } - - public function execute( - HarbormasterBuild $build, - HarbormasterBuildTarget $build_target) { - $viewer = PhabricatorUser::getOmnipotentUser(); - - $settings = $this->getSettings(); - $variables = $build_target->getVariables(); - - $artifact = $build_target->loadArtifact($settings['hostartifact']); - $impl = $artifact->getArtifactImplementation(); - $lease = $impl->loadArtifactLease($viewer); - - $this->platform = $lease->getAttribute('platform'); - - $command = $this->mergeVariables( - array($this, 'escapeCommand'), - $settings['command'], - $variables); - - $this->platform = null; - - $interface = $lease->getInterface('command'); - - $future = $interface->getExecFuture('%C', $command); - - $log_stdout = $build->createLog($build_target, 'remote', 'stdout'); - $log_stderr = $build->createLog($build_target, 'remote', 'stderr'); - - $start_stdout = $log_stdout->start(); - $start_stderr = $log_stderr->start(); - - $build_update = 5; - - // Read the next amount of available output every second. - $futures = new FutureIterator(array($future)); - foreach ($futures->setUpdateInterval(1) as $key => $future_iter) { - if ($future_iter === null) { - - // Check to see if we should abort. - if ($build_update <= 0) { - $build->reload(); - if ($this->shouldAbort($build, $build_target)) { - $future->resolveKill(); - throw new HarbormasterBuildAbortedException(); - } else { - $build_update = 5; - } - } else { - $build_update -= 1; - } - - // Command is still executing. - - // Read more data as it is available. - list($stdout, $stderr) = $future->read(); - $log_stdout->append($stdout); - $log_stderr->append($stderr); - $future->discardBuffers(); - } else { - // Command execution is complete. - - // Get the return value so we can log that as well. - list($err) = $future->resolve(); - - // Retrieve the last few bits of information. - list($stdout, $stderr) = $future->read(); - $log_stdout->append($stdout); - $log_stderr->append($stderr); - $future->discardBuffers(); - - break; - } - } - - $log_stdout->finalize($start_stdout); - $log_stderr->finalize($start_stderr); - - if ($err) { - throw new HarbormasterBuildFailureException(); - } - } - - public function getArtifactInputs() { - return array( - array( - 'name' => pht('Run on Host'), - 'key' => $this->getSetting('hostartifact'), - 'type' => HarbormasterHostArtifact::ARTIFACTCONST, - ), - ); - } - - public function getFieldSpecifications() { - return array( - 'command' => array( - 'name' => pht('Command'), - 'type' => 'text', - 'required' => true, - 'caption' => pht( - "Under Windows, this is executed under PowerShell. ". - "Under UNIX, this is executed using the user's shell."), - ), - 'hostartifact' => array( - 'name' => pht('Host'), - 'type' => 'text', - 'required' => true, - ), - ); - } - -} diff --git a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php deleted file mode 100644 index b14d0975f7..0000000000 --- a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php +++ /dev/null @@ -1,74 +0,0 @@ -getSettings(); - - // Create the lease. - $lease = id(new DrydockLease()) - ->setResourceType('host') - ->setOwnerPHID($build_target->getPHID()) - ->setAttributes( - array( - 'platform' => $settings['platform'], - )) - ->queueForActivation(); - - // Wait until the lease is fulfilled. - // TODO: This will throw an exception if the lease can't be fulfilled; - // we should treat that as build failure not build error. - $lease->waitUntilActive(); - - // Create the associated artifact. - $artifact = $build_target->createArtifact( - PhabricatorUser::getOmnipotentUser(), - $settings['name'], - HarbormasterHostArtifact::ARTIFACTCONST, - array( - 'drydockLeasePHID' => $lease->getPHID(), - )); - } - - public function getArtifactOutputs() { - return array( - array( - 'name' => pht('Leased Host'), - 'key' => $this->getSetting('name'), - 'type' => HarbormasterHostArtifact::ARTIFACTCONST, - ), - ); - } - - public function getFieldSpecifications() { - return array( - 'name' => array( - 'name' => pht('Artifact Name'), - 'type' => 'text', - 'required' => true, - ), - 'platform' => array( - 'name' => pht('Platform'), - 'type' => 'text', - 'required' => true, - ), - ); - } - -}