diff --git a/src/applications/config/check/PhabricatorSetupCheckStorage.php b/src/applications/config/check/PhabricatorSetupCheckStorage.php --- a/src/applications/config/check/PhabricatorSetupCheckStorage.php +++ b/src/applications/config/check/PhabricatorSetupCheckStorage.php @@ -2,6 +2,9 @@ final class PhabricatorSetupCheckStorage extends PhabricatorSetupCheck { + /** + * @phutil-external-symbol class PhabricatorStartup + */ protected function executeChecks() { $upload_limit = PhabricatorEnv::getEnvConfig('storage.upload-size-limit'); if (!$upload_limit) { @@ -16,8 +19,64 @@ ->setName(pht('Upload Limit Not Yet Configured')) ->setMessage($message) ->addPhabricatorConfig('storage.upload-size-limit'); + } else { + $memory_limit = PhabricatorStartup::getOldMemoryLimit(); + if ($memory_limit && ((int)$memory_limit > 0)) { + $memory_limit_bytes = phutil_parse_bytes($memory_limit); + $memory_usage_bytes = memory_get_usage(); + $upload_limit_bytes = phutil_parse_bytes($upload_limit); + + $available_bytes = ($memory_limit_bytes - $memory_usage_bytes); + + if ($upload_limit_bytes > $available_bytes) { + $summary = pht( + 'Your PHP memory limit is configured in a way that may prevent '. + 'you from uploading large files.'); + + $message = pht( + 'When you upload a file via drag-and-drop or the API, the entire '. + 'file is buffered into memory before being written to permanent '. + 'storage. Phabricator needs memory available to store these '. + 'files while they are uploaded, but PHP is currently configured '. + 'to limit the available memory.'. + "\n\n". + 'Your Phabricator %s is currently set to a larger value (%s) than '. + 'the amount of available memory (%s) that a PHP process has '. + 'available to use, so uploads via drag-and-drop and the API will '. + 'hit the memory limit before they hit other limits.'. + "\n\n". + '(Note that the application itself must also fit in available '. + 'memory, so not all of the memory under the memory limit is '. + 'available for buffering file uploads.)'. + "\n\n". + "The easiest way to resolve this issue is to set %s to %s in your ". + "PHP configuration, to disable the memory limit. There is ". + "usually little or no value to using this option to limit ". + "Phabricator process memory.". + "\n\n". + "You can also increase the limit, or decrease %s, or ignore this ". + "issue and accept that these upload mechanisms will be limited ". + "in the size of files they can handle.", + phutil_tag('tt', array(), 'storage.upload-size-limit'), + phutil_format_bytes($upload_limit_bytes), + phutil_format_bytes($available_bytes), + phutil_tag('tt', array(), 'memory_limit'), + phutil_tag('tt', array(), '-1'), + phutil_tag('tt', array(), 'storage.upload-size-limit')); + + $this + ->newIssue('php.memory_limit.upload') + ->setName(pht('Memory Limit Restricts File Uploads')) + ->setSummary($summary) + ->setMessage($message) + ->addPHPConfig('memory_limit') + ->addPHPConfigOriginalValue('memory_limit', $memory_limit) + ->addPhabricatorConfig('storage.upload-size-limit'); + } + } } + $local_path = PhabricatorEnv::getEnvConfig('storage.local-disk.path'); if (!$local_path) { return; diff --git a/src/applications/config/issue/PhabricatorSetupIssue.php b/src/applications/config/issue/PhabricatorSetupIssue.php --- a/src/applications/config/issue/PhabricatorSetupIssue.php +++ b/src/applications/config/issue/PhabricatorSetupIssue.php @@ -16,6 +16,7 @@ private $phpConfig = array(); private $commands = array(); private $mysqlConfig = array(); + private $originalPHPConfigValues = array(); public function addCommand($command) { $this->commands[] = $command; @@ -82,6 +83,28 @@ return $this; } + /** + * Set an explicit value to display when showing the user PHP configuration + * values. + * + * If Phabricator has changed a value by the time a config issue is raised, + * you can provide the original value here so the UI makes sense. For example, + * we alter `memory_limit` during startup, so if the original value is not + * provided it will look like it is always set to `-1`. + * + * @param string PHP configuration option to provide a value for. + * @param string Explicit value to show in the UI. + * @return this + */ + public function addPHPConfigOriginalValue($php_config, $value) { + $this->originalPHPConfigValues[$php_config] = $value; + return $this; + } + + public function getPHPConfigOriginalValue($php_config, $default = null) { + return idx($this->originalPHPConfigValues, $php_config, $default); + } + public function getPHPConfig() { return $this->phpConfig; } diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php --- a/src/applications/config/view/PhabricatorSetupIssueView.php +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -26,7 +26,7 @@ $configs = $issue->getPHPConfig(); if ($configs) { - $description[] = $this->renderPHPConfig($configs); + $description[] = $this->renderPHPConfig($configs, $issue); } $configs = $issue->getMySQLConfig(); @@ -243,7 +243,7 @@ )); } - private function renderPHPConfig(array $configs) { + private function renderPHPConfig(array $configs, $issue) { $table_info = phutil_tag( 'p', array(), @@ -253,7 +253,9 @@ $dict = array(); foreach ($configs as $key) { - $dict[$key] = ini_get($key); + $dict[$key] = $issue->getPHPConfigOriginalValue( + $key, + ini_get($key)); } $table = $this->renderValueTable($dict); diff --git a/support/PhabricatorStartup.php b/support/PhabricatorStartup.php --- a/support/PhabricatorStartup.php +++ b/support/PhabricatorStartup.php @@ -41,6 +41,7 @@ private static $globals = array(); private static $capturingOutput; private static $rawInput; + private static $oldMemoryLimit; // TODO: For now, disable rate limiting entirely by default. We need to // iterate on it a bit for Conduit, some of the specific score levels, and @@ -310,6 +311,7 @@ */ private static function setupPHP() { error_reporting(E_ALL | E_STRICT); + self::$oldMemoryLimit = ini_get('memory_limit'); ini_set('memory_limit', -1); // If we have libxml, disable the incredibly dangerous entity loader. @@ -318,6 +320,14 @@ } } + + /** + * @task validation + */ + public static function getOldMemoryLimit() { + return self::$oldMemoryLimit; + } + /** * @task validation */