diff --git a/.gitignore b/.gitignore index 279cb9427d..a90e868e6f 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,7 @@ # NPM local packages /support/aphlict/server/node_modules/ + +# Places for users to add custom resources. +/resources/cows/custom/* +/resources/figlet/custom/* diff --git a/resources/celerity/map.php b/resources/celerity/map.php index cb661a9bd9..12b63a615d 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ return array( 'differential.pkg.css' => '2de124c9', 'differential.pkg.js' => '6223dd9d', 'diffusion.pkg.css' => 'f45955ed', - 'diffusion.pkg.js' => '0115b37c', + 'diffusion.pkg.js' => 'ca1c8b5a', 'maniphest.pkg.css' => '4845691a', 'maniphest.pkg.js' => '3ec6a6d5', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', @@ -371,7 +371,7 @@ return array( 'rsrc/js/application/diffusion/behavior-jump-to.js' => '73d09eef', 'rsrc/js/application/diffusion/behavior-load-blame.js' => '42126667', 'rsrc/js/application/diffusion/behavior-locate-file.js' => '6d3e1947', - 'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => '2b228192', + 'rsrc/js/application/diffusion/behavior-pull-lastmodified.js' => 'f01586dc', 'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => 'e5822781', 'rsrc/js/application/files/behavior-icon-composer.js' => '8ef9ab58', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888', @@ -574,7 +574,7 @@ return array( 'javelin-behavior-diffusion-commit-graph' => '9007c197', 'javelin-behavior-diffusion-jump-to' => '73d09eef', 'javelin-behavior-diffusion-locate-file' => '6d3e1947', - 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', + 'javelin-behavior-diffusion-pull-lastmodified' => 'f01586dc', 'javelin-behavior-doorkeeper-tag' => 'e5822781', 'javelin-behavior-durable-column' => 'c72aa091', 'javelin-behavior-error-log' => '6882e80a', @@ -998,13 +998,6 @@ return array( 'javelin-install', 'javelin-util', ), - '2b228192' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-workflow', - 'javelin-json', - ), '2b8de964' => array( 'javelin-install', 'javelin-util', @@ -1939,6 +1932,13 @@ return array( 'javelin-install', 'javelin-util', ), + 'f01586dc' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-workflow', + 'javelin-json', + ), 'f24a53cb' => array( 'phui-fontkit-css', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 61b4d1f390..184a484929 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1011,6 +1011,7 @@ phutil_register_library_map(array( 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', 'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php', + 'HarbormasterDrydockBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php', 'HarbormasterDrydockCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php', 'HarbormasterDrydockLeaseArtifact' => 'applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php', 'HarbormasterExecFuture' => 'applications/harbormaster/future/HarbormasterExecFuture.php', @@ -4813,6 +4814,7 @@ phutil_register_library_map(array( 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', 'HarbormasterDAO' => 'PhabricatorLiskDAO', + 'HarbormasterDrydockBuildStepGroup' => 'HarbormasterBuildStepGroup', 'HarbormasterDrydockCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterDrydockLeaseArtifact' => 'HarbormasterArtifact', 'HarbormasterExecFuture' => 'Future', diff --git a/src/applications/diffusion/controller/DiffusionLastModifiedController.php b/src/applications/diffusion/controller/DiffusionLastModifiedController.php index 3487733c8b..b23abf1903 100644 --- a/src/applications/diffusion/controller/DiffusionLastModifiedController.php +++ b/src/applications/diffusion/controller/DiffusionLastModifiedController.php @@ -98,12 +98,10 @@ final class DiffusionLastModifiedController extends DiffusionController { $modified = DiffusionView::linkCommit( $drequest->getRepository(), $commit->getCommitIdentifier()); - $date = phabricator_date($epoch, $viewer); - $time = phabricator_time($epoch, $viewer); + $date = phabricator_datetime($epoch, $viewer); } else { $modified = ''; $date = ''; - $time = ''; } $data = $commit->getCommitData(); @@ -137,7 +135,6 @@ final class DiffusionLastModifiedController extends DiffusionController { $return = array( 'commit' => $modified, 'date' => $date, - 'time' => $time, 'author' => $author, 'details' => $details, ); diff --git a/src/applications/diffusion/view/DiffusionBranchTableView.php b/src/applications/diffusion/view/DiffusionBranchTableView.php index d053d5bb33..0f4594e576 100644 --- a/src/applications/diffusion/view/DiffusionBranchTableView.php +++ b/src/applications/diffusion/view/DiffusionBranchTableView.php @@ -21,6 +21,11 @@ final class DiffusionBranchTableView extends DiffusionView { $drequest = $this->getDiffusionRequest(); $current_branch = $drequest->getBranch(); $repository = $drequest->getRepository(); + $commits = $this->commits; + $viewer = $this->getUser(); + + $buildables = $this->loadBuildables($commits); + $have_builds = false; $can_close_branches = ($repository->isHg()); @@ -31,13 +36,21 @@ final class DiffusionBranchTableView extends DiffusionView { $rows = array(); $rowc = array(); foreach ($this->branches as $branch) { - $commit = idx($this->commits, $branch->getCommitIdentifier()); + $commit = idx($commits, $branch->getCommitIdentifier()); if ($commit) { $details = $commit->getSummary(); - $datetime = phabricator_datetime($commit->getEpoch(), $this->user); + $datetime = phabricator_datetime($commit->getEpoch(), $viewer); + $buildable = idx($buildables, $commit->getPHID()); + if ($buildable) { + $build_status = $this->renderBuildable($buildable); + $have_builds = true; + } else { + $build_status = null; + } } else { $datetime = null; $details = null; + $build_status = null; } switch ($repository->shouldSkipAutocloseBranch($branch->getShortName())) { @@ -86,16 +99,7 @@ final class DiffusionBranchTableView extends DiffusionView { } $rows[] = array( - phutil_tag( - 'a', - array( - 'href' => $drequest->generateURI( - array( - 'action' => 'history', - 'branch' => $branch->getShortName(), - )), - ), - pht('History')), + $this->linkBranchHistory($branch->getShortName()), phutil_tag( 'a', array( @@ -109,10 +113,11 @@ final class DiffusionBranchTableView extends DiffusionView { self::linkCommit( $drequest->getRepository(), $branch->getCommitIdentifier()), + $build_status, $status, + AphrontTableView::renderSingleDisplayLine($details), $status_icon, $datetime, - AphrontTableView::renderSingleDisplayLine($details), ); if ($branch->getShortName() == $current_branch) { $rowc[] = 'highlighted'; @@ -124,33 +129,37 @@ final class DiffusionBranchTableView extends DiffusionView { $view = new AphrontTableView($rows); $view->setHeaders( array( - pht('History'), + null, pht('Branch'), pht('Head'), + null, pht('State'), - pht(''), - pht('Modified'), pht('Details'), + null, + pht('Committed'), )); $view->setColumnClasses( array( '', 'pri', '', - '', - '', + 'icon', '', 'wide', + '', + '', )); $view->setColumnVisibility( array( true, true, true, + $have_builds, $can_close_branches, )); $view->setRowClasses($rowc); return $view->render(); } + } diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php index a8eb745fe4..71cc659897 100644 --- a/src/applications/diffusion/view/DiffusionBrowseTableView.php +++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php @@ -31,6 +31,8 @@ final class DiffusionBrowseTableView extends DiffusionView { $show_edit = false; foreach ($this->paths as $path) { + $history_link = $this->linkHistory($path->getPath()); + $dir_slash = null; $file_type = $path->getFileType(); if ($file_type == DifferentialChangeType::FILE_DIRECTORY) { @@ -67,7 +69,6 @@ final class DiffusionBrowseTableView extends DiffusionView { 'lint' => celerity_generate_unique_node_id(), 'commit' => celerity_generate_unique_node_id(), 'date' => celerity_generate_unique_node_id(), - 'time' => celerity_generate_unique_node_id(), 'author' => celerity_generate_unique_node_id(), 'details' => celerity_generate_unique_node_id(), ); @@ -78,13 +79,13 @@ final class DiffusionBrowseTableView extends DiffusionView { } $rows[] = array( + $history_link, $browse_link, idx($dict, 'lint'), $dict['commit'], $dict['author'], $dict['details'], $dict['date'], - $dict['time'], ); } @@ -108,29 +109,29 @@ final class DiffusionBrowseTableView extends DiffusionView { $view = new AphrontTableView($rows); $view->setHeaders( array( + null, pht('Path'), ($lint ? $lint : pht('Lint')), pht('Modified'), pht('Author/Committer'), pht('Details'), - pht('Date'), - pht('Time'), + pht('Committed'), )); $view->setColumnClasses( array( + 'nudgeright', + '', + '', '', - 'n', - 'n', '', 'wide', '', - 'right', )); $view->setColumnVisibility( array( true, - $show_lint, true, + $show_lint, true, true, true, @@ -140,11 +141,11 @@ final class DiffusionBrowseTableView extends DiffusionView { $view->setDeviceVisibility( array( true, - false, true, false, true, false, + true, false, )); diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php index 80989efdf2..314bfb435c 100644 --- a/src/applications/diffusion/view/DiffusionHistoryTableView.php +++ b/src/applications/diffusion/view/DiffusionHistoryTableView.php @@ -7,12 +7,10 @@ final class DiffusionHistoryTableView extends DiffusionView { private $handles = array(); private $isHead; private $parents; - private $buildCache; public function setHistory(array $history) { assert_instances_of($history, 'DiffusionPathChange'); $this->history = $history; - $this->buildCache = null; return $this; } @@ -62,33 +60,14 @@ final class DiffusionHistoryTableView extends DiffusionView { return $this; } - public function loadBuildablesOnDemand() { - if ($this->buildCache !== null) { - return $this->buildCache; - } - - $commits_to_builds = array(); - - $commits = mpull($this->history, 'getCommit'); - - $commit_phids = mpull($commits, 'getPHID'); - - $buildables = id(new HarbormasterBuildableQuery()) - ->setViewer($this->getUser()) - ->withBuildablePHIDs($commit_phids) - ->withManualBuildables(false) - ->execute(); - - $this->buildCache = mpull($buildables, null, 'getBuildablePHID'); - - return $this->buildCache; - } - public function render() { $drequest = $this->getDiffusionRequest(); $viewer = $this->getUser(); + $buildables = $this->loadBuildables(mpull($this->history, 'getCommit')); + $has_any_build = false; + $show_revisions = PhabricatorApplication::isClassInstalledForViewer( 'PhabricatorDifferentialApplication', $viewer); @@ -110,11 +89,9 @@ final class DiffusionHistoryTableView extends DiffusionView { $epoch = $history->getEpoch(); if ($epoch) { - $date = phabricator_date($epoch, $this->user); - $time = phabricator_time($epoch, $this->user); + $committed = phabricator_datetime($epoch, $viewer); } else { - $date = null; - $time = null; + $committed = null; } $data = $history->getCommitData(); @@ -160,36 +137,9 @@ final class DiffusionHistoryTableView extends DiffusionView { $build = null; if ($show_builds) { - $buildable_lookup = $this->loadBuildablesOnDemand(); - $buildable = idx($buildable_lookup, $commit->getPHID()); + $buildable = idx($buildables, $commit->getPHID()); if ($buildable !== null) { - $icon = HarbormasterBuildable::getBuildableStatusIcon( - $buildable->getBuildableStatus()); - $color = HarbormasterBuildable::getBuildableStatusColor( - $buildable->getBuildableStatus()); - $name = HarbormasterBuildable::getBuildableStatusName( - $buildable->getBuildableStatus()); - - $icon_view = id(new PHUIIconView()) - ->setIconFont($icon.' '.$color); - - $tooltip_view = javelin_tag( - 'span', - array( - 'sigil' => 'has-tooltip', - 'meta' => array('tip' => $name), - ), - $icon_view); - - Javelin::initBehavior('phabricator-tooltips'); - - $href_view = phutil_tag( - 'a', - array('href' => '/'.$buildable->getMonogram()), - $tooltip_view); - - $build = $href_view; - + $build = $this->renderBuildable($buildable); $has_any_build = true; } } @@ -214,8 +164,7 @@ final class DiffusionHistoryTableView extends DiffusionView { null), $author, $summary, - $date, - $time, + $committed, ); } @@ -226,30 +175,28 @@ final class DiffusionHistoryTableView extends DiffusionView { null, pht('Commit'), null, - pht('Revision'), + null, pht('Author/Committer'), pht('Details'), - pht('Date'), - pht('Time'), + pht('Committed'), )); $view->setColumnClasses( array( 'threads', 'nudgeright', - 'n', + '', 'icon', - 'n', + '', '', 'wide', '', - 'right', )); $view->setColumnVisibility( array( $graph ? true : false, true, true, - true, + $has_any_build, $show_revisions, )); $view->setDeviceVisibility( @@ -262,7 +209,6 @@ final class DiffusionHistoryTableView extends DiffusionView { false, true, false, - false, )); return $view->render(); } diff --git a/src/applications/diffusion/view/DiffusionTagListView.php b/src/applications/diffusion/view/DiffusionTagListView.php index 668453f88e..3b27284f5e 100644 --- a/src/applications/diffusion/view/DiffusionTagListView.php +++ b/src/applications/diffusion/view/DiffusionTagListView.php @@ -29,6 +29,8 @@ final class DiffusionTagListView extends DiffusionView { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); + $buildables = $this->loadBuildables($this->commits); + $has_builds = false; $rows = array(); foreach ($this->tags as $tag) { @@ -80,30 +82,56 @@ final class DiffusionTagListView extends DiffusionView { } } + $build = null; + if ($commit) { + $buildable = idx($buildables, $commit->getPHID()); + if ($buildable) { + $build = $this->renderBuildable($buildable); + $has_builds = true; + } + } + + $history = $this->linkTagHistory($tag->getName()); + $rows[] = array( + $history, $tag_link, $commit_link, - $description, + $build, $author, + $description, phabricator_datetime($tag->getEpoch(), $this->user), ); } - $table = new AphrontTableView($rows); - $table->setHeaders( - array( - pht('Tag'), - pht('Commit'), - pht('Description'), - pht('Author'), - pht('Created'), - )); - $table->setColumnClasses( - array( - 'pri', - '', - 'wide', - )); + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + null, + pht('Tag'), + pht('Commit'), + null, + pht('Author'), + pht('Description'), + pht('Created'), + )) + ->setColumnClasses( + array( + 'nudgeright', + 'pri', + '', + '', + '', + 'wide', + )) + ->setColumnVisibility( + array( + true, + true, + true, + $has_builds, + )); + return $table->render(); } diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php index b7b3599a2d..83fdc1f7e5 100644 --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -20,6 +20,30 @@ abstract class DiffusionView extends AphrontView { 'path' => $path, )); + return $this->renderHistoryLink($href); + } + + final public function linkBranchHistory($branch) { + $href = $this->getDiffusionRequest()->generateURI( + array( + 'action' => 'history', + 'branch' => $branch, + )); + + return $this->renderHistoryLink($href); + } + + final public function linkTagHistory($tag) { + $href = $this->getDiffusionRequest()->generateURI( + array( + 'action' => 'history', + 'commit' => $tag, + )); + + return $this->renderHistoryLink($href); + } + + private function renderHistoryLink($href) { return javelin_tag( 'a', array( @@ -31,7 +55,7 @@ abstract class DiffusionView extends AphrontView { 'align' => 'E', ), ), - id(new PHUIIconView())->setIconFont('fa-list-ul blue')); + id(new PHUIIconView())->setIconFont('fa-history bluegrey')); } final public function linkBrowse($path, array $details = array()) { @@ -170,4 +194,58 @@ abstract class DiffusionView extends AphrontView { return hsprintf('%s', $name); } + final protected function renderBuildable(HarbormasterBuildable $buildable) { + $status = $buildable->getBuildableStatus(); + + $icon = HarbormasterBuildable::getBuildableStatusIcon($status); + $color = HarbormasterBuildable::getBuildableStatusColor($status); + $name = HarbormasterBuildable::getBuildableStatusName($status); + + $icon_view = id(new PHUIIconView()) + ->setIconFont($icon.' '.$color); + + $tooltip_view = javelin_tag( + 'span', + array( + 'sigil' => 'has-tooltip', + 'meta' => array('tip' => $name), + ), + $icon_view); + + Javelin::initBehavior('phabricator-tooltips'); + + return phutil_tag( + 'a', + array('href' => '/'.$buildable->getMonogram()), + $tooltip_view); + } + + final protected function loadBuildables(array $commits) { + assert_instances_of($commits, 'PhabricatorRepositoryCommit'); + + if (!$commits) { + return array(); + } + + $viewer = $this->getUser(); + + $harbormaster_app = 'PhabricatorHarbormasterApplication'; + $have_harbormaster = PhabricatorApplication::isClassInstalledForViewer( + $harbormaster_app, + $viewer); + + if ($have_harbormaster) { + $buildables = id(new HarbormasterBuildableQuery()) + ->setViewer($viewer) + ->withBuildablePHIDs(mpull($commits, 'getPHID')) + ->withManualBuildables(false) + ->execute(); + $buildables = mpull($buildables, null, 'getBuildablePHID'); + } else { + $buildables = array(); + } + + return $buildables; + } + } diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php index e458020600..e62c7b7558 100644 --- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php @@ -262,13 +262,14 @@ final class DrydockAlmanacServiceHostBlueprintImplementation array( DrydockResourceStatus::STATUS_PENDING, DrydockResourceStatus::STATUS_ACTIVE, + DrydockResourceStatus::STATUS_BROKEN, DrydockResourceStatus::STATUS_RELEASED, )) ->execute(); $allocated_phids = array(); foreach ($pool as $resource) { - $allocated_phids[] = $resource->getAttribute('almanacDevicePHID'); + $allocated_phids[] = $resource->getAttribute('almanacBindingPHID'); } $allocated_phids = array_fuse($allocated_phids); diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php index 513d7b37f9..0f2e0aad44 100644 --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -19,6 +19,10 @@ abstract class DrydockBlueprintImplementation extends Phobject { return array(); } + public function getViewer() { + return PhabricatorUser::getOmnipotentUser(); + } + /* -( Lease Acquisition )-------------------------------------------------- */ @@ -288,7 +292,7 @@ abstract class DrydockBlueprintImplementation extends Phobject { } protected function newLease(DrydockBlueprint $blueprint) { - return id(new DrydockLease()); + return DrydockLease::initializeNewLease(); } protected function requireActiveLease(DrydockLease $lease) { @@ -310,4 +314,67 @@ abstract class DrydockBlueprintImplementation extends Phobject { } } + + /** + * Apply standard limits on resource allocation rate. + * + * @param DrydockBlueprint The blueprint requesting an allocation. + * @return bool True if further allocations should be limited. + */ + protected function shouldLimitAllocatingPoolSize( + DrydockBlueprint $blueprint) { + + // TODO: If this mechanism sticks around, these values should be + // configurable by the blueprint implementation. + + // Limit on total number of active resources. + $total_limit = 1; + + // Always allow at least this many allocations to be in flight at once. + $min_allowed = 1; + + // Allow this fraction of allocating resources as a fraction of active + // resources. + $growth_factor = 0.25; + + $resource = new DrydockResource(); + $conn_r = $resource->establishConnection('r'); + + $counts = queryfx_all( + $conn_r, + 'SELECT status, COUNT(*) N FROM %T WHERE blueprintPHID = %s', + $resource->getTableName(), + $blueprint->getPHID()); + $counts = ipull($counts, 'N', 'status'); + + $n_alloc = idx($counts, DrydockResourceStatus::STATUS_PENDING, 0); + $n_active = idx($counts, DrydockResourceStatus::STATUS_ACTIVE, 0); + $n_broken = idx($counts, DrydockResourceStatus::STATUS_BROKEN, 0); + $n_released = idx($counts, DrydockResourceStatus::STATUS_RELEASED, 0); + + // If we're at the limit on total active resources, limit additional + // allocations. + $n_total = ($n_alloc + $n_active + $n_broken + $n_released); + if ($n_total >= $total_limit) { + return true; + } + + // If the number of in-flight allocations is fewer than the minimum number + // of allowed allocations, don't impose a limit. + if ($n_alloc < $min_allowed) { + return false; + } + + $allowed_alloc = (int)ceil($n_active * $growth_factor); + + // If the number of in-flight allocation is fewer than the number of + // allowed allocations according to the pool growth factor, don't impose + // a limit. + if ($n_alloc < $allowed_alloc) { + return false; + } + + return true; + } + } diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php index 7a34e2d3bc..30dd6fe4c5 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -29,6 +29,17 @@ final class DrydockWorkingCopyBlueprintImplementation public function canAllocateResourceForLease( DrydockBlueprint $blueprint, DrydockLease $lease) { + $viewer = $this->getViewer(); + + if ($this->shouldLimitAllocatingPoolSize($blueprint)) { + return false; + } + + // TODO: If we have a pending resource which is compatible with the + // configuration for this lease, prevent a new allocation? Otherwise the + // queue can fill up with copies of requests from the same lease. But + // maybe we can deal with this with "pre-leasing"? + return true; } @@ -37,6 +48,12 @@ final class DrydockWorkingCopyBlueprintImplementation DrydockResource $resource, DrydockLease $lease) { + // Don't hand out leases on working copies which have not activated, since + // it may take an arbitrarily long time for them to acquire a host. + if (!$resource->isActive()) { + return false; + } + $need_map = $lease->getAttribute('repositories.map'); if (!is_array($need_map)) { return false; @@ -104,8 +121,13 @@ final class DrydockWorkingCopyBlueprintImplementation $host_lease = $this->newLease($blueprint) ->setResourceType('host') ->setOwnerPHID($resource_phid) - ->setAttribute('workingcopy.resourcePHID', $resource_phid) - ->queueForActivation(); + ->setAttribute('workingcopy.resourcePHID', $resource_phid); + + $resource + ->setAttribute('host.leasePHID', $host_lease->getPHID()) + ->save(); + + $host_lease->queueForActivation(); // TODO: Add some limits to the number of working copies we can have at // once? @@ -121,7 +143,6 @@ final class DrydockWorkingCopyBlueprintImplementation return $resource ->setAttribute('repositories.map', $map) - ->setAttribute('host.leasePHID', $host_lease->getPHID()) ->allocateResource(); } @@ -165,7 +186,13 @@ final class DrydockWorkingCopyBlueprintImplementation DrydockBlueprint $blueprint, DrydockResource $resource) { - $lease = $this->loadHostLease($resource); + try { + $lease = $this->loadHostLease($resource); + } catch (Exception $ex) { + // If we can't load the lease, assume we don't need to take any actions + // to destroy it. + return; + } // Destroy the lease on the host. $lease->releaseOnDestruction(); @@ -310,8 +337,10 @@ final class DrydockWorkingCopyBlueprintImplementation } private function loadRepositories(array $phids) { + $viewer = $this->getViewer(); + $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->setViewer($viewer) ->withPHIDs($phids) ->execute(); $repositories = mpull($repositories, null, 'getPHID'); @@ -343,7 +372,7 @@ final class DrydockWorkingCopyBlueprintImplementation } private function loadHostLease(DrydockResource $resource) { - $viewer = PhabricatorUser::getOmnipotentUser(); + $viewer = $this->getViewer(); $lease_phid = $resource->getAttribute('host.leasePHID'); diff --git a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php index 1faf762ae8..46ab965b36 100644 --- a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php +++ b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php @@ -19,7 +19,7 @@ final class DrydockLeaseWaitingForResourcesLogType extends DrydockLogType { return pht( 'Waiting for available resources from: %s.', - $viewer->renderHandleList($blueprint_phids)); + $viewer->renderHandleList($blueprint_phids)->render()); } } diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php index aa2ea753f0..01ccb0a5c4 100644 --- a/src/applications/drydock/storage/DrydockLease.php +++ b/src/applications/drydock/storage/DrydockLease.php @@ -19,6 +19,16 @@ final class DrydockLease extends DrydockDAO private $activateWhenAcquired = false; private $slotLocks = array(); + public static function initializeNewLease() { + $lease = new DrydockLease(); + + // Pregenerate a PHID so that the caller can set something up to release + // this lease before queueing it for activation. + $lease->setPHID($lease->generatePHID()); + + return $lease; + } + /** * Flag this lease to be released when its destructor is called. This is * mostly useful if you have a script which acquires, uses, and then releases @@ -232,6 +242,7 @@ final class DrydockLease extends DrydockDAO } $this->openTransaction(); + try { DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks); $this->slotLocks = array(); @@ -247,16 +258,12 @@ final class DrydockLease extends DrydockDAO throw $ex; } - try { - $this - ->setResourcePHID($resource->getPHID()) - ->attachResource($resource) - ->setStatus($new_status) - ->save(); - } catch (Exception $ex) { - $this->killTransaction(); - throw $ex; - } + $this + ->setResourcePHID($resource->getPHID()) + ->attachResource($resource) + ->setStatus($new_status) + ->save(); + $this->saveTransaction(); $this->isAcquired = true; @@ -295,12 +302,24 @@ final class DrydockLease extends DrydockDAO $this->openTransaction(); - $this - ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE) - ->save(); - + try { DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks); $this->slotLocks = array(); + } catch (DrydockSlotLockException $ex) { + $this->killTransaction(); + + $this->logEvent( + DrydockSlotLockFailureLogType::LOGCONST, + array( + 'locks' => $ex->getLockMap(), + )); + + throw $ex; + } + + $this + ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE) + ->save(); $this->saveTransaction(); diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php index da97a089d9..3eceec8b77 100644 --- a/src/applications/drydock/storage/DrydockResource.php +++ b/src/applications/drydock/storage/DrydockResource.php @@ -113,13 +113,6 @@ final class DrydockResource extends DrydockDAO } public function allocateResource() { - if ($this->getID()) { - throw new Exception( - pht( - 'Trying to allocate a resource which has already been persisted. '. - 'Only new resources may be allocated.')); - } - // We expect resources to have a pregenerated PHID, as they should have // been created by a call to DrydockBlueprint->newResourceTemplate(). if (!$this->getPHID()) { @@ -155,9 +148,14 @@ final class DrydockResource extends DrydockDAO } catch (DrydockSlotLockException $ex) { $this->killTransaction(); - // NOTE: We have to log this on the blueprint, as the resource is not - // going to be saved so the PHID will vanish. - $this->getBlueprint()->logEvent( + if ($this->getID()) { + $log_target = $this; + } else { + // If we don't have an ID, we have to log this on the blueprint, as the + // resource is not going to be saved so the PHID will vanish. + $log_target = $this->getBlueprint(); + } + $log_target->logEvent( DrydockSlotLockFailureLogType::LOGCONST, array( 'locks' => $ex->getLockMap(), @@ -166,14 +164,9 @@ final class DrydockResource extends DrydockDAO throw $ex; } - try { - $this - ->setStatus($new_status) - ->save(); - } catch (Exception $ex) { - $this->killTransaction(); - throw $ex; - } + $this + ->setStatus($new_status) + ->save(); $this->saveTransaction(); @@ -210,12 +203,24 @@ final class DrydockResource extends DrydockDAO $this->openTransaction(); - $this - ->setStatus(DrydockResourceStatus::STATUS_ACTIVE) - ->save(); - + try { DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks); $this->slotLocks = array(); + } catch (DrydockSlotLockException $ex) { + $this->killTransaction(); + + $this->logEvent( + DrydockSlotLockFailureLogType::LOGCONST, + array( + 'locks' => $ex->getLockMap(), + )); + + throw $ex; + } + + $this + ->setStatus(DrydockResourceStatus::STATUS_ACTIVE) + ->save(); $this->saveTransaction(); @@ -288,6 +293,15 @@ final class DrydockResource extends DrydockDAO } } + public function isActive() { + switch ($this->getStatus()) { + case DrydockResourceStatus::STATUS_ACTIVE: + return true; + } + + return false; + } + public function logEvent($type, array $data = array()) { $log = id(new DrydockLog()) ->setEpoch(PhabricatorTime::getNow()) diff --git a/src/applications/drydock/storage/DrydockSlotLock.php b/src/applications/drydock/storage/DrydockSlotLock.php index 2f980660aa..7e4e320946 100644 --- a/src/applications/drydock/storage/DrydockSlotLock.php +++ b/src/applications/drydock/storage/DrydockSlotLock.php @@ -149,6 +149,7 @@ final class DrydockSlotLock extends DrydockDAO { // time we should be able to figure out which locks are already held. $held = self::loadHeldLocks($locks); $held = mpull($held, 'getOwnerPHID', 'getLockKey'); + throw new DrydockSlotLockException($held); } } diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php index b6ac6cec52..a93997259d 100644 --- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php @@ -535,7 +535,7 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { // If this lease has been acquired but not activated, queue a task to // activate it. if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACQUIRED) { - PhabricatorWorker::scheduleTask( + $this->queueTask( __CLASS__, array( 'leasePHID' => $lease->getPHID(), @@ -691,7 +691,14 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { ->setStatus(DrydockLeaseStatus::STATUS_BROKEN) ->save(); - $lease->scheduleUpdate(); + $this->queueTask( + __CLASS__, + array( + 'leasePHID' => $lease->getPHID(), + ), + array( + 'objectPHID' => $lease->getPHID(), + )); $lease->logEvent( DrydockLeaseActivationFailureLogType::LOGCONST, diff --git a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php index bd3868e7db..e7a2ac41e6 100644 --- a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php +++ b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php @@ -29,7 +29,9 @@ abstract class HarbormasterDrydockLeaseArtifact } public function willCreateArtifact(PhabricatorUser $actor) { - $this->loadArtifactLease($actor); + // We don't load the lease here because it's expected that artifacts are + // created before leases actually exist. This guarantees that the leases + // will be cleaned up. } public function loadArtifactLease(PhabricatorUser $viewer) { @@ -51,7 +53,15 @@ abstract class HarbormasterDrydockLeaseArtifact } public function releaseArtifact(PhabricatorUser $actor) { - $lease = $this->loadArtifactLease($actor); + try { + $lease = $this->loadArtifactLease($actor); + } catch (Exception $ex) { + // If we can't load the lease, treat it as already released. Artifacts + // are generated before leases are queued, so it's possible to arrive + // here under normal conditions. + return; + } + if (!$lease->canRelease()) { return; } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php index dd43aa0bee..98494d881d 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php @@ -61,7 +61,7 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController { $is_new = (!$plan->getID()); if ($is_new) { $title = pht('New Build Plan'); - $cancel_uri = $this->getApplicationURI(); + $cancel_uri = $this->getApplicationURI('plan/'); $save_button = pht('Create Build Plan'); } else { $id = $plan->getID(); diff --git a/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php index 0b19704197..6eb63bb39c 100644 --- a/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php @@ -12,7 +12,7 @@ final class HarbormasterDrydockCommandBuildStepImplementation } public function getBuildStepGroupKey() { - return HarbormasterPrototypeBuildStepGroup::GROUPKEY; + return HarbormasterDrydockBuildStepGroup::GROUPKEY; } public function getDescription() { diff --git a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php index 001a981a60..73ea19bd7a 100644 --- a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php @@ -12,7 +12,7 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation } public function getBuildStepGroupKey() { - return HarbormasterPrototypeBuildStepGroup::GROUPKEY; + return HarbormasterDrydockBuildStepGroup::GROUPKEY; } public function execute( @@ -41,7 +41,7 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation $working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation()) ->getType(); - $lease = id(new DrydockLease()) + $lease = DrydockLease::initializeNewLease() ->setResourceType($working_copy_type) ->setOwnerPHID($build_target->getPHID()); @@ -54,6 +54,18 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation $lease->setAwakenTaskIDs(array($task_id)); } + // TODO: Maybe add a method to mark artifacts like this as pending? + + // Create the artifact now so that the lease is always disposed of, even + // if this target is aborted. + $build_target->createArtifact( + $viewer, + $settings['name'], + HarbormasterWorkingCopyArtifact::ARTIFACTCONST, + array( + 'drydockLeasePHID' => $lease->getPHID(), + )); + $lease->queueForActivation(); $build_target @@ -73,14 +85,6 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation 'Lease "%s" never activated.', $lease->getPHID())); } - - $artifact = $build_target->createArtifact( - $viewer, - $settings['name'], - HarbormasterWorkingCopyArtifact::ARTIFACTCONST, - array( - 'drydockLeasePHID' => $lease->getPHID(), - )); } public function getArtifactOutputs() { diff --git a/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php b/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php new file mode 100644 index 0000000000..606a47a114 --- /dev/null +++ b/src/applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php @@ -0,0 +1,25 @@ +setIconFont('fa-bars') ->setDropdownMenu($actions); + $header_name = phutil_tag( + 'a', + array( + 'href' => $handle->getURI(), + ), + $handle->getName()); + $header = id(new PHUIHeaderView()) ->setUser($viewer) ->setEpoch($answer->getDateModified()) - ->setHeader($handle->getName()) + ->setHeader($header_name) ->addActionLink($action_button) ->setImage($handle->getImageURI()) ->setImageURL($handle->getURI()); diff --git a/src/infrastructure/daemon/workers/PhabricatorWorker.php b/src/infrastructure/daemon/workers/PhabricatorWorker.php index c448e7221a..473649ba19 100644 --- a/src/infrastructure/daemon/workers/PhabricatorWorker.php +++ b/src/infrastructure/daemon/workers/PhabricatorWorker.php @@ -160,8 +160,7 @@ abstract class PhabricatorWorker extends Phobject { try { $worker->doWork(); foreach ($worker->getQueuedTasks() as $queued_task) { - list($queued_class, $queued_data, $queued_priority) = $queued_task; - $queued_options = array('priority' => $queued_priority); + list($queued_class, $queued_data, $queued_options) = $queued_task; self::scheduleTask($queued_class, $queued_data, $queued_options); } break; @@ -220,11 +219,14 @@ abstract class PhabricatorWorker extends Phobject { * * @param string Task class to queue. * @param array Data for the followup task. - * @param int|null Priority for the followup task. + * @param array Options for the followup task. * @return this */ - final protected function queueTask($class, array $data, $priority = null) { - $this->queuedTasks[] = array($class, $data, $priority); + final protected function queueTask( + $class, + array $data, + array $options = array()) { + $this->queuedTasks[] = array($class, $data, $options); return $this; } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php index 3826075276..2acc8452ea 100644 --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php @@ -42,7 +42,8 @@ final class PhabricatorWorkerManagementExecuteWorkflow $task->getDataID()); $task->setData($task_data->getData()); - $console->writeOut( + echo tsprintf( + "%s\n", pht( 'Executing task %d (%s)...', $task->getID(), diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php index 6c2f5b4461..57a1794ec3 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php @@ -217,13 +217,14 @@ final class PhabricatorWorkerActiveTask extends PhabricatorWorkerTask { // so execute it out here and just let the exception escape. if ($did_succeed) { foreach ($worker->getQueuedTasks() as $task) { - list($class, $data) = $task; - PhabricatorWorker::scheduleTask( - $class, - $data, - array( - 'priority' => (int)$this->getPriority(), - )); + list($class, $data, $options) = $task; + + // Default the new task priority to our own priority. + $options = $options + array( + 'priority' => (int)$this->getPriority(), + ); + + PhabricatorWorker::scheduleTask($class, $data, $options); } } diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index 055b85f0df..beda39c21c 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1398,6 +1398,11 @@ final class PhabricatorUSEnglishTranslation 'Setting retention policy for "%s" to %s days.', ), + 'Waiting %s second(s) for lease to activate.' => array( + 'Waiting a second for lease to activate.', + 'Waiting %s seconds for lease to activate.', + ), + ); } diff --git a/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php b/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php index 66801d9578..f24e1e7b3d 100644 --- a/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php +++ b/src/infrastructure/markup/interpreter/PhabricatorRemarkupCowsayBlockInterpreter.php @@ -28,10 +28,16 @@ final class PhabricatorRemarkupCowsayBlockInterpreter ->setText($content) ->renderCow(); - if ($this->getEngine()->isTextMode()) { + $engine = $this->getEngine(); + + if ($engine->isTextMode()) { return $result; } + if ($engine->isHTMLMailMode()) { + return phutil_tag('pre', array(), $result); + } + return phutil_tag( 'div', array( diff --git a/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php b/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php index e823d7bdeb..ee794bcc73 100644 --- a/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php +++ b/src/infrastructure/markup/interpreter/PhabricatorRemarkupFigletBlockInterpreter.php @@ -27,10 +27,16 @@ final class PhabricatorRemarkupFigletBlockInterpreter $result = $figlet->lineEcho($content); - if ($this->getEngine()->isTextMode()) { + $engine = $this->getEngine(); + + if ($engine->isTextMode()) { return $result; } + if ($engine->isHTMLMailMode()) { + return phutil_tag('pre', array(), $result); + } + return phutil_tag( 'div', array( diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php index d2fbc95326..8e373d0cf6 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php @@ -60,9 +60,9 @@ final class PhabricatorStorageManagementProbeWorkflow $overall); $table->addRow(array( - 'name' => phutil_console_format('**%s**', $db), - 'size' => phutil_console_format('**%s**', $database_size), - 'percentage' => phutil_console_format('**%s**', $database_percentage), + 'name' => tsprintf('**%s**', $db), + 'size' => tsprintf('**%s**', $database_size), + 'percentage' => tsprintf('**%s**', $database_percentage), )); $data[$db] = isort($data[$db], '_totalSize'); foreach ($data[$db] as $table_name => $info) { @@ -82,9 +82,9 @@ final class PhabricatorStorageManagementProbeWorkflow $overall, $overall); $table->addRow(array( - 'name' => phutil_console_format('**%s**', pht('TOTAL')), - 'size' => phutil_console_format('**%s**', $overall_size), - 'percentage' => phutil_console_format('**%s**', $overall_percentage), + 'name' => tsprintf('**%s**', pht('TOTAL')), + 'size' => tsprintf('**%s**', $overall_size), + 'percentage' => tsprintf('**%s**', $overall_percentage), )); $table->draw(); diff --git a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js index aef3dce6b2..dc097487fd 100644 --- a/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js +++ b/webroot/rsrc/js/application/diffusion/behavior-pull-lastmodified.js @@ -16,7 +16,16 @@ JX.behavior('diffusion-pull-lastmodified', function(config) { if (!config.map[k][l]) { continue; } - JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l])); + try { + JX.DOM.setContent(JX.$(config.map[k][l]), JX.$H(r[k][l])); + } catch (ex) { + // The way this works is weird and sometimes the components get + // out of sync. Fail gently until we can eventually improve the + // underlying mechanism. + + // In particular, we currently may generate lint information + // without generating a lint column. See T9524. + } } } })