Add lint support to 'arc svn-hook-pre-commit'.

Summary: The biggest blocker on getting rid of arc in trunk is that the lint
rules in the commit hooks are still running the old version. Push arc
commit hook support toward some reasonable state of approximately working.

Test Plan:

Reviewers:

CC:

src/lint/engine/base/ArcanistLintEngine.php

@@ -27,6 +27,7 @@ abstract class ArcanistLintEngine {
   private $minimumSeverity = null;
 
   private $changedLines = array();
+  private $commitHookMode = false;
 
   public function __construct() {
 
@@ -59,6 +60,16 @@ abstract class ArcanistLintEngine {
     return idx($this->changedLines, $path);
   }
 
+  public function setFileData($data) {
+    $this->fileData = $data + $this->fileData;
+    return $this;
+  }
+
+  public function setCommitHookMode($mode) {
+    $this->commitHookMode = $mode;
+    return $this;
+  }
+
   protected function loadData($path) {
     if (!isset($this->fileData[$path])) {
       $disk_path = $this->getFilePathOnDisk($path);
@@ -67,6 +78,15 @@ abstract class ArcanistLintEngine {
     return $this->fileData[$path];
   }
 
+  public function pathExists($path) {
+    if ($this->getCommitHookMode()) {
+      return (idx($this->fileData, $path) !== null);
+    } else {
+      $disk_path = $this->getFilePathOnDisk($path);
+      return Filesystem::pathExists($disk_path);
+    }
+  }
+
   public function getFilePathOnDisk($path) {
     return Filesystem::resolvePath(
       $path,
@@ -79,7 +99,7 @@ abstract class ArcanistLintEngine {
   }
 
   public function getCommitHookMode() {
-    return false;
+    return $this->commitHookMode;
   }
 
   public function run() {

src/lint/engine/phutil/PhutilLintEngine.php

@@ -93,9 +93,4 @@ class PhutilLintEngine extends ArcanistLintEngine {
     return $linters;
   }
 
-  public function pathExists($path) {
-    $disk_path = $this->getFilePathOnDisk($path);
-    return Filesystem::pathExists($disk_path);
-  }
-
 }

src/lint/engine/phutil/__init__.php

@@ -7,14 +7,12 @@
 
 
 phutil_require_module('arcanist', 'lint/engine/base');
+phutil_require_module('arcanist', 'lint/linter/apachelicense');
 phutil_require_module('arcanist', 'lint/linter/filename');
 phutil_require_module('arcanist', 'lint/linter/generated');
 phutil_require_module('arcanist', 'lint/linter/phutilmodule');
 phutil_require_module('arcanist', 'lint/linter/text');
 phutil_require_module('arcanist', 'lint/linter/xhpast');
-phutil_require_module('arcanist', 'lint/linter/apachelicense');
-
-phutil_require_module('phutil', 'filesystem');
 
 
 phutil_require_source('PhutilLintEngine.php');

src/workflow/amend/ArcanistAmendWorkflow.php

@@ -95,6 +95,10 @@ EOTEXT
       }
     }
 
+    // TODO: The old 'arc amend' had a check here to see if you were running
+    // 'arc amend' with an explicit revision but HEAD already had another
+    // revision in it. Maybe this is worth restoring?
+
     $conduit = $this->getConduit();
     $message = $conduit->callMethodSynchronous(
       'differential.getcommitmessage',

src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php

@@ -20,7 +20,7 @@ class ArcanistSvnHookPreCommitWorkflow extends ArcanistBaseWorkflow {
 
   public function getCommandHelp() {
     return phutil_console_format(<<<EOTEXT
-      **svn-hook-pre-receive** __repository__ __transaction__
+      **svn-hook-pre-commit** __repository__ __transaction__
           Supports: svn
           You can install this as an SVN pre-commit hook.
 EOTEXT
@@ -48,8 +48,12 @@ EOTEXT
       $transaction,
       $repository);
 
+    if (strpos($commit_message, '@bypass-lint') !== false) {
+      return 0;
+    }
+
+
     // TODO: Do stuff with commit message.
-    var_dump($commit_message);
 
     list($changed) = execx(
       'svnlook changed --transaction %s %s',
@@ -119,6 +123,7 @@ EOTEXT
       }
     }
 
+
     if ($failed && $resolved) {
       $failed_paths = '        '.implode("\n        ", $failed);
       $resolved_paths = '        '.implode("\n        ", array_keys($resolved));
@@ -134,7 +139,7 @@ EOTEXT
 
     if (!$resolved) {
       // None of the affected paths are beneath a .arcconfig file.
-      return 3;
+      return 0;
     }
 
     $groups = array();
@@ -155,11 +160,19 @@ EOTEXT
         $message);
     }
 
-    $project_root = key($groups);
+    $config_file = key($groups);
+    $project_root = dirname($config_file);
     $paths = reset($groups);
 
+    list($config) = execx(
+      'svnlook cat --transaction %s %s %s',
+      $transaction,
+      $repository,
+      $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,
@@ -168,9 +181,51 @@ EOTEXT
       $data[$path] = $err ? null : $filedata;
     }
 
-    // TODO: Do stuff with data.
-    var_dump($data);
+    $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile(
+      $project_root,
+      $config);
 
-    return 1;
+    $lint_engine = $working_copy->getConfig('lint_engine');
+    if (!$lint_engine) {
+      return 0;
+    }
+
+    PhutilSymbolLoader::loadClass($lint_engine);
+
+    $engine = newv($lint_engine, array());
+    $engine->setWorkingCopy($working_copy);
+    $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
+    $engine->setPaths(array_keys($data));
+    $engine->setFileData($data);
+    $engine->setCommitHookMode(true);
+
+    $results = $engine->run();
+
+    $renderer = new ArcanistLintRenderer();
+    $failures = array();
+    foreach ($results as $result) {
+      if (!$result->getMessages()) {
+        continue;
+      }
+      $failures[] = $result;
+    }
+
+    if ($failures) {
+      $at = "@";
+      $msg = phutil_console_format(
+        "\n**LINT ERRORS**\n\n".
+        "This changeset has lint errors. You must fix all lint errors before ".
+        "you can commit.\n\n".
+        "You can add '{$at}bypass-lint' to your commit message to disable ".
+        "lint checks for this commit, or '{$at}nolint' to the file with ".
+        "errors to disable lint for that file.\n\n");
+      echo phutil_console_wrap($msg);
+      foreach ($failures as $result) {
+        echo $renderer->renderLintResult($result);
+      }
+      return 1;
+    }
+
+    return 99;
   }
 }

src/workflow/svn-hook-pre-commit/__init__.php

@@ -7,10 +7,15 @@
 
 
 phutil_require_module('arcanist', 'exception/usage');
+phutil_require_module('arcanist', 'lint/renderer');
+phutil_require_module('arcanist', 'lint/severity');
 phutil_require_module('arcanist', 'workflow/base');
+phutil_require_module('arcanist', 'workingcopyidentity');
 
 phutil_require_module('phutil', 'console');
 phutil_require_module('phutil', 'future/exec');
+phutil_require_module('phutil', 'symbols');
+phutil_require_module('phutil', 'utils');
 
 
 phutil_require_source('ArcanistSvnHookPreCommitWorkflow.php');

src/workingcopyidentity/ArcanistWorkingCopyIdentity.php

@@ -31,29 +31,38 @@ class ArcanistWorkingCopyIdentity {
         continue;
       }
       $proj_raw = Filesystem::readFile($config_file);
-      $proj = json_decode($proj_raw, true);
-      if (!is_array($proj)) {
-        throw new Exception(
-          "Unable to parse '.arcconfig' file in '{$dir}'. The file contents ".
-          "should be valid JSON.");
-      }
-      $required_keys = array(
-        'project_id',
-      );
-      foreach ($required_keys as $key) {
-        if (!array_key_exists($key, $proj)) {
-          throw new Exception(
-            "Required key '{$key}' is missing from '.arcconfig' file in ".
-            "'{$dir}'.");
-        }
-      }
-      $config = $proj;
+      $config = self::parseRawConfigFile($proj_raw);
       $project_root = $dir;
       break;
     }
     return new ArcanistWorkingCopyIdentity($project_root, $config);
   }
 
+  public static function newFromRootAndConfigFile($root, $config_raw) {
+    $config = self::parseRawConfigFile($config_raw);
+    return new ArcanistWorkingCopyIdentity($root, $config);
+  }
+
+  private static function parseRawConfigFile($raw_config) {
+    $proj = json_decode($raw_config, true);
+    if (!is_array($proj)) {
+      throw new Exception(
+        "Unable to parse '.arcconfig' file in '{$dir}'. The file contents ".
+        "should be valid JSON.");
+    }
+    $required_keys = array(
+      'project_id',
+    );
+    foreach ($required_keys as $key) {
+      if (!array_key_exists($key, $proj)) {
+        throw new Exception(
+          "Required key '{$key}' is missing from '.arcconfig' file in ".
+          "'{$dir}'.");
+      }
+    }
+    return $proj;
+  }
+
   protected function __construct($root, array $config) {
     $this->projectRoot    = $root;
     $this->projectConfig  = $config;