From 3a559ddd1398c0050469b3a77bfb0f10eda88e4a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 May 2011 16:30:22 -0700 Subject: [PATCH] 'arc liberate', convenience wrapper for various libphutil operations Summary: The story for creating and maintaining libphutil libraries and modules is pretty terrible right now: you need to know a bunch of secret scripts and dark magic. Provide 'arc liberate' which endeavors to always do the right thing and put a library in the correct state. Test Plan: Ran liberate on libphutil, arcanist, phabricator; created new libphutil libraries, added classes to them, liberated everything, introduced errors etc and liberated that stuff, nothing was obviously broken in a terrible way..? Reviewed By: aran Reviewers: jungejason, tuomaspelkonen, aran CC: aran, epriestley Differential Revision: 269 --- scripts/phutil_analyzer.php | 1 + scripts/phutil_mapper.php | 35 +- src/__phutil_library_map__.php | 4 + src/lint/engine/base/ArcanistLintEngine.php | 2 +- .../liberate/ArcanistLiberateLintEngine.php | 38 ++ src/lint/engine/liberate/__init__.php | 13 + .../ArcanistPhutilModuleLinter.php | 14 +- src/workflow/help/ArcanistHelpWorkflow.php | 3 + .../liberate/ArcanistLiberateWorkflow.php | 336 ++++++++++++++++++ src/workflow/liberate/__init__.php | 26 ++ 10 files changed, 461 insertions(+), 11 deletions(-) create mode 100644 src/lint/engine/liberate/ArcanistLiberateLintEngine.php create mode 100644 src/lint/engine/liberate/__init__.php create mode 100644 src/workflow/liberate/ArcanistLiberateWorkflow.php create mode 100644 src/workflow/liberate/__init__.php diff --git a/scripts/phutil_analyzer.php b/scripts/phutil_analyzer.php index e519ae13..80f5201f 100755 --- a/scripts/phutil_analyzer.php +++ b/scripts/phutil_analyzer.php @@ -33,6 +33,7 @@ $builtin = array( 'print' => true, 'exit' => true, 'die' => true, + 'phutil_load_library' => true, ), 'interface' => array_fill_keys($builtin_interfaces, true), ); diff --git a/scripts/phutil_mapper.php b/scripts/phutil_mapper.php index 7edf1133..7ad38d3f 100755 --- a/scripts/phutil_mapper.php +++ b/scripts/phutil_mapper.php @@ -19,6 +19,16 @@ require_once dirname(__FILE__).'/__init_script__.php'; +$liberate_mode = false; +for ($ii = 0; $ii < $argc; $ii++) { + if ($argv[$ii] == '--find-paths-for-liberate') { + $liberate_mode = true; + unset($argv[$ii]); + } +} +$argv = array_values($argv); +$argc = count($argv); + if ($argc != 2) { $self = basename($argv[0]); echo "usage: {$self} \n"; @@ -35,6 +45,10 @@ if (!@file_exists($root.'/__phutil_library_init__.php')) { throw new Exception("Provided path is not a phutil library."); } +if ($liberate_mode) { + ob_start(); +} + echo "Finding phutil modules...\n"; $files = id(new FileFinder($root)) ->withType('f') @@ -81,19 +95,24 @@ if ($cache) { $specs = array(); -$futures = array(); +$need_update = array(); foreach ($signatures as $module => $signature) { if (isset($signature_cache[$module]) && $signature_cache[$module]['signature'] == $signature) { $specs[$module] = $signature_cache[$module]; } else { - $futures[$module] = new ExecFuture( - '%s %s', - dirname(__FILE__).'/phutil_analyzer.php', - $root.'/'.$module); + $need_update[$module] = true; } } +$futures = array(); +foreach ($need_update as $module => $ignored) { + $futures[$module] = new ExecFuture( + '%s %s', + dirname(__FILE__).'/phutil_analyzer.php', + $root.'/'.$module); +} + if ($futures) { echo "Found ".count($specs)." modules in cache; ". "analyzing ".count($futures)." modified modules"; @@ -180,6 +199,12 @@ echo "Writing library map file...\n"; Filesystem::writeFile($root.'/__phutil_library_map__.php', $map_file); +if ($liberate_mode) { + ob_get_clean(); + echo json_encode(array_keys($need_update))."\n"; + return; +} + echo "Writing module cache...\n"; Filesystem::writeFile( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 848a4575..cf5667c8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -36,6 +36,8 @@ phutil_register_library_map(array( 'ArcanistGitAPI' => 'repository/api/git', 'ArcanistGitHookPreReceiveWorkflow' => 'workflow/git-hook-pre-receive', 'ArcanistHelpWorkflow' => 'workflow/help', + 'ArcanistLiberateLintEngine' => 'lint/engine/liberate', + 'ArcanistLiberateWorkflow' => 'workflow/liberate', 'ArcanistLicenseLinter' => 'lint/linter/license', 'ArcanistLintEngine' => 'lint/engine/base', 'ArcanistLintJSONRenderer' => 'lint/renderer', @@ -97,6 +99,8 @@ phutil_register_library_map(array( 'ArcanistGitAPI' => 'ArcanistRepositoryAPI', 'ArcanistGitHookPreReceiveWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistHelpWorkflow' => 'ArcanistBaseWorkflow', + 'ArcanistLiberateLintEngine' => 'ArcanistLintEngine', + 'ArcanistLiberateWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistLicenseLinter' => 'ArcanistLinter', 'ArcanistLintWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistLinterTestCase' => 'ArcanistPhutilTestCase', diff --git a/src/lint/engine/base/ArcanistLintEngine.php b/src/lint/engine/base/ArcanistLintEngine.php index 472d4350..6cae48f6 100644 --- a/src/lint/engine/base/ArcanistLintEngine.php +++ b/src/lint/engine/base/ArcanistLintEngine.php @@ -182,7 +182,7 @@ abstract class ArcanistLintEngine { // through the lint messages and doing this load only if any of them // have original/replacement text or something like that. try { - $this->fileData[$path] = Filesystem::readFile($path); + $this->fileData[$path] = Filesystem::readFile($disk_path); $result->setData($this->fileData[$path]); } catch (FilesystemException $ex) { // Ignore this, it's noncritical that we access this data and it diff --git a/src/lint/engine/liberate/ArcanistLiberateLintEngine.php b/src/lint/engine/liberate/ArcanistLiberateLintEngine.php new file mode 100644 index 00000000..111ad43a --- /dev/null +++ b/src/lint/engine/liberate/ArcanistLiberateLintEngine.php @@ -0,0 +1,38 @@ +getPaths() as $path) { + $module_linter->addPath($path); + } + + return array($module_linter); + } + +} diff --git a/src/lint/engine/liberate/__init__.php b/src/lint/engine/liberate/__init__.php new file mode 100644 index 00000000..0fe14a7b --- /dev/null +++ b/src/lint/engine/liberate/__init__.php @@ -0,0 +1,13 @@ + $future) { + foreach (Futures($futures)->limit(16) as $key => $future) { $requirements[$key] = $future->resolveJSON(); } @@ -203,7 +203,7 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter { } } - foreach (Futures($futures) as $key => $future) { + foreach (Futures($futures)->limit(16) as $key => $future) { $dependencies[$key] = $future->resolveJSON(); } @@ -378,13 +378,17 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter { $need_functions, $drop_modules); $init_path = $this->getModulePathOnDisk($key).'/__init__.php'; - $try_path = Filesystem::readablePath($init_path); - if (Filesystem::pathExists($try_path)) { + + $root = $this->getEngine()->getWorkingCopy()->getProjectRoot(); + $try_path = Filesystem::readablePath($init_path, $root); + $full_path = Filesystem::resolvePath($try_path, $root); + if (Filesystem::pathExists($full_path)) { $init_path = $try_path; - $old_file = Filesystem::readFile($init_path); + $old_file = Filesystem::readFile($full_path); } else { $old_file = ''; } + $this->willLintPath($init_path); $message = $this->raiseLintAtOffset( null, diff --git a/src/workflow/help/ArcanistHelpWorkflow.php b/src/workflow/help/ArcanistHelpWorkflow.php index 5d5d7760..d4421e82 100644 --- a/src/workflow/help/ArcanistHelpWorkflow.php +++ b/src/workflow/help/ArcanistHelpWorkflow.php @@ -78,6 +78,9 @@ EOTEXT if ($argument == '*') { continue; } + if (!empty($spec['hide'])) { + continue; + } if (isset($spec['param'])) { if (isset($spec['short'])) { $optref[] = phutil_console_format( diff --git a/src/workflow/liberate/ArcanistLiberateWorkflow.php b/src/workflow/liberate/ArcanistLiberateWorkflow.php new file mode 100644 index 00000000..645cee4c --- /dev/null +++ b/src/workflow/liberate/ArcanistLiberateWorkflow.php @@ -0,0 +1,336 @@ + array( + 'help' => + "Drop the module cache before liberating. This will completely ". + "reanalyze the entire library. Thorough, but slow!", + ), + 'force-update' => array( + 'help' => + "Force the library map to be updated, even in the presence of ". + "lint errors.", + ), + 'remap' => array( + 'hide' => true, + 'help' => + "Internal. Run the remap step of liberation. You do not need to ". + "run this unless you are debugging the workflow.", + ), + 'verify' => array( + 'hide' => true, + 'help' => + "Internal. Run the verify step of liberation. You do not need to ". + "run this unless you are debugging the workflow.", + ), + '*' => 'argv', + ); + } + + public function run() { + $argv = $this->getArgument('argv'); + if (count($argv) > 1) { + throw new ArcanistUsageException( + "Provide only one path to 'arc liberate'. The path should be a ". + "directory where you want to create or update a libphutil library."); + } else if (count($argv) == 0) { + $path = getcwd(); + } else { + $path = reset($argv); + } + + $is_remap = $this->getArgument('remap'); + $is_verify = $this->getArgument('verify'); + + $path = Filesystem::resolvePath($path); + + if (Filesystem::pathExists($path) && is_dir($path)) { + $init = id(new FileFinder($path)) + ->withPath('*/__phutil_library_init__.php') + ->find(); + } else { + $init = null; + } + + if ($init) { + if (count($init) > 1) { + throw new ArcanistUsageException( + "Specified directory contains more than one libphutil library. Use ". + "a more specific path."); + } + $path = Filesystem::resolvePath(dirname(reset($init)), $path); + } else { + $found = false; + foreach (Filesystem::walkToRoot($path) as $dir) { + if (Filesystem::pathExists($dir.'/__phutil_library_init__.php')) { + $path = $dir; + break; + } + } + if (!$found) { + echo "No library currently exists at that path...\n"; + $this->liberateCreateDirectory($path); + $this->liberateCreateLibrary($path); + } + } + + if ($this->getArgument('remap')) { + return $this->liberateRunRemap($path); + } + + if ($this->getArgument('verify')) { + return $this->liberateRunVerify($path); + } + + $readable = Filesystem::readablePath($path); + echo "Using library root at '{$readable}'...\n"; + + if ($this->getArgument('all')) { + echo "Dropping module cache...\n"; + Filesystem::remove($path.'/.phutil_module_cache'); + } + + echo "Mapping library...\n"; + + // Force a rebuild of the library map before running lint. The remap + // operation will load the map before regenerating it, so if a class has + // been renamed (say, from OldClass to NewClass) this rebuild will + // cause the initial remap to see NewClass and correctly remove includes + // caused by use of OldClass. + $this->liberateGetChangedPaths($path); + + $arc_bin = $this->getScriptPath('bin/arc'); + + do { + $future = new ExecFuture( + '%s liberate --remap -- %s', + $arc_bin, + $path); + $wrote = $future->resolveJSON(); + foreach ($wrote as $wrote_path) { + echo "Updated '{$wrote_path}'...\n"; + } + } while ($wrote); + + echo "Verifying library...\n"; + + $err = 0; + $cmd = csprintf('%s liberate --verify -- %s', $arc_bin, $path); + passthru($cmd, $err); + + $do_update = (!$err || $this->getArgument('force-update')); + + if ($do_update) { + echo "Finalizing library map...\n"; + execx('%s %s', $this->getPhutilMapperLocation(), $path); + } + + if ($err) { + if ($do_update) { + echo phutil_console_format( + "** WARNING ** Library update forced, but lint ". + "failures remain.\n"); + } else { + echo phutil_console_format( + "** UNRESOLVED LINT ERRORS ** This library has ". + "unresolved lint failures. The library map was not updated. Use ". + "--force-update to force an update.\n"); + } + } else { + echo phutil_console_format( + "** OKAY ** Library updated.\n"); + } + + return $err; + } + + private function liberateLintModules($path, array $changed) { + $engine = $this->liberateBuildLintEngine($path, $changed); + if ($engine) { + return $engine->run(); + } else { + return array(); + } + } + + private function liberateWritePatches(array $results) { + $wrote = array(); + + foreach ($results as $result) { + if ($result->isPatchable()) { + $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); + $patcher->writePatchToDisk(); + $wrote[] = $result->getPath(); + } + } + + return $wrote; + } + + private function liberateBuildLintEngine($path, array $changed) { + $lint_map = array(); + foreach ($changed as $module) { + $module_path = $path.'/'.$module; + $files = Filesystem::listDirectory($module_path); + $lint_map[$module] = $files; + } + + $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( + $path, + json_encode( + array( + 'project_id' => '__arcliberate__', + )), + 'arc liberate'); + + $engine = new ArcanistLiberateLintEngine(); + $engine->setWorkingCopy($working_copy); + + $lint_paths = array(); + foreach ($lint_map as $module => $files) { + foreach ($files as $file) { + $lint_paths[] = $module.'/'.$file; + } + } + + if (!$lint_paths) { + return null; + } + + $engine->setPaths($lint_paths); + $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + + return $engine; + } + + + private function liberateCreateDirectory($path) { + if (Filesystem::pathExists($path)) { + if (!is_dir($path)) { + throw new ArcanistUsageException( + "Provide a directory to create or update a libphutil library in."); + } + return; + } + + echo "The directory '{$path}' does not exist."; + if (!phutil_console_confirm('Do you want to create it?')) { + throw new ArcanistUsageException("Cancelled."); + } + + execx('mkdir -p %s', $path); + } + + private function liberateCreateLibrary($path) { + $init_path = $path.'/__phutil_library_init__.php'; + if (Filesystem::pathExists($init_path)) { + return; + } + + echo "Creating new libphutil library in '{$path}'.\n"; + echo "Choose a name for the new library.\n"; + do { + $name = phutil_console_prompt('What do you want to name this library?'); + if (preg_match('/^[a-z]+$/', $name)) { + break; + } else { + echo "Library name should contain only lowercase letters.\n"; + } + } while (true); + + $template = + "getPhutilMapperLocation(); + $future = new ExecFuture('%s %s --find-paths-for-liberate', $mapper, $path); + return $future->resolveJSON(); + } + + private function getScriptPath($script) { + $root = dirname(phutil_get_library_root('arcanist')); + return $root.'/'.$script; + } + + private function getPhutilMapperLocation() { + return $this->getScriptPath('scripts/phutil_mapper.php'); + } + + private function liberateRunRemap($path) { + phutil_load_library($path); + + $paths = $this->liberateGetChangedPaths($path); + $results = $this->liberateLintModules($path, $paths); + $wrote = $this->liberateWritePatches($results); + + echo json_encode($wrote, true); + + return 0; + } + + private function liberateRunVerify($path) { + phutil_load_library($path); + + $paths = $this->liberateGetChangedPaths($path); + $results = $this->liberateLintModules($path, $paths); + + $renderer = new ArcanistLintRenderer(); + + $unresolved = false; + foreach ($results as $result) { + foreach ($result->getMessages() as $message) { + echo $renderer->renderLintResult($result); + $unresolved = true; + break; + } + } + + return (int)$unresolved; + } + +} diff --git a/src/workflow/liberate/__init__.php b/src/workflow/liberate/__init__.php new file mode 100644 index 00000000..aab0b048 --- /dev/null +++ b/src/workflow/liberate/__init__.php @@ -0,0 +1,26 @@ +