From 8b7a5157f8388f8222ac463798407ff9a9da6f27 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 31 May 2012 11:41:39 -0700 Subject: [PATCH] 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 | 195 +++++++++++------- src/workflow/base/ArcanistBaseWorkflow.php | 12 ++ .../get-config/ArcanistGetConfigWorkflow.php | 3 +- src/workflow/lint/ArcanistLintWorkflow.php | 2 +- .../set-config/ArcanistSetConfigWorkflow.php | 97 ++++++++- src/workflow/unit/ArcanistUnitWorkflow.php | 2 +- .../ArcanistWorkingCopyIdentity.php | 59 +++++- 7 files changed, 288 insertions(+), 82 deletions(-) diff --git a/scripts/arcanist.php b/scripts/arcanist.php index b6fdcd27..c48e5c1e 100755 --- a/scripts/arcanist.php +++ b/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'); - } - if ($libs) { - foreach ($libs as $name => $location) { + // 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 + // from running "arc set-config" to correct them. + 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 - // 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. - - $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 '. - ' '. - '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); - } - } + // Load libraries in ".arcconfig". Libraries here must load. + arcanist_load_libraries( + $working_copy->getConfig('phutil_libraries'), + $must_load = true, + $lib_source = 'the "phutil_libraries" setting in ".arcconfig"', + $working_copy, + $config_trace_mode); } $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); + } + } +} diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index 426631b1..0193a2bf 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/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; + } + } diff --git a/src/workflow/get-config/ArcanistGetConfigWorkflow.php b/src/workflow/get-config/ArcanistGetConfigWorkflow.php index 497fc409..8ba82ea2 100644 --- a/src/workflow/get-config/ArcanistGetConfigWorkflow.php +++ b/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; diff --git a/src/workflow/lint/ArcanistLintWorkflow.php b/src/workflow/lint/ArcanistLintWorkflow.php index 0f6c7065..43c35ddb 100644 --- a/src/workflow/lint/ArcanistLintWorkflow.php +++ b/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 ". diff --git a/src/workflow/set-config/ArcanistSetConfigWorkflow.php b/src/workflow/set-config/ArcanistSetConfigWorkflow.php index 4effedde..6c415bad 100644 --- a/src/workflow/set-config/ArcanistSetConfigWorkflow.php +++ b/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}'!"); + } + } + } diff --git a/src/workflow/unit/ArcanistUnitWorkflow.php b/src/workflow/unit/ArcanistUnitWorkflow.php index 02c873fe..4dcc44de 100644 --- a/src/workflow/unit/ArcanistUnitWorkflow.php +++ b/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( diff --git a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php index c2400404..5adae0c6 100644 --- a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php +++ b/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])) { - return $this->projectConfig[$key]; + +/* -( Config )------------------------------------------------------------- */ + + + /** + * 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); + } + + + /** + * 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; } - return null; + + // 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); } }