Attach commit diff to its revision

Summary:
This attaches commit diff to its associated revision as any other diff.
The consequence is that the revision page now shows the actual commit instead of the last diff. It may be disturbing but it is desired.
Another consequence is that lint and unit results are displayed as skipped after the revision was committed. I want to fix it somehow.

My next plan is to automatically diff against the last normal diff and include the link to this diff in commit e-mail if non-empty.

Test Plan:
  reparse.php

Diff against last normal diff.

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran, Koolvin, jungejason

Maniphest Tasks: T201

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

src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php

@@ -58,6 +58,15 @@ final class DifferentialRevisionViewController extends DifferentialController {
       }
     }
 
+    $target_manual = $target;
+    if (!$target_id) {
+      foreach ($diffs as $diff) {
+        if ($diff->getCreationMethod() != 'commit') {
+          $target_manual = $diff;
+        }
+      }
+    }
+
     $diffs = mpull($diffs, null, 'getID');
     if (empty($diffs[$diff_vs])) {
       $diff_vs = null;
@@ -65,7 +74,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
 
     list($aux_fields, $props) = $this->loadAuxiliaryFieldsAndProperties(
       $revision,
-      $target,
+      $target_manual,
       array(
         'local:commits',
         'arc:unit',
@@ -205,18 +214,17 @@ final class DifferentialRevisionViewController extends DifferentialController {
         newv($custom_renderer_class, array());
       $actions = array_merge(
         $actions,
-        $custom_renderer->generateActionLinks($revision, $target));
+        $custom_renderer->generateActionLinks($revision, $target_manual));
     }
 
     $whitespace = $request->getStr(
       'whitespace',
       DifferentialChangesetParser::WHITESPACE_IGNORE_ALL);
 
-    $arc_project = $target->loadArcanistProject();
+    $arc_project = $target_manual->loadArcanistProject();
 
     if ($arc_project) {
       $symbol_indexes = $this->buildSymbolIndexes(
-        $target,
         $arc_project,
         $visible_changesets);
       $repository = $arc_project->loadRepository();
@@ -704,7 +712,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
   }
 
   private function buildSymbolIndexes(
-    DifferentialDiff $target,
     PhabricatorRepositoryArcanistProject $arc_project,
     array $visible_changesets) {
     assert_instances_of($visible_changesets, 'DifferentialChangeset');

src/applications/differential/view/revisionupdatehistory/DifferentialRevisionUpdateHistoryView.php

@@ -24,7 +24,8 @@ final class DifferentialRevisionUpdateHistoryView extends AphrontView {
   private $selectedWhitespace;
   private $user;
 
-  public function setDiffs($diffs) {
+  public function setDiffs(array $diffs) {
+    assert_instances_of($diffs, 'DifferentialDiff');
     $this->diffs = $diffs;
     return $this;
   }

src/applications/repository/worker/commitmessageparser/base/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -92,13 +92,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
           DifferentialRevision::TABLE_COMMIT,
           $revision->getID(),
           $commit->getPHID());
-
+        $commit_is_new = $conn_w->getAffectedRows();
-        $status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
-        $should_close = ($revision->getStatus() != $status_closed) &&
-                        (!$repository->getDetail('disable-autoclose', false));
-
-        if ($should_close) {
-          $revision->setDateCommitted($commit->getEpoch());
 
         $message = null;
         $committer = $data->getCommitDetail('authorPHID');
@@ -106,6 +100,13 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
           $committer = $revision->getAuthorPHID();
           $message = 'Closed by '.$data->getAuthorName().'.';
         }
+
+        $status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
+        $should_close = ($revision->getStatus() != $status_closed) &&
+                        (!$repository->getDetail('disable-autoclose', false));
+
+        if ($should_close) {
+          $revision->setDateCommitted($commit->getEpoch());
           $editor = new DifferentialCommentEditor(
             $revision,
             $committer,
@@ -113,9 +114,48 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
           $editor->setIsDaemonWorkflow(true);
           $editor->setMessage($message)->save();
         }
+
+        if ($commit_is_new) {
+          $this->attachToRevision($revision, $committer);
         }
       }
     }
+  }
+
+  private function attachToRevision(
+    DifferentialRevision $revision,
+    $committer) {
+
+    $drequest = DiffusionRequest::newFromDictionary(array(
+      'repository' => $this->repository,
+      'commit' => $this->commit->getCommitIdentifier(),
+    ));
+
+    $raw_diff = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest)
+      ->loadRawDiff();
+
+    $changes = id(new ArcanistDiffParser())->parseDiff($raw_diff);
+    $diff = DifferentialDiff::newFromRawChanges($changes)
+      ->setRevisionID($revision->getID())
+      ->setAuthorPHID($committer)
+      ->setCreationMethod('commit')
+      ->setSourceControlSystem($this->repository->getVersionControlSystem())
+      ->setLintStatus(DifferentialLintStatus::LINT_SKIP)
+      ->setUnitStatus(DifferentialUnitStatus::UNIT_SKIP)
+      ->setDateCreated($this->commit->getEpoch())
+      ->setDescription(
+        'Commit r'.
+        $this->repository->getCallsign().
+        $this->commit->getCommitIdentifier());
+
+    $parents = DiffusionCommitParentsQuery::newFromDiffusionRequest($drequest)
+      ->loadParents();
+    if ($parents) {
+      $diff->setSourceControlBaseRevision(head_key($parents));
+    }
+
+    $diff->save();
+  }
 
   /**
    * When querying for revisions by hash, more than one revision may be found.

src/applications/repository/worker/commitmessageparser/base/__init__.php

@@ -7,11 +7,18 @@
 
 
 phutil_require_module('arcanist', 'differential/constants/revisionstatus');
+phutil_require_module('arcanist', 'parser/diff');
 
 phutil_require_module('phabricator', 'applications/differential/constants/action');
+phutil_require_module('phabricator', 'applications/differential/constants/lintstatus');
+phutil_require_module('phabricator', 'applications/differential/constants/unitstatus');
 phutil_require_module('phabricator', 'applications/differential/editor/comment');
 phutil_require_module('phabricator', 'applications/differential/query/revision');
+phutil_require_module('phabricator', 'applications/differential/storage/diff');
 phutil_require_module('phabricator', 'applications/differential/storage/revision');
+phutil_require_module('phabricator', 'applications/diffusion/query/parents/base');
+phutil_require_module('phabricator', 'applications/diffusion/query/rawdiff/base');
+phutil_require_module('phabricator', 'applications/diffusion/request/base');
 phutil_require_module('phabricator', 'applications/repository/storage/commitdata');
 phutil_require_module('phabricator', 'applications/repository/worker/base');
 phutil_require_module('phabricator', 'storage/queryfx');