From 1c1f749eba216e906845c706197c6159fecb6ba4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 16 Sep 2011 11:42:45 -0700 Subject: [PATCH] Add an "arcanist.projectinfo" Conduit call Summary: We currently rely on "remote_hooks_enabled" in .arcconfig to determine whether commands like "arc amend" and "arc merge" should imply "arc mark-committed". However, this is a historical artifact that is now bad for a bunch of reasons: - The option name is confusing, it really means 'repository is tracked'. - The option is hard to discover and generally sucks. - We can empirically determine the right answer since we now know if a project is in a tracked repository. Add a call which arcanist can make on these workflows to figure out if it is interacting with a project in a tracked repository or not. Also added an "isTracked()" convenience method to reduce the number of magic strings all over the place. Test Plan: Ran "arcanist.projectinfo" for nonexistent, untracked and tracked projects. Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran Reviewed By: Makinde CC: aran, epriestley, Makinde Differential Revision: 945 --- .../daemon/phabricator_daemon_launcher.php | 2 +- src/__phutil_library_map__.php | 4 + .../base/ConduitAPI_arcanist_Method.php | 24 ++++++ .../conduit/method/arcanist/base/__init__.php | 12 +++ ...ConduitAPI_arcanist_projectinfo_Method.php | 73 +++++++++++++++++++ .../method/arcanist/projectinfo/__init__.php | 16 ++++ .../home/DiffusionHomeController.php | 2 +- .../PhabricatorRepositoryEditController.php | 4 +- .../PhabricatorRepositoryListController.php | 2 +- .../PhabricatorRepositoryPullLocalDaemon.php | 2 +- .../repository/PhabricatorRepository.php | 4 + 11 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 src/applications/conduit/method/arcanist/base/ConduitAPI_arcanist_Method.php create mode 100644 src/applications/conduit/method/arcanist/base/__init__.php create mode 100644 src/applications/conduit/method/arcanist/projectinfo/ConduitAPI_arcanist_projectinfo_Method.php create mode 100644 src/applications/conduit/method/arcanist/projectinfo/__init__.php diff --git a/scripts/daemon/phabricator_daemon_launcher.php b/scripts/daemon/phabricator_daemon_launcher.php index 5fe297a6cd..1d14db556a 100755 --- a/scripts/daemon/phabricator_daemon_launcher.php +++ b/scripts/daemon/phabricator_daemon_launcher.php @@ -243,7 +243,7 @@ function phd_load_tracked_repositories() { $repositories = id(new PhabricatorRepository())->loadAll(); foreach ($repositories as $key => $repository) { - if (!$repository->getDetail('tracking-enabled')) { + if (!$repository->isTracked()) { unset($repositories[$key]); } } diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ab26999da9..86c3246cb1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -86,6 +86,8 @@ phutil_register_library_map(array( 'CelerityStaticResourceResponse' => 'infrastructure/celerity/response', 'ConduitAPIMethod' => 'applications/conduit/method/base', 'ConduitAPIRequest' => 'applications/conduit/protocol/request', + 'ConduitAPI_arcanist_Method' => 'applications/conduit/method/arcanist/base', + 'ConduitAPI_arcanist_projectinfo_Method' => 'applications/conduit/method/arcanist/projectinfo', 'ConduitAPI_conduit_connect_Method' => 'applications/conduit/method/conduit/connect', 'ConduitAPI_conduit_getcertificate_Method' => 'applications/conduit/method/conduit/getcertificate', 'ConduitAPI_conduit_ping_Method' => 'applications/conduit/method/conduit/ping', @@ -783,6 +785,8 @@ phutil_register_library_map(array( 'AphrontTypeaheadTemplateView' => 'AphrontView', 'AphrontWebpageResponse' => 'AphrontResponse', 'CelerityResourceController' => 'AphrontController', + 'ConduitAPI_arcanist_Method' => 'ConduitAPIMethod', + 'ConduitAPI_arcanist_projectinfo_Method' => 'ConduitAPI_arcanist_Method', 'ConduitAPI_conduit_connect_Method' => 'ConduitAPIMethod', 'ConduitAPI_conduit_getcertificate_Method' => 'ConduitAPIMethod', 'ConduitAPI_conduit_ping_Method' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/method/arcanist/base/ConduitAPI_arcanist_Method.php b/src/applications/conduit/method/arcanist/base/ConduitAPI_arcanist_Method.php new file mode 100644 index 0000000000..84ba3e2837 --- /dev/null +++ b/src/applications/conduit/method/arcanist/base/ConduitAPI_arcanist_Method.php @@ -0,0 +1,24 @@ + 'required string', + ); + } + + public function defineReturnType() { + return 'nonempty dict'; + } + + public function defineErrorTypes() { + return array( + 'ERR-BAD-ARCANIST-PROJECT' => 'No such project exists.', + ); + } + + protected function execute(ConduitAPIRequest $request) { + $name = $request->getValue('name'); + + $project = id(new PhabricatorRepositoryArcanistProject())->loadOneWhere( + 'name = %s', + $name); + + if (!$project) { + throw new ConduitException('ERR-BAD-ARCANIST-PROJECT'); + } + + $repository = $project->loadRepository(); + + $repository_phid = null; + $tracked = false; + if ($repository) { + $repository_phid = $repository->getPHID(); + $tracked = $repository->isTracked(); + } + + return array( + 'name' => $project->getName(), + 'phid' => $project->getPHID(), + 'repositoryPHID' => $repository_phid, + 'tracked' => $tracked, + ); + } + +} diff --git a/src/applications/conduit/method/arcanist/projectinfo/__init__.php b/src/applications/conduit/method/arcanist/projectinfo/__init__.php new file mode 100644 index 0000000000..1dd5441591 --- /dev/null +++ b/src/applications/conduit/method/arcanist/projectinfo/__init__.php @@ -0,0 +1,16 @@ +loadAll(); foreach ($repositories as $key => $repository) { - if (!$repository->getDetail('tracking-enabled')) { + if (!$repository->isTracked()) { unset($repositories[$key]); } } diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index 801f84baec..1b55461aec 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php @@ -327,7 +327,7 @@ class PhabricatorRepositoryEditController $error_view->appendChild( 'Tracking changes were saved. You may need to restart the daemon '. 'before changes will take effect.'); - } else if (!$repository->getDetail('tracking-enabled')) { + } else if (!$repository->isTracked()) { $error_view = new AphrontErrorView(); $error_view->setSeverity(AphrontErrorView::SEVERITY_WARNING); $error_view->setTitle('Repository Not Tracked'); @@ -381,7 +381,7 @@ class PhabricatorRepositoryEditController 'enabled' => 'Enabled', )) ->setValue( - $repository->getDetail('tracking-enabled') + $repository->isTracked() ? 'enabled' : 'disabled')) ->appendChild(''); diff --git a/src/applications/repository/controller/list/PhabricatorRepositoryListController.php b/src/applications/repository/controller/list/PhabricatorRepositoryListController.php index c9b83d67b1..5aa8d5d1dd 100644 --- a/src/applications/repository/controller/list/PhabricatorRepositoryListController.php +++ b/src/applications/repository/controller/list/PhabricatorRepositoryListController.php @@ -34,7 +34,7 @@ class PhabricatorRepositoryListController $rows = array(); foreach ($repos as $repo) { - if ($repo->getDetail('tracking-enabled')) { + if ($repo->isTracked()) { $diffusion_link = phutil_render_tag( 'a', array( diff --git a/src/applications/repository/daemon/pulllocal/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/pulllocal/PhabricatorRepositoryPullLocalDaemon.php index 8146788498..dd02d3ee8c 100644 --- a/src/applications/repository/daemon/pulllocal/PhabricatorRepositoryPullLocalDaemon.php +++ b/src/applications/repository/daemon/pulllocal/PhabricatorRepositoryPullLocalDaemon.php @@ -39,7 +39,7 @@ abstract class PhabricatorRepositoryPullLocalDaemon "repository '{$repo_name}' is a '{$repo_type_name}' repository."); } - $tracked = $repository->getDetail('tracking-enabled'); + $tracked = $repository->isTracked(); if (!$tracked) { throw new Exception("Tracking is not enabled for this repository."); } diff --git a/src/applications/repository/storage/repository/PhabricatorRepository.php b/src/applications/repository/storage/repository/PhabricatorRepository.php index 1775f7ba0f..3be0dca297 100644 --- a/src/applications/repository/storage/repository/PhabricatorRepository.php +++ b/src/applications/repository/storage/repository/PhabricatorRepository.php @@ -293,4 +293,8 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO { return ($protocol == 'http' || $protocol == 'https'); } + public function isTracked() { + return $this->getDetail('tracking-enabled', false); + } + }