From 1040046f3a8e0338396b23005cee5e914544177b Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 10:07:48 -0800 Subject: [PATCH 1/7] Support --load-phutil-library in arcanist. Summary: Test Plan: Reviewers: CC: --- scripts/arcanist.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/arcanist.php b/scripts/arcanist.php index 6f1265ba..99af52e8 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -32,12 +32,17 @@ phutil_require_module('arcanist', 'repository/api/base'); $config_trace_mode = false; $args = array_slice($argv, 1); +$load = array(); +$matches = null; foreach ($args as $key => $arg) { if ($arg == '--') { break; } else if ($arg == '--trace') { unset($args[$key]); $config_trace_mode = true; + } else if (preg_match('/^--load-phutil-library=(.*)$/', $arg, $matches)) { + unset($args[$key]); + $load['?'] = $matches[1]; } } @@ -54,7 +59,11 @@ try { } $working_copy = ArcanistWorkingCopyIdentity::newFromPath($_SERVER['PWD']); - $libs = $working_copy->getConfig('phutil_libraries'); + if ($load) { + $libs = $load; + } else { + $libs = $working_copy->getConfig('phutil_libraries'); + } if ($libs) { foreach ($libs as $name => $location) { if ($config_trace_mode) { From 834b375e47d88e550cb94e67bdd3de1b6f734bfb Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 11:51:06 -0800 Subject: [PATCH 2/7] Improve bash completion for commands like 'arc lint' and 'arc unit'. Summary: Test Plan: Reviewers: CC: --- .../ArcanistShellCompleteWorkflow.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php b/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php index 6b117761..9ce06663 100644 --- a/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php +++ b/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php @@ -123,6 +123,7 @@ EOTEXT } return 0; } else { + $output = array(); foreach ($arguments as $argument => $spec) { if ($argument == '*') { @@ -135,8 +136,23 @@ EOTEXT } $output[] = '--'.$argument; } - - echo implode(' ', $output)."\n"; + + $cur = idx($argv, $pos, ''); + $any_match = false; + foreach ($output as $possible) { + if (!strncmp($possible, $cur, strlen($cur))) { + $any_match = true; + } + } + + if (!$any_match && isset($arguments['*'])) { + // TODO: the '*' specifier should probably have more details about + // whether or not it is a list of files. Since it almost always is in + // practice, assume FILE for now. + echo "FILE\n"; + } else { + echo implode(' ', $output)."\n"; + } return 0; } } From 2f3791294607e668ffe3ca5cebbcdb9b3fcddadd Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 11:53:13 -0800 Subject: [PATCH 3/7] Expose the already-functional "--engine" flag for the unit workflow. Summary: Test Plan: Reviewers: CC: --- src/workflow/unit/ArcanistUnitWorkflow.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/workflow/unit/ArcanistUnitWorkflow.php b/src/workflow/unit/ArcanistUnitWorkflow.php index e4d517e8..46d632ed 100644 --- a/src/workflow/unit/ArcanistUnitWorkflow.php +++ b/src/workflow/unit/ArcanistUnitWorkflow.php @@ -36,6 +36,11 @@ EOTEXT public function getArguments() { return array( + 'engine' => array( + 'param' => 'classname', + 'help' => + "Override configured unit engine for this project." + ), '*' => 'paths', ); } From 2354d544e35e002bd1a5eaee33ae9d42baa3608e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 11:53:34 -0800 Subject: [PATCH 4/7] Restore hook configuration in lint unit tests. Make working copy setup an external. Summary: Test Plan: Reviewers: CC: --- .../__tests__/ArcanistApacheLicenseLinterTestCase.php | 6 +++++- src/lint/linter/base/test/ArcanistLinterTestCase.php | 11 ++++------- .../xhpast/__tests__/ArcanistXHPASTLinterTestCase.php | 6 +++++- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/lint/linter/apachelicense/__tests__/ArcanistApacheLicenseLinterTestCase.php b/src/lint/linter/apachelicense/__tests__/ArcanistApacheLicenseLinterTestCase.php index b06c3ec1..96643aea 100644 --- a/src/lint/linter/apachelicense/__tests__/ArcanistApacheLicenseLinterTestCase.php +++ b/src/lint/linter/apachelicense/__tests__/ArcanistApacheLicenseLinterTestCase.php @@ -20,7 +20,11 @@ class ArcanistApacheLicenseLinterTestCase extends ArcanistLinterTestCase { public function testApacheLicenseLint() { $linter = new ArcanistApacheLicenseLinter(); - return $this->executeTestsInDirectory(dirname(__FILE__).'/data/', $linter); + $working_copy = ArcanistWorkingCopyIdentity::newFromPath(__FILE__); + return $this->executeTestsInDirectory( + dirname(__FILE__).'/data/', + $linter, + $working_copy); } } diff --git a/src/lint/linter/base/test/ArcanistLinterTestCase.php b/src/lint/linter/base/test/ArcanistLinterTestCase.php index 10d6a291..8b02bdf1 100644 --- a/src/lint/linter/base/test/ArcanistLinterTestCase.php +++ b/src/lint/linter/base/test/ArcanistLinterTestCase.php @@ -18,17 +18,15 @@ abstract class ArcanistLinterTestCase extends ArcanistPhutilTestCase { - public function executeTestsInDirectory($root, $linter) { + public function executeTestsInDirectory($root, $linter, $working_copy) { foreach (Filesystem::listDirectory($root, $hidden = false) as $file) { - $this->lintFile($root.$file, $linter); + $this->lintFile($root.$file, $linter, $working_copy); } } - private function lintFile($file, $linter) { + private function lintFile($file, $linter, $working_copy) { $linter = clone $linter; - $working_copy = ArcanistWorkingCopyIdentity::newFromPath(__FILE__); - $contents = Filesystem::readFile($file); $contents = explode("~~~~~~~~~~\n", $contents); if (count($contents) < 2) { @@ -77,8 +75,7 @@ abstract class ArcanistLinterTestCase extends ArcanistPhutilTestCase { $engine->setWorkingCopy($working_copy); $engine->setPaths(array($path)); -// TODO: restore this -// $engine->setCommitHookMode(idx($config, 'hook', false)); + $engine->setCommitHookMode(idx($config, 'hook', false)); $linter->addPath($path); $linter->addData($path, $data); diff --git a/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterTestCase.php b/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterTestCase.php index e04eec3b..ccc71831 100644 --- a/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterTestCase.php +++ b/src/lint/linter/xhpast/__tests__/ArcanistXHPASTLinterTestCase.php @@ -20,7 +20,11 @@ class ArcanistXHPASTLinterTestCase extends ArcanistLinterTestCase { public function testXHPASTLint() { $linter = new ArcanistXHPASTLinter(); - return $this->executeTestsInDirectory(dirname(__FILE__).'/data/', $linter); + $working_copy = ArcanistWorkingCopyIdentity::newFromPath(__FILE__); + return $this->executeTestsInDirectory( + dirname(__FILE__).'/data/', + $linter, + $working_copy); } } From ba6091f9457b5f9109e63cdd84db86d9d6a08a9c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 17:26:51 -0800 Subject: [PATCH 5/7] Add @nocommit and tests to Text linter. Summary: Test Plan: Reviewers: CC: --- src/__phutil_library_map__.php | 2 ++ .../apachelicense/__tests__/__init__.php | 1 + src/lint/linter/base/test/__init__.php | 1 - src/lint/linter/text/ArcanistTextLinter.php | 23 ++++++++++++++ .../__tests__/ArcanistTextLinterTestCase.php | 30 +++++++++++++++++++ src/lint/linter/text/__tests__/__init__.php | 14 +++++++++ .../__tests__/data/nocommit-hook.lint-test | 8 +++++ .../__tests__/data/nocommit-nohook.lint-test | 2 ++ src/lint/linter/xhpast/__tests__/__init__.php | 1 + 9 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 src/lint/linter/text/__tests__/ArcanistTextLinterTestCase.php create mode 100644 src/lint/linter/text/__tests__/__init__.php create mode 100644 src/lint/linter/text/__tests__/data/nocommit-hook.lint-test create mode 100644 src/lint/linter/text/__tests__/data/nocommit-nohook.lint-test diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2b53d7b8..d0d784a4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -59,6 +59,7 @@ phutil_register_library_map(array( 'ArcanistSubversionAPI' => 'repository/api/subversion', 'ArcanistSvnHookPreCommitWorkflow' => 'workflow/svn-hook-pre-commit', 'ArcanistTextLinter' => 'lint/linter/text', + 'ArcanistTextLinterTestCase' => 'lint/linter/text/__tests__', 'ArcanistUnitTestResult' => 'unit/result', 'ArcanistUnitWorkflow' => 'workflow/unit', 'ArcanistUsageException' => 'exception/usage', @@ -104,6 +105,7 @@ phutil_register_library_map(array( 'ArcanistSubversionAPI' => 'ArcanistRepositoryAPI', 'ArcanistSvnHookPreCommitWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistTextLinter' => 'ArcanistLinter', + 'ArcanistTextLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistUnitWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistUserAbortException' => 'ArcanistUsageException', 'ArcanistXHPASTLinter' => 'ArcanistLinter', diff --git a/src/lint/linter/apachelicense/__tests__/__init__.php b/src/lint/linter/apachelicense/__tests__/__init__.php index d9c8c50f..49d0c3a4 100644 --- a/src/lint/linter/apachelicense/__tests__/__init__.php +++ b/src/lint/linter/apachelicense/__tests__/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('arcanist', 'lint/linter/apachelicense'); phutil_require_module('arcanist', 'lint/linter/base/test'); +phutil_require_module('arcanist', 'workingcopyidentity'); phutil_require_source('ArcanistApacheLicenseLinterTestCase.php'); diff --git a/src/lint/linter/base/test/__init__.php b/src/lint/linter/base/test/__init__.php index c51dbca4..3ae02c6e 100644 --- a/src/lint/linter/base/test/__init__.php +++ b/src/lint/linter/base/test/__init__.php @@ -9,7 +9,6 @@ phutil_require_module('arcanist', 'lint/engine/test'); phutil_require_module('arcanist', 'lint/patcher'); phutil_require_module('arcanist', 'unit/engine/phutil/testcase'); -phutil_require_module('arcanist', 'workingcopyidentity'); phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'utils'); diff --git a/src/lint/linter/text/ArcanistTextLinter.php b/src/lint/linter/text/ArcanistTextLinter.php index 61ccad07..23c24c77 100644 --- a/src/lint/linter/text/ArcanistTextLinter.php +++ b/src/lint/linter/text/ArcanistTextLinter.php @@ -24,6 +24,7 @@ class ArcanistTextLinter extends ArcanistLinter { const LINT_EOF_NEWLINE = 4; const LINT_BAD_CHARSET = 5; const LINT_TRAILING_WHITESPACE = 6; + const LINT_NO_COMMIT = 7; private $maxLineLength = 80; @@ -54,6 +55,7 @@ class ArcanistTextLinter extends ArcanistLinter { self::LINT_EOF_NEWLINE => 'File Does Not End in Newline', self::LINT_BAD_CHARSET => 'Bad Charset', self::LINT_TRAILING_WHITESPACE => 'Trailing Whitespace', + self::LINT_NO_COMMIT => 'Explicit @no'.'commit', ); } @@ -74,6 +76,10 @@ class ArcanistTextLinter extends ArcanistLinter { $this->lintLineLength($path); $this->lintEOFNewline($path); $this->lintTrailingWhitespace($path); + + if ($this->getEngine()->getCommitHookMode()) { + $this->lintNoCommit($path); + } } protected function lintNewlines($path) { @@ -181,4 +187,21 @@ class ArcanistTextLinter extends ArcanistLinter { } } + private function lintNoCommit($path) { + $data = $this->getData($path); + + $deadly = '@no'.'commit'; + + $offset = strpos($data, $deadly); + if ($offset !== false) { + $this->raiseLintAtOffset( + $offset, + self::LINT_NO_COMMIT, + 'This file is explicitly marked as "'.$deadly.'", which blocks '. + 'commits.', + $deadly); + } + } + + } diff --git a/src/lint/linter/text/__tests__/ArcanistTextLinterTestCase.php b/src/lint/linter/text/__tests__/ArcanistTextLinterTestCase.php new file mode 100644 index 00000000..caff6243 --- /dev/null +++ b/src/lint/linter/text/__tests__/ArcanistTextLinterTestCase.php @@ -0,0 +1,30 @@ +executeTestsInDirectory( + dirname(__FILE__).'/data/', + $linter, + $working_copy); + } + +} diff --git a/src/lint/linter/text/__tests__/__init__.php b/src/lint/linter/text/__tests__/__init__.php new file mode 100644 index 00000000..773f78c6 --- /dev/null +++ b/src/lint/linter/text/__tests__/__init__.php @@ -0,0 +1,14 @@ + Date: Wed, 16 Feb 2011 18:47:17 -0800 Subject: [PATCH 6/7] Fix two common points of confusion: - show "push upstream" instructions after arc amend - parse 'diffcamp revision' Summary: Test Plan: Reviewers: CC: --- .../commitmessage/ArcanistDifferentialCommitMessage.php | 3 ++- src/workflow/amend/ArcanistAmendWorkflow.php | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/differential/commitmessage/ArcanistDifferentialCommitMessage.php b/src/differential/commitmessage/ArcanistDifferentialCommitMessage.php index 5b45084f..728397ca 100644 --- a/src/differential/commitmessage/ArcanistDifferentialCommitMessage.php +++ b/src/differential/commitmessage/ArcanistDifferentialCommitMessage.php @@ -30,8 +30,9 @@ class ArcanistDifferentialCommitMessage { $obj = new ArcanistDifferentialCommitMessage(); $obj->rawCorpus = $corpus; + // TODO: Remove "Diffcamp" backward compatibility. $match = null; - if (preg_match('/^Differential Revision:\s*D?(\d+)/m', $corpus, $match)) { + if (preg_match('/^(?:Differential|DiffCamp) Revision:\s*D?(\d+)/im', $corpus, $match)) { $obj->revisionID = (int)$match[1]; } diff --git a/src/workflow/amend/ArcanistAmendWorkflow.php b/src/workflow/amend/ArcanistAmendWorkflow.php index afa5c605..a41f3b46 100644 --- a/src/workflow/amend/ArcanistAmendWorkflow.php +++ b/src/workflow/amend/ArcanistAmendWorkflow.php @@ -126,6 +126,10 @@ EOTEXT } } + echo phutil_console_wrap( + "You may now push this commit upstream, as appropriate (e.g. with ". + "'git push', or 'git svn dcommit', or by printing and faxing it).\n"); + return 0; } From e0de194e11ba47eca5e2a776296b77d642c1ef2f Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 16 Feb 2011 18:49:45 -0800 Subject: [PATCH 7/7] Move this block into a more sensible branch. Summary: Test Plan: Reviewers: CC: --- src/workflow/amend/ArcanistAmendWorkflow.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/workflow/amend/ArcanistAmendWorkflow.php b/src/workflow/amend/ArcanistAmendWorkflow.php index a41f3b46..4c14013f 100644 --- a/src/workflow/amend/ArcanistAmendWorkflow.php +++ b/src/workflow/amend/ArcanistAmendWorkflow.php @@ -124,11 +124,11 @@ EOTEXT array($revision_id)); $mark_workflow->run(); } - } - echo phutil_console_wrap( - "You may now push this commit upstream, as appropriate (e.g. with ". - "'git push', or 'git svn dcommit', or by printing and faxing it).\n"); + echo phutil_console_wrap( + "You may now push this commit upstream, as appropriate (e.g. with ". + "'git push', or 'git svn dcommit', or by printing and faxing it).\n"); + } return 0; }