diff --git a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php index 22038e7cf6..44455cb203 100644 --- a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php +++ b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php @@ -3,6 +3,8 @@ final class PhabricatorSetupCheckExtraConfig extends PhabricatorSetupCheck { protected function executeChecks() { + $ancient_config = self::getAncientConfig(); + $all_keys = PhabricatorEnv::getAllConfigKeys(); $all_keys = array_keys($all_keys); sort($all_keys); @@ -13,20 +15,132 @@ final class PhabricatorSetupCheckExtraConfig extends PhabricatorSetupCheck { if (isset($defined_keys[$key])) { continue; } - $summary = pht("This option is not recognized. It may be misspelled."); - $message = pht( - "The configuration option '%s' is not recognized. It may be ". - "misspelled, or it might have existed in an older version of ". - "Phabricator. It has no effect, and should be corrected or deleted.", - $key); - $this - ->newIssue('config.unknown.'.$key) - ->setShortName(pht('Unknown Config')) - ->setName(pht('Unknown Configuration Option "%s"', $key)) - ->setSummary($summary) - ->setMessage($message) - ->addPhabricatorConfig($key); + if (isset($ancient_config[$key])) { + $summary = pht( + 'This option has been removed. You may delete it at your '. + 'convenience.'); + $message = pht( + "The configuration option '%s' has been removed. You may delete ". + "it at your convenience.". + "\n\n%s", + $key, + $ancient_config[$key]); + $short = pht('Obsolete Config'); + $name = pht('Obsolete Configuration Option "%s"', $key); + } else { + $summary = pht("This option is not recognized. It may be misspelled."); + $message = pht( + "The configuration option '%s' is not recognized. It may be ". + "misspelled, or it might have existed in an older version of ". + "Phabricator. It has no effect, and should be corrected or deleted.", + $key); + $short = pht('Unknown Config'); + $name = pht('Unknown Configuration Option "%s"', $key); + } + + $issue = $this->newIssue('config.unknown.'.$key) + ->setShortName($short) + ->setName($name) + ->setSummary($summary); + + $stack = PhabricatorEnv::getConfigSourceStack(); + $stack = $stack->getStack(); + + $found = array(); + $found_local = false; + $found_database = false; + + foreach ($stack as $source_key => $source) { + $value = $source->getKeys(array($key)); + if ($value) { + $found[] = $source->getName(); + if ($source instanceof PhabricatorConfigDatabaseSource) { + $found_database = true; + } + if ($source instanceof PhabricatorConfigLocalSource) { + $found_local = true; + } + } + } + + $message = $message."\n\n".pht( + "This configuration value is defined in in these %d ". + "configuration source(s): %s.", + count($found), + implode(', ', $found)); + $issue->setMessage($message); + + if ($found_local) { + $command = csprintf('phabricator/ $ ./bin/config delete %s', $key); + $issue->addCommand($command); + } + + if ($found_database) { + $issue->addPhabricatorConfig($key); + } } } + + /** + * Return a map of deleted config options. Keys are option keys; values are + * explanations of what happened to the option. + */ + public static function getAncientConfig() { + $reason_auth = pht( + 'This option has been migrated to the "Auth" application. Your old '. + 'configuration is still in effect, but now stored in "Auth" instead of '. + 'configuration. Going forward, you can manage authentication from '. + 'the web UI.'); + + $auth_config = array( + 'controller.oauth-registration', + 'auth.password-auth-enabled', + 'facebook.auth-enabled', + 'facebook.registration-enabled', + 'facebook.auth-permanent', + 'facebook.application-id', + 'facebook.application-secret', + 'facebook.require-https-auth', + 'github.auth-enabled', + 'github.registration-enabled', + 'github.auth-permanent', + 'github.application-id', + 'github.application-secret', + 'google.auth-enabled', + 'google.registration-enabled', + 'google.auth-permanent', + 'google.application-id', + 'google.application-secret', + 'ldap.auth-enabled', + 'ldap.hostname', + 'ldap.port', + 'ldap.base_dn', + 'ldap.search_attribute', + 'ldap.search-first', + 'ldap.username-attribute', + 'ldap.real_name_attributes', + 'ldap.activedirectory_domain', + 'ldap.version', + 'ldap.referrals', + 'ldap.anonymous-user-name', + 'ldap.anonymous-user-password', + 'ldap.start-tls', + 'disqus.auth-enabled', + 'disqus.registration-enabled', + 'disqus.auth-permanent', + 'disqus.application-id', + 'disqus.application-secret', + 'phabricator.oauth-uri', + 'phabricator.auth-enabled', + 'phabricator.registration-enabled', + 'phabricator.auth-permanent', + 'phabricator.application-id', + 'phabricator.application-secret', + ); + + $ancient_config = array_fill_keys($auth_config, $reason_auth); + + return $ancient_config; + } } diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php index 1dcfc3796f..fcc73511fd 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -16,6 +16,18 @@ final class PhabricatorConfigEditController $options = PhabricatorApplicationConfigOptions::loadAllOptions(); if (empty($options[$this->key])) { + $ancient = PhabricatorSetupCheckExtraConfig::getAncientConfig(); + if (isset($ancient[$this->key])) { + $desc = pht( + "This configuration has been removed. You can safely delete ". + "it.\n\n%s", + $ancient[$this->key]); + } else { + $desc = pht( + "This configuration option is unknown. It may be misspelled, ". + "or have existed in a previous version of Phabricator."); + } + // This may be a dead config entry, which existed in the past but no // longer exists. Allow it to be edited so it can be reviewed and // deleted. @@ -23,10 +35,7 @@ final class PhabricatorConfigEditController ->setKey($this->key) ->setType('wild') ->setDefault(null) - ->setDescription( - pht( - "This configuration option is unknown. It may be misspelled, ". - "or have existed in a previous version of Phabricator.")); + ->setDescription($desc); $group = null; $group_uri = $this->getApplicationURI(); } else { @@ -465,20 +474,6 @@ final class PhabricatorConfigEditController $stack = PhabricatorEnv::getConfigSourceStack(); $stack = $stack->getStack(); - /* - - TODO: Once DatabaseSource lands, do this: - - foreach ($stack as $key => $source) { - unset($stack[$key]); - if ($source instanceof PhabricatorConfigDatabaseSource) { - break; - } - } - - */ - - $table = array(); $table[] = hsprintf( '%s%s', diff --git a/src/applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php index d765b5ffe9..cf1f05be74 100644 --- a/src/applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php +++ b/src/applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php @@ -33,13 +33,6 @@ final class PhabricatorConfigManagementDeleteWorkflow "Too many arguments: expected one key."); } - $options = PhabricatorApplicationConfigOptions::loadAllOptions(); - if (empty($options[$key])) { - throw new PhutilArgumentUsageException( - "No such configuration key '{$key}'! Use `config list` to list all ". - "keys."); - } - $config = new PhabricatorConfigLocalSource(); $values = $config->getKeys(array($key)); if (!$values) { diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php index 5736c66c43..ffa21ecd40 100644 --- a/src/applications/config/view/PhabricatorSetupIssueView.php +++ b/src/applications/config/view/PhabricatorSetupIssueView.php @@ -177,7 +177,7 @@ final class PhabricatorSetupIssueView extends AphrontView { } else { $update = array(); foreach ($configs as $config) { - if (!idx($options, $config) || $options[$config]->getLocked()) { + if (idx($options, $config) && $options[$config]->getLocked()) { continue; } $link = phutil_tag(