From 62e527482b12dce11e9ee2b9f7a1b7874a0ade98 Mon Sep 17 00:00:00 2001 From: Kunal Bhalla Date: Tue, 20 Dec 2011 20:26:05 -0800 Subject: [PATCH] [arc svn-hook-pre-commit] Access working copy Summary: Creates a new hook API that can be used to interface with SVN/Git/Mercurial in the context of a commit hook. Currently only adds a function to read the modified file data in a Subversion commit hook. An object of this API is created in the SvnHookPreCommitWorkflow and passed on the Lint Engine which then uses it to access current file data, of the way the APIs seem to be structured); linters use the getData function which is essentially a wrapper around the engine's call, with another layer of caching. Task ID: #770556 Blame Rev: Test Plan: - Create a local svn repository and add a minimal hook to run the local version of arc to test commits (http://phabricator.com/docs/arcanist/article/Installing_Arcanist_SVN_Hooks.html) - Create a temporary repository that can trigger any of the linters available, and test against a temporary linter by committing against the test repository: the linter should be able to access all required files by using loadData/getData in the LintEngine and Linter. Revert Plan: Tags: lint, svn-hook-pre-commit Reviewers: jungejason, asukhachev, epriestley, aran Reviewed By: epriestley CC: aran, jungejason, epriestley, kunalb, asukhachev Differential Revision: https://secure.phabricator.com/D1256 --- src/__phutil_library_map__.php | 3 ++ src/lint/engine/base/ArcanistLintEngine.php | 20 +++++++-- src/lint/linter/base/ArcanistLinter.php | 7 +-- src/lint/linter/base/__init__.php | 2 - .../hookapi/base/ArcanistHookAPI.php | 24 +++++++++++ src/repository/hookapi/base/__init__.php | 23 ++++++++++ .../subversion/ArcanistSubversionHookAPI.php | 43 +++++++++++++++++++ .../hookapi/subversion/__init__.php | 14 ++++++ .../ArcanistSvnHookPreCommitWorkflow.php | 21 +++------ src/workflow/svn-hook-pre-commit/__init__.php | 1 + 10 files changed, 133 insertions(+), 25 deletions(-) create mode 100644 src/repository/hookapi/base/ArcanistHookAPI.php create mode 100644 src/repository/hookapi/base/__init__.php create mode 100644 src/repository/hookapi/subversion/ArcanistSubversionHookAPI.php create mode 100644 src/repository/hookapi/subversion/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bf97f652..7a620a74 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -42,6 +42,7 @@ phutil_register_library_map(array( 'ArcanistGitAPI' => 'repository/api/git', 'ArcanistGitHookPreReceiveWorkflow' => 'workflow/git-hook-pre-receive', 'ArcanistHelpWorkflow' => 'workflow/help', + 'ArcanistHookAPI' => 'repository/hookapi/base', 'ArcanistInstallCertificateWorkflow' => 'workflow/install-certificate', 'ArcanistLiberateLintEngine' => 'lint/engine/liberate', 'ArcanistLiberateWorkflow' => 'workflow/liberate', @@ -78,6 +79,7 @@ phutil_register_library_map(array( 'ArcanistRepositoryAPI' => 'repository/api/base', 'ArcanistShellCompleteWorkflow' => 'workflow/shell-complete', 'ArcanistSubversionAPI' => 'repository/api/subversion', + 'ArcanistSubversionHookAPI' => 'repository/hookapi/subversion', 'ArcanistSvnHookPreCommitWorkflow' => 'workflow/svn-hook-pre-commit', 'ArcanistTextLinter' => 'lint/linter/text', 'ArcanistTextLinterTestCase' => 'lint/linter/text/__tests__', @@ -145,6 +147,7 @@ phutil_register_library_map(array( 'ArcanistPyLintLinter' => 'ArcanistLinter', 'ArcanistShellCompleteWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistSubversionAPI' => 'ArcanistRepositoryAPI', + 'ArcanistSubversionHookAPI' => 'ArcanistHookAPI', 'ArcanistSvnHookPreCommitWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistTextLinter' => 'ArcanistLinter', 'ArcanistTextLinterTestCase' => 'ArcanistLinterTestCase', diff --git a/src/lint/engine/base/ArcanistLintEngine.php b/src/lint/engine/base/ArcanistLintEngine.php index 308cdd50..1db0007c 100644 --- a/src/lint/engine/base/ArcanistLintEngine.php +++ b/src/lint/engine/base/ArcanistLintEngine.php @@ -69,6 +69,7 @@ abstract class ArcanistLintEngine { private $changedLines = array(); private $commitHookMode = false; + private $hookAPI; public function __construct() { @@ -115,10 +116,23 @@ abstract class ArcanistLintEngine { return $this; } - protected function loadData($path) { + public function setHookAPI(ArcanistHookAPI $hook_api) { + $this->hookAPI = $hook_api; + } + + public function getHookAPI() { + return $this->hookAPI; + } + + public function loadData($path) { if (!isset($this->fileData[$path])) { - $disk_path = $this->getFilePathOnDisk($path); - $this->fileData[$path] = Filesystem::readFile($disk_path); + if ($this->getCommitHookMode()) { + $this->fileData[$path] = $this->getHookAPI() + ->getCurrentFileData($path); + } else { + $disk_path = $this->getFilePathOnDisk($path); + $this->fileData[$path] = Filesystem::readFile($disk_path); + } } return $this->fileData[$path]; } diff --git a/src/lint/linter/base/ArcanistLinter.php b/src/lint/linter/base/ArcanistLinter.php index 67bfd6c0..638085d8 100644 --- a/src/lint/linter/base/ArcanistLinter.php +++ b/src/lint/linter/base/ArcanistLinter.php @@ -67,12 +67,7 @@ abstract class ArcanistLinter { protected function getData($path) { if (!array_key_exists($path, $this->data)) { - $disk_path = $this->getEngine()->getFilePathOnDisk($path); - if ($disk_path) { - $this->data[$path] = Filesystem::readFile($disk_path); - } else { - throw new Exception("Data is not provided for path '{$path}'!"); - } + $this->data[$path] = $this->getEngine()->loadData($path); } return $this->data[$path]; } diff --git a/src/lint/linter/base/__init__.php b/src/lint/linter/base/__init__.php index 1b364bf5..ade0834a 100644 --- a/src/lint/linter/base/__init__.php +++ b/src/lint/linter/base/__init__.php @@ -9,7 +9,5 @@ phutil_require_module('arcanist', 'lint/message'); phutil_require_module('arcanist', 'lint/severity'); -phutil_require_module('phutil', 'filesystem'); - phutil_require_source('ArcanistLinter.php'); diff --git a/src/repository/hookapi/base/ArcanistHookAPI.php b/src/repository/hookapi/base/ArcanistHookAPI.php new file mode 100644 index 00000000..0fcd2b6f --- /dev/null +++ b/src/repository/hookapi/base/ArcanistHookAPI.php @@ -0,0 +1,24 @@ +root = $root; + $this->transaction = $transaction; + $this->repository = $repository; + } + + public function getCurrentFileData($path) { + list($err, $file) = exec_manual( + 'svnlook cat --transaction %s %s %s', + $this->transaction, + $this->repository, + $this->root . "/$path"); + + return ($err? null : $file); + } +} diff --git a/src/repository/hookapi/subversion/__init__.php b/src/repository/hookapi/subversion/__init__.php new file mode 100644 index 00000000..23bc5354 --- /dev/null +++ b/src/repository/hookapi/subversion/__init__.php @@ -0,0 +1,14 @@ +getConfig('lint_engine'); if (!$lint_engine) { return 0; @@ -204,9 +197,9 @@ EOTEXT $engine = newv($lint_engine, array()); $engine->setWorkingCopy($working_copy); $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR); - $engine->setPaths(array_keys($data)); - $engine->setFileData($data); + $engine->setPaths($paths); $engine->setCommitHookMode(true); + $engine->setHookAPI($repository_api); try { $results = $engine->run(); diff --git a/src/workflow/svn-hook-pre-commit/__init__.php b/src/workflow/svn-hook-pre-commit/__init__.php index f80fe11c..1dacc48c 100644 --- a/src/workflow/svn-hook-pre-commit/__init__.php +++ b/src/workflow/svn-hook-pre-commit/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'lint/renderer'); phutil_require_module('arcanist', 'lint/severity'); +phutil_require_module('arcanist', 'repository/hookapi/subversion'); phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('arcanist', 'workingcopyidentity');