From 6d631577f23c245f73a53d9e35b43ceb24494d51 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 21 Mar 2012 17:10:05 -0700
Subject: [PATCH] Detect changes in merge commits as the diff of the merge and
 the first parent

Summary:
Currently, we use "git log" to detect the change list for all commits, but this produces no output for merge commits.

Instead, parse them as changes against the first parent (the merge destination). This produces generally sensible/expected behavior, and is consistent with what GitHub does.

We need to special-case the first commit because it doesn't have parents.

NOTE: This is a parser change so you need to run `./scripts/repository/reparse.php --all <callsign> --change` to reparse merge commits in already-imported repositories after updating.

Test Plan: Reparsed a merge commit, a non-merge commit, and the first commit in the Phabricator repository.

Reviewers: btrahan, gschmidt

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T961

Differential Revision: https://secure.phabricator.com/D1985
---
 ...rRepositoryGitCommitChangeParserWorker.php | 36 ++++++++++++++++---
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/applications/repository/worker/commitchangeparser/git/PhabricatorRepositoryGitCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/git/PhabricatorRepositoryGitCommitChangeParserWorker.php
index 25a841fd4a..6ddbb67223 100644
--- a/src/applications/repository/worker/commitchangeparser/git/PhabricatorRepositoryGitCommitChangeParserWorker.php
+++ b/src/applications/repository/worker/commitchangeparser/git/PhabricatorRepositoryGitCommitChangeParserWorker.php
@@ -30,13 +30,39 @@ final class PhabricatorRepositoryGitCommitChangeParserWorker
       return;
     }
 
-    // NOTE: "--pretty=format: " is to disable log output, we only want the
-    // part we get from "--raw".
-    list($raw) = $repository->execxLocalCommand(
-      'log -n1 -M -C -B --find-copies-harder --raw -t '.
-        '--abbrev=40 --pretty=format: %s',
+    // 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
+    // "git diff" because it has the right behavior for merge commits, but
+    // it requires the commit to have a parent that we can diff against. The
+    // first commit doesn't, so "commit^" is not a valid ref.
+    list($parents) = $repository->execxLocalCommand(
+      'log -n1 --format=%s %s',
+      '%P',
       $commit->getCommitIdentifier());
 
+    $use_log = !strlen(trim($parents));
+    if ($use_log) {
+      // This is the first commit so we need to use "log". We know it's not a
+      // merge commit because it couldn't be merging anything, so this is safe.
+
+      // NOTE: "--pretty=format: " is to disable diff output, we only want the
+      // part we get from "--raw".
+      list($raw) = $repository->execxLocalCommand(
+        'log -n1 -M -C -B --find-copies-harder --raw -t '.
+          '--pretty=format: --abbrev=40 %s',
+        $commit->getCommitIdentifier());
+    } else {
+      // Otherwise, we can use "diff", which will give us output for merges.
+      // We diff against the first parent, as this is generally the expectation
+      // and results in sensible behavior.
+      list($raw) = $repository->execxLocalCommand(
+        'diff -n1 -M -C -B --find-copies-harder --raw -t '.
+          '--abbrev=40 %s^1 %s',
+        $commit->getCommitIdentifier(),
+        $commit->getCommitIdentifier());
+    }
+
     $changes = array();
     $move_away = array();
     $copy_away = array();