From 3f13e36182871ac6684e6e9fdb128c38a97abe57 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 12 Jan 2011 15:45:17 -0800 Subject: [PATCH] Update arcanist to use the PhutilSymbolLoader. Summary: This should also fix the bug with double help for certain commands Test Plan: Reviewers: CC: --- scripts/arcanist.php | 4 +-- scripts/phutil_analyzer.php | 6 ++-- src/configuration/ArcanistConfiguration.php | 26 ++++++++++------ src/configuration/__init__.php | 2 +- .../ArcanistPhutilModuleLinter.php | 30 +++++++++++-------- src/lint/linter/phutilmodule/__init__.php | 2 +- .../api/subversion/ArcanistSubversionAPI.php | 6 ++-- .../engine/phutil/PhutilUnitTestEngine.php | 25 ++++++++++------ src/unit/engine/phutil/__init__.php | 2 +- src/workflow/lint/ArcanistLintWorkflow.php | 6 +--- src/workflow/lint/__init__.php | 2 +- ...p => ArcanistSvnHookPreCommitWorkflow.php} | 26 ++++++++-------- src/workflow/svn-hook-pre-commit/__init__.php | 3 +- src/workflow/unit/ArcanistUnitWorkflow.php | 7 +---- src/workflow/unit/__init__.php | 3 +- 15 files changed, 81 insertions(+), 69 deletions(-) rename src/workflow/svn-hook-pre-commit/{ArcanistSvnHoookPreCommitWorkflow.php => ArcanistSvnHookPreCommitWorkflow.php} (98%) diff --git a/scripts/arcanist.php b/scripts/arcanist.php index 5091c897..b838da1d 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -23,7 +23,7 @@ phutil_require_module('phutil', 'conduit/client'); phutil_require_module('phutil', 'console'); phutil_require_module('phutil', 'future/exec'); phutil_require_module('phutil', 'filesystem'); -phutil_require_module('phutil', 'autoload'); +phutil_require_module('phutil', 'symbols'); phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'configuration'); @@ -69,7 +69,7 @@ try { $config = $working_copy->getConfig('arcanist_configuration'); if ($config) { - phutil_autoload_class($config); + PhutilSymbolLoader::loadClass($config); $config = new $config(); } else { $config = new ArcanistConfiguration(); diff --git a/scripts/phutil_analyzer.php b/scripts/phutil_analyzer.php index 67b679d7..a31b8d37 100755 --- a/scripts/phutil_analyzer.php +++ b/scripts/phutil_analyzer.php @@ -23,7 +23,9 @@ $builtin_functions = get_defined_functions(); $builtin_functions = $builtin_functions['internal']; $builtin = array( - 'class' => array_fill_keys($builtin_classes, true), + 'class' => array_fill_keys($builtin_classes, true) + array( + 'PhutilBootloader' => true, + ), 'function' => array_fill_keys($builtin_functions, true) + array( 'empty' => true, 'isset' => true, @@ -31,8 +33,6 @@ $builtin = array( 'print' => true, 'exit' => true, 'die' => true, - - 'phutil_module_exists' => true, ), 'interface' => array_fill_keys($builtin_interfaces, true), ); diff --git a/src/configuration/ArcanistConfiguration.php b/src/configuration/ArcanistConfiguration.php index 17a4d4fe..e8659267 100644 --- a/src/configuration/ArcanistConfiguration.php +++ b/src/configuration/ArcanistConfiguration.php @@ -29,12 +29,7 @@ class ArcanistConfiguration { return null; } - if (!phutil_module_exists('arcanist', 'workflow/'.$command)) { - return null; - } - $workflow_class = 'Arcanist'.ucfirst($command).'Workflow'; - $workflow_class = preg_replace_callback( '/-([a-z])/', array( @@ -43,18 +38,31 @@ class ArcanistConfiguration { ), $workflow_class); - phutil_autoload_class($workflow_class); + $symbols = id(new PhutilSymbolLoader()) + ->setType('class') + ->setName($workflow_class) + ->setLibrary('arcanist') + ->selectAndLoadSymbols(); + + if (!$symbols) { + return null; + } return newv($workflow_class, array()); } public function buildAllWorkflows() { - $classes = phutil_find_class_descendants('ArcanistBaseWorkflow'); + $symbols = id(new PhutilSymbolLoader()) + ->setType('class') + ->setAncestorClass('ArcanistBaseWorkflow') + ->setLibrary('arcanist') + ->selectAndLoadSymbols(); + $workflows = array(); - foreach ($classes as $class) { + foreach ($symbols as $symbol) { + $class = $class['name']; $name = preg_replace('/^Arcanist(\w+)Workflow$/', '\1', $class); $name = strtolower($name); - phutil_autoload_class($class); $workflows[$name] = newv($class, array()); } diff --git a/src/configuration/__init__.php b/src/configuration/__init__.php index 7c8a7b8d..27e15bce 100644 --- a/src/configuration/__init__.php +++ b/src/configuration/__init__.php @@ -6,7 +6,7 @@ -phutil_require_module('phutil', 'autoload'); +phutil_require_module('phutil', 'symbols'); phutil_require_module('phutil', 'utils'); diff --git a/src/lint/linter/phutilmodule/ArcanistPhutilModuleLinter.php b/src/lint/linter/phutilmodule/ArcanistPhutilModuleLinter.php index 85077b3f..8c3318d9 100644 --- a/src/lint/linter/phutilmodule/ArcanistPhutilModuleLinter.php +++ b/src/lint/linter/phutilmodule/ArcanistPhutilModuleLinter.php @@ -241,13 +241,16 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter { $places); if ($type == 'class' || $type == 'interface') { - $class_spec = PhutilLibraryMapRegistry::findClass( - $library = null, - $name); - if ($class_spec) { + $loader = new PhutilSymbolLoader(); + $loader->setType($type); + $loader->setName($name); + $symbols = $loader->selectSymbolsWithoutLoading(); + if ($symbols) { + $class_spec = reset($symbols); try { - $loaded = phutil_autoload_class($name); - } catch (PhutilLibraryLoadException $ex) { + $loader->selectAndLoadSymbols(); + $loaded = true; + } catch (PhutilMissingSymbolException $ex) { $loaded = false; } if ($loaded) { @@ -279,13 +282,16 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter { } } } else { - $func_spec = PhutilLibraryMapRegistry::findFunction( - $library = null, - $name); - if ($func_spec) { + $loader = new PhutilSymbolLoader(); + $loader->setType($type); + $loader->setName($name); + $symbols = $loader->selectSymbolsWithoutLoading(); + if ($symbols) { + $func_spec = reset($symbols); try { - $loaded = phutil_autoload_function($name); - } catch (PhutilLibraryLoadException $ex) { + $loader->selectAndLoadSymbols(); + $loaded = true; + } catch (PhutilMissingSymbolException $ex) { $loaded = false; } if ($loaded) { diff --git a/src/lint/linter/phutilmodule/__init__.php b/src/lint/linter/phutilmodule/__init__.php index 97c77273..28cd14ce 100644 --- a/src/lint/linter/phutilmodule/__init__.php +++ b/src/lint/linter/phutilmodule/__init__.php @@ -9,12 +9,12 @@ phutil_require_module('arcanist', 'lint/linter/base'); phutil_require_module('arcanist', 'lint/severity'); -phutil_require_module('phutil', 'autoload'); phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'future'); phutil_require_module('phutil', 'future/exec'); phutil_require_module('phutil', 'moduleutils'); phutil_require_module('phutil', 'parser/xhpast/bin'); +phutil_require_module('phutil', 'symbols'); phutil_require_source('ArcanistPhutilModuleLinter.php'); diff --git a/src/repository/api/subversion/ArcanistSubversionAPI.php b/src/repository/api/subversion/ArcanistSubversionAPI.php index 84430686..b3f80b15 100644 --- a/src/repository/api/subversion/ArcanistSubversionAPI.php +++ b/src/repository/api/subversion/ArcanistSubversionAPI.php @@ -171,7 +171,7 @@ class ArcanistSubversionAPI extends ArcanistRepositoryAPI { // To reproduce, do: // // $ ln -s working_copy working_link - // $ svn info working_copy # ok + // $ svn info working_copy # ok // $ svn info working_link # fails // // Work around this by cd-ing into the directory before executing @@ -411,7 +411,7 @@ EODIFF; return $blame; } - + public function getOriginalFileData($path) { // SVN issues warnings for nonexistent paths, directories, etc., but still // returns no error code. However, for new paths in the working copy it @@ -425,7 +425,7 @@ EODIFF; } return $stdout; } - + public function getCurrentFileData($path) { $full_path = $this->getPath($path); if (Filesystem::pathExists($full_path)) { diff --git a/src/unit/engine/phutil/PhutilUnitTestEngine.php b/src/unit/engine/phutil/PhutilUnitTestEngine.php index 8938e086..41c9ca8d 100644 --- a/src/unit/engine/phutil/PhutilUnitTestEngine.php +++ b/src/unit/engine/phutil/PhutilUnitTestEngine.php @@ -20,6 +20,8 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine { public function run() { + $bootloader = PhutilBootloader::getInstance(); + $tests = array(); foreach ($this->getPaths() as $path) { $library_root = phutil_get_library_root_for_path($path); @@ -41,7 +43,10 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine { if (basename($library_path) == '__tests__') { // Okay, this is a __tests__ module. } else { - if (phutil_module_exists($library_name, $library_path.'/__tests__')) { + $exists = $bootloader->moduleExists( + $library_name, + $library_path.'/__tests__'); + if ($exists) { // This is a module which has a __tests__ module in it. $path .= '/__tests__'; } else { @@ -72,14 +77,16 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine { } $run_tests = array(); - $all_test_classes = phutil_find_class_descendants('ArcanistPhutilTestCase'); - $all_test_classes = array_fill_keys($all_test_classes, true); foreach ($tests as $test) { - $local_classes = phutil_find_classes_declared_in_module( - $test['library'], - $test['module']); - $local_classes = array_fill_keys($local_classes, true); - $run_tests += array_intersect($local_classes, $all_test_classes); + $symbols = id(new PhutilSymbolLoader()) + ->setType('class') + ->setLibrary($test['library']) + ->setModule($test['module']) + ->setAncestorClass('ArcanistPhutilTestCase') + ->selectAndLoadSymbols(); + foreach ($symbols as $symbol) { + $run_tests[$symbol['name']] = true; + } } $run_tests = array_keys($run_tests); @@ -90,7 +97,7 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine { $results = array(); foreach ($run_tests as $test_class) { - phutil_autoload_class($test_class); + PhutilSymbolLoader::loadClass($test_class); $test_case = newv($test_class, array()); $results[] = $test_case->run(); } diff --git a/src/unit/engine/phutil/__init__.php b/src/unit/engine/phutil/__init__.php index 11320952..c7db7cc4 100644 --- a/src/unit/engine/phutil/__init__.php +++ b/src/unit/engine/phutil/__init__.php @@ -9,9 +9,9 @@ phutil_require_module('arcanist', 'exception/usage/noeffect'); phutil_require_module('arcanist', 'unit/engine/base'); -phutil_require_module('phutil', 'autoload'); phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'moduleutils'); +phutil_require_module('phutil', 'symbols'); phutil_require_module('phutil', 'utils'); diff --git a/src/workflow/lint/ArcanistLintWorkflow.php b/src/workflow/lint/ArcanistLintWorkflow.php index 573cede7..c031e68c 100644 --- a/src/workflow/lint/ArcanistLintWorkflow.php +++ b/src/workflow/lint/ArcanistLintWorkflow.php @@ -136,11 +136,7 @@ EOTEXT "specify a lint engine."); } - $ok = phutil_autoload_class($engine); - if (!$ok) { - throw new ArcanistUsageException( - "Configured lint engine '{$engine}' could not be loaded."); - } + PhutilSymbolLoader::loadClass($engine); $engine = newv($engine, array()); $engine->setWorkingCopy($working_copy); diff --git a/src/workflow/lint/__init__.php b/src/workflow/lint/__init__.php index 9f6a024a..4560471f 100644 --- a/src/workflow/lint/__init__.php +++ b/src/workflow/lint/__init__.php @@ -14,12 +14,12 @@ phutil_require_module('arcanist', 'lint/severity'); phutil_require_module('arcanist', 'repository/api/base'); phutil_require_module('arcanist', 'workflow/base'); -phutil_require_module('phutil', 'autoload'); phutil_require_module('phutil', 'console'); phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'filesystem/filelist'); phutil_require_module('phutil', 'filesystem/tempfile'); phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'symbols'); phutil_require_module('phutil', 'utils'); phutil_require_module('phutil', 'xsprintf/csprintf'); diff --git a/src/workflow/svn-hook-pre-commit/ArcanistSvnHoookPreCommitWorkflow.php b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php similarity index 98% rename from src/workflow/svn-hook-pre-commit/ArcanistSvnHoookPreCommitWorkflow.php rename to src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php index 2e13768d..fdc0c654 100644 --- a/src/workflow/svn-hook-pre-commit/ArcanistSvnHoookPreCommitWorkflow.php +++ b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php @@ -46,12 +46,12 @@ EOTEXT // TODO: Do stuff with commit message. var_dump($commit_message); - + list($changed) = execx( 'svnlook changed --transaction %s %s', $transaction, $repository); - + $paths = array(); $changed = explode("\n", trim($changed)); foreach ($changed as $line) { @@ -59,13 +59,13 @@ EOTEXT preg_match('/^..\s*(.*)$/', $line, $matches); $paths[$matches[1]] = strlen($matches[1]); } - + $resolved = array(); $failed = array(); $missing = array(); $found = array(); asort($paths); - + foreach ($paths as $path => $length) { foreach ($resolved as $rpath => $root) { if (!strncmp($path, $rpath, strlen($rpath))) { @@ -74,12 +74,12 @@ EOTEXT } } $config = $path; - + if (basename($config) == '.arcconfig') { $resolved[$config] = $config; continue; } - + $config = rtrim($config, '/'); $last_config = $config; do { @@ -109,12 +109,12 @@ EOTEXT } $last_config = $config; } while (true); - + if (empty($resolved[$path])) { $failed[] = $path; } } - + if ($failed && $resolved) { $failed_paths = ' '.implode("\n ", $failed); $resolved_paths = ' '.implode("\n ", array_keys($resolved)); @@ -127,12 +127,12 @@ EOTEXT "Files not in projects:\n\n". $failed_paths); } - + if (!$resolved) { // None of the affected paths are beneath a .arcconfig file. return 3; } - + $groups = array(); foreach ($resolved as $path => $project) { $groups[$project][] = $path; @@ -150,10 +150,10 @@ EOTEXT "only that project.\n\n". $message); } - + $project_root = key($groups); $paths = reset($groups); - + $data = array(); foreach ($paths as $path) { list($err, $filedata) = exec_manual( @@ -163,7 +163,7 @@ EOTEXT $path); $data[$path] = $err ? null : $filedata; } - + // TODO: Do stuff with data. var_dump($data); diff --git a/src/workflow/svn-hook-pre-commit/__init__.php b/src/workflow/svn-hook-pre-commit/__init__.php index 921b10ec..e3f69614 100644 --- a/src/workflow/svn-hook-pre-commit/__init__.php +++ b/src/workflow/svn-hook-pre-commit/__init__.php @@ -6,10 +6,11 @@ +phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('phutil', 'console'); phutil_require_module('phutil', 'future/exec'); -phutil_require_source('ArcanistSvnHoookPreCommitWorkflow.php'); +phutil_require_source('ArcanistSvnHookPreCommitWorkflow.php'); diff --git a/src/workflow/unit/ArcanistUnitWorkflow.php b/src/workflow/unit/ArcanistUnitWorkflow.php index 0424afbf..800d4d0b 100644 --- a/src/workflow/unit/ArcanistUnitWorkflow.php +++ b/src/workflow/unit/ArcanistUnitWorkflow.php @@ -62,12 +62,6 @@ EOTEXT "to specify a unit test engine."); } - $ok = phutil_autoload_class($engine_class); - if (!$ok) { - throw new ArcanistUsageException( - "Configured unit test engine '{$engine_class}' could not be loaded."); - } - $repository_api = $this->getRepositoryAPI(); if ($this->getArgument('paths')) { @@ -79,6 +73,7 @@ EOTEXT $paths = array_keys($paths); } + PhutilSymbolLoader::loadClass($engine_class); $engine = newv($engine_class, array()); $engine->setWorkingCopy($working_copy); $engine->setPaths($paths); diff --git a/src/workflow/unit/__init__.php b/src/workflow/unit/__init__.php index 9ac1daea..b408e2ae 100644 --- a/src/workflow/unit/__init__.php +++ b/src/workflow/unit/__init__.php @@ -6,13 +6,12 @@ -phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'exception/usage/noengine'); phutil_require_module('arcanist', 'unit/result'); phutil_require_module('arcanist', 'workflow/base'); -phutil_require_module('phutil', 'autoload'); phutil_require_module('phutil', 'console'); +phutil_require_module('phutil', 'symbols'); phutil_require_module('phutil', 'utils');