From 96839d35f49c76e8e266af69bbbdcb4c013b9af9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 30 Dec 2012 06:37:49 -0800 Subject: [PATCH] Detect and raise setup warnings from within Phabricator Summary: This is basicaly a light version of D4286. The major problem with D4286 is that it's a huge leap and completely replaces the setup process in one step. Instead, I want to do this: - Add the post-setup warnings (yellow bar with "6 unresolved warnings..."). - Copy all setup checks into post-setup warnings (so every check has an old-style check and a new-style check). - Run that for a little bit and make sure it's stable. - Implement fatal post-setup checks (the red screen, vs the yellow bar). - Run that for a little bit. - Nuke setup mode and delete all the old checks. This should give us a bunch of very gradual steps toward the brave new world of simpler setup. Test Plan: - Faked APC setup failures, saw warnings raise. - Verified that this runs after restart (get + set). - Verified that this costs us only one cache hit after first-run (get only). Reviewers: btrahan, codeblock, vrana, chad Reviewed By: codeblock CC: aran Maniphest Tasks: T2228 Differential Revision: https://secure.phabricator.com/D4295 --- src/__phutil_library_map__.php | 10 + .../PhabricatorApplicationConfig.php | 6 +- .../config/check/PhabricatorSetupCheck.php | 79 +++++ .../config/check/PhabricatorSetupCheckAPC.php | 37 +++ .../PhabricatorConfigIssueListController.php | 65 ++++ .../PhabricatorConfigIssueViewController.php | 81 +++++ .../config/issue/PhabricatorSetupIssue.php | 113 +++++++ .../config/view/PhabricatorSetupIssueView.php | 290 ++++++++++++++++++ webroot/index.php | 2 + .../css/application/config/setup-issue.css | 86 ++++++ 10 files changed, 768 insertions(+), 1 deletion(-) create mode 100644 src/applications/config/check/PhabricatorSetupCheck.php create mode 100644 src/applications/config/check/PhabricatorSetupCheckAPC.php create mode 100644 src/applications/config/controller/PhabricatorConfigIssueListController.php create mode 100644 src/applications/config/controller/PhabricatorConfigIssueViewController.php create mode 100644 src/applications/config/issue/PhabricatorSetupIssue.php create mode 100644 src/applications/config/view/PhabricatorSetupIssueView.php create mode 100644 webroot/rsrc/css/application/config/setup-issue.css diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6834b6cb97..5e23240701 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -686,6 +686,8 @@ phutil_register_library_map(array( 'PhabricatorConfigEntry' => 'applications/config/storage/PhabricatorConfigEntry.php', 'PhabricatorConfigEntryDAO' => 'applications/config/storage/PhabricatorConfigEntryDAO.php', 'PhabricatorConfigFileSource' => 'infrastructure/env/PhabricatorConfigFileSource.php', + 'PhabricatorConfigIssueListController' => 'applications/config/controller/PhabricatorConfigIssueListController.php', + 'PhabricatorConfigIssueViewController' => 'applications/config/controller/PhabricatorConfigIssueViewController.php', 'PhabricatorConfigListController' => 'applications/config/controller/PhabricatorConfigListController.php', 'PhabricatorConfigLocalSource' => 'infrastructure/env/PhabricatorConfigLocalSource.php', 'PhabricatorConfigManagementSetWorkflow' => 'infrastructure/env/management/PhabricatorConfigManagementSetWorkflow.php', @@ -1135,6 +1137,10 @@ phutil_register_library_map(array( 'PhabricatorSettingsPanelSSHKeys' => 'applications/settings/panel/PhabricatorSettingsPanelSSHKeys.php', 'PhabricatorSettingsPanelSearchPreferences' => 'applications/settings/panel/PhabricatorSettingsPanelSearchPreferences.php', 'PhabricatorSetup' => 'infrastructure/PhabricatorSetup.php', + 'PhabricatorSetupCheck' => 'applications/config/check/PhabricatorSetupCheck.php', + 'PhabricatorSetupCheckAPC' => 'applications/config/check/PhabricatorSetupCheckAPC.php', + 'PhabricatorSetupIssue' => 'applications/config/issue/PhabricatorSetupIssue.php', + 'PhabricatorSetupIssueView' => 'applications/config/view/PhabricatorSetupIssueView.php', 'PhabricatorSlowvoteChoice' => 'applications/slowvote/storage/PhabricatorSlowvoteChoice.php', 'PhabricatorSlowvoteComment' => 'applications/slowvote/storage/PhabricatorSlowvoteComment.php', 'PhabricatorSlowvoteController' => 'applications/slowvote/controller/PhabricatorSlowvoteController.php', @@ -2002,6 +2008,8 @@ phutil_register_library_map(array( 'PhabricatorConfigEntry' => 'PhabricatorConfigEntryDAO', 'PhabricatorConfigEntryDAO' => 'PhabricatorLiskDAO', 'PhabricatorConfigFileSource' => 'PhabricatorConfigProxySource', + 'PhabricatorConfigIssueListController' => 'PhabricatorConfigController', + 'PhabricatorConfigIssueViewController' => 'PhabricatorConfigController', 'PhabricatorConfigListController' => 'PhabricatorConfigController', 'PhabricatorConfigLocalSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigManagementSetWorkflow' => 'PhabricatorConfigManagementWorkflow', @@ -2409,6 +2417,8 @@ phutil_register_library_map(array( 'PhabricatorSettingsPanelProfile' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelSSHKeys' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelSearchPreferences' => 'PhabricatorSettingsPanel', + 'PhabricatorSetupCheckAPC' => 'PhabricatorSetupCheck', + 'PhabricatorSetupIssueView' => 'AphrontView', 'PhabricatorSlowvoteChoice' => 'PhabricatorSlowvoteDAO', 'PhabricatorSlowvoteComment' => 'PhabricatorSlowvoteDAO', 'PhabricatorSlowvoteController' => 'PhabricatorController', diff --git a/src/applications/config/application/PhabricatorApplicationConfig.php b/src/applications/config/application/PhabricatorApplicationConfig.php index 88e78dd3c7..e069d1b602 100644 --- a/src/applications/config/application/PhabricatorApplicationConfig.php +++ b/src/applications/config/application/PhabricatorApplicationConfig.php @@ -25,8 +25,12 @@ final class PhabricatorApplicationConfig extends PhabricatorApplication { public function getRoutes() { return array( '/config/' => array( - '' => 'PhabricatorConfigListController', + '' => 'PhabricatorConfigListController', 'edit/(?P[\w\.\-]+)/' => 'PhabricatorConfigEditController', + 'issue/' => array( + '' => 'PhabricatorConfigIssueListController', + '(?P[^/]+)/' => 'PhabricatorConfigIssueViewController', + ), ), ); } diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php new file mode 100644 index 0000000000..187236c174 --- /dev/null +++ b/src/applications/config/check/PhabricatorSetupCheck.php @@ -0,0 +1,79 @@ +setIssueKey($key); + + $this->issues[$key] = $issue; + + return $issue; + } + + final public function getIssues() { + return $this->issues; + } + + final public function runSetupChecks() { + $this->issues = array(); + $this->executeChecks(); + } + + final public static function getOpenSetupIssueCount() { + $cache = PhabricatorCaches::getSetupCache(); + return $cache->getKey('phabricator.setup.issues'); + } + + final public static function setOpenSetupIssueCount($count) { + $cache = PhabricatorCaches::getSetupCache(); + $cache->setKey('phabricator.setup.issues', $count); + } + + final public static function willProcessRequest() { + $issue_count = self::getOpenSetupIssueCount(); + if ($issue_count !== null) { + // We've already run setup checks, didn't hit any fatals, and then set + // an issue count. This means we're good and don't need to do any extra + // work. + return null; + } + + $issues = self::runAllChecks(); + + self::setOpenSetupIssueCount(count($issues)); + + return null; + } + + final public static function runAllChecks() { + $symbols = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorSetupCheck') + ->setConcreteOnly(true) + ->selectAndLoadSymbols(); + + $checks = array(); + foreach ($symbols as $symbol) { + $checks[] = newv($symbol['name'], array()); + } + + $issues = array(); + foreach ($checks as $check) { + $check->runSetupChecks(); + foreach ($check->getIssues() as $key => $issue) { + if (isset($issues[$key])) { + throw new Exception( + "Two setup checks raised an issue with key '{$key}'!"); + } + $issues[$key] = $issue; + } + } + + return $issues; + } + +} diff --git a/src/applications/config/check/PhabricatorSetupCheckAPC.php b/src/applications/config/check/PhabricatorSetupCheckAPC.php new file mode 100644 index 0000000000..3a583e5b84 --- /dev/null +++ b/src/applications/config/check/PhabricatorSetupCheckAPC.php @@ -0,0 +1,37 @@ +newIssue('extension.apc') + ->setShortName(pht('APC')) + ->setName(pht("PHP Extension 'APC' Not Installed")) + ->setMessage($message) + ->addPHPExtension('apc'); + return; + } + + if (!ini_get('apc.enabled')) { + $summary = pht("Enabling APC will dramatically improve performance."); + $message = pht( + "The PHP extension 'APC' is installed, but not enabled in your PHP ". + "configuration. Enabling it will dramatically improve Phabricator ". + "performance. Edit the 'apc.enabled' setting to enable the extension."); + + $this + ->newIssue('extension.apc.enabled') + ->setShortName(pht('APC Disabled')) + ->setName(pht("PHP Extension 'APC' Not Enabled")) + ->setSummary($summary) + ->setMessage($message) + ->addPHPConfig('apc.enabled'); + } + + } +} diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/PhabricatorConfigIssueListController.php new file mode 100644 index 0000000000..fd794e3e71 --- /dev/null +++ b/src/applications/config/controller/PhabricatorConfigIssueListController.php @@ -0,0 +1,65 @@ +getRequest(); + $user = $request->getUser(); + + $nav = $this->buildSideNavView(); + + $issues = PhabricatorSetupCheck::runAllChecks(); + PhabricatorSetupCheck::setOpenSetupIssueCount(count($issues)); + + $list = $this->buildIssueList($issues); + $list->setNoDataString(pht("There are no open setup issues.")); + + $header = id(new PhabricatorHeaderView()) + ->setHeader(pht('Open Phabricator Setup Issues')); + + $nav->appendChild( + array( + $header, + $list, + )); + + $title = pht('Setup Issues'); + + $crumbs = $this + ->buildApplicationCrumbs($nav) + ->addCrumb( + id(new PhabricatorCrumbView()) + ->setName($title) + ->setHref($this->getApplicationURI('issue/'))); + + $nav->setCrumbs($crumbs); + + return $this->buildApplicationPage( + $nav, + array( + 'title' => $title, + 'device' => true, + ) + ); + } + + private function buildIssueList(array $issues) { + assert_instances_of($issues, 'PhabricatorSetupIssue'); + $list = new PhabricatorObjectItemListView(); + + foreach ($issues as $issue) { + $href = $this->getApplicationURI('/issue/'.$issue->getIssueKey().'/'); + $item = id(new PhabricatorObjectItemView()) + ->setHeader($issue->getName()) + ->setHref($href) + ->setBarColor('yellow') + ->addIcon('warning', pht('Setup Warning')) + ->addAttribute($issue->getSummary()); + $list->addItem($item); + } + + return $list; + } + +} diff --git a/src/applications/config/controller/PhabricatorConfigIssueViewController.php b/src/applications/config/controller/PhabricatorConfigIssueViewController.php new file mode 100644 index 0000000000..cf5c6ea449 --- /dev/null +++ b/src/applications/config/controller/PhabricatorConfigIssueViewController.php @@ -0,0 +1,81 @@ +issueKey = $data['key']; + } + + public function processRequest() { + $request = $this->getRequest(); + $user = $request->getUser(); + + $nav = $this->buildSideNavView(); + + $issues = PhabricatorSetupCheck::runAllChecks(); + PhabricatorSetupCheck::setOpenSetupIssueCount(count($issues)); + + if (empty($issues[$this->issueKey])) { + $content = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setTitle(pht('Issue Resolved')) + ->appendChild(pht('This setup issue has been resolved. ')) + ->appendChild( + phutil_render_tag( + 'a', + array( + 'href' => $this->getApplicationURI('issue/'), + ), + pht('Return to Open Issue List'))); + $title = pht('Resolved Issue'); + } else { + $issue = $issues[$this->issueKey]; + $content = $this->renderIssue($issue); + $title = $issue->getShortName(); + } + + $nav->appendChild($content); + + $crumbs = $this + ->buildApplicationCrumbs($nav) + ->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht('Setup Issues')) + ->setHref($this->getApplicationURI('issue/'))) + ->addCrumb( + id(new PhabricatorCrumbView()) + ->setName($title) + ->setHref($request->getRequestURI())); + + $nav->setCrumbs($crumbs); + + return $this->buildApplicationPage( + $nav, + array( + 'title' => $title, + 'device' => true, + ) + ); + } + + private function renderIssue(PhabricatorSetupIssue $issue) { + require_celerity_resource('setup-issue-css'); + + $view = new PhabricatorSetupIssueView(); + $view->setIssue($issue); + $view; + + $container = phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-background', + ), + $view->render()); + + return $container; + } + +} diff --git a/src/applications/config/issue/PhabricatorSetupIssue.php b/src/applications/config/issue/PhabricatorSetupIssue.php new file mode 100644 index 0000000000..fec0cd0be6 --- /dev/null +++ b/src/applications/config/issue/PhabricatorSetupIssue.php @@ -0,0 +1,113 @@ +commands[] = $command; + return $this; + } + + public function getCommands() { + return $this->commands; + } + + public function setShortName($short_name) { + $this->shortName = $short_name; + return $this; + } + + public function getShortName() { + if ($this->shortName === null) { + return $this->getName(); + } + return $this->shortName; + } + + public function setName($name) { + $this->name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setSummary($summary) { + $this->summary = $summary; + return $this; + } + + public function getSummary() { + if ($this->summary === null) { + return $this->getMessage(); + } + return $this->summary; + } + + public function setIssueKey($issue_key) { + $this->issueKey = $issue_key; + return $this; + } + + public function getIssueKey() { + return $this->issueKey; + } + + public function setIsFatal($is_fatal) { + $this->isFatal = $is_fatal; + return $this; + } + + public function getIsFatal() { + return $this->isFatal; + } + + public function addPHPConfig($php_config) { + $this->phpConfig[] = $php_config; + return $this; + } + + public function getPHPConfig() { + return $this->phpConfig; + } + + public function addPhabricatorConfig($phabricator_config) { + $this->phabricatorConfig[] = $phabricator_config; + return $this; + } + + public function getPhabricatorConfig() { + return $this->phabricatorConfig; + } + + public function addPHPExtension($php_extension) { + $this->phpExtensions[] = $php_extension; + return $this; + } + + public function getPHPExtensions() { + return $this->phpExtensions; + } + + public function setMessage($message) { + $this->message = $message; + return $this; + } + + public function getMessage() { + return $this->message; + } + +} diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php new file mode 100644 index 0000000000..c4f2f14eaa --- /dev/null +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -0,0 +1,290 @@ +issue = $issue; + return $this; + } + + public function getIssue() { + return $this->issue; + } + + public function render() { + $issue = $this->getIssue(); + + $description = phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-instructions', + ), + nl2br(phutil_escape_html($issue->getMessage()))); + + $configs = $issue->getPhabricatorConfig(); + if ($configs) { + $description .= $this->renderPhabricatorConfig($configs); + } + + $configs = $issue->getPHPConfig(); + if ($configs) { + $description .= $this->renderPHPConfig($configs); + } + + $commands = $issue->getCommands(); + if ($commands) { + $run_these = pht("Run these %d command(s):", count($commands)); + $description .= phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-config', + ), + phutil_render_tag('p', array(), $run_these). + phutil_render_tag('pre', array(), implode("\n", $commands))); + } + + $extensions = $issue->getPHPExtensions(); + if ($extensions) { + $install_these = pht( + "Install these %d PHP extension(s):", count($extensions)); + + $install_info = pht( + "You can usually install a PHP extension using apt-get or ". + "yum. Common package names are ". + "php-extname or php5-extname. ". + "Try commands like these:"); + + // TODO: We should do a better job of detecting how to install extensions + // on the current system. + $install_commands = array( + "$ sudo apt-get install php5-extname # Debian / Ubuntu", + "$ sudo yum install php5-extname # Red Hat / Derivatives", + ); + $install_commands = implode("\n", $install_commands); + + $fallback_info = pht( + "If those commands don't work, try Google. The process of installing ". + "PHP extensions is not specific to Phabricator, and any instructions ". + "you can find for installing them on your system should work. On Mac ". + "OS X, you might want to try Homebrew."); + + $restart_info = pht( + "After installing new PHP extensions, restart your webserver ". + "for the changes to take effect."); + + $description .= phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-config', + ), + phutil_render_tag('p', array(), $install_these). + phutil_render_tag('pre', array(), implode("\n", $extensions)). + phutil_render_tag('p', array(), $install_info). + phutil_render_tag('pre', array(), $install_commands). + phutil_render_tag('p', array(), $fallback_info). + phutil_render_tag('p', array(), $restart_info)); + + } + + $next = phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-next', + ), + pht('To continue, resolve this problem and reload the page.')); + + $name = phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-name', + ), + phutil_escape_html($issue->getName())); + + return phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue', + ), + $name.$description.$next); + } + + private function renderPhabricatorConfig(array $configs) { + $table_info = phutil_render_tag( + 'p', + array(), + pht( + "The current Phabricator configuration has these %d value(s):", + count($configs))); + + $table = array(); + foreach ($configs as $key) { + $table[] = ''; + $table[] = ''.phutil_escape_html($key).''; + + $value = PhabricatorEnv::getEnvConfig($key); + if ($value === null) { + $value = 'null'; + } else if ($value === false) { + $value = 'false'; + } else if ($value === true) { + $value = 'true'; + } else { + $value = phutil_escape_html($value); + } + + $table[] = ''.$value.''; + $table[] = ''; + } + + $table = phutil_render_tag( + 'table', + array( + + ), + implode("\n", $table)); + + $update_info = phutil_render_tag( + 'p', + array(), + pht( + "To update these %d value(s), run these command(s) from the command ". + "line:", + count($configs))); + + $update = array(); + foreach ($configs as $key) { + $cmd = 'phabricator/ $ ./bin/config set '. + phutil_escape_html($key).' '. + 'value'; + $update[] = $cmd; + } + $update = phutil_render_tag('pre', array(), implode("\n", $update)); + + return phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-config', + ), + self::renderSingleView( + array( + $table_info, + $table, + $update_info, + $update, + ))); + } + + private function renderPHPConfig(array $configs) { + $table_info = phutil_render_tag( + 'p', + array(), + pht( + "The current PHP configuration has these %d value(s):", + count($configs))); + + $table = array(); + foreach ($configs as $key) { + $table[] = ''; + $table[] = ''.phutil_escape_html($key).''; + + $value = ini_get($key); + if ($value === null) { + $value = 'null'; + } else if ($value === false) { + $value = 'false'; + } else if ($value === true) { + $value = 'true'; + } else if ($value === '') { + $value = '(empty string)'; + } else { + $value = phutil_escape_html($value); + } + + $table[] = ''.$value.''; + $table[] = ''; + } + + $table = phutil_render_tag( + 'table', + array( + + ), + implode("\n", $table)); + + ob_start(); + phpinfo(); + $phpinfo = ob_get_clean(); + + + $rex = '@Loaded Configuration File\s*(.*?)@i'; + $matches = null; + + $ini_loc = null; + if (preg_match($rex, $phpinfo, $matches)) { + $ini_loc = trim($matches[1]); + } + + $rex = '@Additional \.ini files parsed\s*(.*?)@i'; + + $more_loc = array(); + if (preg_match($rex, $phpinfo, $matches)) { + $more_loc = trim($matches[1]); + if ($more_loc == '(none)') { + $more_loc = array(); + } else { + $more_loc = preg_split('/\s*,\s*/', $more_loc); + } + } + + if (!$ini_loc) { + $info = phutil_render_tag( + 'p', + array(), + pht( + "To update these %d value(s), edit your PHP configuration file.", + count($configs))); + } else { + $info = phutil_render_tag( + 'p', + array(), + pht( + "To update these %d value(s), edit your PHP configuration file, ". + "located here:", + count($configs))); + $info .= phutil_render_tag( + 'pre', + array(), + phutil_escape_html($ini_loc)); + } + + if ($more_loc) { + $info .= phutil_render_tag( + 'p', + array(), + pht( + "PHP also loaded these configuration file(s):", + count($more_loc))); + $info .= phutil_render_tag( + 'pre', + array(), + phutil_escape_html(implode("\n", $more_loc))); + } + + $info .= phutil_render_tag( + 'p', + array(), + pht( + "After editing the PHP configuration, restart your webserver ". + "for the changes to take effect.")); + + return phutil_render_tag( + 'div', + array( + 'class' => 'setup-issue-config', + ), + $table_info.$table.$info); + } + +} diff --git a/webroot/index.php b/webroot/index.php index 5c9e89f3d3..fc60165d93 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -38,6 +38,8 @@ try { return; } + PhabricatorSetupCheck::willProcessRequest(); + phabricator_detect_bad_base_uri(); $host = $_SERVER['HTTP_HOST']; diff --git a/webroot/rsrc/css/application/config/setup-issue.css b/webroot/rsrc/css/application/config/setup-issue.css new file mode 100644 index 0000000000..58c34d997f --- /dev/null +++ b/webroot/rsrc/css/application/config/setup-issue.css @@ -0,0 +1,86 @@ +/** + * @provides setup-issue-css + */ + + +.setup-issue-background { + background-color: #edecef; + padding: 1em 0; +} + +.setup-issue { + border: 1px solid #35393d; + margin: 15px auto; + max-width: 760px; + background: #ffffff; + box-shadow: 1px 2px 2px rgba(0, 0, 0, 0.25); +} + +.setup-issue p { + margin: 1em 0; +} + +.setup-issue table { + width: 90%; + margin: auto; + border-collapse: collapse; + border: 1px solid #dfdfdf; +} + +.setup-issue table th { + text-align: right; + width: 30%; + background: #efefef; + border: 1px solid #dfdfdf; + padding: 8px; +} + +.setup-issue table td { + border: 1px solid #dfdfdf; + padding: 8px; +} + +.setup-issue pre { + width: 84%; + margin: auto; + border: 1px solid #dfdfdf; + padding: 10px 3%; + background: #efefef; +} + +.setup-issue tt { + color: #666666; +} + +.setup-issue em { + font-weight: bold; +} + +.setup-issue-instructions { + font-size: 15px; + padding: 20px; + line-height: 1.4em; + background: #efefef; + border-bottom: 1px solid #bfbfbf; +} + +.setup-issue-name { + padding: 15px; + background: #35393d; + color: #ffffff; + font-size: 15px; + font-weight: bold; +} + +.setup-issue-next { + padding: 15px; + background: #35393d; + text-align: center; + font-size: 16px; + color: #ffffff; +} + +.setup-issue-config { + margin: 15px 0; + padding: 0 20px; +}