Allow global config to load libraries and set test engines

Summary:
Khan Academy is looking into lint configuration, but doesn't use ".arcconfig" because they have a large number of repositories. Making configuration more flexible generally gives us more options for onboarding installs.

  - Currently, only project config (".arcconfig") can load libraries. Allow user config ("~/.arcrc") to load libraries as well.
  - Currently, only project config can set lint/unit engines. Allow user config to set default lint/unit engines.
  - Add some type checking to "arc set-config".
  - Add "arc set-config --show".

Test Plan:
  - **load**
    - Ran `arc set-config load xxx`, got error about format.
    - Ran `arc set-config load ["apple"]`, got warning on running 'arc' commands (no such library) but was able to run 'arc set-config' again to clear it.
    - Ran `arc set-config load ["/path/to/a/lib/src/"]`, worked.
    - Ran `arc list --trace`, verified my library loaded in addition to `.arcconfig` libraries.
    - Ran `arc list --load-phutil-library=xxx --trace`, verified only that library loaded.
    - Ran `arc list --trace --load-phutil-library=apple --trace`, got hard error about bad library.
    - Set `.arcconfig` to point at a bad library, verified hard error.
  - **lint.engine** / **unit.engine**
    - Removed lint engine from `.arcconfig`, ran "arc lint", got a run with specified engine.
    - Removed unit engine from `.arcconfig`, ran "arc unit", got a run with specified engine.
  - **--show**
    - Ran `arc set-config --show`.
  - **misc**
    - Ran `arc get-config`.

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

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

scripts/arcanist.php

@@ -59,87 +59,46 @@ try {
     throw new ArcanistUsageException("No command provided. Try 'arc help'.");
   }
 
+  $global_config = ArcanistBaseWorkflow::readGlobalArcConfig();
   $working_copy = ArcanistWorkingCopyIdentity::newFromPath($working_directory);
+
+  // Load additional libraries, which can provide new classes like configuration
+  // overrides, linters and lint engines, unit test engines, etc.
+
+  // If the user specified "--load-phutil-library" one or more times from
+  // the command line, we load those libraries **instead** of whatever else
+  // is configured. This is basically a debugging feature to let you force
+  // specific libraries to load regardless of the state of the world.
   if ($load) {
-    $libs = $load;
+    // Load the flag libraries. These must load, since the user specified them
+    // explicitly.
+    arcanist_load_libraries(
+      $load,
+      $must_load = true,
+      $lib_source = 'a "--load-phutil-library" flag',
+      $working_copy,
+      $config_trace_mode);
   } else {
-    $libs = $working_copy->getConfig('phutil_libraries');
+    // Load libraries in global 'load' config, as per "arc set-config load". We
-  }
+    // need to fail softly if these break because errors would prevent the user
-  if ($libs) {
+    // from running "arc set-config" to correct them.
-    foreach ($libs as $name => $location) {
+    arcanist_load_libraries(
+      idx($global_config, 'load', array()),
+      $must_load = false,
+      $lib_source = 'the "load" setting in global config',
+      $working_copy,
+      $config_trace_mode);
 
-      // Try to resolve the library location. We look in several places, in
+    // Load libraries in ".arcconfig". Libraries here must load.
-      // order:
+    arcanist_load_libraries(
-      //
+      $working_copy->getConfig('phutil_libraries'),
-      //  1. Inside the working copy. This is for phutil libraries within the
+      $must_load = true,
-      //     project. For instance "library/src" will resolve to
+      $lib_source = 'the "phutil_libraries" setting in ".arcconfig"',
-      //     "./library/src" if it exists.
+      $working_copy,
-      //  2. In the same directory as the working copy. This allows you to
+      $config_trace_mode);
-      //     check out a library alongside a working copy and reference it.
-      //     If we haven't resolved yet, "library/src" will try to resolve to
-      //     "../library/src" if it exists.
-      //  3. Using normal libphutil resolution rules. Generally, this means
-      //     that it checks for libraries next to libphutil, then libraries
-      //     in the PHP include_path.
-
-      $resolved = false;
-
-      // Check inside the working copy.
-      $resolved_location = Filesystem::resolvePath(
-        $location,
-        $working_copy->getProjectRoot());
-      if (Filesystem::pathExists($resolved_location)) {
-        $location = $resolved_location;
-        $resolved = true;
-      }
-
-      // If we didn't find anything, check alongside the working copy.
-      if (!$resolved) {
-        $resolved_location = Filesystem::resolvePath(
-          $location,
-          dirname($working_copy->getProjectRoot()));
-        if (Filesystem::pathExists($resolved_location)) {
-          $location = $resolved_location;
-          $resolved = true;
-        }
-      }
-
-      if ($config_trace_mode) {
-        echo "Loading phutil library '{$name}' from '{$location}'...\n";
-      }
-
-      try {
-        phutil_load_library($location);
-      } catch (PhutilBootloaderException $ex) {
-        $error_msg = sprintf(
-          'Failed to load library "%s" at location "%s". Please check the '.
-          '"phutil_libraries" setting in your .arcconfig file. Refer to '.
-          '<http://www.phabricator.com/docs/phabricator/article/'.
-          'Arcanist_User_Guide_Configuring_a_New_Project.html> '.
-          'for more information.',
-          $name,
-          $location);
-        throw new ArcanistUsageException($error_msg);
-      } catch (PhutilLibraryConflictException $ex) {
-        if ($ex->getLibrary() != 'arcanist') {
-          throw $ex;
-        }
-
-        $arc_dir = dirname(dirname(__FILE__));
-        $error_msg =
-          "You are trying to run one copy of Arcanist on another copy of ".
-          "Arcanist. This operation is not supported. To execute Arcanist ".
-          "operations against this working copy, run './bin/arc' (from the ".
-          "current working copy) not some other copy of 'arc' (you ran one ".
-          "from '{$arc_dir}').";
-
-        throw new ArcanistUsageException($error_msg);
-      }
-    }
   }
 
   $user_config = ArcanistBaseWorkflow::readUserConfigurationFile();
-  $global_config = ArcanistBaseWorkflow::readGlobalArcConfig();
 
   $config = $working_copy->getConfig('arcanist_configuration');
   if ($config) {
@@ -438,3 +397,91 @@ function die_with_bad_php($message) {
   echo "\n\n";
   exit(1);
 }
+
+
+function arcanist_load_libraries(
+  $load,
+  $must_load,
+  $lib_source,
+  ArcanistWorkingCopyIdentity $working_copy,
+  $config_trace_mode) {
+
+  if (!$load) {
+    return;
+  }
+
+  foreach ($load as $location) {
+
+    // Try to resolve the library location. We look in several places, in
+    // order:
+    //
+    //  1. Inside the working copy. This is for phutil libraries within the
+    //     project. For instance "library/src" will resolve to
+    //     "./library/src" if it exists.
+    //  2. In the same directory as the working copy. This allows you to
+    //     check out a library alongside a working copy and reference it.
+    //     If we haven't resolved yet, "library/src" will try to resolve to
+    //     "../library/src" if it exists.
+    //  3. Using normal libphutil resolution rules. Generally, this means
+    //     that it checks for libraries next to libphutil, then libraries
+    //     in the PHP include_path.
+    //
+    // Note that absolute paths will just resolve absolutely through rule (1).
+
+    $resolved = false;
+
+    // Check inside the working copy. This also checks absolute paths, since
+    // they'll resolve absolute and just ignore the project root.
+    $resolved_location = Filesystem::resolvePath(
+      $location,
+      $working_copy->getProjectRoot());
+    if (Filesystem::pathExists($resolved_location)) {
+      $location = $resolved_location;
+      $resolved = true;
+    }
+
+    // If we didn't find anything, check alongside the working copy.
+    if (!$resolved) {
+      $resolved_location = Filesystem::resolvePath(
+        $location,
+        dirname($working_copy->getProjectRoot()));
+      if (Filesystem::pathExists($resolved_location)) {
+        $location = $resolved_location;
+        $resolved = true;
+      }
+    }
+
+    if ($config_trace_mode) {
+      echo "Loading phutil library from '{$location}'...\n";
+    }
+
+    $error = null;
+    try {
+      phutil_load_library($location);
+    } catch (PhutilBootloaderException $ex) {
+      $error = "Failed to load phutil library at location '{$location}'. ".
+               "This library is specified by {$lib_source}. Check that the ".
+               "setting is correct and the library is located in the right ".
+               "place.";
+      if ($must_load) {
+        throw new ArcanistUsageException($error);
+      } else {
+        file_put_contents(
+          'php://stderr',
+          phutil_console_wrap('WARNING: '.$error."\n\n"));
+      }
+    } catch (PhutilLibraryConflictException $ex) {
+      if ($ex->getLibrary() != 'arcanist') {
+        throw $ex;
+      }
+      $arc_dir = dirname(dirname(__FILE__));
+      $error =
+        "You are trying to run one copy of Arcanist on another copy of ".
+        "Arcanist. This operation is not supported. To execute Arcanist ".
+        "operations against this working copy, run './bin/arc' (from the ".
+        "current working copy) not some other copy of 'arc' (you ran one ".
+        "from '{$arc_dir}').";
+      throw new ArcanistUsageException($error);
+    }
+  }
+}

src/workflow/base/ArcanistBaseWorkflow.php

@@ -1153,4 +1153,16 @@ abstract class ArcanistBaseWorkflow {
     return $this->repositoryEncoding;
   }
 
+  protected static function formatConfigValueForDisplay($value) {
+    if (is_array($value)) {
+      // TODO: Both json_encode() and PhutilJSON do a bad job with one-liners.
+      // PhutilJSON splits them across a bunch of lines, while json_encode()
+      // escapes all kinds of stuff like "/". It would be nice if PhutilJSON
+      // had a mode for pretty one-liners.
+      $value = json_encode($value);
+      return $value;
+    }
+    return $value;
+  }
+
 }

src/workflow/get-config/ArcanistGetConfigWorkflow.php

@@ -57,7 +57,8 @@ EOTEXT
     }
 
     foreach ($keys as $key) {
-      echo "{$key} = ".idx($config, $key)."\n";
+      $val = self::formatConfigValueForDisplay(idx($config, $key));
+      echo "{$key} = {$val}\n";
     }
 
     return 0;

src/workflow/lint/ArcanistLintWorkflow.php

@@ -139,7 +139,7 @@ EOTEXT
 
     $engine = $this->getArgument('engine');
     if (!$engine) {
-      $engine = $working_copy->getConfig('lint_engine');
+      $engine = $working_copy->getConfigFromAnySource('lint.engine');
       if (!$engine) {
         throw new ArcanistNoEngineException(
           "No lint engine configured for this project. Edit .arcconfig to ".

src/workflow/set-config/ArcanistSetConfigWorkflow.php

@@ -37,20 +37,31 @@ EOTEXT
 
           Values are written to '~/.arcrc' on Linux and Mac OS X, and an
           undisclosed location on Windows.
+
+          With __--show__, a description of supported configuration values
+          is shown.
 EOTEXT
       );
   }
 
   public function getArguments() {
     return array(
+      'show' => array(
+        'help' => 'Show available configuration values.',
+      ),
       '*' => 'argv',
     );
   }
 
   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.");
+      throw new ArcanistUsageException(
+        "Specify a key and a value, or --show.");
     }
 
     $config = self::readGlobalArcConfig();
@@ -73,9 +84,14 @@ EOTEXT
         echo "Deleted key '{$key}' (was '{$old}').\n";
       }
     } else {
+      $val = $this->parse($key, $val);
+
       $config[$key] = $val;
       self::writeGlobalArcConfig($config);
 
+      $val = self::formatConfigValueForDisplay($val);
+      $old = self::formatConfigValueForDisplay($old);
+
       if ($old === null) {
         echo "Set key '{$key}' = '{$val}'.\n";
       } else {
@@ -86,4 +102,83 @@ EOTEXT
     return 0;
   }
 
+  private function show() {
+    $keys = array(
+      'default'   => array(
+        'help' =>
+          'The URI of a Phabricator install to connect to by default, if '.
+          'arc is run in a project without a Phabricator URI or run outside '.
+          'of a project.',
+        'example' => 'http://phabricator.example.com/',
+      ),
+      'load'      => array(
+        'help' =>
+          'A list of paths to phutil libraries that should be loaded at '.
+          'startup. This can be used to make classes available, like lint or '.
+          'unit test engines.',
+        'example' => '["/var/arc/customlib/src"]',
+      ),
+      'lint.engine' => array(
+        'help' =>
+          'The name of a default lint engine to use, if no lint engine is '.
+          'specified by the current project.',
+        'example' => 'ExampleLintEngine',
+      ),
+      'unit.engine' => array(
+        'help' =>
+          'The name of a default unit test engine to use, if no unit test '.
+          'engine is specified by the current project.',
+        'example' => 'ExampleUnitTestEngine',
+      ),
+    );
+
+    $config = self::readGlobalArcConfig();
+
+    foreach ($keys as $key => $spec) {
+      $type = $this->getType($key);
+
+      $value = idx($config, $key);
+      $value = self::formatConfigValueForDisplay($value);
+
+      echo phutil_console_format("**__%s__** (%s)\n\n", $key, $type);
+      echo phutil_console_format("           Example: %s\n", $spec['example']);
+      if (strlen($value)) {
+        echo phutil_console_format("    Global Setting: %s\n", $value);
+      }
+      echo "\n";
+      echo phutil_console_wrap($spec['help'], 4);
+      echo "\n\n\n";
+    }
+
+    return 0;
+  }
+
+  private function getType($key) {
+    static $types = array(
+      'load'  => 'list',
+    );
+
+    return idx($types, $key, 'string');
+  }
+
+  private function parse($key, $val) {
+    $type = $this->getType($key);
+
+    switch ($type) {
+      case 'string':
+        return $val;
+      case 'list':
+        $val = json_decode($val, true);
+        if (!is_array($val)) {
+          $example = '["apple", "banana", "cherry"]';
+          throw new ArcanistUsageException(
+            "Value for key '{$key}' must be specified as a JSON-encoded ".
+            "list. Example: {$example}");
+        }
+        return $val;
+      default:
+        throw new Exception("Unknown config key type '{$type}'!");
+    }
+  }
+
 }

src/workflow/unit/ArcanistUnitWorkflow.php

@@ -103,7 +103,7 @@ EOTEXT
 
     $engine_class = $this->getArgument(
       'engine',
-      $working_copy->getConfig('unit_engine'));
+      $working_copy->getConfigFromAnySource('unit.engine'));
 
     if (!$engine_class) {
       throw new ArcanistNoEngineException(

src/workingcopyidentity/ArcanistWorkingCopyIdentity.php

@@ -19,6 +19,9 @@
 /**
  * Interfaces with basic information about the working copy.
  *
+ *
+ * @task config
+ *
  * @group workingcopy
  */
 final class ArcanistWorkingCopyIdentity {
@@ -108,11 +111,59 @@ final class ArcanistWorkingCopyIdentity {
     return $this->getConfig('conduit_uri');
   }
 
-  public function getConfig($key) {
+
-    if (!empty($this->projectConfig[$key])) {
+/* -(  Config  )------------------------------------------------------------- */
-      return $this->projectConfig[$key];
+
+
+  /**
+   * Read a configuration directive from project configuration. This reads ONLY
+   * permanent project configuration (i.e., ".arcconfig"), not other
+   * configuration sources. See @{method:getConfigFromAnySource} to read from
+   * user configuration.
+   *
+   * @param key   Key to read.
+   * @param wild  Default value if key is not found.
+   * @return wild Value, or default value if not found.
+   *
+   * @task config
+   */
+  public function getConfig($key, $default = null) {
+    return idx($this->projectConfig, $key, $default);
   }
-    return null;
+
+
+  /**
+   * Read a configuration directive from any available configuration source.
+   * In contrast to @{method:getConfig}, this will look for the directive in
+   * user configuration in addition to project configuration.
+   *
+   * @param key   Key to read.
+   * @param wild  Default value if key is not found.
+   * @return wild Value, or default value if not found.
+   *
+   * @task config
+   */
+  public function getConfigFromAnySource($key, $default = null) {
+    $pval = $this->getConfig($key);
+    if ($pval !== null) {
+      return $pval;
+    }
+
+    // Test for older names.
+
+    static $deprecated_names = array(
+      'lint.engine' => 'lint_engine',
+      'unit.engine' => 'unit_engine',
+    );
+    if (isset($deprecated_names[$key])) {
+      $pval = $this->getConfig($deprecated_names[$key]);
+      if ($pval !== null) {
+        return $pval;
+      }
+    }
+
+    $global_config = ArcanistBaseWorkflow::readGlobalArcConfig();
+    return idx($global_config, $key, $default);
   }
 
 }