Mark IMPORTED_CHANGE more consistently

Summary:
See <https://github.com/facebook/phabricator/issues/425>. There are some ways that the change parsers may not reach `finishParse()`, but we now need them to in order to mark the commit imported, advance the progress bar, and eventually kick the repository out of IMPORTING status.

Take all the copy/pasted code in the parsers and move it into the parent. Specifically, this is:

  - Printing a status message about starting a parse;
  - checking for bad commits;
  - queueing the next parse stage; and
  - marking the import step complete.

Test Plan: Used `reparse.php --change` to reparse Git, SVN and Mercurial repos.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

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

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php

@@ -9,6 +9,31 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker
     return 60 * 60 * 24;
   }
 
+  abstract protected function parseCommitChanges(
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryCommit $commit);
+
+  protected function parseCommit(
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryCommit $commit) {
+
+    $identifier = $commit->getCommitIdentifier();
+    $callsign = $repository->getCallsign();
+    $full_name = 'r'.$callsign.$identifier;
+
+    $this->log("Parsing %s...\n", $full_name);
+    if ($this->isBadCommit($full_name)) {
+      $this->log("This commit is marked bad!");
+      $result = null;
+    } else {
+      $result = $this->parseCommitChanges($repository, $commit);
+    }
+
+    $this->finishParse();
+
+    return $result;
+  }
+
   public static function lookupOrCreatePaths(array $paths) {
     $repository = new PhabricatorRepository();
     $conn_w = $repository->establishConnection('w');

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryGitCommitChangeParserWorker.php

@@ -3,17 +3,10 @@
 final class PhabricatorRepositoryGitCommitChangeParserWorker
   extends PhabricatorRepositoryCommitChangeParserWorker {
 
-  protected function parseCommit(
+  protected function parseCommitChanges(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
-    $full_name = 'r'.$repository->getCallsign().$commit->getCommitIdentifier();
-    echo "Parsing {$full_name}...\n";
-    if ($this->isBadCommit($full_name)) {
-      echo "This commit is marked bad!\n";
-      return;
-    }
-
     // Check if the commit has parents. We're testing to see whether it is the
     // first commit in history (in which case we must use "git log") or some
     // other commit (in which case we can use "git diff"). We'd rather use
@@ -272,8 +265,6 @@ final class PhabricatorRepositoryGitCommitChangeParserWorker
         PhabricatorRepository::TABLE_PATHCHANGE,
         implode(', ', $sql_chunk));
     }
-
-    $this->finishParse();
   }
 
 }

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryMercurialCommitChangeParserWorker.php

@@ -3,17 +3,10 @@
 final class PhabricatorRepositoryMercurialCommitChangeParserWorker
   extends PhabricatorRepositoryCommitChangeParserWorker {
 
-  protected function parseCommit(
+  protected function parseCommitChanges(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
-    $full_name = 'r'.$repository->getCallsign().$commit->getCommitIdentifier();
-    echo "Parsing {$full_name}...\n";
-    if ($this->isBadCommit($full_name)) {
-      echo "This commit is marked bad!\n";
-      return;
-    }
-
     list($stdout) = $repository->execxLocalCommand(
       'status -C --change %s',
       $commit->getCommitIdentifier());
@@ -307,8 +300,6 @@ final class PhabricatorRepositoryMercurialCommitChangeParserWorker
         PhabricatorRepository::TABLE_PATHCHANGE,
         implode(', ', $sql_chunk));
     }
-
-    $this->finishParse();
   }
 
   private function mercurialPathExists(

src/applications/repository/worker/commitchangeparser/PhabricatorRepositorySvnCommitChangeParserWorker.php

@@ -3,7 +3,7 @@
 final class PhabricatorRepositorySvnCommitChangeParserWorker
   extends PhabricatorRepositoryCommitChangeParserWorker {
 
-  protected function parseCommit(
+  protected function parseCommitChanges(
     PhabricatorRepository $repository,
     PhabricatorRepositoryCommit $commit) {
 
@@ -27,15 +27,6 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker
     $uri = $repository->getDetail('remote-uri');
     $svn_commit = $commit->getCommitIdentifier();
 
-    $callsign = $repository->getCallsign();
-    $full_name = 'r'.$callsign.$svn_commit;
-    echo "Parsing {$full_name}...\n";
-
-    if ($this->isBadCommit($full_name)) {
-      echo "This commit is marked bad!\n";
-      return;
-    }
-
     // Pull the top-level path changes out of "svn log". This is pretty
     // straightforward; just parse the XML log.
     $log = $this->getSVNLogXMLObject($uri, $svn_commit, $verbose = true);
@@ -368,8 +359,6 @@ final class PhabricatorRepositorySvnCommitChangeParserWorker
 
     $this->writeChanges($repository, $commit, $effects, $path_map, $commit_map);
     $this->writeBrowse($repository, $commit, $effects, $path_map);
-
-    $this->finishParse();
   }
 
   private function writeChanges(