Improve 'arc get-config', 'arc set-config' and show more config info with --trace

Summary:
Fixes T4952. Several issues:

  - You review configuration values with `arc set-config --show`. This makes no sense and never has, I think it just predated `arc get-config` or was easier or something.
    - Instead, review values with `arc get-config` and review details with `arc get-config --verbose`.
  - Show better and more detailed information about all config sources.
  - Establish and show default values from a new "default" source.
  - With `--trace` include more information about attempts to read configuration files.

Test Plan:
Ran `arc get-config --trace --verbose` in various working directories and received sensible-looking output.

{F156247}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4952

Differential Revision: https://secure.phabricator.com/D9172

src/configuration/ArcanistConfigurationManager.php

@@ -23,6 +23,7 @@ final class ArcanistConfigurationManager {
   const CONFIG_SOURCE_PROJECT = 'project';
   const CONFIG_SOURCE_USER    = 'user';
   const CONFIG_SOURCE_SYSTEM  = 'system';
+  const CONFIG_SOURCE_DEFAULT = 'default';
 
   public function getProjectConfig($key) {
     if ($this->workingCopy) {
@@ -112,6 +113,11 @@ final class ArcanistConfigurationManager {
         $settings->willReadValue($key, $pval);
     }
 
+    $default_config = $this->readDefaultConfig();
+    if (array_key_exists($key, $default_config)) {
+      $results[self::CONFIG_SOURCE_DEFAULT] = $default_config[$key];
+    }
+
     return $results;
   }
 
@@ -153,37 +159,59 @@ final class ArcanistConfigurationManager {
    * @{method:readUserArcConfig}.
    */
   public function readUserConfigurationFile() {
-    $user_config = array();
+    static $user_config;
-    $user_config_path = self::getUserConfigurationFileLocation();
+    if ($user_config === null) {
-    if (Filesystem::pathExists($user_config_path)) {
+      $user_config = array();
+      $user_config_path = self::getUserConfigurationFileLocation();
 
-      if (!phutil_is_windows()) {
+      $console = PhutilConsole::getConsole();
-        $mode = fileperms($user_config_path);
+
-        if (!$mode) {
+      if (Filesystem::pathExists($user_config_path)) {
-          throw new Exception("Unable to get perms of '{$user_config_path}'!");
+        $console->writeLog(
-        }
+          "%s\n",
-        if ($mode & 0177) {
+          pht(
-          // Mode should allow only owner access.
+            'Config: Reading user configuration file "%s"...',
-          $prompt = "File permissions on your ~/.arcrc are too open. ".
+            $user_config_path));
-                    "Fix them by chmod'ing to 600?";
+
-          if (!phutil_console_confirm($prompt, $default_no = false)) {
+        if (!phutil_is_windows()) {
-            throw new ArcanistUsageException("Set ~/.arcrc to file mode 600.");
+          $mode = fileperms($user_config_path);
+          if (!$mode) {
+            throw new Exception(
+              pht(
+                'Unable to read file permissions for "%s"!',
+                $user_config_path));
           }
-          execx('chmod 600 %s', $user_config_path);
+          if ($mode & 0177) {
+            // Mode should allow only owner access.
+            $prompt = "File permissions on your ~/.arcrc are too open. ".
+                      "Fix them by chmod'ing to 600?";
+            if (!phutil_console_confirm($prompt, $default_no = false)) {
+              throw new ArcanistUsageException(
+                'Set ~/.arcrc to file mode 600.');
+            }
+            execx('chmod 600 %s', $user_config_path);
 
-          // Drop the stat cache so we don't read the old permissions if
+            // Drop the stat cache so we don't read the old permissions if
-          // we end up here again. If we don't do this, we may prompt the user
+            // we end up here again. If we don't do this, we may prompt the user
-          // to fix permissions multiple times.
+            // to fix permissions multiple times.
-          clearstatcache();
+            clearstatcache();
+          }
         }
+
+        $user_config_data = Filesystem::readFile($user_config_path);
+        $user_config = json_decode($user_config_data, true);
+        if (!is_array($user_config)) {
+          throw new ArcanistUsageException(
+            "Your '~/.arcrc' file is not a valid JSON file.");
+        }
+      } else {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Config: Did not find user configuration at "%s".',
+            $user_config_path));
       }
 
-      $user_config_data = Filesystem::readFile($user_config_path);
-      $user_config = json_decode($user_config_data, true);
-      if (!is_array($user_config)) {
-        throw new ArcanistUsageException(
-          "Your '~/.arcrc' file is not a valid JSON file.");
-      }
     }
     return $user_config;
   }
@@ -247,15 +275,38 @@ final class ArcanistConfigurationManager {
   }
 
   public function readSystemArcConfig() {
-    $system_config = array();
+    static $system_config;
-    $system_config_path = self::getSystemArcConfigLocation();
+    if ($system_config === null) {
-    if (Filesystem::pathExists($system_config_path)) {
+      $system_config = array();
-      $file = Filesystem::readFile($system_config_path);
+      $system_config_path = self::getSystemArcConfigLocation();
-      if ($file) {
+
-        $system_config = json_decode($file, true);
+      $console = PhutilConsole::getConsole();
+
+      if (Filesystem::pathExists($system_config_path)) {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Config: Reading system configuration file "%s"...',
+            $system_config_path));
+        $file = Filesystem::readFile($system_config_path);
+        if ($file) {
+          $system_config = json_decode($file, true);
+        }
+      } else {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Config: Did not find system configuration at "%s".',
+            $system_config_path));
       }
     }
+
     return $system_config;
   }
 
+  public function readDefaultConfig() {
+    $settings = new ArcanistSettings();
+    return $settings->getDefaultSettings();
+  }
+
 }

src/configuration/ArcanistSettings.php

@@ -31,6 +31,7 @@ final class ArcanistSettings {
           'startup. This can be used to make classes available, like lint or '.
           'unit test engines.',
         'example' => '["/var/arc/customlib/src"]',
+        'default' => array(),
       ),
       'repository.callsign' => array(
         'type' => 'string',
@@ -104,6 +105,7 @@ final class ArcanistSettings {
           "report incorrect results, particularly while developing linters. ".
           "This is probably worth enabling only if your linters are very slow.",
         'example' => 'false',
+        'default' => false,
       ),
       'history.immutable' => array(
         'type' => 'bool',
@@ -113,6 +115,7 @@ final class ArcanistSettings {
           'amending or rebasing). Defaults to true in Mercurial and false in '.
           'Git. This setting has no effect in Subversion.',
         'example' => 'false',
+        'default' => false,
       ),
       'editor' => array(
         'type' => 'string',
@@ -134,6 +137,7 @@ final class ArcanistSettings {
         'help' => 'List of domains to blindly trust SSL certificates for. '.
                   'Disables peer verification.',
         'example' => '["secure.mycompany.com"]',
+        'default' => array(),
       ),
       'browser' => array(
         'type' => 'string',
@@ -145,6 +149,7 @@ final class ArcanistSettings {
         'type' => 'list',
         'help' => 'List of event listener classes to install at startup.',
         'example' => '["ExampleEventListener"]',
+        'default' => array(),
       ),
       'http.basicauth.user' => array(
         'type' => 'string',
@@ -167,6 +172,7 @@ final class ArcanistSettings {
           'to restore their working directory from the local stash if '.
           'an Arcanist operation causes an unrecoverable error.',
         'example' => 'false',
+        'default' => false,
       ),
     );
   }
@@ -195,6 +201,16 @@ final class ArcanistSettings {
     return idx($this->getOption($key), 'legacy');
   }
 
+  public function getDefaultSettings() {
+    $defaults = array();
+    foreach ($this->getOptions() as $key => $option) {
+      if (array_key_exists('default', $option)) {
+        $defaults[$key] = $option['default'];
+      }
+    }
+    return $defaults;
+  }
+
   public function willWriteValue($key, $value) {
     $type = $this->getType($key);
     switch ($type) {

src/workflow/ArcanistGetConfigWorkflow.php

@@ -2,8 +2,6 @@
 
 /**
  * Read configuration settings.
- *
- * @group workflow
  */
 final class ArcanistGetConfigWorkflow extends ArcanistBaseWorkflow {
 
@@ -13,7 +11,7 @@ final class ArcanistGetConfigWorkflow extends ArcanistBaseWorkflow {
 
   public function getCommandSynopses() {
     return phutil_console_format(<<<EOTEXT
-      **get-config** -- [__name__ ...]
+      **get-config** [__options__] -- [__name__ ...]
 EOTEXT
       );
   }
@@ -23,12 +21,18 @@ EOTEXT
           Supports: cli
           Reads an arc configuration option. With no argument, reads all
           options.
+
+          With __--verbose__, shows detailed information about one or more
+          options.
 EOTEXT
       );
   }
 
   public function getArguments() {
     return array(
+      'verbose' => array(
+        'help' => pht('Show detailed information about options.'),
+      ),
       '*' => 'argv',
     );
   }
@@ -39,56 +43,130 @@ EOTEXT
 
   public function run() {
     $argv = $this->getArgument('argv');
+    $verbose = $this->getArgument('verbose');
 
     $settings = new ArcanistSettings();
 
     $configuration_manager = $this->getConfigurationManager();
     $configs = array(
-      ArcanistConfigurationManager::CONFIG_SOURCE_SYSTEM  =>
+      ArcanistConfigurationManager::CONFIG_SOURCE_LOCAL =>
-        $configuration_manager->readSystemArcConfig(),
+        $configuration_manager->readLocalArcConfig(),
-      ArcanistConfigurationManager::CONFIG_SOURCE_USER  =>
-        $configuration_manager->readUserArcConfig(),
       ArcanistConfigurationManager::CONFIG_SOURCE_PROJECT =>
         $this->getWorkingCopy()->readProjectConfig(),
-      ArcanistConfigurationManager::CONFIG_SOURCE_LOCAL   =>
+      ArcanistConfigurationManager::CONFIG_SOURCE_USER =>
-        $configuration_manager->readLocalArcConfig(),
+        $configuration_manager->readUserArcConfig(),
+      ArcanistConfigurationManager::CONFIG_SOURCE_SYSTEM =>
+        $configuration_manager->readSystemArcConfig(),
+      ArcanistConfigurationManager::CONFIG_SOURCE_DEFAULT =>
+        $configuration_manager->readDefaultConfig(),
     );
 
     if ($argv) {
       $keys = $argv;
     } else {
       $keys = array_mergev(array_map('array_keys', $configs));
+      $keys = array_merge($keys, $settings->getAllKeys());
       $keys = array_unique($keys);
       sort($keys);
     }
 
+    $console = PhutilConsole::getConsole();
     $multi = (count($keys) > 1);
 
     foreach ($keys as $key) {
-      if ($multi) {
+      $console->writeOut("**%s**\n\n", $key);
-        echo "{$key}\n";
+
-      }
+      if ($verbose) {
-      foreach ($configs as $name => $config) {
+        $help = $settings->getHelp($key);
-        switch ($name) {
+        if (!$help) {
-          case ArcanistConfigurationManager::CONFIG_SOURCE_PROJECT:
+          $help = pht(
-            // Respect older names in project config.
+            '(This configuration value is not recognized by arc. It may '.
-            $val = $this->getWorkingCopy()->getProjectConfig($key);
+            'be misspelled or out of date.)');
-            break;
-          default:
-            $val = idx($config, $key);
-            break;
         }
-        if ($val === null) {
+
-          continue;
+        $console->writeOut("%s\n\n", phutil_console_wrap($help, 4));
-        }
+
-        $val = $settings->formatConfigValueForDisplay($key, $val);
+        $console->writeOut(
-        printf("% 10.10s: %s\n", $name, $val);
+          "%s: %s\n\n",
+          sprintf('% 20.20s', pht('Example Value')),
+          $settings->getExample($key));
+
       }
+
+      $values = array();
+      foreach ($configs as $config_key => $config) {
+        if (array_key_exists($key, $config)) {
+          $values[$config_key] = $config[$key];
+        } else {
+          // If we didn't find a value, look for a legacy value.
+          $source_project = ArcanistConfigurationManager::CONFIG_SOURCE_PROJECT;
+          if ($config_key === $source_project) {
+            $legacy_name = $settings->getLegacyName($key);
+            if (array_key_exists($legacy_name, $config)) {
+              $values[$config_key] = $config[$legacy_name];
+            }
+          }
+        }
+      }
+
+      $console->writeOut(
+        '%s: ',
+        sprintf('% 20.20s', pht('Current Value')));
+
+      if ($values) {
+        $value = head($values);
+        $value = $settings->formatConfigValueForDisplay($key, $value);
+        $console->writeOut("%s\n", $value);
+      } else {
+        $console->writeOut("-\n");
+      }
+
+      $console->writeOut(
+        '%s: ',
+        sprintf('% 20.20s', pht('Current Source')));
+
+      if ($values) {
+        $source = head_key($values);
+        $console->writeOut("%s\n", $source);
+      } else {
+        $console->writeOut("-\n");
+      }
+
+      if ($verbose) {
+        $console->writeOut("\n");
+
+        foreach ($configs as $name => $config) {
+          $have_value = false;
+          if (array_key_exists($name, $values)) {
+            $have_value = true;
+            $value = $values[$name];
+          }
+
+          $console->writeOut(
+            '%s: ',
+            sprintf('% 20.20s', pht('%s Value', $name)));
+
+          if ($have_value) {
+            $console->writeOut(
+              "%s\n",
+              $settings->formatConfigValueForDisplay($key, $value));
+          } else {
+            $console->writeOut("-\n");
+          }
+        }
+      }
+
       if ($multi) {
         echo "\n";
       }
     }
 
+    if (!$verbose) {
+      $console->writeOut(
+        "%s\n",
+        pht('(Run with --verbose for more details.)'));
+    }
+
     return 0;
   }
 

src/workflow/ArcanistSetConfigWorkflow.php

@@ -2,8 +2,6 @@
 
 /**
  * Write configuration settings.
- *
- * @group workflow
  */
 final class ArcanistSetConfigWorkflow extends ArcanistBaseWorkflow {
 
@@ -31,18 +29,12 @@ EOTEXT
           User values are written to '~/.arcrc' on Linux and Mac OS X, and an
           undisclosed location on Windows.  Local values are written to an arc
           directory under either .git, .hg, or .svn as appropriate.
-
-          With __--show__, a description of supported configuration values
-          is shown.
 EOTEXT
       );
   }
 
   public function getArguments() {
     return array(
-      'show' => array(
-        'help' => 'Show available configuration values.',
-      ),
       'local' => array(
         'help' => 'Set a local config value instead of a user one',
       ),
@@ -55,14 +47,10 @@ EOTEXT
   }
 
   public function run() {
-    if ($this->getArgument('show')) {
-      return $this->show();
-    }
-
     $argv = $this->getArgument('argv');
     if (count($argv) != 2) {
       throw new ArcanistUsageException(
-        "Specify a key and a value, or --show.");
+        pht('Specify a key and a value.'));
     }
     $configuration_manager = $this->getConfigurationManager();
 
@@ -124,44 +112,4 @@ EOTEXT
     return 0;
   }
 
-  private function show() {
-    $config_manager = $this->getConfigurationManager();
-
-    $settings = new ArcanistSettings();
-
-    $keys = $settings->getAllKeys();
-    sort($keys);
-    foreach ($keys as $key) {
-      $type = $settings->getType($key);
-      $example = $settings->getExample($key);
-      $help = $settings->getHelp($key);
-
-      $config = $config_manager->getConfigFromAllSources($key);
-
-      $source = head_key($config);
-
-      $value = head($config);
-      $value = $settings->formatConfigValueForDisplay($key, $value);
-
-      echo phutil_console_format("**__%s__** (%s)\n\n", $key, $type);
-      if ($example !== null) {
-        echo phutil_console_format("           Example: %s\n", $example);
-      }
-      if (strlen($value)) {
-        if (strlen($source)) {
-          $source = pht('(from %s config)', $source);
-        }
-        echo phutil_console_format(
-          "   Current Setting: %s %s\n",
-          $value,
-          $source);
-      }
-      echo "\n";
-      echo phutil_console_wrap($help, 4);
-      echo "\n\n\n";
-    }
-
-    return 0;
-  }
-
 }

src/workingcopyidentity/ArcanistWorkingCopyIdentity.php

@@ -113,8 +113,10 @@ final class ArcanistWorkingCopyIdentity {
 
     $console = PhutilConsole::getConsole();
 
+    $looked_in = array();
     foreach ($config_paths as $config_path) {
       $config_file = $config_path.'/.arcconfig';
+      $looked_in[] = $config_file;
       if (Filesystem::pathExists($config_file)) {
         // We always need to examine the filesystem to look for `.arcconfig`
         // so we can set the project root correctly. We might or might not
@@ -133,7 +135,20 @@ final class ArcanistWorkingCopyIdentity {
     }
 
     if ($config === null) {
-      // We didn't find a ".arcconfig" anywhere, so just use an empty array.
+      if ($looked_in) {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Working Copy: Unable to find .arcconfig in any of these '.
+            'locations: %s.',
+            implode(', ', $looked_in)));
+      } else {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Working Copy: No candidate locations for .arcconfig from '.
+            'this working directory.'));
+      }
       $config = array();
     }
 
@@ -299,12 +314,28 @@ final class ArcanistWorkingCopyIdentity {
       $local_path = Filesystem::resolvePath(
         'arc/config',
         $this->localMetaDir);
+
+      $console = PhutilConsole::getConsole();
+
       if (Filesystem::pathExists($local_path)) {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Config: Reading local configuration file "%s"...',
+            $local_path));
+
         $file = Filesystem::readFile($local_path);
         if ($file) {
           return json_decode($file, true);
         }
+      } else {
+        $console->writeLog(
+          "%s\n",
+          pht(
+            'Config: Did not find local configuration at "%s".',
+            $local_path));
       }
+
     }
     return array();
   }