Simplify and optimize save_lint.php

Test Plan: Ran it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3933

scripts/repository/save_lint.php

@@ -3,101 +3,51 @@
 
 require_once dirname(__FILE__).'/../__init_script__.php';
 
-if (function_exists('posix_isatty') && posix_isatty(STDIN)) {
+$synopsis = <<<EOT
-  $command = 'xargs -0 arc lint --output json | '.__FILE__;
+**save_lint.php**
-  echo "Usage: git ls-files -z | {$command}\n";
+    Discover lint problems and save them to database so that they can
-  echo "Usage: git diff --name-only -z | {$command}\n"; // TODO: Handle deletes.
+    be displayed in Diffusion.
-  echo "Purpose: Save all lint errors to database.\n";
-  exit(1);
-}
 
-$working_copy = ArcanistWorkingCopyIdentity::newFromPath('.');
+EOT;
-$api = ArcanistRepositoryAPI::newAPIFromWorkingCopyIdentity($working_copy);
-$svn_root = id(new PhutilURI($api->getSourceControlPath()))->getPath();
 
-$project_id = $working_copy->getProjectID();
+$args = id(new PhutilArgumentParser($argv))
-$project = id(new PhabricatorRepositoryArcanistProject())
+  ->setTagline('save lint errors to database')
-  ->loadOneWhere('name = %s', $project_id);
+  ->setSynopsis($synopsis)
-if (!$project || !$project->getRepositoryID()) {
+  ->parseStandardArguments()
-  throw new Exception("Couldn't find repository for {$project_id}.");
+  ->parse(array(
-}
+    array(
+      'name' => 'all',
+      'help' =>
+        "Discover problems in the whole repository instead of just changes ".
+        "since the last run.",
+    ),
+    array(
+      'name' => 'arc',
+      'param' => 'path',
+      'default' => 'arc',
+      'help' => "Path to Arcanist executable.",
+    ),
+    array(
+      'name' => 'severity',
+      'param' => 'string',
+      'default' => ArcanistLintSeverity::SEVERITY_ADVICE,
+      'help' => "Minimum severity, one of ArcanistLintSeverity constants.",
+    ),
+    array(
+      'name' => 'chunk-size',
+      'param' => 'number',
+      'default' => 256,
+      'help' => "Number of paths passed to `arc` at once.",
+    ),
+  ));
 
-$branch_name = $api->getBranchName();
+echo "Saving lint errors to database...\n";
-$branch = id(new PhabricatorRepositoryBranch())->loadOneWhere(
-  'repositoryID = %d AND name = %s',
-  $project->getRepositoryID(),
-  $branch_name);
-if (!$branch) {
-  $branch = id(new PhabricatorRepositoryBranch())
-    ->setRepositoryID($project->getRepositoryID())
-    ->setName($branch_name);
-}
-$branch->setLintCommit($api->getWorkingCopyRevision());
-$branch->save();
-$conn = $branch->establishConnection('w');
 
-$inserts = array();
+$count = id(new DiffusionLintSaveRunner())
+  ->setAll($args->getArg('all', false))
+  ->setArc($args->getArg('arc'))
+  ->setSeverity($args->getArg('severity'))
+  ->setChunkSize($args->getArg('chunk-size'))
+  ->run('.');
 
-while ($json = fgets(STDIN)) {
+echo "\nProcessed {$count} files.\n";
-  $paths = json_decode(rtrim($json, "\n"), true);
-  if (!is_array($paths)) {
-    throw new Exception("Invalid JSON: {$json}");
-  }
-
-  if (!$paths) {
-    continue;
-  }
-
-  $conn->openTransaction();
-
-  foreach (array_chunk(array_keys($paths), 1024) as $some_paths) {
-    $full_paths = array();
-    foreach ($some_paths as $path) {
-      $full_paths[] = $svn_root.'/'.$path;
-    }
-    queryfx(
-      $conn,
-      'DELETE FROM %T WHERE branchID = %d AND path IN (%Ls)',
-      PhabricatorRepository::TABLE_LINTMESSAGE,
-      $branch->getID(),
-      $full_paths);
-  }
-
-  foreach ($paths as $path => $messages) {
-    // TODO: Handle multiple $json for a single path. Don't save duplicates.
-
-    foreach ($messages as $message) {
-      $inserts[] = qsprintf(
-        $conn,
-        '(%d, %s, %d, %s, %s, %s, %s)',
-        $branch->getID(),
-        $svn_root.'/'.$path,
-        idx($message, 'line', 0),
-        idx($message, 'code', ''),
-        idx($message, 'severity', ''),
-        idx($message, 'name', ''),
-        idx($message, 'description', ''));
-
-      if (count($inserts) >= 256) {
-        save_lint_messages($conn, $inserts);
-        $inserts = array();
-      }
-    }
-  }
-
-  $conn->saveTransaction();
-}
-
-save_lint_messages($conn, $inserts);
-
-function save_lint_messages($conn, array $inserts) {
-  if ($inserts) {
-    queryfx(
-      $conn,
-      'INSERT INTO %T
-        (branchID, path, line, code, severity, name, description)
-        VALUES %Q',
-      PhabricatorRepository::TABLE_LINTMESSAGE,
-      implode(', ', $inserts));
-  }
-}

src/__phutil_library_map__.php

@@ -362,6 +362,7 @@ phutil_register_library_map(array(
     'DiffusionLastModifiedQuery' => 'applications/diffusion/query/lastmodified/DiffusionLastModifiedQuery.php',
     'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
     'DiffusionLintDetailsController' => 'applications/diffusion/controller/DiffusionLintDetailsController.php',
+    'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
     'DiffusionMercurialBranchQuery' => 'applications/diffusion/query/branch/DiffusionMercurialBranchQuery.php',
     'DiffusionMercurialBrowseQuery' => 'applications/diffusion/query/browse/DiffusionMercurialBrowseQuery.php',
     'DiffusionMercurialCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php',

src/applications/diffusion/DiffusionLintSaveRunner.php

@@ -0,0 +1,198 @@
+<?php
+
+final class DiffusionLintSaveRunner {
+  private $arc = 'arc';
+  private $severity = ArcanistLintSeverity::SEVERITY_ADVICE;
+  private $all = false;
+  private $chunkSize = 256;
+
+  private $svnRoot;
+  private $lintCommit;
+  private $branch;
+  private $conn;
+  private $deletes = array();
+  private $inserts = array();
+
+
+  public function setArc($path) {
+    $this->arc = $path;
+    return $this;
+  }
+
+  public function setSeverity($string) {
+    $this->severity = $string;
+    return $this;
+  }
+
+  public function setAll($bool) {
+    $this->all = $bool;
+    return $this;
+  }
+
+  public function setChunkSize($number) {
+    $this->chunkSize = $number;
+    return $this;
+  }
+
+
+  public function run($dir) {
+    $working_copy = ArcanistWorkingCopyIdentity::newFromPath($dir);
+    $api = ArcanistRepositoryAPI::newAPIFromWorkingCopyIdentity($working_copy);
+    $this->svnRoot = id(new PhutilURI($api->getSourceControlPath()))->getPath();
+
+    $project_id = $working_copy->getProjectID();
+    $project = id(new PhabricatorRepositoryArcanistProject())
+      ->loadOneWhere('name = %s', $project_id);
+    if (!$project || !$project->getRepositoryID()) {
+      throw new Exception("Couldn't find repository for {$project_id}.");
+    }
+
+    $branch_name = $api->getBranchName();
+    $this->branch = new PhabricatorRepositoryBranch();
+    $this->conn = $this->branch->establishConnection('w');
+    $this->branch = $this->branch->loadOneWhere(
+      'repositoryID = %d AND name = %s',
+      $project->getRepositoryID(),
+      $branch_name);
+
+    $this->lintCommit = null;
+    if (!$this->branch) {
+      $this->branch = id(new PhabricatorRepositoryBranch())
+        ->setRepositoryID($project->getRepositoryID())
+        ->setName($branch_name)
+        ->save();
+    } else if (!$this->all) {
+      $this->lintCommit = $this->branch->getLintCommit();
+    }
+
+    if ($this->lintCommit) {
+      try {
+        $all_files = $api->getChangedFiles($this->lintCommit);
+      } catch (ArcanistCapabilityNotSupportedException $ex) {
+        $this->lintCommit = null;
+      }
+    }
+
+    if (!$this->lintCommit) {
+      $where = ($this->svnRoot
+        ? qsprintf($this->conn, 'AND path LIKE %>', $this->svnRoot.'/')
+        : '');
+      queryfx(
+        $this->conn,
+        'DELETE FROM %T WHERE branchID = %d %Q',
+        PhabricatorRepository::TABLE_LINTMESSAGE,
+        $this->branch->getID(),
+        $where);
+      $all_files = $api->getAllFiles();
+    }
+
+    $this->deletes = array();
+    $this->inserts = array();
+    $count = 0;
+
+    $files = array();
+    foreach ($all_files as $file => $val) {
+      $count++;
+      if (!$this->lintCommit) {
+        $file = $val;
+      } else {
+        $this->deletes[] = $this->svnRoot.'/'.$file;
+        if ($val & ArcanistRepositoryAPI::FLAG_DELETED) {
+          continue;
+        }
+      }
+      $files[$file] = $file;
+
+      if (count($files) >= $this->chunkSize) {
+        $this->runArcLint($files);
+        $files = array();
+      }
+    }
+
+    $this->runArcLint($files);
+    $this->saveLintMessages();
+    $this->branch->setLintCommit($api->getWorkingCopyRevision());
+    $this->branch->save();
+
+    return $count;
+  }
+
+
+  private function runArcLint(array $files) {
+    if (!$files) {
+      return;
+    }
+
+    echo '.';
+    try {
+      $future = new ExecFuture(
+        '%C lint --severity %s --output json %Ls',
+        $this->arc,
+        $this->severity,
+        $files);
+
+      foreach (new LinesOfALargeExecFuture($future) as $json) {
+        $paths = json_decode($json, true);
+        if (!is_array($paths)) {
+          fprintf(STDERR, "Invalid JSON: {$json}\n");
+          continue;
+        }
+
+        foreach ($paths as $path => $messages) {
+          if (!isset($files[$path])) {
+            continue;
+          }
+
+          foreach ($messages as $message) {
+            $this->inserts[] = qsprintf(
+              $this->conn,
+              '(%d, %s, %d, %s, %s, %s, %s)',
+              $this->branch->getID(),
+              $this->svnRoot.'/'.$path,
+              idx($message, 'line', 0),
+              idx($message, 'code', ''),
+              idx($message, 'severity', ''),
+              idx($message, 'name', ''),
+              idx($message, 'description', ''));
+          }
+
+          if (count($this->deletes) >= 1024 || count($this->inserts) >= 256) {
+            $this->saveLintMessages($this->branch);
+            $this->deletes = array();
+            $this->inserts = array();
+          }
+        }
+      }
+
+    } catch (Exception $ex) {
+      fprintf(STDERR, $ex->getMessage()."\n");
+    }
+  }
+
+
+  private function saveLintMessages() {
+    $this->conn->openTransaction();
+
+    foreach (array_chunk($this->deletes, 1024) as $paths) {
+      queryfx(
+        $this->conn,
+        'DELETE FROM %T WHERE branchID = %d AND path IN (%Ls)',
+        PhabricatorRepository::TABLE_LINTMESSAGE,
+        $this->branch->getID(),
+        $paths);
+    }
+
+    foreach (array_chunk($this->inserts, 256) as $values) {
+      queryfx(
+        $this->conn,
+        'INSERT INTO %T
+          (branchID, path, line, code, severity, name, description)
+          VALUES %Q',
+        PhabricatorRepository::TABLE_LINTMESSAGE,
+        implode(', ', $values));
+    }
+
+    $this->conn->saveTransaction();
+  }
+
+}