diff --git a/scripts/arcanist.php b/scripts/arcanist.php index 1ef7f431..4f772a63 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -2,7 +2,7 @@ getConfig('immutable_history') === true); } + /** + * Workflows like 'lint' and 'unit' operate on a list of working copy paths. + * The user can either specify the paths explicitly ("a.js b.php"), or by + * specfifying a revision ("--rev a3f10f1f") to select all paths modified + * since that revision, or by omitting both and letting arc choose the + * default relative revision. + * + * This method takes the user's selections and returns the paths that the + * workflow should act upon. + * + * @param list List of explicitly provided paths. + * @param string|null Revision name, if provided. + * @return list List of paths the workflow should act on. + */ + protected function selectPathsForWorkflow(array $paths, $rev) { + if ($paths) { + $working_copy = $this->getWorkingCopy(); + foreach ($paths as $key => $path) { + $full_path = Filesystem::resolvePath($path); + if (!Filesystem::pathExists($full_path)) { + throw new ArcanistUsageException("Path '{$path}' does not exist!"); + } + $relative_path = Filesystem::readablePath( + $full_path, + $working_copy->getProjectRoot()); + $paths[$key] = $relative_path; + } + } else { + $repository_api = $this->getRepositoryAPI(); + if ($rev) { + $repository_api->parseRelativeLocalCommit(array($rev)); + } + + $paths = $repository_api->getWorkingCopyStatus(); + foreach ($paths as $path => $flags) { + if ($flags & ArcanistRepositoryAPI::FLAG_UNTRACKED) { + unset($paths[$path]); + } + } + $paths = array_keys($paths); + } + + return array_values($paths); + } + } diff --git a/src/workflow/base/__init__.php b/src/workflow/base/__init__.php index b59c83a1..f23596cf 100644 --- a/src/workflow/base/__init__.php +++ b/src/workflow/base/__init__.php @@ -14,6 +14,7 @@ phutil_require_module('arcanist', 'parser/bundle'); phutil_require_module('arcanist', 'parser/diff'); phutil_require_module('arcanist', 'parser/diff/change'); phutil_require_module('arcanist', 'parser/diff/changetype'); +phutil_require_module('arcanist', 'repository/api/base'); phutil_require_module('phutil', 'conduit/client'); phutil_require_module('phutil', 'console'); diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index 449ba706..f538978b 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -956,9 +956,8 @@ EOTEXT echo "Linting...\n"; try { $argv = $this->getPassthruArgumentsAsArgv('lint'); - if ($repository_api instanceof ArcanistSubversionAPI) { - $argv = array_merge($argv, array_keys($paths)); - } else { + if ($repository_api->supportsRelativeLocalCommits()) { + $argv[] = '--rev'; $argv[] = $repository_api->getRelativeCommit(); } $lint_workflow = $this->buildChildWorkflow('lint', $argv); @@ -1021,8 +1020,9 @@ EOTEXT echo "Running unit tests...\n"; try { $argv = $this->getPassthruArgumentsAsArgv('unit'); - if ($repository_api instanceof ArcanistSubversionAPI) { - $argv = array_merge($argv, array_keys($paths)); + if ($repository_api->supportsRelativeLocalCommits()) { + $argv[] = '--rev'; + $argv[] = $repository_api->getRelativeCommit(); } $this->unitWorkflow = $this->buildChildWorkflow('unit', $argv); $unit_result = $this->unitWorkflow->run(); diff --git a/src/workflow/lint/ArcanistLintWorkflow.php b/src/workflow/lint/ArcanistLintWorkflow.php index f50a88cf..8dc5c937 100644 --- a/src/workflow/lint/ArcanistLintWorkflow.php +++ b/src/workflow/lint/ArcanistLintWorkflow.php @@ -1,7 +1,7 @@ "Show all lint warnings, not just those on changed lines." ), + 'rev' => array( + 'param' => 'revision', + 'help' => "Lint changes since a specific revision.", + 'supports' => array( + 'git', + 'hg', + ), + 'nosupport' => array( + 'svn' => "Lint does not currently support --rev in SVN.", + ), + ), 'output' => array( 'param' => 'format', 'help' => @@ -90,6 +101,10 @@ EOTEXT return true; } + public function requiresRepositoryAPI() { + return true; + } + public function run() { $working_copy = $this->getWorkingCopy(); @@ -103,61 +118,23 @@ EOTEXT } } - $should_lint_all = $this->getArgument('lintall'); + $rev = $this->getArgument('rev'); + $paths = $this->getArgument('paths'); - $repository_api = null; - if (!$should_lint_all) { - try { - $repository_api = ArcanistRepositoryAPI::newAPIFromWorkingCopyIdentity( - $working_copy); - $this->setRepositoryAPI($repository_api); - } catch (ArcanistUsageException $ex) { - throw new ArcanistUsageException( - $ex->getMessage()."\n\n". - "Use '--lintall' to ignore working copy changes when running lint."); - } - - if ($repository_api instanceof ArcanistSubversionAPI) { - $paths = $repository_api->getWorkingCopyStatus(); - $list = new FileList($this->getArgument('paths')); - foreach ($paths as $path => $flags) { - if (!$list->contains($path)) { - unset($paths[$path]); - } - } - } else if ($repository_api->supportsRelativeLocalCommits()) { - $repository_api->parseRelativeLocalCommit( - $this->getArgument('paths', array())); - $paths = $repository_api->getWorkingCopyStatus(); - } else { - throw new Exception("Unknown VCS!"); - } - - foreach ($paths as $path => $flags) { - if ($flags & ArcanistRepositoryAPI::FLAG_UNTRACKED) { - unset($paths[$path]); - } - } - - $paths = array_keys($paths); - } else { - $paths = $this->getArgument('paths'); - if (empty($paths)) { - throw new ArcanistUsageException( - "You must specify one or more files to lint when using '--lintall'."); - } - foreach ($paths as $key => $path) { - $full_path = Filesystem::resolvePath($path); - if (!Filesystem::pathExists($full_path)) { - throw new ArcanistUsageException("Path '{$path}' does not exist!"); - } - $relative_path = Filesystem::readablePath( - $full_path, - $working_copy->getProjectRoot()); - $paths[$key] = $relative_path; - } + if ($rev && $paths) { + throw new ArcanistUsageException("Specify either --rev or paths."); } + $should_lint_all = $this->getArgument('lintall'); + if ($paths) { + // NOTE: When the user specifies paths, we imply --lintall and show all + // warnings for the paths in question. This is easier to deal with for + // us and less confusing for users. + $should_lint_all = true; + } + + $paths = $this->selectPathsForWorkflow($paths, $rev); + PhutilSymbolLoader::loadClass($engine); if (!is_subclass_of($engine, 'ArcanistLintEngine')) { throw new ArcanistUsageException( @@ -262,6 +239,7 @@ EOTEXT } } + $repository_api = $this->getRepositoryAPI(); if ($wrote_to_disk && ($repository_api instanceof ArcanistGitAPI) && $this->shouldAmendChanges) { diff --git a/src/workflow/lint/__init__.php b/src/workflow/lint/__init__.php index 4560471f..b5c858d9 100644 --- a/src/workflow/lint/__init__.php +++ b/src/workflow/lint/__init__.php @@ -11,12 +11,10 @@ phutil_require_module('arcanist', 'exception/usage/noengine'); phutil_require_module('arcanist', 'lint/patcher'); phutil_require_module('arcanist', 'lint/renderer'); phutil_require_module('arcanist', 'lint/severity'); -phutil_require_module('arcanist', 'repository/api/base'); phutil_require_module('arcanist', 'workflow/base'); 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'); diff --git a/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php b/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php index fa185391..7cd8463e 100644 --- a/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php +++ b/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php @@ -1,7 +1,7 @@ array( + 'param' => 'revision', + 'help' => "Run unit tests covering changes since a specific revision.", + 'supports' => array( + 'git', + 'hg', + ), + 'nosupport' => array( + 'svn' => "Arc unit does not currently support --rev in SVN.", + ), + ), 'engine' => array( 'param' => 'classname', 'help' => @@ -79,23 +91,10 @@ EOTEXT "to specify a unit test engine."); } - $repository_api = $this->getRepositoryAPI(); + $paths = $this->getArgument('paths'); + $rev = $this->getArgument('rev'); - if ($this->getArgument('paths')) { - // TODO: deal with git stuff - $paths = $this->getArgument('paths'); - } else { - $paths = $repository_api->getWorkingCopyStatus(); - - // TODO: clean this up - foreach ($paths as $path => $mask) { - if ($mask & ArcanistRepositoryAPI::FLAG_UNTRACKED) { - unset($paths[$path]); - } - } - - $paths = array_keys($paths); - } + $paths = $this->selectPathsForWorkflow($paths, $rev); PhutilSymbolLoader::loadClass($engine_class); if (!is_subclass_of($engine_class, 'ArcanistBaseUnitTestEngine')) { diff --git a/src/workflow/unit/__init__.php b/src/workflow/unit/__init__.php index cafa3c73..bc9e52a6 100644 --- a/src/workflow/unit/__init__.php +++ b/src/workflow/unit/__init__.php @@ -8,7 +8,6 @@ phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'exception/usage/noengine'); -phutil_require_module('arcanist', 'repository/api/base'); phutil_require_module('arcanist', 'unit/result'); phutil_require_module('arcanist', 'workflow/base');