1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-01 19:22:41 +01:00

[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
This commit is contained in:
Kunal Bhalla 2011-12-20 20:26:05 -08:00
parent 7af16f42cd
commit 62e527482b
10 changed files with 133 additions and 25 deletions

View file

@ -42,6 +42,7 @@ phutil_register_library_map(array(
'ArcanistGitAPI' => 'repository/api/git', 'ArcanistGitAPI' => 'repository/api/git',
'ArcanistGitHookPreReceiveWorkflow' => 'workflow/git-hook-pre-receive', 'ArcanistGitHookPreReceiveWorkflow' => 'workflow/git-hook-pre-receive',
'ArcanistHelpWorkflow' => 'workflow/help', 'ArcanistHelpWorkflow' => 'workflow/help',
'ArcanistHookAPI' => 'repository/hookapi/base',
'ArcanistInstallCertificateWorkflow' => 'workflow/install-certificate', 'ArcanistInstallCertificateWorkflow' => 'workflow/install-certificate',
'ArcanistLiberateLintEngine' => 'lint/engine/liberate', 'ArcanistLiberateLintEngine' => 'lint/engine/liberate',
'ArcanistLiberateWorkflow' => 'workflow/liberate', 'ArcanistLiberateWorkflow' => 'workflow/liberate',
@ -78,6 +79,7 @@ phutil_register_library_map(array(
'ArcanistRepositoryAPI' => 'repository/api/base', 'ArcanistRepositoryAPI' => 'repository/api/base',
'ArcanistShellCompleteWorkflow' => 'workflow/shell-complete', 'ArcanistShellCompleteWorkflow' => 'workflow/shell-complete',
'ArcanistSubversionAPI' => 'repository/api/subversion', 'ArcanistSubversionAPI' => 'repository/api/subversion',
'ArcanistSubversionHookAPI' => 'repository/hookapi/subversion',
'ArcanistSvnHookPreCommitWorkflow' => 'workflow/svn-hook-pre-commit', 'ArcanistSvnHookPreCommitWorkflow' => 'workflow/svn-hook-pre-commit',
'ArcanistTextLinter' => 'lint/linter/text', 'ArcanistTextLinter' => 'lint/linter/text',
'ArcanistTextLinterTestCase' => 'lint/linter/text/__tests__', 'ArcanistTextLinterTestCase' => 'lint/linter/text/__tests__',
@ -145,6 +147,7 @@ phutil_register_library_map(array(
'ArcanistPyLintLinter' => 'ArcanistLinter', 'ArcanistPyLintLinter' => 'ArcanistLinter',
'ArcanistShellCompleteWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistShellCompleteWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistSubversionAPI' => 'ArcanistRepositoryAPI', 'ArcanistSubversionAPI' => 'ArcanistRepositoryAPI',
'ArcanistSubversionHookAPI' => 'ArcanistHookAPI',
'ArcanistSvnHookPreCommitWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistSvnHookPreCommitWorkflow' => 'ArcanistBaseWorkflow',
'ArcanistTextLinter' => 'ArcanistLinter', 'ArcanistTextLinter' => 'ArcanistLinter',
'ArcanistTextLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistTextLinterTestCase' => 'ArcanistLinterTestCase',

View file

@ -69,6 +69,7 @@ abstract class ArcanistLintEngine {
private $changedLines = array(); private $changedLines = array();
private $commitHookMode = false; private $commitHookMode = false;
private $hookAPI;
public function __construct() { public function __construct() {
@ -115,11 +116,24 @@ abstract class ArcanistLintEngine {
return $this; 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])) { if (!isset($this->fileData[$path])) {
if ($this->getCommitHookMode()) {
$this->fileData[$path] = $this->getHookAPI()
->getCurrentFileData($path);
} else {
$disk_path = $this->getFilePathOnDisk($path); $disk_path = $this->getFilePathOnDisk($path);
$this->fileData[$path] = Filesystem::readFile($disk_path); $this->fileData[$path] = Filesystem::readFile($disk_path);
} }
}
return $this->fileData[$path]; return $this->fileData[$path];
} }

View file

@ -67,12 +67,7 @@ abstract class ArcanistLinter {
protected function getData($path) { protected function getData($path) {
if (!array_key_exists($path, $this->data)) { if (!array_key_exists($path, $this->data)) {
$disk_path = $this->getEngine()->getFilePathOnDisk($path); $this->data[$path] = $this->getEngine()->loadData($path);
if ($disk_path) {
$this->data[$path] = Filesystem::readFile($disk_path);
} else {
throw new Exception("Data is not provided for path '{$path}'!");
}
} }
return $this->data[$path]; return $this->data[$path];
} }

View file

@ -9,7 +9,5 @@
phutil_require_module('arcanist', 'lint/message'); phutil_require_module('arcanist', 'lint/message');
phutil_require_module('arcanist', 'lint/severity'); phutil_require_module('arcanist', 'lint/severity');
phutil_require_module('phutil', 'filesystem');
phutil_require_source('ArcanistLinter.php'); phutil_require_source('ArcanistLinter.php');

View file

@ -0,0 +1,24 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* API while running in the context of a commit hook
*/
abstract class ArcanistHookAPI {
abstract public function getCurrentFileData($path);
}

View file

@ -0,0 +1,23 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_source('ArcanistHookAPI.php');

View file

@ -0,0 +1,43 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* Interfaces with Subversion while running as a commit hook.
*/
class ArcanistSubversionHookAPI extends ArcanistHookAPI {
protected $root;
protected $transaction;
protected $repository;
public function __construct($root, $transaction, $repository) {
$this->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);
}
}

View file

@ -0,0 +1,14 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('arcanist', 'repository/hookapi/base');
phutil_require_module('phutil', 'future/exec');
phutil_require_source('ArcanistSubversionHookAPI.php');

View file

@ -131,7 +131,6 @@ EOTEXT
} }
} }
if ($failed && $resolved) { if ($failed && $resolved) {
$failed_paths = ' '.implode("\n ", $failed); $failed_paths = ' '.implode("\n ", $failed);
$resolved_paths = ' '.implode("\n ", array_keys($resolved)); $resolved_paths = ' '.implode("\n ", array_keys($resolved));
@ -178,22 +177,16 @@ EOTEXT
$repository, $repository,
$config_file); $config_file);
$data = array();
foreach ($paths as $path) {
// TODO: This should be done in parallel.
list($err, $filedata) = exec_manual(
'svnlook cat --transaction %s %s %s',
$transaction,
$repository,
$path);
$data[$path] = $err ? null : $filedata;
}
$working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile(
$project_root, $project_root,
$config, $config,
$config_file." (svnlook: {$transaction} {$repository})"); $config_file." (svnlook: {$transaction} {$repository})");
$repository_api = new ArcanistSubversionHookAPI(
$project_root,
$transaction,
$repository);
$lint_engine = $working_copy->getConfig('lint_engine'); $lint_engine = $working_copy->getConfig('lint_engine');
if (!$lint_engine) { if (!$lint_engine) {
return 0; return 0;
@ -204,9 +197,9 @@ EOTEXT
$engine = newv($lint_engine, array()); $engine = newv($lint_engine, array());
$engine->setWorkingCopy($working_copy); $engine->setWorkingCopy($working_copy);
$engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR); $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
$engine->setPaths(array_keys($data)); $engine->setPaths($paths);
$engine->setFileData($data);
$engine->setCommitHookMode(true); $engine->setCommitHookMode(true);
$engine->setHookAPI($repository_api);
try { try {
$results = $engine->run(); $results = $engine->run();

View file

@ -9,6 +9,7 @@
phutil_require_module('arcanist', 'exception/usage'); phutil_require_module('arcanist', 'exception/usage');
phutil_require_module('arcanist', 'lint/renderer'); phutil_require_module('arcanist', 'lint/renderer');
phutil_require_module('arcanist', 'lint/severity'); phutil_require_module('arcanist', 'lint/severity');
phutil_require_module('arcanist', 'repository/hookapi/subversion');
phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('arcanist', 'workflow/base');
phutil_require_module('arcanist', 'workingcopyidentity'); phutil_require_module('arcanist', 'workingcopyidentity');