diff --git a/README.md b/README.md index 165c9cbac8..c2375d4340 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -**Phabricator** is an open source collection of web applications which help software companies build better software. +**Phabricator** is a collection of web applications which help software companies build better software. Phabricator includes applications for: @@ -18,25 +18,13 @@ Phabricator is developed and maintained by [Phacility](http://phacility.com). ---------- -**BUG REPORTS** +**SUPPORT RESOURCES** -Please update your install to **HEAD** before filing bug reports. Follow our [bug reporting guide](https://secure.phabricator.com/book/phabcontrib/article/bug_reports/) for complete instructions. +For resources on filing bugs, requesting features, reporting security issues, and getting other kinds of support, see [Support Resources](https://secure.phabricator.com/book/phabricator/article/support/). -**FEATURE REQUESTS** +**NO PULL REQUESTS!** -We're big fans of feature requests that state core problems, not just 'add this'. We've compiled a short guide to effective upstream requests [here](https://secure.phabricator.com/book/phabcontrib/article/feature_requests/). - -**COMMUNITY CHAT** - -Please visit our [IRC Channel (#phabricator on FreeNode)](irc://chat.freenode.net/phabricator) to talk with other members of the Phabricator community. There might be someone there who can help you with setup issues or what image to choose for a macro. - -**SECURITY ISSUES** - -Phabricator participates in HackerOne and may pay out for various issues reported there. You can find out more information on our [HackerOne page](https://hackerone.com/phabricator). - -**PULL REQUESTS** - -We do not accept pull requests through GitHub. If you would like to contribute code, please read our [Contributor's Guide](https://secure.phabricator.com/book/phabcontrib/article/contributing_code/) for more information. +We do not accept pull requests through GitHub. If you would like to contribute code, please read our [Contributor's Guide](https://secure.phabricator.com/book/phabcontrib/article/contributing_code/). **LICENSE** diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 176bf6509c..fe97bdc236 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'c65b251d', + 'core.pkg.css' => '15e557bc', 'core.pkg.js' => '47dc9ebb', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '2de124c9', @@ -18,14 +18,14 @@ return array( 'maniphest.pkg.js' => '3ec6a6d5', 'rsrc/css/aphront/aphront-bars.css' => '231ac33c', 'rsrc/css/aphront/dark-console.css' => '6378ef3d', - 'rsrc/css/aphront/dialog-view.css' => 'fe58b18d', + 'rsrc/css/aphront/dialog-view.css' => 'be0e3a46', 'rsrc/css/aphront/lightbox-attachment.css' => '7acac05d', 'rsrc/css/aphront/list-filter-view.css' => '5d6f0526', 'rsrc/css/aphront/multi-column.css' => 'fd18389d', 'rsrc/css/aphront/notification.css' => '9c279160', 'rsrc/css/aphront/panel-view.css' => '8427b78d', 'rsrc/css/aphront/phabricator-nav-view.css' => 'a24cb589', - 'rsrc/css/aphront/table-view.css' => '63985f5b', + 'rsrc/css/aphront/table-view.css' => '61543e7a', 'rsrc/css/aphront/tokenizer.css' => '04875312', 'rsrc/css/aphront/tooltip.css' => '7672b60f', 'rsrc/css/aphront/typeahead-browse.css' => 'd8581d2c', @@ -71,7 +71,7 @@ return array( 'rsrc/css/application/feed/feed.css' => 'ecd4ec57', 'rsrc/css/application/files/global-drag-and-drop.css' => '697324ad', 'rsrc/css/application/flag/flag.css' => '5337623f', - 'rsrc/css/application/harbormaster/harbormaster.css' => '49d64eb4', + 'rsrc/css/application/harbormaster/harbormaster.css' => 'b0758ca5', 'rsrc/css/application/herald/herald-test.css' => 'a52e323e', 'rsrc/css/application/herald/herald.css' => '826075fa', 'rsrc/css/application/maniphest/batch-editor.css' => 'b0f0b6d5', @@ -104,7 +104,7 @@ return array( 'rsrc/css/application/tokens/tokens.css' => '3d0f239e', 'rsrc/css/application/uiexample/example.css' => '528b19de', 'rsrc/css/core/core.css' => 'a76cefc9', - 'rsrc/css/core/remarkup.css' => 'fa3a8225', + 'rsrc/css/core/remarkup.css' => '8d341238', 'rsrc/css/core/syntax.css' => '9fd11da8', 'rsrc/css/core/z-index.css' => '57ddcaa2', 'rsrc/css/diviner/diviner-shared.css' => '5a337049', @@ -141,7 +141,7 @@ return array( 'rsrc/css/phui/phui-object-item-list-view.css' => '26c30d3f', 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', - 'rsrc/css/phui/phui-property-list-view.css' => '03904f6b', + 'rsrc/css/phui/phui-property-list-view.css' => '27b2849e', 'rsrc/css/phui/phui-remarkup-preview.css' => '867f85b3', 'rsrc/css/phui/phui-spacing.css' => '042804d6', 'rsrc/css/phui/phui-status.css' => '888cedb8', @@ -489,11 +489,11 @@ return array( 'almanac-css' => 'dbb9b3af', 'aphront-bars' => '231ac33c', 'aphront-dark-console-css' => '6378ef3d', - 'aphront-dialog-view-css' => 'fe58b18d', + 'aphront-dialog-view-css' => 'be0e3a46', 'aphront-list-filter-view-css' => '5d6f0526', 'aphront-multi-column-view-css' => 'fd18389d', 'aphront-panel-view-css' => '8427b78d', - 'aphront-table-view-css' => '63985f5b', + 'aphront-table-view-css' => '61543e7a', 'aphront-tokenizer-control-css' => '04875312', 'aphront-tooltip-css' => '7672b60f', 'aphront-typeahead-control-css' => '0e403212', @@ -528,7 +528,7 @@ return array( 'font-lato' => '5ab1a46a', 'font-roboto-slab' => 'f24a53cb', 'global-drag-and-drop-css' => '697324ad', - 'harbormaster-css' => '49d64eb4', + 'harbormaster-css' => 'b0758ca5', 'herald-css' => '826075fa', 'herald-rule-editor' => '91a6031b', 'herald-test-css' => 'a52e323e', @@ -735,7 +735,7 @@ return array( 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => '6920d200', - 'phabricator-remarkup-css' => 'fa3a8225', + 'phabricator-remarkup-css' => '8d341238', 'phabricator-search-results-css' => '7dea472c', 'phabricator-shaped-request' => '7cbe244b', 'phabricator-side-menu-view-css' => 'bec2458e', @@ -792,7 +792,7 @@ return array( 'phui-object-item-list-view-css' => '26c30d3f', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', - 'phui-property-list-view-css' => '03904f6b', + 'phui-property-list-view-css' => '27b2849e', 'phui-remarkup-preview-css' => '867f85b3', 'phui-spacing-css' => '042804d6', 'phui-status-list-view-css' => '888cedb8', diff --git a/resources/sql/autopatches/20150906.mailinglist.sql b/resources/sql/autopatches/20150906.mailinglist.sql new file mode 100644 index 0000000000..4f349eadd4 --- /dev/null +++ b/resources/sql/autopatches/20150906.mailinglist.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_metamta.metamta_mailinglist; diff --git a/resources/sql/autopatches/20151023.harborpolicy.1.sql b/resources/sql/autopatches/20151023.harborpolicy.1.sql new file mode 100644 index 0000000000..516cd1af00 --- /dev/null +++ b/resources/sql/autopatches/20151023.harborpolicy.1.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildplan + ADD viewPolicy VARBINARY(64) NOT NULL; + +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildplan + ADD editPolicy VARBINARY(64) NOT NULL; diff --git a/resources/sql/autopatches/20151023.harborpolicy.2.php b/resources/sql/autopatches/20151023.harborpolicy.2.php new file mode 100644 index 0000000000..133bcb6ee7 --- /dev/null +++ b/resources/sql/autopatches/20151023.harborpolicy.2.php @@ -0,0 +1,21 @@ +establishConnection('w'); + +$view_policy = PhabricatorPolicies::getMostOpenPolicy(); +queryfx( + $conn_w, + 'UPDATE %T SET viewPolicy = %s WHERE viewPolicy = %s', + $table->getTableName(), + $view_policy, + ''); + +$edit_policy = id(new PhabricatorHarbormasterApplication()) + ->getPolicy(HarbormasterCreatePlansCapability::CAPABILITY); +queryfx( + $conn_w, + 'UPDATE %T SET editPolicy = %s WHERE editPolicy = %s', + $table->getTableName(), + $edit_policy, + ''); diff --git a/resources/sql/autopatches/20151023.patchduration.sql b/resources/sql/autopatches/20151023.patchduration.sql new file mode 100644 index 0000000000..3e0c363931 --- /dev/null +++ b/resources/sql/autopatches/20151023.patchduration.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_meta_data.patch_status + ADD duration BIGINT UNSIGNED; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d00b3a377d..398d212913 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -416,6 +416,7 @@ phutil_register_library_map(array( 'DifferentialLocalCommitsView' => 'applications/differential/view/DifferentialLocalCommitsView.php', 'DifferentialManiphestTasksField' => 'applications/differential/customfield/DifferentialManiphestTasksField.php', 'DifferentialModernHunk' => 'applications/differential/storage/DifferentialModernHunk.php', + 'DifferentialNextStepField' => 'applications/differential/customfield/DifferentialNextStepField.php', 'DifferentialParseCacheGarbageCollector' => 'applications/differential/garbagecollector/DifferentialParseCacheGarbageCollector.php', 'DifferentialParseCommitMessageConduitAPIMethod' => 'applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php', 'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php', @@ -709,6 +710,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryRemarkupRule' => 'applications/diffusion/remarkup/DiffusionRepositoryRemarkupRule.php', 'DiffusionRepositorySymbolsController' => 'applications/diffusion/controller/DiffusionRepositorySymbolsController.php', 'DiffusionRepositoryTag' => 'applications/diffusion/data/DiffusionRepositoryTag.php', + 'DiffusionRepositoryTestAutomationController' => 'applications/diffusion/controller/DiffusionRepositoryTestAutomationController.php', 'DiffusionRequest' => 'applications/diffusion/request/DiffusionRequest.php', 'DiffusionResolveRefsConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionResolveRefsConduitAPIMethod.php', 'DiffusionResolveUserQuery' => 'applications/diffusion/query/DiffusionResolveUserQuery.php', @@ -909,6 +911,7 @@ phutil_register_library_map(array( 'DrydockSlotLock' => 'applications/drydock/storage/DrydockSlotLock.php', 'DrydockSlotLockException' => 'applications/drydock/exception/DrydockSlotLockException.php', 'DrydockSlotLockFailureLogType' => 'applications/drydock/logtype/DrydockSlotLockFailureLogType.php', + 'DrydockTestRepositoryOperation' => 'applications/drydock/operation/DrydockTestRepositoryOperation.php', 'DrydockWebrootInterface' => 'applications/drydock/interface/webroot/DrydockWebrootInterface.php', 'DrydockWorker' => 'applications/drydock/worker/DrydockWorker.php', 'DrydockWorkingCopyBlueprintImplementation' => 'applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php', @@ -991,6 +994,8 @@ phutil_register_library_map(array( 'HarbormasterBuildPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPHIDType.php', 'HarbormasterBuildPlan' => 'applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php', 'HarbormasterBuildPlanDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildPlanDatasource.php', + 'HarbormasterBuildPlanDefaultEditCapability' => 'applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php', + 'HarbormasterBuildPlanDefaultViewCapability' => 'applications/harbormaster/capability/HarbormasterBuildPlanDefaultViewCapability.php', 'HarbormasterBuildPlanEditor' => 'applications/harbormaster/editor/HarbormasterBuildPlanEditor.php', 'HarbormasterBuildPlanPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildPlanPHIDType.php', 'HarbormasterBuildPlanQuery' => 'applications/harbormaster/query/HarbormasterBuildPlanQuery.php', @@ -1036,6 +1041,7 @@ phutil_register_library_map(array( 'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php', 'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php', 'HarbormasterCreateArtifactConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterCreateArtifactConduitAPIMethod.php', + 'HarbormasterCreatePlansCapability' => 'applications/harbormaster/capability/HarbormasterCreatePlansCapability.php', 'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php', 'HarbormasterDrydockBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterDrydockBuildStepGroup.php', 'HarbormasterDrydockCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterDrydockCommandBuildStepImplementation.php', @@ -1049,7 +1055,6 @@ phutil_register_library_map(array( 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php', 'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php', 'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php', - 'HarbormasterManagePlansCapability' => 'applications/harbormaster/capability/HarbormasterManagePlansCapability.php', 'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php', 'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php', 'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php', @@ -1076,6 +1081,7 @@ phutil_register_library_map(array( 'HarbormasterStepAddController' => 'applications/harbormaster/controller/HarbormasterStepAddController.php', 'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php', 'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php', + 'HarbormasterStepViewController' => 'applications/harbormaster/controller/HarbormasterStepViewController.php', 'HarbormasterTargetEngine' => 'applications/harbormaster/engine/HarbormasterTargetEngine.php', 'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php', 'HarbormasterTestBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterTestBuildStepGroup.php', @@ -1914,6 +1920,7 @@ phutil_register_library_map(array( 'PhabricatorConfigTransaction' => 'applications/config/storage/PhabricatorConfigTransaction.php', 'PhabricatorConfigTransactionQuery' => 'applications/config/query/PhabricatorConfigTransactionQuery.php', 'PhabricatorConfigValidationException' => 'applications/config/exception/PhabricatorConfigValidationException.php', + 'PhabricatorConfigVersionsModule' => 'applications/config/module/PhabricatorConfigVersionsModule.php', 'PhabricatorConfigWelcomeController' => 'applications/config/controller/PhabricatorConfigWelcomeController.php', 'PhabricatorConpherenceApplication' => 'applications/conpherence/application/PhabricatorConpherenceApplication.php', 'PhabricatorConpherencePreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorConpherencePreferencesSettingsPanel.php', @@ -2403,7 +2410,6 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAMailableDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php', 'PhabricatorMetaMTAMailableFunctionDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAMailableFunctionDatasource.php', 'PhabricatorMetaMTAMailgunReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php', - 'PhabricatorMetaMTAMailingList' => 'applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php', 'PhabricatorMetaMTAMemberQuery' => 'applications/metamta/query/PhabricatorMetaMTAMemberQuery.php', 'PhabricatorMetaMTAPermanentFailureException' => 'applications/metamta/exception/PhabricatorMetaMTAPermanentFailureException.php', 'PhabricatorMetaMTAReceivedMail' => 'applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php', @@ -4154,6 +4160,7 @@ phutil_register_library_map(array( 'DifferentialLocalCommitsView' => 'AphrontView', 'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField', 'DifferentialModernHunk' => 'DifferentialHunk', + 'DifferentialNextStepField' => 'DifferentialCustomField', 'DifferentialParseCacheGarbageCollector' => 'PhabricatorGarbageCollector', 'DifferentialParseCommitMessageConduitAPIMethod' => 'DifferentialConduitAPIMethod', 'DifferentialParseRenderTestCase' => 'PhabricatorTestCase', @@ -4461,6 +4468,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'DiffusionRepositorySymbolsController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryTag' => 'Phobject', + 'DiffusionRepositoryTestAutomationController' => 'DiffusionRepositoryEditController', 'DiffusionRequest' => 'Phobject', 'DiffusionResolveRefsConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', 'DiffusionResolveUserQuery' => 'Phobject', @@ -4701,6 +4709,7 @@ phutil_register_library_map(array( 'DrydockSlotLock' => 'DrydockDAO', 'DrydockSlotLockException' => 'Exception', 'DrydockSlotLockFailureLogType' => 'DrydockLogType', + 'DrydockTestRepositoryOperation' => 'DrydockRepositoryOperationType', 'DrydockWebrootInterface' => 'DrydockInterface', 'DrydockWorker' => 'PhabricatorWorker', 'DrydockWorkingCopyBlueprintImplementation' => 'DrydockBlueprintImplementation', @@ -4815,6 +4824,8 @@ phutil_register_library_map(array( 'PhabricatorSubscribableInterface', ), 'HarbormasterBuildPlanDatasource' => 'PhabricatorTypeaheadDatasource', + 'HarbormasterBuildPlanDefaultEditCapability' => 'PhabricatorPolicyCapability', + 'HarbormasterBuildPlanDefaultViewCapability' => 'PhabricatorPolicyCapability', 'HarbormasterBuildPlanEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildPlanPHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildPlanQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -4874,6 +4885,7 @@ phutil_register_library_map(array( 'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod', 'HarbormasterController' => 'PhabricatorController', 'HarbormasterCreateArtifactConduitAPIMethod' => 'HarbormasterConduitAPIMethod', + 'HarbormasterCreatePlansCapability' => 'PhabricatorPolicyCapability', 'HarbormasterDAO' => 'PhabricatorLiskDAO', 'HarbormasterDrydockBuildStepGroup' => 'HarbormasterBuildStepGroup', 'HarbormasterDrydockCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation', @@ -4887,7 +4899,6 @@ phutil_register_library_map(array( 'HarbormasterLeaseWorkingCopyBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterLintMessagesController' => 'HarbormasterController', 'HarbormasterLintPropertyView' => 'AphrontView', - 'HarbormasterManagePlansCapability' => 'PhabricatorPolicyCapability', 'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow', 'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow', @@ -4914,6 +4925,7 @@ phutil_register_library_map(array( 'HarbormasterStepAddController' => 'HarbormasterController', 'HarbormasterStepDeleteController' => 'HarbormasterController', 'HarbormasterStepEditController' => 'HarbormasterController', + 'HarbormasterStepViewController' => 'HarbormasterController', 'HarbormasterTargetEngine' => 'Phobject', 'HarbormasterTargetWorker' => 'HarbormasterWorker', 'HarbormasterTestBuildStepGroup' => 'HarbormasterBuildStepGroup', @@ -5893,6 +5905,7 @@ phutil_register_library_map(array( 'PhabricatorConfigTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorConfigValidationException' => 'Exception', + 'PhabricatorConfigVersionsModule' => 'PhabricatorConfigModule', 'PhabricatorConfigWelcomeController' => 'PhabricatorConfigController', 'PhabricatorConpherenceApplication' => 'PhabricatorApplication', 'PhabricatorConpherencePreferencesSettingsPanel' => 'PhabricatorSettingsPanel', @@ -6456,7 +6469,6 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAMailableDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorMetaMTAMailableFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorMetaMTAMailgunReceiveController' => 'PhabricatorMetaMTAController', - 'PhabricatorMetaMTAMailingList' => 'PhabricatorMetaMTADAO', 'PhabricatorMetaMTAMemberQuery' => 'PhabricatorQuery', 'PhabricatorMetaMTAPermanentFailureException' => 'Exception', 'PhabricatorMetaMTAReceivedMail' => 'PhabricatorMetaMTADAO', diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php index 026129404e..6f0518a3fb 100644 --- a/src/aphront/AphrontRequest.php +++ b/src/aphront/AphrontRequest.php @@ -28,7 +28,7 @@ final class AphrontRequest extends Phobject { private $applicationConfiguration; private $site; private $controller; - private $uriData; + private $uriData = array(); private $cookiePrefix; public function __construct($host, $path) { diff --git a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php index 7747a73866..659fb80edb 100644 --- a/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php +++ b/src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php @@ -132,10 +132,7 @@ abstract class PhabricatorAphlictManagementWorkflow if (posix_getuid() == 0) { throw new PhutilArgumentUsageException( - pht( - // TODO: Update this message after a while. - 'The notification server should not be run as root. It no '. - 'longer requires access to privileged ports.')); + pht('The notification server should not be run as root.')); } // Make sure we can write to the PID file. diff --git a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php index 6dfa49d860..ccc66ffeb7 100644 --- a/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php +++ b/src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php @@ -84,6 +84,22 @@ final class PhabricatorAuthOneTimeLoginController ->addCancelButton('/login/email/', pht('Send Another Email')); } + if (!$target_user->canEstablishWebSessions()) { + return $this->newDialog() + ->setTitle(pht('Unable to Establish Web Session')) + ->setShortTitle(pht('Login Failure')) + ->appendParagraph( + pht( + 'You are trying to gain access to an account ("%s") that can not '. + 'establish a web session.', + $target_user->getUsername())) + ->appendParagraph( + pht( + 'Special users like daemons and mailing lists are not permitted '. + 'to log in via the web. Log in as a normal user instead.')) + ->addCancelButton('/'); + } + if ($request->isFormPost()) { // If we have an email bound into this URI, verify email so that clicking // the link in the "Welcome" email is good enough, without requiring users diff --git a/src/applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php b/src/applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php index ad843d56ce..a31b108f08 100644 --- a/src/applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php +++ b/src/applications/auth/management/PhabricatorAuthManagementRecoverWorkflow.php @@ -71,6 +71,16 @@ final class PhabricatorAuthManagementRecoverWorkflow $can_recover)); } + if (!$user->canEstablishWebSessions()) { + throw new PhutilArgumentUsageException( + pht( + 'This account ("%s") can not establish web sessions, so it is '. + 'not possible to generate a functional recovery link. Special '. + 'accounts like daemons and mailing lists can not log in via the '. + 'web UI.', + $username)); + } + $engine = new PhabricatorAuthSessionEngine(); $onetime_uri = $engine->getOneTimeLoginURI( $user, diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php index 75ecc06239..cc792358ad 100644 --- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php +++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php @@ -319,9 +319,11 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck { ->addMySQLConfig('innodb_buffer_pool_size'); } + $conn_w = id(new PhabricatorUser())->establishConnection('w'); + $ok = PhabricatorStorageManagementAPI::isCharacterSetAvailableOnConnection( 'utf8mb4', - id(new PhabricatorUser())->establishConnection('w')); + $conn_w); if (!$ok) { $summary = pht( 'You are using an old version of MySQL, and should upgrade.'); @@ -339,6 +341,28 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck { ->setMessage($message); } + $info = queryfx_one( + $conn_w, + 'SELECT UNIX_TIMESTAMP() epoch'); + + $epoch = (int)$info['epoch']; + $local = PhabricatorTime::getNow(); + $delta = (int)abs($local - $epoch); + if ($delta > 60) { + $this->newIssue('mysql.clock') + ->setName(pht('Major Web/Database Clock Skew')) + ->setSummary( + pht( + 'This host is set to a very different time than the database.')) + ->setMessage( + pht( + 'The database host and this host ("%s") disagree on the current '. + 'time by more than 60 seconds (absolute skew is %s seconds). '. + 'Check that the current time is set correctly everywhere.', + php_uname('n'), + new PhutilNumber($delta))); + } + } protected function shouldUseMySQLSearchEngine() { diff --git a/src/applications/config/controller/PhabricatorConfigAllController.php b/src/applications/config/controller/PhabricatorConfigAllController.php index 17bf65fd7b..d805168eb1 100644 --- a/src/applications/config/controller/PhabricatorConfigAllController.php +++ b/src/applications/config/controller/PhabricatorConfigAllController.php @@ -58,31 +58,10 @@ final class PhabricatorConfigAllController $panel->setHeaderText(pht('Current Settings')); $panel->setTable($table); - $versions = $this->loadVersions(); - - $version_property_list = id(new PHUIPropertyListView()); - foreach ($versions as $version) { - list($name, $hash) = $version; - $version_property_list->addProperty($name, $hash); - } - - $object_box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Current Version')) - ->addPropertyList($version_property_list); - - $phabricator_root = dirname(phutil_get_library_root('phabricator')); - $version_path = $phabricator_root.'/conf/local/VERSION'; - if (Filesystem::pathExists($version_path)) { - $version_from_file = Filesystem::readFile($version_path); - $version_property_list->addProperty( - pht('Local Version'), - $version_from_file); - } $nav = $this->buildSideNavView(); $nav->selectFilter('all/'); $nav->setCrumbs($crumbs); - $nav->appendChild($object_box); $nav->appendChild($panel); @@ -93,42 +72,4 @@ final class PhabricatorConfigAllController )); } - private function loadVersions() { - $specs = array( - array( - 'name' => pht('Phabricator Version'), - 'root' => 'phabricator', - ), - array( - 'name' => pht('Arcanist Version'), - 'root' => 'arcanist', - ), - array( - 'name' => pht('libphutil Version'), - 'root' => 'phutil', - ), - ); - - $futures = array(); - foreach ($specs as $key => $spec) { - $root = dirname(phutil_get_library_root($spec['root'])); - $futures[$key] = id(new ExecFuture('git log --format=%%H -n 1 --')) - ->setCWD($root); - } - - $results = array(); - foreach ($futures as $key => $future) { - list($err, $stdout) = $future->resolve(); - if (!$err) { - $name = trim($stdout); - } else { - $name = pht('Unknown'); - } - $results[$key] = array($specs[$key]['name'], $name); - } - - return array_select_keys($results, array_keys($specs)); - } - - } diff --git a/src/applications/config/module/PhabricatorConfigPHIDModule.php b/src/applications/config/module/PhabricatorConfigPHIDModule.php index 6c363e3bee..4c5c09f9c3 100644 --- a/src/applications/config/module/PhabricatorConfigPHIDModule.php +++ b/src/applications/config/module/PhabricatorConfigPHIDModule.php @@ -18,9 +18,35 @@ final class PhabricatorConfigPHIDModule extends PhabricatorConfigModule { $rows = array(); foreach ($types as $key => $type) { + $class_name = $type->getPHIDTypeApplicationClass(); + if ($class_name !== null) { + $app = PhabricatorApplication::getByClass($class_name); + $app_name = $app->getName(); + + $icon = $app->getFontIcon(); + if ($icon) { + $app_icon = id(new PHUIIconView())->setIconFont($icon); + } else { + $app_icon = null; + } + } else { + $app_name = null; + $app_icon = null; + } + + $icon = $type->getTypeIcon(); + if ($icon) { + $type_icon = id(new PHUIIconView())->setIconFont($icon); + } else { + $type_icon = null; + } + $rows[] = array( $type->getTypeConstant(), get_class($type), + $app_icon, + $app_name, + $type_icon, $type->getTypeName(), ); } @@ -30,12 +56,18 @@ final class PhabricatorConfigPHIDModule extends PhabricatorConfigModule { array( pht('Constant'), pht('Class'), + null, + pht('Application'), + null, pht('Name'), )) ->setColumnClasses( array( null, 'pri', + 'icon', + null, + 'icon', 'wide', )); diff --git a/src/applications/config/module/PhabricatorConfigVersionsModule.php b/src/applications/config/module/PhabricatorConfigVersionsModule.php new file mode 100644 index 0000000000..b3240aec51 --- /dev/null +++ b/src/applications/config/module/PhabricatorConfigVersionsModule.php @@ -0,0 +1,79 @@ +getViewer(); + + + $versions = $this->loadVersions(); + + $version_property_list = id(new PHUIPropertyListView()); + foreach ($versions as $version) { + list($name, $hash) = $version; + $version_property_list->addProperty($name, $hash); + } + + $object_box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Current Versions')) + ->addPropertyList($version_property_list); + + $phabricator_root = dirname(phutil_get_library_root('phabricator')); + $version_path = $phabricator_root.'/conf/local/VERSION'; + if (Filesystem::pathExists($version_path)) { + $version_from_file = Filesystem::readFile($version_path); + $version_property_list->addProperty( + pht('Local Version'), + $version_from_file); + } + + return $object_box; + } + + private function loadVersions() { + $specs = array( + array( + 'name' => pht('Phabricator Version'), + 'root' => 'phabricator', + ), + array( + 'name' => pht('Arcanist Version'), + 'root' => 'arcanist', + ), + array( + 'name' => pht('libphutil Version'), + 'root' => 'phutil', + ), + ); + + $futures = array(); + foreach ($specs as $key => $spec) { + $root = dirname(phutil_get_library_root($spec['root'])); + $futures[$key] = id(new ExecFuture('git log --format=%%H -n 1 --')) + ->setCWD($root); + } + + $results = array(); + foreach ($futures as $key => $future) { + list($err, $stdout) = $future->resolve(); + if (!$err) { + $name = trim($stdout); + } else { + $name = pht('Unknown'); + } + $results[$key] = array($specs[$key]['name'], $name); + } + + return array_select_keys($results, array_keys($specs)); + } + +} diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php index a2bb63a2a2..dd864d523f 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -25,6 +25,8 @@ final class PhabricatorDifferentialConfigOptions $custom_field_type = 'custom:PhabricatorCustomFieldConfigOptionType'; $fields = array( + new DifferentialNextStepField(), + new DifferentialTitleField(), new DifferentialSummaryField(), new DifferentialTestPlanField(), diff --git a/src/applications/differential/controller/DifferentialRevisionOperationController.php b/src/applications/differential/controller/DifferentialRevisionOperationController.php index 6ec5b8edec..a5d5b5f98f 100644 --- a/src/applications/differential/controller/DifferentialRevisionOperationController.php +++ b/src/applications/differential/controller/DifferentialRevisionOperationController.php @@ -18,44 +18,13 @@ final class DifferentialRevisionOperationController $detail_uri = "/D{$id}"; - $repository = $revision->getRepository(); - if (!$repository) { - return $this->rejectOperation( - $revision, - pht('No Repository'), - pht( - 'This revision is not associated with a known repository. Only '. - 'revisions associated with a tracked repository can be landed '. - 'automatically.')); - } - - if (!$repository->canPerformAutomation()) { - return $this->rejectOperation( - $revision, - pht('No Repository Automation'), - pht( - 'The repository this revision is associated with ("%s") is not '. - 'configured to support automation. Configure automation for the '. - 'repository to enable revisions to be landed automatically.', - $repository->getMonogram())); - } - - // TODO: At some point we should allow installs to give "land reviewed - // code" permission to more users than "push any commit", because it is - // a much less powerful operation. For now, just require push so this - // doesn't do anything users can't do on their own. - $can_push = PhabricatorPolicyFilter::hasCapability( - $viewer, - $repository, - DiffusionPushCapability::CAPABILITY); - if (!$can_push) { - return $this->rejectOperation( - $revision, - pht('Unable to Push'), - pht( - 'You do not have permission to push to the repository this '. - 'revision is associated with ("%s"), so you can not land it.', - $repository->getMonogram())); + $op = new DrydockLandRepositoryOperation(); + $barrier = $op->getBarrierToLanding($viewer, $revision); + if ($barrier) { + return $this->newDialog() + ->setTitle($barrier['title']) + ->appendParagraph($barrier['body']) + ->addCancelButton($detail_uri); } if ($request->isFormPost()) { @@ -64,8 +33,7 @@ final class DifferentialRevisionOperationController // occurs. $diff = $revision->getActiveDiff(); - - $op = new DrydockLandRepositoryOperation(); + $repository = $revision->getRepository(); $operation = DrydockRepositoryOperation::initializeNewOperation($op) ->setAuthorPHID($viewer->getPHID()) @@ -96,18 +64,4 @@ final class DifferentialRevisionOperationController ->addSubmitButton(pht('Mutate Repository Unpredictably')); } - private function rejectOperation( - DifferentialRevision $revision, - $title, - $body) { - - $id = $revision->getID(); - $detail_uri = "/D{$id}"; - - return $this->newDialog() - ->setTitle($title) - ->appendParagraph($body) - ->addCancelButton($detail_uri); - } - } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index f6c90362df..621464cc9b 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1047,18 +1047,31 @@ final class DifferentialRevisionViewController extends DifferentialController { $operations = id(new DrydockRepositoryOperationQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($revision->getPHID())) - ->withOperationStates( + ->withOperationTypes( array( - DrydockRepositoryOperation::STATE_WAIT, - DrydockRepositoryOperation::STATE_WORK, - DrydockRepositoryOperation::STATE_FAIL, + DrydockLandRepositoryOperation::OPCONST, )) ->execute(); if (!$operations) { return null; } - $operation = head(msort($operations, 'getID')); + $state_fail = DrydockRepositoryOperation::STATE_FAIL; + + // We're going to show the oldest operation which hasn't failed, or the + // most recent failure if they're all failures. + $operations = msort($operations, 'getID'); + foreach ($operations as $operation) { + if ($operation->getOperationState() != $state_fail) { + break; + } + } + + // If we found a completed operation, don't render anything. We don't want + // to show an older error after the thing worked properly. + if ($operation->isDone()) { + return null; + } $box_view = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Active Operations')); diff --git a/src/applications/differential/customfield/DifferentialNextStepField.php b/src/applications/differential/customfield/DifferentialNextStepField.php new file mode 100644 index 0000000000..bf0ed5972d --- /dev/null +++ b/src/applications/differential/customfield/DifferentialNextStepField.php @@ -0,0 +1,65 @@ +getFieldName(); + } + + public function renderPropertyViewValue(array $handles) { + $revision = $this->getObject(); + $diff = $revision->getActiveDiff(); + + $status = $revision->getStatus(); + if ($status != ArcanistDifferentialRevisionStatus::ACCEPTED) { + return null; + } + + $local_vcs = $diff->getSourceControlSystem(); + switch ($local_vcs) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $bookmark = $diff->getBookmark(); + if (strlen($bookmark)) { + $next_step = csprintf('arc land %R', $bookmark); + } else { + $next_step = csprintf('arc land'); + } + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $branch = $diff->getBranch(); + if (strlen($branch)) { + $next_step = csprintf('arc land %R', $branch); + } else { + $next_step = csprintf('arc land'); + } + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $next_step = csprintf('arc commit'); + break; + default: + return null; + } + + $next_step = phutil_tag('tt', array(), (string)$next_step); + + return $next_step; + } + +} diff --git a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php index 7dcf3f462d..2cf3eaa4e5 100644 --- a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php +++ b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php @@ -26,7 +26,9 @@ final class DifferentialLandingActionMenuEventListener } private function renderRevisionAction(PhutilEvent $event) { - if (!$this->canUseApplication($event->getUser())) { + $viewer = $event->getUser(); + + if (!$this->canUseApplication($viewer)) { return null; } @@ -40,11 +42,22 @@ final class DifferentialLandingActionMenuEventListener if ($repository->canPerformAutomation()) { $revision_id = $revision->getID(); + $op = new DrydockLandRepositoryOperation(); + $barrier = $op->getBarrierToLanding($viewer, $revision); + + if ($barrier) { + $can_land = false; + } else { + $can_land = true; + } + $action = id(new PhabricatorActionView()) - ->setWorkflow(true) ->setName(pht('Land Revision')) ->setIcon('fa-fighter-jet') - ->setHref("/differential/revision/operation/{$revision_id}/"); + ->setHref("/differential/revision/operation/{$revision_id}/") + ->setWorkflow(true) + ->setDisabled(!$can_land); + $this->addActionMenuItems($event, $action); } @@ -54,7 +67,6 @@ final class DifferentialLandingActionMenuEventListener ->execute(); foreach ($strategies as $strategy) { - $viewer = $event->getUser(); $action = $strategy->createMenuItem($viewer, $revision, $repository); if ($action == null) { continue; diff --git a/src/applications/differential/view/DifferentialRevisionDetailView.php b/src/applications/differential/view/DifferentialRevisionDetailView.php index cf4222dbdb..493691d21a 100644 --- a/src/applications/differential/view/DifferentialRevisionDetailView.php +++ b/src/applications/differential/view/DifferentialRevisionDetailView.php @@ -73,36 +73,6 @@ final class DifferentialRevisionDetailView extends AphrontView { ->setUser($user) ->setObject($revision); - $status = $revision->getStatus(); - $local_vcs = $this->getDiff()->getSourceControlSystem(); - - $next_step = null; - if ($status == ArcanistDifferentialRevisionStatus::ACCEPTED) { - switch ($local_vcs) { - case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - $bookmark = $this->getDiff()->getBookmark(); - $next_step = ($bookmark != '' - ? csprintf('arc land %s', $bookmark) - : 'arc land'); - break; - - case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - $branch = $this->getDiff()->getBranch(); - $next_step = ($branch != '' - ? csprintf('arc land %s', $branch) - : 'arc land'); - break; - - case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - $next_step = 'arc commit'; - break; - } - } - if ($next_step) { - $next_step = phutil_tag('tt', array(), $next_step); - $properties->addProperty(pht('Next Step'), $next_step); - } - $properties->setHasKeyboardShortcuts(true); $properties->setActionList($actions); $this->setActionList($actions); diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php index 09a5b35f19..2e3d2a4110 100644 --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -103,6 +103,7 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { 'symbol/' => 'DiffusionRepositorySymbolsController', 'staging/' => 'DiffusionRepositoryEditStagingController', 'automation/' => 'DiffusionRepositoryEditAutomationController', + 'testautomation/' => 'DiffusionRepositoryTestAutomationController', ), 'pathtree/(?P.*)' => 'DiffusionPathTreeController', 'mirror/' => array( diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditAutomationController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditAutomationController.php index 1c94e8bbc5..8fe5b70f97 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditAutomationController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditAutomationController.php @@ -4,7 +4,7 @@ final class DiffusionRepositoryEditAutomationController extends DiffusionRepositoryEditController { protected function processDiffusionRequest(AphrontRequest $request) { - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $drequest = $this->diffusionRequest; $repository = $drequest->getRepository(); diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php index 5def1c754d..f18ffe4163 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php @@ -688,6 +688,19 @@ final class DiffusionRepositoryEditMainController $this->getRepositoryControllerURI($repository, 'edit/automation/')); $view->addAction($edit); + $can_test = $repository->canPerformAutomation(); + + $test = id(new PhabricatorActionView()) + ->setIcon('fa-gamepad') + ->setName(pht('Test Configuration')) + ->setWorkflow(true) + ->setDisabled(!$can_test) + ->setHref( + $this->getRepositoryControllerURI( + $repository, + 'edit/testautomation/')); + $view->addAction($test); + return $view; } diff --git a/src/applications/diffusion/controller/DiffusionRepositoryTestAutomationController.php b/src/applications/diffusion/controller/DiffusionRepositoryTestAutomationController.php new file mode 100644 index 0000000000..6a9039889e --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionRepositoryTestAutomationController.php @@ -0,0 +1,78 @@ +getViewer(); + $drequest = $this->diffusionRequest; + $repository = $drequest->getRepository(); + + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withIDs(array($repository->getID())) + ->executeOne(); + if (!$repository) { + return new Aphront404Response(); + } + + $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/'); + + if (!$repository->canPerformAutomation()) { + return $this->newDialog() + ->setTitle(pht('Automation Not Configured')) + ->appendParagraph( + pht( + 'You can not run a configuration test for this repository '. + 'because you have not configured repository automation yet. '. + 'Configure it first, then test the configuration.')) + ->addCancelButton($edit_uri); + } + + if ($request->isFormPost()) { + $op = new DrydockTestRepositoryOperation(); + + $operation = DrydockRepositoryOperation::initializeNewOperation($op) + ->setAuthorPHID($viewer->getPHID()) + ->setObjectPHID($repository->getPHID()) + ->setRepositoryPHID($repository->getPHID()) + ->setRepositoryTarget('none:') + ->save(); + + $operation->scheduleUpdate(); + + $operation_id = $operation->getID(); + $operation_uri = "/drydock/operation/{$operation_id}/"; + + return id(new AphrontRedirectResponse()) + ->setURI($operation_uri); + } + + return $this->newDialog() + ->setTitle(pht('Test Automation Configuration')) + ->appendParagraph( + pht( + 'This configuration test will build a working copy of the '. + 'repository and perform some basic validation. If it works, '. + 'your configuration is substantially correct.')) + ->appendParagraph( + pht( + 'The test will not perform any writes against the repository, so '. + 'write operations may still fail even if the test passes. This '. + 'test covers building and reading working copies, but not writing '. + 'to them.')) + ->appendParagraph( + pht( + 'If you run into write failures despite passing this test, '. + 'it suggests that your setup is nearly correct but authentication '. + 'is probably not fully configured.')) + ->addCancelButton($edit_uri) + ->addSubmitButton(pht('Start Test')); + } + +} diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index 5d6b2623b5..e8f499955a 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -72,6 +72,7 @@ final class DiffusionCommitQuery */ public function withRepositoryPHIDs(array $phids) { $this->repositoryPHIDs = $phids; + return $this; } /** diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php index e62c7b7558..19e476c4ba 100644 --- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php @@ -109,11 +109,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation DrydockBlueprint $blueprint, DrydockResource $resource, DrydockLease $lease) { - - if (!DrydockSlotLock::isLockFree($this->getLeaseSlotLock($resource))) { - return false; - } - return true; } @@ -124,7 +119,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation $lease ->setActivateWhenAcquired(true) - ->needSlotLock($this->getLeaseSlotLock($resource)) ->acquireOnResource($resource); } @@ -146,11 +140,6 @@ final class DrydockAlmanacServiceHostBlueprintImplementation return; } - private function getLeaseSlotLock(DrydockResource $resource) { - $resource_phid = $resource->getPHID(); - return "almanac.host.lease({$resource_phid})"; - } - public function getType() { return 'host'; } @@ -188,7 +177,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation } } - public function getFieldSpecifications() { + protected function getCustomFieldSpecifications() { return array( 'almanacServicePHIDs' => array( 'name' => pht('Almanac Services'), @@ -207,7 +196,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation 'credential.type' => PassphraseSSHPrivateKeyTextCredentialType::CREDENTIAL_TYPE, ), - ) + parent::getFieldSpecifications(); + ); } private function loadServices(DrydockBlueprint $blueprint) { diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php index 0c53b19fcf..01c2067280 100644 --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -16,6 +16,26 @@ abstract class DrydockBlueprintImplementation extends Phobject { abstract public function getDescription(); public function getFieldSpecifications() { + $fields = array(); + + $fields += $this->getCustomFieldSpecifications(); + + if ($this->shouldUseConcurrentResourceLimit()) { + $fields += array( + 'allocator.limit' => array( + 'name' => pht('Limit'), + 'caption' => pht( + 'Maximum number of resources this blueprint can have active '. + 'concurrently.'), + 'type' => 'int', + ), + ); + } + + return $fields; + } + + protected function getCustomFieldSpecifications() { return array(); } @@ -316,6 +336,85 @@ abstract class DrydockBlueprintImplementation extends Phobject { } + /** + * Does this implementation use concurrent resource limits? + * + * Implementations can override this method to opt into standard limit + * behavior, which provides a simple concurrent resource limit. + * + * @return bool True to use limits. + */ + protected function shouldUseConcurrentResourceLimit() { + return false; + } + + + /** + * Get the effective concurrent resource limit for this blueprint. + * + * @param DrydockBlueprint Blueprint to get the limit for. + * @return int|null Limit, or `null` for no limit. + */ + protected function getConcurrentResourceLimit(DrydockBlueprint $blueprint) { + if ($this->shouldUseConcurrentResourceLimit()) { + $limit = $blueprint->getFieldValue('allocator.limit'); + $limit = (int)$limit; + if ($limit > 0) { + return $limit; + } else { + return null; + } + } + + return null; + } + + + protected function getConcurrentResourceLimitSlotLock( + DrydockBlueprint $blueprint) { + + $limit = $this->getConcurrentResourceLimit($blueprint); + if ($limit === null) { + return; + } + + $blueprint_phid = $blueprint->getPHID(); + + // TODO: This logic shouldn't do anything awful, but is a little silly. It + // would be nice to unify the "huge limit" and "small limit" cases + // eventually but it's a little tricky. + + // If the limit is huge, just pick a random slot. This is just stopping + // us from exploding if someone types a billion zillion into the box. + if ($limit > 1024) { + $slot = mt_rand(0, $limit - 1); + return "allocator({$blueprint_phid}).limit({$slot})"; + } + + // For reasonable limits, actually check for an available slot. + $locks = DrydockSlotLock::loadLocks($blueprint_phid); + $locks = mpull($locks, null, 'getLockKey'); + + $slots = range(0, $limit - 1); + shuffle($slots); + + foreach ($slots as $slot) { + $slot_lock = "allocator({$blueprint_phid}).limit({$slot})"; + if (empty($locks[$slot_lock])) { + return $slot_lock; + } + } + + // If we found no free slot, just return whatever we checked last (which + // is just a random slot). There's a small chance we'll get lucky and the + // lock will be free by the time we try to take it, but usually we'll just + // fail to grab the lock, throw an appropriate lock exception, and get back + // on the right path to retry later. + return $slot_lock; + } + + + /** * Apply standard limits on resource allocation rate. * @@ -329,7 +428,7 @@ abstract class DrydockBlueprintImplementation extends Phobject { // configurable by the blueprint implementation. // Limit on total number of active resources. - $total_limit = 1; + $total_limit = $this->getConcurrentResourceLimit($blueprint); // Always allow at least this many allocations to be in flight at once. $min_allowed = 1; @@ -358,9 +457,11 @@ abstract class DrydockBlueprintImplementation extends Phobject { // 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 ($total_limit !== null) { + $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 diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php index 1d7776af14..f39353a8f4 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -3,6 +3,8 @@ final class DrydockWorkingCopyBlueprintImplementation extends DrydockBlueprintImplementation { + const PHASE_SQUASHMERGE = 'squashmerge'; + public function isEnabled() { return true; } @@ -125,15 +127,7 @@ final class DrydockWorkingCopyBlueprintImplementation ->setOwnerPHID($resource_phid) ->setAttribute('workingcopy.resourcePHID', $resource_phid) ->setAllowedBlueprintPHIDs($blueprint_phids); - - $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? + $resource->setAttribute('host.leasePHID', $host_lease->getPHID()); $map = $lease->getAttribute('repositories.map'); foreach ($map as $key => $value) { @@ -143,10 +137,18 @@ final class DrydockWorkingCopyBlueprintImplementation 'phid', )); } + $resource->setAttribute('repositories.map', $map); - return $resource - ->setAttribute('repositories.map', $map) - ->allocateResource(); + $slot_lock = $this->getConcurrentResourceLimitSlotLock($blueprint); + if ($slot_lock !== null) { + $resource->needSlotLock($slot_lock); + } + + $resource->allocateResource(); + + $host_lease->queueForActivation(); + + return $resource; } public function activateResource( @@ -237,8 +239,7 @@ final class DrydockWorkingCopyBlueprintImplementation $cmd = array(); $arg = array(); - $cmd[] = 'cd %s'; - $arg[] = "{$root}/repo/{$directory}/"; + $interface->pushWorkingDirectory("{$root}/repo/{$directory}/"); $cmd[] = 'git clean -d --force'; $cmd[] = 'git fetch'; @@ -285,6 +286,15 @@ final class DrydockWorkingCopyBlueprintImplementation if (idx($spec, 'default')) { $default = $directory; } + + $merges = idx($spec, 'merges'); + if ($merges) { + foreach ($merges as $merge) { + $this->applyMerge($lease, $interface, $merge); + } + } + + $interface->popWorkingDirectory(); } if ($default === null) { @@ -333,7 +343,7 @@ final class DrydockWorkingCopyBlueprintImplementation $command_interface = $host_lease->getInterface($type); $path = $lease->getAttribute('workingcopy.default'); - $command_interface->setWorkingDirectory($path); + $command_interface->pushWorkingDirectory($path); return $command_interface; } @@ -393,15 +403,87 @@ final class DrydockWorkingCopyBlueprintImplementation return $lease; } - public function getFieldSpecifications() { + protected function getCustomFieldSpecifications() { return array( 'blueprintPHIDs' => array( 'name' => pht('Use Blueprints'), 'type' => 'blueprints', 'required' => true, ), - ) + parent::getFieldSpecifications(); + ); } + protected function shouldUseConcurrentResourceLimit() { + return true; + } + + private function applyMerge( + DrydockLease $lease, + DrydockCommandInterface $interface, + array $merge) { + + $src_uri = $merge['src.uri']; + $src_ref = $merge['src.ref']; + + $interface->execx( + 'git fetch --no-tags -- %s +%s:%s', + $src_uri, + $src_ref, + $src_ref); + + // NOTE: This can never actually generate a commit because we pass + // "--squash", but git sometimes runs code to check that a username and + // email are configured anyway. + $real_command = csprintf( + 'git -c user.name=%s -c user.email=%s merge --no-stat --squash -- %R', + 'drydock', + 'drydock@phabricator', + $src_ref); + + // Show the user a simplified command if the operation fails and we need to + // report an error. + $show_command = csprintf( + 'git merge --squash -- %R', + $src_ref); + + try { + $interface->execx('%C', $real_command); + } catch (CommandException $ex) { + $this->setWorkingCopyVCSErrorFromCommandException( + $lease, + self::PHASE_SQUASHMERGE, + $show_command, + $ex); + + throw $ex; + } + } + + protected function setWorkingCopyVCSErrorFromCommandException( + DrydockLease $lease, + $phase, + $command, + CommandException $ex) { + + $error = array( + 'phase' => $phase, + 'command' => (string)$command, + 'raw' => (string)$ex->getCommand(), + 'err' => $ex->getError(), + 'stdout' => $ex->getStdout(), + 'stderr' => $ex->getStderr(), + ); + + $lease->setAttribute('workingcopy.vcs.error', $error); + } + + public function getWorkingCopyVCSError(DrydockLease $lease) { + $error = $lease->getAttribute('workingcopy.vcs.error'); + if (!$error) { + return null; + } else { + return $error; + } + } } diff --git a/src/applications/drydock/controller/DrydockRepositoryOperationViewController.php b/src/applications/drydock/controller/DrydockRepositoryOperationViewController.php index e740073dd9..e43ffbd367 100644 --- a/src/applications/drydock/controller/DrydockRepositoryOperationViewController.php +++ b/src/applications/drydock/controller/DrydockRepositoryOperationViewController.php @@ -46,10 +46,15 @@ final class DrydockRepositoryOperationViewController ->setHeader($header) ->addPropertyList($properties); + $status_view = id(new DrydockRepositoryOperationStatusView()) + ->setUser($viewer) + ->setOperation($operation); + return $this->buildApplicationPage( array( $crumbs, $object_box, + $status_view, ), array( 'title' => $title, @@ -83,6 +88,15 @@ final class DrydockRepositoryOperationViewController pht('Object'), $viewer->renderHandle($operation->getObjectPHID())); + $lease_phid = $operation->getWorkingCopyLeasePHID(); + if ($lease_phid) { + $lease_display = $viewer->renderHandle($lease_phid); + } else { + $lease_display = phutil_tag('em', array(), pht('None')); + } + + $view->addProperty(pht('Working Copy'), $lease_display); + return $view; } diff --git a/src/applications/drydock/interface/command/DrydockCommandInterface.php b/src/applications/drydock/interface/command/DrydockCommandInterface.php index 8034e0f732..f6b8b132da 100644 --- a/src/applications/drydock/interface/command/DrydockCommandInterface.php +++ b/src/applications/drydock/interface/command/DrydockCommandInterface.php @@ -4,15 +4,27 @@ abstract class DrydockCommandInterface extends DrydockInterface { const INTERFACE_TYPE = 'command'; - private $workingDirectory; + private $workingDirectoryStack = array(); - public function setWorkingDirectory($working_directory) { - $this->workingDirectory = $working_directory; + public function pushWorkingDirectory($working_directory) { + $this->workingDirectoryStack[] = $working_directory; return $this; } - public function getWorkingDirectory() { - return $this->workingDirectory; + public function popWorkingDirectory() { + if (!$this->workingDirectoryStack) { + throw new Exception( + pht( + 'Unable to pop working directory, directory stack is empty.')); + } + return array_pop($this->workingDirectoryStack); + } + + public function peekWorkingDirectory() { + if ($this->workingDirectoryStack) { + return last($this->workingDirectoryStack); + } + return null; } final public function getInterfaceType() { @@ -38,12 +50,14 @@ abstract class DrydockCommandInterface extends DrydockInterface { abstract public function getExecFuture($command); protected function applyWorkingDirectoryToArgv(array $argv) { - if ($this->getWorkingDirectory() !== null) { + $directory = $this->peekWorkingDirectory(); + + if ($directory !== null) { $cmd = $argv[0]; $cmd = "(cd %s && {$cmd})"; $argv = array_merge( array($cmd), - array($this->getWorkingDirectory()), + array($directory), array_slice($argv, 1)); } diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php index df888f3f6f..48a3e8075f 100644 --- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php +++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php @@ -35,6 +35,28 @@ final class DrydockLandRepositoryOperation } } + public function getWorkingCopyMerges(DrydockRepositoryOperation $operation) { + $repository = $operation->getRepository(); + $merges = array(); + + $object = $operation->getObject(); + if ($object instanceof DifferentialRevision) { + $diff = $this->loadDiff($operation); + $merges[] = array( + 'src.uri' => $repository->getStagingURI(), + 'src.ref' => $diff->getStagingRef(), + ); + } else { + throw new Exception( + pht( + 'Invalid or unknown object ("%s") for land operation, expected '. + 'Differential Revision.', + $operation->getObjectPHID())); + } + + return $merges; + } + public function applyOperation( DrydockRepositoryOperation $operation, DrydockInterface $interface) { @@ -48,36 +70,7 @@ final class DrydockLandRepositoryOperation if ($object instanceof DifferentialRevision) { $revision = $object; - $diff_phid = $operation->getProperty('differential.diffPHID'); - - $diff = id(new DifferentialDiffQuery()) - ->setViewer($viewer) - ->withPHIDs(array($diff_phid)) - ->executeOne(); - if (!$diff) { - throw new Exception( - pht( - 'Unable to load diff "%s".', - $diff_phid)); - } - - $diff_revid = $diff->getRevisionID(); - $revision_id = $revision->getID(); - if ($diff_revid != $revision_id) { - throw new Exception( - pht( - 'Diff ("%s") has wrong revision ID ("%s", expected "%s").', - $diff_phid, - $diff_revid, - $revision_id)); - } - - $cmd[] = 'git fetch --no-tags -- %s +%s:%s'; - $arg[] = $repository->getStagingURI(); - $arg[] = $diff->getStagingRef(); - $arg[] = $diff->getStagingRef(); - - $merge_src = $diff->getStagingRef(); + $diff = $this->loadDiff($operation); $dict = $diff->getDiffAuthorshipDict(); $author_name = idx($dict, 'authorName'); @@ -104,7 +97,6 @@ final class DrydockLandRepositoryOperation switch ($type) { case 'branch': $push_dst = 'refs/heads/'.$name; - $merge_dst = 'refs/remotes/origin/'.$name; break; default: throw new Exception( @@ -116,30 +108,24 @@ final class DrydockLandRepositoryOperation $committer_info = $this->getCommitterInfo($operation); - $cmd[] = 'git checkout %s'; - $arg[] = $merge_dst; + // NOTE: We're doing this commit with "-F -" so we don't run into trouble + // with enormous commit messages which might otherwise exceed the maximum + // size of a command. - $cmd[] = 'git merge --no-stat --squash --ff-only -- %s'; - $arg[] = $merge_src; + $future = $interface->getExecFuture( + 'git -c user.name=%s -c user.email=%s commit --author %s -F - --', + $committer_info['name'], + $committer_info['email'], + "{$author_name} <{$author_email}>"); - $cmd[] = 'git -c user.name=%s -c user.email=%s commit --author %s -m %s'; + $future + ->write($commit_message) + ->resolvex(); - $arg[] = $committer_info['name']; - $arg[] = $committer_info['email']; - - $arg[] = "{$author_name} <{$author_email}>"; - $arg[] = $commit_message; - - $cmd[] = 'git push origin -- %s:%s'; - $arg[] = 'HEAD'; - $arg[] = $push_dst; - - $cmd = implode(' && ', $cmd); - $argv = array_merge(array($cmd), $arg); - - $result = call_user_func_array( - array($interface, 'execx'), - $argv); + $interface->execx( + 'git push origin -- %s:%s', + 'HEAD', + $push_dst); } private function getCommitterInfo(DrydockRepositoryOperation $operation) { @@ -172,4 +158,132 @@ final class DrydockLandRepositoryOperation ); } + private function loadDiff(DrydockRepositoryOperation $operation) { + $viewer = $this->getViewer(); + $revision = $operation->getObject(); + + $diff_phid = $operation->getProperty('differential.diffPHID'); + + $diff = id(new DifferentialDiffQuery()) + ->setViewer($viewer) + ->withPHIDs(array($diff_phid)) + ->executeOne(); + if (!$diff) { + throw new Exception( + pht( + 'Unable to load diff "%s".', + $diff_phid)); + } + + $diff_revid = $diff->getRevisionID(); + $revision_id = $revision->getID(); + if ($diff_revid != $revision_id) { + throw new Exception( + pht( + 'Diff ("%s") has wrong revision ID ("%s", expected "%s").', + $diff_phid, + $diff_revid, + $revision_id)); + } + + return $diff; + } + + public function getBarrierToLanding( + PhabricatorUser $viewer, + DifferentialRevision $revision) { + + $repository = $revision->getRepository(); + if (!$repository) { + return array( + 'title' => pht('No Repository'), + 'body' => pht( + 'This revision is not associated with a known repository. Only '. + 'revisions associated with a tracked repository can be landed '. + 'automatically.'), + ); + } + + if (!$repository->canPerformAutomation()) { + return array( + 'title' => pht('No Repository Automation'), + 'body' => pht( + 'The repository this revision is associated with ("%s") is not '. + 'configured to support automation. Configure automation for the '. + 'repository to enable revisions to be landed automatically.', + $repository->getMonogram()), + ); + } + + // TODO: At some point we should allow installs to give "land reviewed + // code" permission to more users than "push any commit", because it is + // a much less powerful operation. For now, just require push so this + // doesn't do anything users can't do on their own. + $can_push = PhabricatorPolicyFilter::hasCapability( + $viewer, + $repository, + DiffusionPushCapability::CAPABILITY); + if (!$can_push) { + return array( + 'title' => pht('Unable to Push'), + 'body' => pht( + 'You do not have permission to push to the repository this '. + 'revision is associated with ("%s"), so you can not land it.', + $repository->getMonogram()), + ); + } + + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + if ($revision->getStatus() != $status_accepted) { + return array( + 'title' => pht('Revision Not Accepted'), + 'body' => pht( + 'This revision is still under review. Only revisions which have '. + 'been accepted may land.'), + ); + } + + // Check for other operations. Eventually this should probably be more + // general (e.g., it's OK to land to multiple different branches + // simultaneously) but just put this in as a sanity check for now. + $other_operations = id(new DrydockRepositoryOperationQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($revision->getPHID())) + ->withOperationTypes( + array( + $this->getOperationConstant(), + )) + ->withOperationStates( + array( + DrydockRepositoryOperation::STATE_WAIT, + DrydockRepositoryOperation::STATE_WORK, + DrydockRepositoryOperation::STATE_DONE, + )) + ->execute(); + + if ($other_operations) { + $any_done = false; + foreach ($other_operations as $operation) { + if ($operation->isDone()) { + $any_done = true; + break; + } + } + + if ($any_done) { + return array( + 'title' => pht('Already Complete'), + 'body' => pht('This revision has already landed.'), + ); + } else { + return array( + 'title' => pht('Already In Flight'), + 'body' => pht('This revision is already landing.'), + ); + } + } + + return null; + } + } diff --git a/src/applications/drydock/operation/DrydockRepositoryOperationType.php b/src/applications/drydock/operation/DrydockRepositoryOperationType.php index 1ae6d6e4f8..7ec3689aff 100644 --- a/src/applications/drydock/operation/DrydockRepositoryOperationType.php +++ b/src/applications/drydock/operation/DrydockRepositoryOperationType.php @@ -16,6 +16,10 @@ abstract class DrydockRepositoryOperationType extends Phobject { DrydockRepositoryOperation $operation, PhabricatorUser $viewer); + public function getWorkingCopyMerges(DrydockRepositoryOperation $operation) { + return array(); + } + final public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; diff --git a/src/applications/drydock/operation/DrydockTestRepositoryOperation.php b/src/applications/drydock/operation/DrydockTestRepositoryOperation.php new file mode 100644 index 0000000000..0b0efbad0a --- /dev/null +++ b/src/applications/drydock/operation/DrydockTestRepositoryOperation.php @@ -0,0 +1,53 @@ +getRepository(); + switch ($operation->getOperationState()) { + case DrydockRepositoryOperation::STATE_WAIT: + return pht( + 'Waiting to test configuration for %s...', + $repository->getMonogram()); + case DrydockRepositoryOperation::STATE_WORK: + return pht( + 'Testing configuration for %s. This may take a moment if Drydock '. + 'has to clone the repository for the first time.', + $repository->getMonogram()); + case DrydockRepositoryOperation::STATE_DONE: + return pht( + 'Success! Automation is configured properly and Drydock can '. + 'operate on %s.', + $repository->getMonogram()); + } + } + + public function applyOperation( + DrydockRepositoryOperation $operation, + DrydockInterface $interface) { + $repository = $operation->getRepository(); + + if ($repository->isGit()) { + $interface->execx('git status'); + } else if ($repository->isHg()) { + $interface->execx('hg status'); + } else if ($repository->isSVN()) { + $interface->execx('svn status'); + } else { + throw new PhutilMethodNotImplementedException(); + } + } + +} diff --git a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php index b72e654603..38e1b4787b 100644 --- a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php +++ b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php @@ -7,6 +7,7 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { private $objectPHIDs; private $repositoryPHIDs; private $operationStates; + private $operationTypes; public function withIDs(array $ids) { $this->ids = $ids; @@ -33,6 +34,11 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { return $this; } + public function withOperationTypes(array $types) { + $this->operationTypes = $types; + return $this; + } + public function newResultObject() { return new DrydockRepositoryOperation(); } @@ -139,6 +145,13 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { $this->operationStates); } + if ($this->operationTypes !== null) { + $where[] = qsprintf( + $conn, + 'operationType IN (%Ls)', + $this->operationTypes); + } + return $where; } diff --git a/src/applications/drydock/storage/DrydockAuthorization.php b/src/applications/drydock/storage/DrydockAuthorization.php index 8872ef7f6f..ea1160b23b 100644 --- a/src/applications/drydock/storage/DrydockAuthorization.php +++ b/src/applications/drydock/storage/DrydockAuthorization.php @@ -66,7 +66,7 @@ final class DrydockAuthorization extends DrydockDAO public static function getBlueprintStateIcon($state) { $map = array( - self::BLUEPRINTAUTH_REQUESTED => 'fa-exclamation-circle indigo', + self::BLUEPRINTAUTH_REQUESTED => 'fa-exclamation-circle pink', self::BLUEPRINTAUTH_AUTHORIZED => 'fa-check-circle green', self::BLUEPRINTAUTH_DECLINED => 'fa-times red', ); diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php index af475c5d61..0dd4e36b48 100644 --- a/src/applications/drydock/storage/DrydockLease.php +++ b/src/applications/drydock/storage/DrydockLease.php @@ -429,10 +429,7 @@ final class DrydockLease extends DrydockDAO $this->scheduleUpdate($expires); } - $awaken_ids = $this->getAttribute('internal.awakenTaskIDs'); - if (is_array($awaken_ids) && $awaken_ids) { - PhabricatorWorker::awakenTaskIDs($awaken_ids); - } + $this->awakenTasks(); } public function logEvent($type, array $data = array()) { @@ -454,6 +451,19 @@ final class DrydockLease extends DrydockDAO return $log->save(); } + /** + * Awaken yielded tasks after a state change. + * + * @return this + */ + public function awakenTasks() { + $awaken_ids = $this->getAttribute('internal.awakenTaskIDs'); + if (is_array($awaken_ids) && $awaken_ids) { + PhabricatorWorker::awakenTaskIDs($awaken_ids); + } + + return $this; + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/drydock/storage/DrydockRepositoryOperation.php b/src/applications/drydock/storage/DrydockRepositoryOperation.php index 2ee09f3227..83e660ffa9 100644 --- a/src/applications/drydock/storage/DrydockRepositoryOperation.php +++ b/src/applications/drydock/storage/DrydockRepositoryOperation.php @@ -99,7 +99,7 @@ final class DrydockRepositoryOperation extends DrydockDAO public static function getOperationStateIcon($state) { $map = array( self::STATE_WAIT => 'fa-clock-o', - self::STATE_WORK => 'fa-refresh blue', + self::STATE_WORK => 'fa-plane ph-spin blue', self::STATE_DONE => 'fa-check green', self::STATE_FAIL => 'fa-times red', ); @@ -158,6 +158,32 @@ final class DrydockRepositoryOperation extends DrydockDAO return false; } + public function isDone() { + return ($this->getOperationState() === self::STATE_DONE); + } + + public function getWorkingCopyMerges() { + return $this->getImplementation()->getWorkingCopyMerges( + $this); + } + + public function setWorkingCopyLeasePHID($lease_phid) { + return $this->setProperty('exec.leasePHID', $lease_phid); + } + + public function getWorkingCopyLeasePHID() { + return $this->getProperty('exec.leasePHID'); + } + + public function setWorkingCopyVCSError(array $error) { + return $this->setProperty('exec.workingcopy.error', $error); + } + + public function getWorkingCopyVCSError() { + return $this->getProperty('exec.workingcopy.error'); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/drydock/view/DrydockObjectAuthorizationView.php b/src/applications/drydock/view/DrydockObjectAuthorizationView.php index 261829bfe5..25c1d582a3 100644 --- a/src/applications/drydock/view/DrydockObjectAuthorizationView.php +++ b/src/applications/drydock/view/DrydockObjectAuthorizationView.php @@ -47,6 +47,7 @@ final class DrydockObjectAuthorizationView extends AphrontView { $authorizations = array(); } + $warnings = array(); $items = array(); foreach ($blueprint_phids as $phid) { $authorization = idx($authorizations, $phid); @@ -65,10 +66,28 @@ final class DrydockObjectAuthorizationView extends AphrontView { null, DrydockAuthorization::getBlueprintStateName($state)); + switch ($state) { + case DrydockAuthorization::BLUEPRINTAUTH_REQUESTED: + case DrydockAuthorization::BLUEPRINTAUTH_DECLINED: + $warnings[] = $authorization; + break; + } + $items[] = $item; } $status = new PHUIStatusListView(); + + if ($warnings) { + $status->addItem( + id(new PHUIStatusItemView()) + ->setIcon('fa-exclamation-triangle', 'pink') + ->setTarget( + pht( + 'WARNING: There are %s unapproved authorization(s)!', + new PhutilNumber(count($warnings))))); + } + foreach ($items as $item) { $status->addItem($item); } diff --git a/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php b/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php index 3bdb529c20..bf1040afe6 100644 --- a/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php +++ b/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php @@ -73,12 +73,104 @@ final class DrydockRepositoryOperationStatusView if ($state != DrydockRepositoryOperation::STATE_FAIL) { $item->addAttribute($operation->getOperationCurrentStatus($viewer)); } else { - // TODO: Make this more useful. - $item->addAttribute(pht('Operation encountered an error.')); + $vcs_error = $operation->getWorkingCopyVCSError(); + if ($vcs_error) { + switch ($vcs_error['phase']) { + case DrydockWorkingCopyBlueprintImplementation::PHASE_SQUASHMERGE: + $message = pht( + 'This change did not merge cleanly. This usually indicates '. + 'that the change is out of date and needs to be updated.'); + break; + default: + $message = pht( + 'Operation encountered an error while performing repository '. + 'operations.'); + break; + } + + $item->addAttribute($message); + + $table = $this->renderVCSErrorTable($vcs_error); + list($links, $info) = $this->renderDetailToggles($table); + + $item->addAttribute($links); + $item->appendChild($info); + } else { + $item->addAttribute(pht('Operation encountered an error.')); + } } return id(new PHUIObjectItemListView()) ->addItem($item); } + private function renderVCSErrorTable(array $vcs_error) { + $rows = array(); + $rows[] = array(pht('Command'), $vcs_error['command']); + $rows[] = array(pht('Error'), $vcs_error['err']); + $rows[] = array(pht('Stdout'), $vcs_error['stdout']); + $rows[] = array(pht('Stderr'), $vcs_error['stderr']); + + $table = id(new AphrontTableView($rows)) + ->setColumnClasses( + array( + 'header', + 'wide prewrap', + )); + + return $table; + } + + private function renderDetailToggles(AphrontTableView $table) { + $show_id = celerity_generate_unique_node_id(); + $hide_id = celerity_generate_unique_node_id(); + $info_id = celerity_generate_unique_node_id(); + + Javelin::initBehavior('phabricator-reveal-content'); + + $show_details = javelin_tag( + 'a', + array( + 'id' => $show_id, + 'href' => '#', + 'sigil' => 'reveal-content', + 'mustcapture' => true, + 'meta' => array( + 'hideIDs' => array($show_id), + 'showIDs' => array($hide_id, $info_id), + ), + ), + pht('Show Details')); + + $hide_details = javelin_tag( + 'a', + array( + 'id' => $hide_id, + 'href' => '#', + 'sigil' => 'reveal-content', + 'mustcapture' => true, + 'style' => 'display: none', + 'meta' => array( + 'hideIDs' => array($hide_id, $info_id), + 'showIDs' => array($show_id), + ), + ), + pht('Hide Details')); + + $info = javelin_tag( + 'div', + array( + 'id' => $info_id, + 'style' => 'display: none', + ), + $table); + + $links = array( + $show_details, + $hide_details, + ); + + return array($links, $info); + } + } diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php index 7d8cc98219..8f34a88342 100644 --- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php @@ -309,17 +309,18 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { return array(); } - $query = id(new DrydockBlueprintQuery()) - ->setViewer($viewer) - ->withBlueprintClasses(array_keys($impls)) - ->withDisabled(false); - $blueprint_phids = $lease->getAllowedBlueprintPHIDs(); if (!$blueprint_phids) { $lease->logEvent(DrydockLeaseNoBlueprintsLogType::LOGCONST); return array(); } + $query = id(new DrydockBlueprintQuery()) + ->setViewer($viewer) + ->withPHIDs($blueprint_phids) + ->withBlueprintClasses(array_keys($impls)) + ->withDisabled(false); + // The Drydock application itself is allowed to authorize anything. This // is primarily used for leases generated by CLI administrative tools. $drydock_phid = id(new PhabricatorDrydockApplication())->getPHID(); @@ -751,6 +752,15 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { ->setStatus(DrydockLeaseStatus::STATUS_BROKEN) ->save(); + $lease->logEvent( + DrydockLeaseActivationFailureLogType::LOGCONST, + array( + 'class' => get_class($ex), + 'message' => $ex->getMessage(), + )); + + $lease->awakenTasks(); + $this->queueTask( __CLASS__, array( @@ -760,13 +770,6 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { 'objectPHID' => $lease->getPHID(), )); - $lease->logEvent( - DrydockLeaseActivationFailureLogType::LOGCONST, - array( - 'class' => get_class($ex), - 'message' => $ex->getMessage(), - )); - throw new PhabricatorWorkerPermanentFailureException( pht( 'Permanent failure while activating lease ("%s"): %s', @@ -796,6 +799,8 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { ->save(); $lease->logEvent(DrydockLeaseDestroyedLogType::LOGCONST); + + $lease->awakenTasks(); } } diff --git a/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php b/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php index 556119f6b3..6160575fd0 100644 --- a/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php @@ -53,6 +53,9 @@ final class DrydockRepositoryOperationUpdateWorker // waiting for a lease we're holding. try { + $operation->getImplementation() + ->setViewer($viewer); + $lease = $this->loadWorkingCopyLease($operation); $interface = $lease->getInterface( @@ -61,9 +64,6 @@ final class DrydockRepositoryOperationUpdateWorker // No matter what happens here, destroy the lease away once we're done. $lease->releaseOnDestruction(true); - $operation->getImplementation() - ->setViewer($viewer); - $operation->applyOperation($interface); } catch (PhabricatorWorkerYieldException $ex) { @@ -90,6 +90,9 @@ final class DrydockRepositoryOperationUpdateWorker // TODO: This is very similar to leasing in Harbormaster, maybe we can // share some of the logic? + $working_copy = new DrydockWorkingCopyBlueprintImplementation(); + $working_copy_type = $working_copy->getType(); + $lease_phid = $operation->getProperty('exec.leasePHID'); if ($lease_phid) { $lease = id(new DrydockLeaseQuery()) @@ -103,9 +106,6 @@ final class DrydockRepositoryOperationUpdateWorker $lease_phid)); } } else { - $working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation()) - ->getType(); - $repository = $operation->getRepository(); $allowed_phids = $repository->getAutomationBlueprintPHIDs(); @@ -127,7 +127,7 @@ final class DrydockRepositoryOperationUpdateWorker } $operation - ->setProperty('exec.leasePHID', $lease->getPHID()) + ->setWorkingCopyLeasePHID($lease->getPHID()) ->save(); $lease->queueForActivation(); @@ -138,6 +138,13 @@ final class DrydockRepositoryOperationUpdateWorker } if (!$lease->isActive()) { + $vcs_error = $working_copy->getWorkingCopyVCSError($lease); + if ($vcs_error) { + $operation + ->setWorkingCopyVCSError($vcs_error) + ->save(); + } + throw new PhabricatorWorkerPermanentFailureException( pht( 'Lease "%s" never activated.', @@ -158,6 +165,9 @@ final class DrydockRepositoryOperationUpdateWorker 'branch' => $name, ); break; + case 'none': + $spec = array(); + break; default: throw new Exception( pht( @@ -166,6 +176,8 @@ final class DrydockRepositoryOperationUpdateWorker $target)); } + $spec['merges'] = $operation->getWorkingCopyMerges(); + $map = array(); $map[$repository->getCloneName()] = array( 'phid' => $repository->getPHID(), diff --git a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php index 59b8ce6444..c9026e722f 100644 --- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php +++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php @@ -61,6 +61,7 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication { 'add/(?:(?P\d+)/)?' => 'HarbormasterStepAddController', 'new/(?P\d+)/(?P[^/]+)/' => 'HarbormasterStepEditController', + 'view/(?P\d+)/' => 'HarbormasterStepViewController', 'edit/(?:(?P\d+)/)?' => 'HarbormasterStepEditController', 'delete/(?:(?P\d+)/)?' => 'HarbormasterStepDeleteController', ), @@ -95,8 +96,16 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication { protected function getCustomCapabilities() { return array( - HarbormasterManagePlansCapability::CAPABILITY => array( - 'caption' => pht('Can create and manage build plans.'), + HarbormasterCreatePlansCapability::CAPABILITY => array( + 'default' => PhabricatorPolicies::POLICY_ADMIN, + ), + HarbormasterBuildPlanDefaultViewCapability::CAPABILITY => array( + 'template' => HarbormasterBuildPlanPHIDType::TYPECONST, + 'capability' => PhabricatorPolicyCapability::CAN_VIEW, + ), + HarbormasterBuildPlanDefaultEditCapability::CAPABILITY => array( + 'template' => HarbormasterBuildPlanPHIDType::TYPECONST, + 'capability' => PhabricatorPolicyCapability::CAN_EDIT, 'default' => PhabricatorPolicies::POLICY_ADMIN, ), ); diff --git a/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php b/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php new file mode 100644 index 0000000000..fa9e10c956 --- /dev/null +++ b/src/applications/harbormaster/capability/HarbormasterBuildPlanDefaultEditCapability.php @@ -0,0 +1,12 @@ +getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($request->getURIData('id'))) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php index 98494d881d..a4f177e7dc 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php @@ -5,9 +5,6 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController { public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $id = $request->getURIData('id'); if ($id) { $plan = id(new HarbormasterBuildPlanQuery()) @@ -23,23 +20,42 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController { return new Aphront404Response(); } } else { + $this->requireApplicationCapability( + HarbormasterCreatePlansCapability::CAPABILITY); + $plan = HarbormasterBuildPlan::initializeNewBuildPlan($viewer); } $e_name = true; $v_name = $plan->getName(); + $v_view = $plan->getViewPolicy(); + $v_edit = $plan->getEditPolicy(); $validation_exception = null; if ($request->isFormPost()) { $xactions = array(); $v_name = $request->getStr('name'); + $v_view = $request->getStr('viewPolicy'); + $v_edit = $request->getStr('editPolicy'); + $e_name = null; + $type_name = HarbormasterBuildPlanTransaction::TYPE_NAME; + $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; + $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; $xactions[] = id(new HarbormasterBuildPlanTransaction()) ->setTransactionType($type_name) ->setNewValue($v_name); + $xactions[] = id(new HarbormasterBuildPlanTransaction()) + ->setTransactionType($type_view) + ->setNewValue($v_view); + + $xactions[] = id(new HarbormasterBuildPlanTransaction()) + ->setTransactionType($type_edit) + ->setNewValue($v_edit); + $editor = id(new HarbormasterBuildPlanEditor()) ->setActor($viewer) ->setContinueOnNoEffect(true) @@ -71,19 +87,37 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController { $save_button = pht('Save Build Plan'); } + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($plan) + ->execute(); + $form = id(new AphrontFormView()) ->setUser($viewer) - ->appendChild( + ->appendControl( id(new AphrontFormTextControl()) ->setLabel(pht('Plan Name')) ->setName('name') ->setError($e_name) - ->setValue($v_name)); - - $form->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue($save_button) - ->addCancelButton($cancel_uri)); + ->setValue($v_name)) + ->appendControl( + id(new AphrontFormPolicyControl()) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) + ->setPolicyObject($plan) + ->setPolicies($policies) + ->setValue($v_view) + ->setName('viewPolicy')) + ->appendControl( + id(new AphrontFormPolicyControl()) + ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) + ->setPolicyObject($plan) + ->setPolicies($policies) + ->setValue($v_edit) + ->setName('editPolicy')) + ->appendControl( + id(new AphrontFormSubmitControl()) + ->setValue($save_button) + ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) ->setHeaderText($title) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanListController.php b/src/applications/harbormaster/controller/HarbormasterPlanListController.php index e0f9ce07c7..a67debd533 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanListController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanListController.php @@ -42,7 +42,7 @@ final class HarbormasterPlanListController extends HarbormasterPlanController { $crumbs = parent::buildApplicationCrumbs(); $can_create = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); + HarbormasterCreatePlansCapability::CAPABILITY); $crumbs->addAction( id(new PHUIListItemView()) diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php index be6d138cf7..6655fd6806 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php @@ -4,19 +4,16 @@ final class HarbormasterPlanRunController extends HarbormasterController { public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan_id = $request->getURIData('id'); - // NOTE: At least for now, this only requires the "Can Manage Plans" - // capability, not the "Can Edit" capability. Possibly it should have - // a more stringent requirement, though. - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($plan_id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$plan) { return new Aphront404Response(); diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php index 680594277d..ef8f98bdb2 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -3,7 +3,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { public function handleRequest(AphrontRequest $request) { - $viewer = $this->getviewer(); + $viewer = $this->getViewer(); $id = $request->getURIData('id'); $plan = id(new HarbormasterBuildPlanQuery()) @@ -81,16 +81,11 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->execute(); $steps = mpull($steps, null, 'getPHID'); - $has_manage = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - - $has_edit = PhabricatorPolicyFilter::hasCapability( + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $plan, PhabricatorPolicyCapability::CAN_EDIT); - $can_edit = ($has_manage && $has_edit); - $step_list = id(new PHUIObjectItemListView()) ->setUser($viewer) ->setNoDataString( @@ -122,16 +117,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setStatusIcon('fa-warning red') ->addAttribute(pht( 'This step has an invalid implementation (%s).', - $step->getClassName())) - ->addAction( - id(new PHUIListItemView()) - ->setIcon('fa-times') - ->addSigil('harbormaster-build-step-delete') - ->setWorkflow(true) - ->setRenderNameAsTooltip(true) - ->setName(pht('Delete')) - ->setHref( - $this->getApplicationURI('step/delete/'.$step->getID().'/'))); + $step->getClassName())); $step_list->addItem($item); continue; } @@ -142,23 +128,9 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { $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) { - $item->setHref($edit_uri); - } - - $item - ->setHref($edit_uri) - ->addAction( - id(new PHUIListItemView()) - ->setIcon('fa-times') - ->addSigil('harbormaster-build-step-delete') - ->setWorkflow(true) - ->setDisabled(!$can_edit) - ->setHref( - $this->getApplicationURI('step/delete/'.$step->getID().'/'))); + $view_uri = $this->getApplicationURI("step/view/{$step_id}/"); + $item->setHref($view_uri); $depends = $step->getStepImplementation()->getDependencies($step); $inputs = $step->getStepImplementation()->getArtifactInputs(); @@ -252,16 +224,11 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setObject($plan) ->setObjectURI($this->getApplicationURI("plan/{$id}/")); - $has_manage = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - - $has_edit = PhabricatorPolicyFilter::hasCapability( + $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $plan, PhabricatorPolicyCapability::CAN_EDIT); - $can_edit = ($has_manage && $has_edit); - $list->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Plan')) @@ -288,7 +255,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { ->setIcon('fa-ban')); } - $can_run = ($has_manage && $plan->canRunManually()); + $can_run = ($can_edit && $plan->canRunManually()); $list->addAction( id(new PhabricatorActionView()) diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php index 21cdc3f12c..a5ec41b2e7 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepAddController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -5,9 +5,6 @@ final class HarbormasterStepAddController extends HarbormasterController { public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($request->getURIData('id'))) diff --git a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php index 94afff9d3b..1aa8c24e4a 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php @@ -5,9 +5,6 @@ final class HarbormasterStepDeleteController extends HarbormasterController { public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - $id = $request->getURIData('id'); $step = id(new HarbormasterBuildStepQuery()) diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php index 9e99cc6c7f..37bef5f411 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -6,9 +6,6 @@ final class HarbormasterStepEditController extends HarbormasterController { $viewer = $this->getViewer(); $id = $request->getURIData('id'); - $this->requireApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - if ($id) { $step = id(new HarbormasterBuildStepQuery()) ->setViewer($viewer) @@ -61,6 +58,12 @@ final class HarbormasterStepEditController extends HarbormasterController { $plan_uri = $this->getApplicationURI('plan/'.$plan->getID().'/'); + if ($is_new) { + $cancel_uri = $plan_uri; + } else { + $cancel_uri = $this->getApplicationURI('step/view/'.$step->getID().'/'); + } + $implementation = $step->getStepImplementation(); $field_list = PhabricatorCustomField::getObjectFields( @@ -122,7 +125,10 @@ final class HarbormasterStepEditController extends HarbormasterController { try { $editor->applyTransactions($step, $xactions); - return id(new AphrontRedirectResponse())->setURI($plan_uri); + + $step_uri = $this->getApplicationURI('step/view/'.$step->getID().'/'); + + return id(new AphrontRedirectResponse())->setURI($step_uri); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; } @@ -165,31 +171,31 @@ final class HarbormasterStepEditController extends HarbormasterController { ->setError($e_description) ->setValue($v_description)); + $crumbs = $this->buildApplicationCrumbs(); + $id = $plan->getID(); + $crumbs->addTextCrumb(pht('Plan %d', $id), $plan_uri); + if ($is_new) { $submit = pht('Create Build Step'); $header = pht('New Step: %s', $implementation->getName()); - $crumb = pht('Add Step'); + $crumbs->addTextCrumb(pht('Add Step')); } else { $submit = pht('Save Build Step'); $header = pht('Edit Step: %s', $implementation->getName()); - $crumb = pht('Edit Step'); + $crumbs->addTextCrumb(pht('Step %d', $step->getID()), $cancel_uri); + $crumbs->addTextCrumb(pht('Edit Step')); } $form->appendChild( id(new AphrontFormSubmitControl()) ->setValue($submit) - ->addCancelButton($plan_uri)); + ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) ->setHeaderText($header) ->setValidationException($validation_exception) ->setForm($form); - $crumbs = $this->buildApplicationCrumbs(); - $id = $plan->getID(); - $crumbs->addTextCrumb(pht('Plan %d', $id), $plan_uri); - $crumbs->addTextCrumb($crumb); - $variables = $this->renderBuildVariablesTable(); if ($is_new) { diff --git a/src/applications/harbormaster/controller/HarbormasterStepViewController.php b/src/applications/harbormaster/controller/HarbormasterStepViewController.php new file mode 100644 index 0000000000..d8d1d3f0b0 --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterStepViewController.php @@ -0,0 +1,125 @@ +getViewer(); + $id = $request->getURIData('id'); + + $step = id(new HarbormasterBuildStepQuery()) + ->setViewer($viewer) + ->withIDs(array($id)) + ->executeOne(); + if (!$step) { + return new Aphront404Response(); + } + $plan = $step->getBuildPlan(); + + $plan_id = $plan->getID(); + $plan_uri = $this->getApplicationURI("plan/{$plan_id}/"); + + $implementation = $step->getStepImplementation(); + + $field_list = PhabricatorCustomField::getObjectFields( + $step, + PhabricatorCustomField::ROLE_VIEW); + $field_list + ->setViewer($viewer) + ->readFieldsFromStorage($step); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb(pht('Plan %d', $plan_id), $plan_uri); + $crumbs->addTextCrumb(pht('Step %d', $id)); + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Build Step %d: %s', $id, $step->getName())); + + $properties = $this->buildPropertyList($step, $field_list); + $actions = $this->buildActionList($step); + $properties->setActionList($actions); + + $box->addPropertyList($properties); + + $timeline = $this->buildTransactionTimeline( + $step, + new HarbormasterBuildStepTransactionQuery()); + $timeline->setShouldTerminate(true); + + return $this->buildApplicationPage( + array( + $crumbs, + $box, + $timeline, + ), + array( + 'title' => pht('Step %d', $id), + )); + } + + private function buildPropertyList( + HarbormasterBuildStep $step, + PhabricatorCustomFieldList $field_list) { + $viewer = $this->getViewer(); + + $view = id(new PHUIPropertyListView()) + ->setUser($viewer) + ->setObject($step); + + $view->addProperty( + pht('Created'), + phabricator_datetime($step->getDateCreated(), $viewer)); + + $field_list->appendFieldsToPropertyList( + $step, + $viewer, + $view); + + $view->invokeWillRenderEvent(); + + $description = $step->getDescription(); + if (strlen($description)) { + $view->addSectionHeader( + pht('Description'), + PHUIPropertyListView::ICON_SUMMARY); + $view->addTextContent( + new PHUIRemarkupView($viewer, $description)); + } + + return $view; + } + + + private function buildActionList(HarbormasterBuildStep $step) { + $viewer = $this->getViewer(); + $id = $step->getID(); + + $list = id(new PhabricatorActionListView()) + ->setUser($viewer) + ->setObject($step); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $step, + PhabricatorPolicyCapability::CAN_EDIT); + + $list->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Step')) + ->setHref($this->getApplicationURI("step/edit/{$id}/")) + ->setWorkflow(!$can_edit) + ->setDisabled(!$can_edit) + ->setIcon('fa-pencil')); + + $list->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Delete Step')) + ->setHref($this->getApplicationURI("step/delete/{$id}/")) + ->setWorkflow(true) + ->setDisabled(!$can_edit) + ->setIcon('fa-times')); + + return $list; + } + + +} diff --git a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php index e3b8a73ad6..3b45ff108c 100644 --- a/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php @@ -15,7 +15,8 @@ final class HarbormasterBuildPlanEditor $types = parent::getTransactionTypes(); $types[] = HarbormasterBuildPlanTransaction::TYPE_NAME; $types[] = HarbormasterBuildPlanTransaction::TYPE_STATUS; - $types[] = PhabricatorTransactions::TYPE_COMMENT; + $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; + $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } diff --git a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php index bc6a52018c..7110d98d6e 100644 --- a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php +++ b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php @@ -15,6 +15,12 @@ final class HarbormasterManagementBuildWorkflow 'param' => 'id', 'help' => pht('ID of build plan to run.'), ), + array( + 'name' => 'background', + 'help' => pht( + 'Submit builds into the build queue normally instead of '. + 'running them in the foreground.'), + ), array( 'name' => 'buildable', 'wildcard' => true, @@ -88,7 +94,10 @@ final class HarbormasterManagementBuildWorkflow "\n %s\n\n", PhabricatorEnv::getProductionURI('/B'.$buildable->getID())); - PhabricatorWorker::setRunAllTasksInProcess(true); + if (!$args->getArg('background')) { + PhabricatorWorker::setRunAllTasksInProcess(true); + } + $buildable->applyPlan($plan, array()); $console->writeOut("%s\n", pht('Done.')); diff --git a/src/applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php b/src/applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php index 92e1980d8c..c3427fe46f 100644 --- a/src/applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php +++ b/src/applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php @@ -34,7 +34,7 @@ final class HarbormasterBuildStepPHIDType extends PhabricatorPHIDType { $handle ->setName($name) ->setFullName(pht('Build Step %d: %s', $id, $name)) - ->setURI("/harbormaster/step/{$id}/edit/"); + ->setURI("/harbormaster/step/view/{$id}/"); } } diff --git a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php index 1bf5b33a0c..82c041f423 100644 --- a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php @@ -41,7 +41,10 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation $working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation()) ->getType(); - $allowed_phids = $build_target->getFieldValue('repositoryPHIDs'); + $allowed_phids = $build_target->getFieldValue('blueprintPHIDs'); + if (!is_array($allowed_phids)) { + $allowed_phids = array(); + } $authorizing_phid = $build_target->getBuildStep()->getPHID(); $lease = DrydockLease::initializeNewLease() @@ -135,6 +138,9 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation } $also_phids = $build_target->getFieldValue('repositoryPHIDs'); + if (!is_array($also_phids)) { + $also_phids = array(); + } $all_phids = $also_phids; $all_phids[] = $repository_phid; diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php index cf3d9ee7d7..11cb6260b7 100644 --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -12,6 +12,8 @@ final class HarbormasterBuildPlan extends HarbormasterDAO protected $name; protected $planStatus; protected $planAutoKey; + protected $viewPolicy; + protected $editPolicy; const STATUS_ACTIVE = 'active'; const STATUS_DISABLED = 'disabled'; @@ -19,10 +21,22 @@ final class HarbormasterBuildPlan extends HarbormasterDAO private $buildSteps = self::ATTACHABLE; public static function initializeNewBuildPlan(PhabricatorUser $actor) { + $app = id(new PhabricatorApplicationQuery()) + ->setViewer($actor) + ->withClasses(array('PhabricatorHarbormasterApplication')) + ->executeOne(); + + $view_policy = $app->getPolicy( + HarbormasterBuildPlanDefaultViewCapability::CAPABILITY); + $edit_policy = $app->getPolicy( + HarbormasterBuildPlanDefaultEditCapability::CAPABILITY); + return id(new HarbormasterBuildPlan()) ->setName('') ->setPlanStatus(self::STATUS_ACTIVE) - ->attachBuildSteps(array()); + ->attachBuildSteps(array()) + ->setViewPolicy($view_policy) + ->setEditPolicy($edit_policy); } protected function getConfiguration() { @@ -156,16 +170,15 @@ final class HarbormasterBuildPlan extends HarbormasterDAO public function getPolicy($capability) { switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: - return PhabricatorPolicies::getMostOpenPolicy(); + if ($this->isAutoplan()) { + return PhabricatorPolicies::getMostOpenPolicy(); + } + return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: - // NOTE: In practice, this policy is always limited by the "Mangage - // Build Plans" policy. - if ($this->isAutoplan()) { return PhabricatorPolicies::POLICY_NOONE; } - - return PhabricatorPolicies::getMostOpenPolicy(); + return $this->getEditPolicy(); } } diff --git a/src/applications/macro/markup/PhabricatorIconRemarkupRule.php b/src/applications/macro/markup/PhabricatorIconRemarkupRule.php index ce1212b0fc..acd87d4136 100644 --- a/src/applications/macro/markup/PhabricatorIconRemarkupRule.php +++ b/src/applications/macro/markup/PhabricatorIconRemarkupRule.php @@ -41,6 +41,7 @@ final class PhabricatorIconRemarkupRule extends PhutilRemarkupRule { $defaults = array( 'color' => null, + 'spin' => false, ); $options = idx($extra, 1, ''); @@ -70,8 +71,16 @@ final class PhabricatorIconRemarkupRule extends PhutilRemarkupRule { $color = null; } + $classes = array(); + $classes[] = $color; + + $spin = $options['spin']; + if ($spin) { + $classes[] = 'ph-spin'; + } + $icon_view = id(new PHUIIconView()) - ->setIconFont('fa-'.$icon, $color); + ->setIconFont('fa-'.$icon, implode(' ', $classes)); return $this->getEngine()->storeText($icon_view); } diff --git a/src/applications/macro/phid/PhabricatorMacroMacroPHIDType.php b/src/applications/macro/phid/PhabricatorMacroMacroPHIDType.php index 7d2dcfc9ad..19c7f5a1fc 100644 --- a/src/applications/macro/phid/PhabricatorMacroMacroPHIDType.php +++ b/src/applications/macro/phid/PhabricatorMacroMacroPHIDType.php @@ -8,6 +8,10 @@ final class PhabricatorMacroMacroPHIDType extends PhabricatorPHIDType { return pht('Image Macro'); } + public function getPHIDTypeApplicationClass() { + return 'PhabricatorMacroApplication'; + } + public function getTypeIcon() { return 'fa-meh-o'; } diff --git a/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php b/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php deleted file mode 100644 index 68d2499ed6..0000000000 --- a/src/applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php +++ /dev/null @@ -1,40 +0,0 @@ - true, - self::CONFIG_COLUMN_SCHEMA => array( - 'name' => 'text128', - 'email' => 'text128', - 'uri' => 'text255?', - ), - self::CONFIG_KEY_SCHEMA => array( - 'key_phid' => null, - 'phid' => array( - 'columns' => array('phid'), - 'unique' => true, - ), - 'email' => array( - 'columns' => array('email'), - 'unique' => true, - ), - 'name' => array( - 'columns' => array('name'), - 'unique' => true, - ), - ), - ) + parent::getConfiguration(); - } - -} diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 30af5057dd..02e3a2f27d 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -72,11 +72,14 @@ final class PhabricatorPeopleProfileController $href = id(new PhutilURI('/conpherence/new/')) ->setQueryParam('participant', $user->getPHID()); + $can_send = $viewer->isLoggedIn(); + $actions->addAction( id(new PhabricatorActionView()) ->setIcon('fa-comments') ->setName(pht('Send Message')) ->setWorkflow(true) + ->setDisabled(!$can_send) ->setHref($href)); } diff --git a/src/applications/ponder/phid/PonderAnswerPHIDType.php b/src/applications/ponder/phid/PonderAnswerPHIDType.php index 0bb0596737..be148af2f2 100644 --- a/src/applications/ponder/phid/PonderAnswerPHIDType.php +++ b/src/applications/ponder/phid/PonderAnswerPHIDType.php @@ -8,6 +8,10 @@ final class PonderAnswerPHIDType extends PhabricatorPHIDType { return pht('Ponder Answer'); } + public function getPHIDTypeApplicationClass() { + return 'PhabricatorPonderApplication'; + } + public function newObject() { return new PonderAnswer(); } diff --git a/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php b/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php index a4d382e170..7d1b8d799c 100644 --- a/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php +++ b/src/applications/subscriptions/view/SubscriptionListDialogBuilder.php @@ -54,15 +54,14 @@ final class SubscriptionListDialogBuilder extends Phobject { ->setUser($this->getViewer()) ->setWidth(AphrontDialogView::WIDTH_FORM) ->setTitle($this->getTitle()) - ->appendChild($this->buildBody($this->getViewer(), $handles)) + ->setObjectList($this->buildBody($this->getViewer(), $handles)) ->addCancelButton($object_handle->getURI(), pht('Close')); } private function buildBody(PhabricatorUser $viewer, array $handles) { $list = id(new PHUIObjectItemListView()) - ->setUser($viewer) - ->setFlush(true); + ->setUser($viewer); foreach ($handles as $handle) { $item = id(new PHUIObjectItemView()) ->setHeader($handle->getFullName()) diff --git a/src/docs/contributor/bug_reports.diviner b/src/docs/contributor/bug_reports.diviner index 0ad3635d87..46ac466d83 100644 --- a/src/docs/contributor/bug_reports.diviner +++ b/src/docs/contributor/bug_reports.diviner @@ -69,11 +69,12 @@ To update Phabricator, use a script like the one described in @{article:Upgrading Phabricator}. **If you can not update** for some reason, please include the version of -Phabricator you are running in your report. The version is just the Git hash -of your local HEAD. You can find the version by running `git show` in -`phabricator/` and copy/pasting the first line of output, or by browsing to -{nav Config > All Settings} in the web UI and copy/pasting the information -at the top. +Phabricator you are running when you file a report. You can find the version in +{nav Config > Versions} in the web UI. + +(The version is just the Git hash of your local HEAD, so you can also find it +by running `git show` in `phabricator/` and looking at the first line of +output.) Supported Issues diff --git a/src/docs/contributor/feature_requests.diviner b/src/docs/contributor/feature_requests.diviner index a5f8b610c0..a1d2a6640c 100644 --- a/src/docs/contributor/feature_requests.diviner +++ b/src/docs/contributor/feature_requests.diviner @@ -114,7 +114,8 @@ We have a lot of users and a small team. Even if your feature is something we're interested in and a good fit for where we want the product to go, it may take us a long time to get around to building it. -We work full time on Phabricator, and our long-term roadmap has many years worth +We work full time on Phabricator, and our long-term roadmap (which we call our +[[ https://secure.phabricator.com/w/starmap/ | Starmap ]]) has many years worth of work. Your feature request is competing against thousands of other requests for priority. @@ -127,7 +128,8 @@ Even if your feature request is simple and has substantial impact for a large number of users, the size of the request queue means that it is mathematically unlikely to be near the top. -You can find some information about how we prioritize in T4778. In particular, +You can find some information about how we prioritize in +[[ https://secure.phabricator.com/w/planning/ | Planning ]]. In particular, we reprioritize frequently and can not accurately predict when we'll build a feature which isn't very near to top of the queue. @@ -137,9 +139,9 @@ give you any updates or predictions about timelines. One day, out of nowhere, your feature will materialize. That day may be a decade from now. You should have realistic expectations about this when filing a feature request. -If you want a concrete timeline, you can build the feature yourself. See -@{article:Contributing Code} for details and alternatives to working with the -upstream. +If you want a concrete timeline, you can work with us to pay for some control +over our roadmap. For details, see +[[ https://secure.phabricator.com/w/prioritization/ | Prioritization ]]. Describe Problems diff --git a/src/docs/user/userguide/differential.diviner b/src/docs/user/userguide/differential.diviner index edf992f720..414ad87a4f 100644 --- a/src/docs/user/userguide/differential.diviner +++ b/src/docs/user/userguide/differential.diviner @@ -50,19 +50,24 @@ just some intern who you don't trust) you can write a Herald rule to automatically CC you on any revisions which match rules (like content, author, files affected, etc.) -= Differential Tips = +Inline Comments +=============== - - You can leave inline comments by clicking the line numbers in the diff. - - You can leave a comment across multiple lines by dragging across the line - numbers. - - Inline comments are initially saved as drafts. They are not submitted until - you submit a comment at the bottom of the page. - - Press "?" to view keyboard shortcuts. +You can leave inline comments by clicking the line number next to a line. For +an in-depth look at inline comments, see +@{article:Differential User Guide: Inline Comments}. -= Next Steps = - - Read the FAQ at @{article:Differential User Guide: FAQ}; or - - learn about handling large changesets at +Next Steps +========== + +Continue by: + + - diving into the details of inline comments in + @{article:Differential User Guide: Inline Comments}; or + - reading the FAQ at @{article:Differential User Guide: FAQ}; or + - learning about handling large changesets in @{article:Differential User Guide: Large Changes}; or - - learn about test plans at @{article:Differential User Guide: Test Plans}; or - - learn more about Herald at @{article:Herald User Guide}. + - learning about test plans in + @{article:Differential User Guide: Test Plans}; or + - learning more about Herald in @{article:Herald User Guide}. diff --git a/src/docs/user/userguide/differential_inlines.diviner b/src/docs/user/userguide/differential_inlines.diviner new file mode 100644 index 0000000000..80ac9891ed --- /dev/null +++ b/src/docs/user/userguide/differential_inlines.diviner @@ -0,0 +1,168 @@ +@title Differential User Guide: Inline Comments +@group userguide + +Guide to inline comments in Differential. + +Overview +======== + +Differential allows reviewers to leave feedback about changes to code inline, +within the body of the diff itself. These comments are called "inline +comments", and can be used to discuss specific parts of a change. + +(NOTE) Click the line number next to a line to leave an inline comment. + +To leave an inline comment, click the line number next to a line when reviewing +a change in Differential. You can also leave a comment on a range of adjacent +lines by clicking one line number and dragging to a nearby line number. + +(NOTE) Other users can't see your comments right away! + +When you make a comment, it is initially an **unsubmitted draft**, indicated by +an "Unsubmitted" badge in the comment header. Other users can't see it yet. +This behavior is different from the behavior of other software you may be +familiar with. + +To publish your inline comments, scroll to the bottom of the page and submit +the form there. All your unsubmitted inline comments will be published when you +do, alongside an optional normal comment and optional action (like accepting +the revision). + +Differential doesn't publish inlines initially because having a draft phase +gives reviewers more time to revise and adjust the inlines, and make their +feedback more cohesive, and contextualize their inlines with discussion in a +normal comment. It also allows Differential to send fewer, more relevant emails +and notifications. + + +Porting / Ghost Comments +======================== + +When a revision is updated, we attempt to port inline comments forward and +display them on the new diff. Ported comments have a pale, ghostly appearance +and include a button which allows you to jump back to the original context +where the comment appeared. + +Ported comments sometimes appear in unexpected locations. There are two major +reasons for this: + + - In the general case, it is not possible to always port comments to the same + lines humans would automatically. + - We are very aggressive about porting comments forward, even in the presence + of heavy changes. This helps prevent mistakes and makes it easier to verify + feedback has been addressed. + +You can disable this behavior in +{nav Settings > Diff Preferences > Show Older Inlines} if you prefer comments +stay anchored to their original diff. + +To understand why porting comments forward is difficult and can produce +unexpected results, and why we choose to be aggressive about it, let's look at +a case where the right behavior is ambiguous. Imagine this code is added as +part of a change: + +```name=important.c +111 ... +112 +113 if (a() || b() || c()) { +114 return; +115 } +116 +117 ... +``` + +Suppose a reviewer leaves this comment on line 113: + +> important.c:113 This is a serious security vulnerability! + +The author later updates the diff, and the code now looks like this, with some +other changes elsewhere so the line numbers have also shifted: + +```name=important.c +140 ... +141 +142 if (a()) { +143 return; +144 } +145 +146 if (b()) { +147 return; +148 } +149 +150 ... +``` + +If we port the inline forward from the first change to the second change, where +should it go? A human would probably do one of three things: + + # Put it on line 142, with the call to `a()`. + # Put it on line 146, with the call to `b()`. + # Don't bring it forward at all. + +A human would choose between (1) and (2) based on context about what `a()` and +`b()` are and what the reviewer meant. The algorithm can not possibly read the +comment and understand which part of the statement it talked about. Humans +might not even agree on which line is the better fit. + +When we choose one of these behaviors, humans will sometimes think the other +behavior was the better one, because they have more information about what +`a()` and `b()` are and what the comment actually meant. The line we choose may +be unexpected, but it is the best the algorithm can do without being able to +actually read the code or understand what the comment means. + +A human might also choose not to bring the comment forward if they knew that +removing `c()` addressed it, but the algorithm can not know that. The call to +`a()` or `b()` may be the dangerous thing the reviewer was talking about, and +we err on the side of caution by bringing comments forward aggressively. + +When a line of code with an inline comment on it changes, we can not know if +the change addressed the inline or not. We take the cautious route and +//always// assume it did not, and that humans need to verify problems have +really been addressed. + +This means that inlines are sometimes ported forward into places that don't +make sense (the code has changed completely), but we won't drop important +inlines just because the structure of the code has changed. + +Here's another case where bringing inlines forward seems desirable. Imagine +this code is added as part of a change: + +```name=warpgate.c +12 ... +13 function_x(); +14 ... +``` + +Suppose a reviewer leaves this comment on line 13: + +> warpgate.c:13 This should be function_y() instead. + +The author later updates the diff, and the code now looks like this: + +```name=warpgate.c +12 ... +13 function_y(); +14 ... +``` + +For the reasons discussed above, we port the comment forward, so it will appear +under line 13. + +We think this is desirable: it makes it trivial for an author or reviewer to +look through the changes and verify that the feedback has really been +addressed, because you can see that the code now uses the proper function. + +This isn't to say that we always do the best we can. There may be cases where +the algorithm can be smarter than it currently is or otherwise produce a better +result. If you find such a case, file a bug report. But it's expected that the +algorithm will sometimes port comments into places that aren't optimal or no +longer make sense, because this is frequently the best behavior available among +the possible alternatives. + + +Next Steps +========== + +Continue by: + + - returning to the @{article:Differential User Guide}. diff --git a/src/docs/user/userguide/differential_land.diviner b/src/docs/user/userguide/differential_land.diviner new file mode 100644 index 0000000000..8d5f30d784 --- /dev/null +++ b/src/docs/user/userguide/differential_land.diviner @@ -0,0 +1,53 @@ +@title Differential User Guide: Automated Landing +@group userguide + +Configuring Phabricator so you can "Land Revision" from the web UI. + + +Overview +======== + +IMPORTANT: This feature is a prototype and has substantial limitations. + +Phabricator can be configured so that approved revisions may be published +directly from the web interface. This can make publishing changes more +convenient, particularly for open source projects where authors may not have +commit access to the repository. This document explains the workflow and how to +configure it. + +When properly configured, a {nav Land Revision} action will appear in +Differential. This action works like `arc land` on the command line, and +merges and publishes the revision. + +This feature has significant limitations: + + - This feature is a prototype. + - This feature is only supported in Git. + - This feature always lands changes onto `master`. + - This feature does not currently provide chain of custody, and what lands + may be arbitrarily different than what is shown in Differential. + +To be landable, a revision must satisfy these requirements: + + - It must belong to a repository which is tracked in Diffusion + (both hosted and imported repositories will work). + - The repository must have a **Staging Area** configured. + - The repository must have **Repository Automation** configured. For + details, see @{article:Drydock User Guide: Repository Automation}. + - The revision must have been created with `arc diff` and pushed to the + configured staging area at creation time. + - The user clicking the "Land Revision" button must have permission to push + to the repository. + +If these requirements are met, the {nav Land Revision} action should be +available in the UI. + + +Next Steps +========== + +Continue by: + + - configuring repository automation with + @{article:Drydock User Guide: Repository Automation}; or + - returning to the @{article:Differential User Guide}. diff --git a/src/docs/user/userguide/drydock.diviner b/src/docs/user/userguide/drydock.diviner index 49b1f1df9e..44a645bf8a 100644 --- a/src/docs/user/userguide/drydock.diviner +++ b/src/docs/user/userguide/drydock.diviner @@ -15,16 +15,19 @@ applications coordinate during complex build and deployment tasks. Typically, you will configure Drydock to enable capabilities in other applications: - Harbormaster can use Drydock to host builds. - - In the future, Differential will be able to use Drydock to perform - server-side merges. + - Differential can use Drydock to perform server-side merges. Users will not normally interact with Drydock directly. +If you want to get started with Drydock right away, see +@{article:Drydock User Guide: Quick Start} for specific instructions on +configuring integrations. + What Drydock Does ================= -Drydock manages working copies, build hosts, and other software and hardware +Drydock manages working copies, hosts, and other software and hardware resources that build and deployment processes may require in order to perform useful work. @@ -49,15 +52,202 @@ doing the actual work. Drydock solves these scaling problems by providing a central allocation framework for //resources//, which are physical or virtual resources like a -build host or a working copy. Processes which need to share hardware or -software can use Drydock to coordinate creation, access, and destruction of -those resources. +host or a working copy. Processes which need to share hardware or software can +use Drydock to coordinate creation, access, and destruction of those resources. Applications ask Drydock for resources matching a description, and it allocates a corresponding resource by either finding a suitable unused resource or creating a new resource. When work completes, the resource is returned to the resource pool or destroyed. + +Getting Started with Drydock +============================ + +In general, you will interact with Drydock by configuring blueprints, which +tell Drydock how to build resources. You can jump into this topic directly +in @{article:Drydock Blueprints}. + +For help on configuring specific application features: + + - to configure server-side merges from Differential, see + @{article:Differential User Guide: Automated Landing}. + +You should also understand the Drydock security model before deploying it +in a production environment. See @{article:Drydock User Guide: Security}. + +The remainder of this document has some additional high-level discussion about +how Drydock works and why it works that way, which may be helpful in +understanding the application as a whole. + + +Drydock Concepts +================ + +The major concepts in Drydock are **Blueprints**, **Resources**, **Leases**, +and the **Allocator**. + +**Blueprints** are configuration that tells Drydock how to create resources: +where it can put them, how to access them, how many it can make at once, who is +allowed to ask for access to them, how to actually build them, how to clean +them up when they are no longer in use, and so on. + +Drydock starts without any blueprints. You'll add blueprints to configure +Drydock and enable it to satisfy requests for resources. You can learn more +about blueprints in @{article:Drydock Blueprints}. + +**Resources** represent things (like hosts or working copies) that Drydock has +created, is managing the lifecycle for, and can give other applications access +to. + +**Leases** are requests for resources with certain qualities by other +applications. For example, Harbormaster may request a working copy of a +particular repository so it can run unit tests. + +The **Allocator** is where Drydock actually does work. It works roughly like +this: + + - An application creates a lease describing a resource it needs, and + uses this lease to ask Drydock for an appropriate resource. + - Drydock looks at free resources to try to find one it can use to satisfy + the request. If it finds one, it marks the resource as in use and gives + the application details about how to access it. + - If it can't find an appropriate resource that already exists, it looks at + the blueprints it has configured to try to build one. If it can, it creates + a new resource, then gives the application access to it. + - Once the application finishes using the resource, it frees it. Depending + on configuration, Drydock may reuse it, destroy it, or hold onto it and + make a decision later. + +Some minor concepts in Drydock are **Slot Locks** and **Repository Operations**. + +**Slot Locks** are simple optimistic locks that most Drydock blueprints use to +avoid race conditions. Their design is not particularly interesting or novel, +they're just a fairly good fit for most of the locking problems that Drydock +blueprints tend to encounter and Drydock provides APIs to make them easy to +work with. + +**Repository Operations** help other applications coordinate writes to +repositories. Multiple applications perform similar kinds of writes, and these +writes require more sequencing/coordination and user feedback than other +operations. + + +Architecture Overview +===================== + +This section describes some of Drydock's design goals and architectural +choices, so you can understand its strengths and weaknesses and which problem +domains it is well or poorly suited for. + +A typical use case for Drydock is giving another application access to a +working copy in order to run a build or unit test operation. Drydock can +satisfy the request and resume execution of application code in 1-2 seconds +under reasonable conditions and with moderate tradeoffs, and can satisfy a +large number of these requests in parallel. + +**Scalable**: Drydock is designed to scale easily to something in the realm of +thousands of hosts in hundreds of pools, and far beyond that with a little +work. + +Drydock is intended to solve resource management problems at very large scales +and minimzes blocking operations, locks, and artificial sequencing. Drydock is +designed to fully utilize an almost arbitrarily large pool of resources and +improve performance roughly linearly with available hardware. + +Because the application assumes that deployment at this scale and complexity +level is typical, you may need to configure more things and do more work than +you would under the simplifying assumptions of small scale. + +**Heavy Resources**: Drydock assumes that resources are relatively +heavyweight and and require a meaningful amount (a second or more) of work to +build, maintain and tear down. It also assumes that leases will often have +substantial lifespans (seconds or minutes) while performing operations. + +Resources like working copies (which typically take several seconds to create +with a command like `git clone`) and VMs (which typically take several seconds +to spin up) are good fits for Drydock and for the problems it is intended to +solve. + +Lease operations like running unit tests, performing builds, executing merges, +generating documentation and running temporary services (which typically last +at least a few seconds) are also good fits for Drydock. + +In both cases, the general concern with lightweight resources and operations is +that Drydock operation overhead is roughly on the order of a second for many +tasks, so overhead from Drydock will be substantial if resources are built and +torn down in a few milliseconds or lease operations require only a fraction of +a second to execute. + +As a rule of thumb, Drydock may be a poor fit for a problem if operations +typically take less than a second to build, execute, and destroy. + +**Focus on Resource Construction**: Drydock is primarily solving a resource +construction problem: something needs a resource matching some description, so +Drydock finds or builds that resource as quickly as possible. + +Drydock generally prioritizes responding to requests quickly over other +concerns, like minimizing waste or performing complex scheduling. Although you +can make adjustments to some of these behaviors, it generally assumes that +resources are cheap compared to the cost of waiting for resource construction. + +This isn't to say that Drydock is grossly wasteful or has a terrible scheduler, +just that efficient utilization and efficient scheduling aren't the primary +problems the design focuses on. + +This prioritization corresponds to scenarios where resources are something like +hosts or working copies, and operations are something like builds, and the cost +of hosts and storage is small compared to the cost of engineer time spent +waiting on jobs to get scheduled. + +Drydock may be a weak fit for a problem if it is bounded by resource +availability and using resources as efficiently as possible is very important. +Drydock generally assumes you will respond to a resource deficit by making more +resources available (usually very cheap), rather than by paying engineers to +wait for operations to complete (usually very expensive). + +**Isolation Tradeoffs**: Drydock assumes that multiple operations running at +similar levels of trust may be interested in reducing isolation to improve +performance, reduce complexity, or satisfy some other similar goal. It does not +guarantee isolation and assumes most operations will not run in total isolation. + +If this isn't true for your use case, you'll need to be careful in configuring +Drydock to make sure that operations are fully isolated and can not interact. +Complete isolation will reduce the performance of the allocator as it will +generally prevent it from reusing resources, which is one of the major ways it +can improve performance. + +You can find more discussion of these tradeoffs in +@{article:Drydock User Guide: Security}. + +**Agentless**: Drydock does not require an agent or daemon to be installed on +hosts. It interacts with hosts over SSH. + +**Very Abstract**: Drydock's design is //extremely// abstract. Resources have +very little hardcoded behavior. The allocator has essentially zero specialized +knowledge about what it is actually doing. + +One aspect of this abstractness is that Drydock is composable, and solves +complex allocation problems by //asking itself// to build the pieces it needs. +To build a working copy, Drydock first asks itself for a suitable host. It +solves this allocation sub-problem, then resolves the original request. + +This allows new types of resources to build on Drydock's existing knowledge of +resource construction by just saying "build one of these other things you +already know how to build, then apply a few adjustments". This also means that +you can tell Drydock about a new way to build hosts (say, bring up VMs from a +different service provider) and the rest of the pipeline can use these new +hosts interchangeably with the old hosts. + +While this design theoretically makes Drydock more powerful and more flexible +than a less abstract approach, abstraction is frequently a double-edged sword. + +Drydock is almost certainly at the extreme upper end of abstraction for tools +in this space, and the level of abstraction may ultimately match poorly with a +particular problem domain. Alternative approaches may give you more specialized +and useful tools for approaching a given problem. + + Next Steps ========== @@ -65,5 +255,6 @@ Continue by: - understanding Drydock security concerns with @{article:Drydock User Guide: Security}; or + - learning about blueprints in @{article:Drydock Blueprints}; or - allowing Phabricator to write to repositories with @{article:Drydock User Guide: Repository Automation}. diff --git a/src/docs/user/userguide/drydock_blueprints.diviner b/src/docs/user/userguide/drydock_blueprints.diviner new file mode 100644 index 0000000000..46c8a27f5e --- /dev/null +++ b/src/docs/user/userguide/drydock_blueprints.diviner @@ -0,0 +1,80 @@ +@title Drydock Blueprints +@group userguide + +Overview of Drydock blueprint types. + + +Overview +======== + +IMPORTANT: Drydock is not a mature application and may be difficult to +configure and use for now. + +Drydock builds and manages various hardware and software resources, like +hosts and repository working copies. Other applications can use these resources +to perform useful work (like running tests or builds). + +For additional disussion of Drydock, see @{article:Drydock User Guide}. + +Drydock can't create any resources until you configure it. You'll configure +Drydock by creating **Blueprints**. Each blueprint tells Drydock how to build +a specific kind of resource, how many it is allowed to build, where it should +build them, who is authorized to request them, and so on. + +You can create a new blueprint in Drydock from the web UI: + +{nav Drydock > Blueprints > New Blueprint} + +Each blueprint builds resources of a specific type, like hosts or repository +working copies. Detailed topic guides are available for each resource type: + +**Hosts**: Hosts are the building block for most other resources. For details, +see @{article:Drydock Blueprints: Hosts}. + +**Working Copies**: Working copies allow Drydock to perform repository +operations like running tests, performing builds, and handling merges. + + +Authorizing Access +================== + +Before objects in other applications can use a blueprint, the blueprint must +authorize them. + +This mostly serves to prevent users with limited access from executing +operations on trusted hosts. For additional discussion, see +@{article:Drydock User Guide: Security}. + +This also broadly prevents Drydock from surprising you by coming up with a +valid but unintended solution to an allocation problem which runs some +operation on resources that are techincally suitable but not desirable. For +example, you may not want your Android builds running on your iPhone build +tier, even if there's no technical reason they can't. + +You can review active authorizations and pending authorization requests in +the "Active Authorizations" section of the blueprint detail screen. + +To approve an authorization, click it and select {nav Approve Authorization}. +Until you do, the requesting object won't be able to access resources from +the blueprint. + +You can also decline an authorization. This prevents use of resources and +removes it from the authorization approval queue. + + +Disabling Blueprints +==================== + +You can disable a blueprint by selecting {nav Disable Blueprint} from the +blueprint detail screen. + +Disabled blueprints will no longer be used for new allocations. However, +existing resources will continue to function. + + +Next Steps +========== + +Continue by: + + - returning to the @{article:Drydock User Guide}. diff --git a/src/docs/user/userguide/drydock_hosts.diviner b/src/docs/user/userguide/drydock_hosts.diviner new file mode 100644 index 0000000000..8bfed7dc60 --- /dev/null +++ b/src/docs/user/userguide/drydock_hosts.diviner @@ -0,0 +1,126 @@ +@title Drydock Blueprints: Hosts +@group userguide + +Guide to configuring Drydock host blueprints. + + +Overview +======== + +IMPORTANT: Drydock is not a mature application and may be difficult to +configure and use for now. + +To give Drydock access to machines so it can perform work, you'll configure +**host blueprints**. These blueprints tell Drydock where to find machines (or +how to build machines) and how to connect to them. + +Once Drydock has access to hosts it can use them to build more interesting and +complex types of resources, like repository working copies. + +Drydock currently supports these kinds of host blueprints: + + - **Almanac Hosts**: Gives Drydock access to a predefined list of hosts. + +Drydock may support additional blueprints in the future. + + +Security +======== + +Drydock can be used to run semi-trusted and untrusted code, and you may want +to isolate specific processes or classes of processes from one another. See +@{article:Drydock User Guide: Security} for discussion of security +concerns and guidance on how to make isolation tradeoffs. + + +General Considerations +====================== + +**You must install software on hosts.** Drydock does not currently handle +installing software on hosts. You'll need to make sure any hosts are configured +properly with any software you need, and have tools like `git`, `hg` or `svn` +that may be required to interact with working copies. + +You do **not** need to install PHP, arcanist, libphutil or Phabricator on the +hosts unless you are specifically running `arc` commands. + +**You must configure authentication.** Drydock also does not handle credentials +for VCS operations. If you're interacting with repositories hosted on +Phabricator, the simplest way to set this up is something like this: + + - Create a new bot user in Phabricator. + - In {nav Settings > SSH Public Keys}, add a public key or generate a + keypair. + - Put the private key on your build hosts as `~/.ssh/id_rsa` for whatever + user you're connecting with. + +This will let processes on the host access Phabricator as the bot user, and +use the bot user's permissions to pull and push changes. + +If you're using hosted repositories from an external service, you can follow +similar steps for that service. + +Note that any processes running under the given user account will have access +to the private key, so you should give the bot the smallest acceptable level of +permissions if you're running semi-trusted or untrusted code like unit tests. + +**You must create a `/var/drydock` directory.** This is hard-coded in Drydock +for now, so you need to create it on the hosts. This can be a symlink to +a different location if you prefer. + + +Almanac Hosts +============= + +The **Almanac Hosts** blueprint type gives Drydock access to a predefined list +of hosts which you configure in the Almanac application. This is the simplest +type of blueprint to set up. + +For more information about Almanac, see @{article:Almanac User Guide}. + +For example, suppose you have `build001.mycompany.com` and +`build002.mycompany.com`, and want to configure Drydock to be able to use these +hosts. To do this: + +**Create Almanac Devices**: Create a device record in Almanac for each your +hosts. + +{nav Almanac > Devices > Create Device} + +Enter the device names (like `build001.mycompany.com`). After creating the +devices, use {nav Add Interface} to configure the ports and IP addresses that +Drydock should connect to over SSH (normally, this is port `22`). + +**Create an Almanac Service**: In the Almanac application, create a new service +to define the pool of devices you want to use. + +{nav Almanac > Services > Create Service} + +Choose the service type **Drydock: Resource Pool**. This will allow Drydock +to use the devices that are bound to the service. + +Now, use {nav Add Binding} to bind all of the devices to the service. + +You can add more hosts to the pool later by binding additional devices, and +Drydock will automatically start using them. Likewise, you can remove bindings +to take hosts out of service. + +**Create a Drydock Blueprint**: Now, create a new blueprint in Drydock. + +{nav Drydock > Blueprints > New Blueprint} + +Choose the **Almanac Hosts** blueprint type. + +In **Almanac Services**, select the service you previously created. For +**Credentials**, select an SSH private key you want Drydock to use to connect +to the hosts. + +Drydock should now be able to build resources from these hosts. + + +Next Steps +========== + +Continue by: + + - returning to @{article:Drydock Blueprints}. diff --git a/src/docs/user/userguide/drydock_quick_start.diviner b/src/docs/user/userguide/drydock_quick_start.diviner new file mode 100644 index 0000000000..4b0bd7110d --- /dev/null +++ b/src/docs/user/userguide/drydock_quick_start.diviner @@ -0,0 +1,74 @@ +@title Drydock User Guide: Quick Start +@group userguide + +Guide to getting Drydock + +Quick Start: Land Revisions +=========================== + +Quick start guide to getting "Land Revision" working in Differential. For +a more detailed guide, see @{article:Drydock User Guide: Repository Automation}. + +Choose a repository you want to enable "Land Revision" for. We'll call this +**Repository X**. + +You need to configure a staging area for this repository if you haven't +already. You can do this in Diffusion in {nav Edit Repository > Edit Staging}. +We'll call this **Staging Area Y**. + +Choose or create a host you want to run merges on. We'll call this +`automation001`. For example, you might bring up a new host in EC2 and +label it `automation001.mycompany.com`. You can use an existing host if you +prefer. + +Create a user account on the host, or choose an existing user account. This is +the user that merges will execute under: Drydock will connect to it and run a +bunch of `git` commands, then ultimately run `git push`. We'll call this user +`builder`. + +Install `git`, `hg` or `svn` if you haven't already and set up private keys +for `builder` so it can pull and push any repositories you want to operate +on. + +If your repository and/or staging area are hosted in Phabricator, you may want +to create a corresponding bot account so you can add keys and give it +permissions. + +At this point you should be able to `ssh builder@automation001` to connect to +the host, and get a normal shell. You should be able to `git clone ...` from +**Repository X** and from **Staging Area Y**, and `git push` to **Repository +X**. If you can't, configure things so you can. + +Now, create a host blueprint for the host. You can find a more detailed +walkthrough in @{article:Drydock Blueprints: Hosts}. Briefly: + + - Create an Almanac device for the host. This should have the IP address and + port for your host. + - Create an Almanac service bound to the device. This should be a Drydock + resource pool service and have a binding to the IP from the previous step. + - Create a Drydock host blueprint which uses the service from the previous + step. It should be configured with an SSH private key that can be used + to connect to `builder@automation001`. + +Then, create a new working copy blueprint which uses the host blueprint you +just made. You can find a more detailed walkthrough in @{article:Drydock +Blueprints: Working Copies}. Authorize the working copy blueprint to use the +host blueprint. + +Finally, configure repository automation for **Repository X**: +{nav Edit Repository > Edit Automation}. Provide the working copy blueprint +from the previous step. Authorize the repository to use the working copy +blueprint. + +After you save changes, click {nav Test Configuration} to test that things +are working properly. + +The "Land Revision" action should now be available on revisions for this +repository. + +Next Steps +========== + +Continue by: + + - returning to @{article:Drydock User Guide}. diff --git a/src/docs/user/userguide/drydock_repository_automation.diviner b/src/docs/user/userguide/drydock_repository_automation.diviner index a16e8d33e9..35ef932cc2 100644 --- a/src/docs/user/userguide/drydock_repository_automation.diviner +++ b/src/docs/user/userguide/drydock_repository_automation.diviner @@ -7,12 +7,19 @@ Configuring repository automation so Phabricator can push commits. Overview ======== -IMPORTANT: This feature is very new and most of the capabilities described +IMPORTANT: This feature is very new and some of the capabilities described in this document are not yet available. This feature as a whole is a prototype. By configuring Drydock and Diffusion appropriately, you can enable **Repository -Automation** for a repository. Once automation is set up, Phabricator will be -able to make changes to the repository. +Automation** for a repository. This will allow Phabricator to make changes +to the repository. + + +Limitations +=========== + + - This feature is a prototype. + - Only Git is supported. Security @@ -29,6 +36,45 @@ with automation. You can read more about this in @{article:Drydock User Guide: Security}. +Configuring Automation +====================== + +To configure automation, use {nav Edit Repository > Edit Automation} from +Diffusion. + +On the configuration screen, specify one or more working copy blueprints in +Drydock (usually, you'll just use one). Repository automation will use working +copies built by these blueprints to perform merges and push changes. + +For more details on configuring these blueprints, see +@{article:Drydock Blueprints: Working Copies}. + +After selecting one or more blueprints, make sure you authorize the repository +to use them. Automation operations won't be able to proceed until you do. The +UI will remind you if you have unauthorized blueprints selected. + + +Testing Configuration +===================== + +Once the blueprints are configured and authorized, use {nav Test Configuration} +to check that things are configured correctly. This will build a working copy +in Drydock, connect to it, and run a trivial command (like `git status`) to +make sure things work. + +If it's the first time you're doing this, it may take a few moments since it +will need to clone a fresh working copy. + +If the test is successful, your configuration is generally in good shape. If +not, it should give you more details about what went wrong. + +Since the test doesn't actually do a push, it's possible that you may have +everything configured properly //except// write access. In this case, you'll +run into a permission error when you try to actually perform a merge or other +similar write. If you do, adjust permissions or credentials appropriately so +the working copy can be pushed from. + + Next Steps ========== diff --git a/src/docs/user/userguide/drydock_working_copies.diviner b/src/docs/user/userguide/drydock_working_copies.diviner new file mode 100644 index 0000000000..2e8eb293c7 --- /dev/null +++ b/src/docs/user/userguide/drydock_working_copies.diviner @@ -0,0 +1,39 @@ +@title Drydock Blueprints: Working Copies +@group userguide + +Guide to configuring Drydock working copy blueprints. + + +Overview +======== + +IMPORTANT: Drydock is not a mature application and may be difficult to +configure and use for now. + +To let Drydock build repository working copies in order to run unit tests and +other similar operations, you'll configure **working copy blueprints**. + +Working Copies +============== + +Working copy blueprints rely on host blueprints, so you'll need to configure +a suitable host blueprint first. See @{article:Drydock Blueprints: Hosts}. + +To configure a working copy blueprint, choose the host blueprints it should +use in **Use Blueprints**. + +You can optionally specify a **Limit**. If you do, the blueprint won't be +allowed to create more than this many simultaneous resources. If you leave +it empty, the blueprint will be able to create an unlimited number of +resources. + +After you save the blueprint, make sure you authorize it to use the selected +host blueprints. It won't be able to acquire host resources until you do. + + +Next Steps +========== + +Continue by: + + - returning to @{article:Drydock Blueprints}. diff --git a/src/docs/user/userguide/remarkup.diviner b/src/docs/user/userguide/remarkup.diviner index ef38552489..ca20751c47 100644 --- a/src/docs/user/userguide/remarkup.diviner +++ b/src/docs/user/userguide/remarkup.diviner @@ -496,6 +496,12 @@ For a list of available icons and colors, check the UIExamples application. [[ http://fortawesome.github.io/Font-Awesome/ | FontAwesome ]], so you can also browse the collection there.) +You can add `spin` to make the icon spin: + + {icon cog spin} + +This renders: {icon cog spin} + = Phriction Documents = diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index a3c1833468..5d3dc359d0 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1420,6 +1420,11 @@ final class PhabricatorUSEnglishTranslation ), ), + 'WARNING: There are %s unapproved authorization(s)!' => array( + 'WARNING: There is an unapproved authorization!', + 'WARNING: There are unapproved authorizations!', + ), + ); } diff --git a/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php b/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php index 0d8fc79bb0..76e8e0a057 100644 --- a/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php +++ b/src/infrastructure/storage/management/PhabricatorStorageManagementAPI.php @@ -17,6 +17,8 @@ final class PhabricatorStorageManagementAPI extends Phobject { const COLLATE_SORT = 'COLLATE_SORT'; const COLLATE_FULLTEXT = 'COLLATE_FULLTEXT'; + const TABLE_STATUS = 'patch_status'; + public function setDisableUTF8MB4($disable_utf8_mb4) { $this->disableUTF8MB4 = $disable_utf8_mb4; return $this; @@ -118,13 +120,26 @@ final class PhabricatorStorageManagementAPI extends Phobject { try { $applied = queryfx_all( $this->getConn('meta_data'), - 'SELECT patch FROM patch_status'); + 'SELECT patch FROM %T', + self::TABLE_STATUS); return ipull($applied, 'patch'); } catch (AphrontQueryException $ex) { return null; } } + public function getPatchDurations() { + try { + $rows = queryfx_all( + $this->getConn('meta_data'), + 'SELECT patch, duration FROM %T WHERE duration IS NOT NULL', + self::TABLE_STATUS); + return ipull($rows, 'duration', 'patch'); + } catch (AphrontQueryException $ex) { + return array(); + } + } + public function createDatabase($fragment) { $info = $this->getCharsetInfo(); @@ -168,13 +183,30 @@ final class PhabricatorStorageManagementAPI extends Phobject { return $legacy; } - public function markPatchApplied($patch) { + public function markPatchApplied($patch, $duration = null) { + $conn = $this->getConn('meta_data'); + queryfx( - $this->getConn('meta_data'), + $conn, 'INSERT INTO %T (patch, applied) VALUES (%s, %d)', - 'patch_status', + self::TABLE_STATUS, $patch, time()); + + // We didn't add this column for a long time, so it may not exist yet. + if ($duration !== null) { + try { + queryfx( + $conn, + 'UPDATE %T SET duration = %d WHERE patch = %s', + self::TABLE_STATUS, + (int)floor($duration * 1000000), + $patch); + } catch (AphrontQueryException $ex) { + // Just ignore this, as it almost certainly indicates that we just + // don't have the column yet. + } + } } public function applyPatch(PhabricatorStoragePatch $patch) { diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php index ebff080bd4..774ae8445c 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php @@ -29,15 +29,26 @@ final class PhabricatorStorageManagementStatusWorkflow ->setShowHeader(false) ->addColumn('id', array('title' => pht('ID'))) ->addColumn('status', array('title' => pht('Status'))) + ->addColumn('duration', array('title' => pht('Duration'))) ->addColumn('type', array('title' => pht('Type'))) ->addColumn('name', array('title' => pht('Name'))); + $durations = $api->getPatchDurations(); + foreach ($patches as $patch) { + $duration = idx($durations, $patch->getFullKey()); + if ($duration === null) { + $duration = '-'; + } else { + $duration = pht('%s us', new PhutilNumber($duration)); + } + $table->addRow(array( 'id' => $patch->getFullKey(), 'status' => in_array($patch->getFullKey(), $applied) ? pht('Applied') : pht('Not Applied'), + 'duration' => $duration, 'type' => $patch->getType(), 'name' => $patch->getName(), )); diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php index 55cf3b223e..d5a4f41cad 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php @@ -187,9 +187,13 @@ final class PhabricatorStorageManagementUpgradeWorkflow echo pht("DRYRUN: Would apply patch '%s'.", $key)."\n"; } else { echo pht("Applying patch '%s'...", $key)."\n"; + + $t_begin = microtime(true); $api->applyPatch($patch); + $t_end = microtime(true); + if (!$skip_mark) { - $api->markPatchApplied($key); + $api->markPatchApplied($key, ($t_end - $t_begin)); } } diff --git a/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php b/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php index 886df14e2c..df48ce3812 100644 --- a/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php +++ b/src/infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php @@ -10,6 +10,7 @@ final class PhabricatorStorageSchemaSpec array( 'patch' => 'text128', 'applied' => 'uint32', + 'duration' => 'uint64?', ), array( 'PRIMARY' => array( diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php index a336e990a2..f8bc3fa060 100644 --- a/src/view/AphrontDialogView.php +++ b/src/view/AphrontDialogView.php @@ -23,6 +23,7 @@ final class AphrontDialogView private $errors = array(); private $flush; private $validationException; + private $objectList; const WIDTH_DEFAULT = 'default'; @@ -132,6 +133,13 @@ final class AphrontDialogView return $this; } + public function setObjectList(PHUIObjectItemListView $list) { + $this->objectList = true; + $box = id(new PHUIObjectBoxView()) + ->setObjectList($list); + return $this->appendChild($box); + } + public function appendParagraph($paragraph) { return $this->appendChild( phutil_tag( @@ -236,15 +244,17 @@ final class AphrontDialogView __CLASS__)); } - $more = $this->class; + $classes = array(); + $classes[] = 'aphront-dialog-view'; + $classes[] = $this->class; if ($this->flush) { - $more .= ' aphront-dialog-flush'; + $classes[] = 'aphront-dialog-flush'; } switch ($this->width) { case self::WIDTH_FORM: case self::WIDTH_FULL: - $more .= ' aphront-dialog-view-width-'.$this->width; + $classes[] = 'aphront-dialog-view-width-'.$this->width; break; case self::WIDTH_DEFAULT: break; @@ -256,11 +266,15 @@ final class AphrontDialogView } if ($this->isStandalone) { - $more .= ' aphront-dialog-view-standalone'; + $classes[] = 'aphront-dialog-view-standalone'; + } + + if ($this->objectList) { + $classes[] = 'aphront-dialog-object-list'; } $attributes = array( - 'class' => 'aphront-dialog-view '.$more, + 'class' => implode(' ', $classes), 'sigil' => 'jx-dialog', ); diff --git a/src/view/phui/PHUIPropertyListView.php b/src/view/phui/PHUIPropertyListView.php index 8a44ecd941..b505bde09f 100644 --- a/src/view/phui/PHUIPropertyListView.php +++ b/src/view/phui/PHUIPropertyListView.php @@ -127,6 +127,7 @@ final class PHUIPropertyListView extends AphrontView { // If we have an action list, make sure we render a property part, even // if there are no properties. Otherwise, the action list won't render. if ($this->actionList) { + $this->classes[] = 'phui-property-list-has-actions'; $have_property_part = false; foreach ($this->parts as $part) { if ($part['type'] == 'property') { diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index 88da74e8e1..3043400357 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -128,3 +128,13 @@ border: 0; border-top: 1px solid {$thinblueborder}; } + +.aphront-dialog-object-list .phui-object-box { + border: none; + padding: 0; + margin: 0; +} + +.aphront-dialog-object-list .aphront-dialog-body { + padding: 0 12px; +} diff --git a/webroot/rsrc/css/aphront/table-view.css b/webroot/rsrc/css/aphront/table-view.css index e0e06317c8..254599625f 100644 --- a/webroot/rsrc/css/aphront/table-view.css +++ b/webroot/rsrc/css/aphront/table-view.css @@ -61,6 +61,7 @@ th.aphront-table-view-sortable-selected { text-align: right; color: {$bluetext}; font-weight: bold; + vertical-align: top; } .aphront-table-view td { diff --git a/webroot/rsrc/css/application/harbormaster/harbormaster.css b/webroot/rsrc/css/application/harbormaster/harbormaster.css index 9878d3f893..6158582ddd 100644 --- a/webroot/rsrc/css/application/harbormaster/harbormaster.css +++ b/webroot/rsrc/css/application/harbormaster/harbormaster.css @@ -3,13 +3,8 @@ */ .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}; + margin: 4px 0 4px 8px; + padding: 8px; } .harbormaster-artifact-summary-header { diff --git a/webroot/rsrc/css/core/remarkup.css b/webroot/rsrc/css/core/remarkup.css index 5c6cfd8c89..3f4516f135 100644 --- a/webroot/rsrc/css/core/remarkup.css +++ b/webroot/rsrc/css/core/remarkup.css @@ -334,6 +334,10 @@ max-width: 100%; } +.phabricator-remarkup .remarkup-table-wrap { + overflow-x: auto; +} + .phabricator-remarkup table.remarkup-table { border-collapse: separate; border-spacing: 1px; diff --git a/webroot/rsrc/css/phui/phui-property-list-view.css b/webroot/rsrc/css/phui/phui-property-list-view.css index 803d572421..3558bf7a9e 100644 --- a/webroot/rsrc/css/phui/phui-property-list-view.css +++ b/webroot/rsrc/css/phui/phui-property-list-view.css @@ -43,7 +43,7 @@ } .device-desktop .phui-property-list-key { - width: 18%; + width: 12%; margin-left: 1%; text-align: right; float: left; @@ -51,6 +51,10 @@ margin-bottom: 4px; } +.device-desktop .phui-property-list-has-actions .phui-property-list-key { + width: 18%; +} + .phui-property-list-properties-wrap.phui-property-list-stacked { width: auto; float: none; @@ -71,12 +75,16 @@ } .device-desktop .phui-property-list-value { - width: 78%; + width: 84%; margin-left: 1%; float: left; margin-bottom: 4px; } +.device-desktop .phui-property-list-has-actions .phui-property-list-value { + width: 78%; +} + .device .phui-property-list-value, .phui-property-list-stacked .phui-property-list-properties .phui-property-list-value { @@ -158,7 +166,7 @@ margin: 2px 0; } -.phui-property-list-properties-wrap { +.phui-property-list-has-actions .phui-property-list-properties-wrap { float: left; width: 78%; }