From 6bcfc212ebd42f306195981b5eee382334dfeabb Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 29 Jan 2019 09:08:32 -0800
Subject: [PATCH] Allow diff change detection to complete for Mercurial changes
 which remove a binary file

Summary:
See PHI1044. When you push a commit, we try to detect if the diff is the same as the last diff in Differential. This is a soft/heuristic check and only used to provide a hint in email ("CHANGED SINCE LAST DIFF").

For some changes, we can end up on this pathway with a binary changeset for a deleted file in the commit, and try to fetch the file data. This will fail since the file has been deleted. I'm not //entirely// sure why this hasn't cropped up before and didn't try to truly build an end-to-end test case, but it's a valid state that we shouldn't fatal in.

When we get here in this state, just detect a change. This is safe, even if it isn't always correct.

(This will be revisited in the future so I'm favoring fixing the broken behavior for now.)

Test Plan: This required some rigging, but I modified `bin/differential extract` to run the `isDiffChangedBeforeCommit()` code, then rigged a commit/diff to hit this case and reproduced the reported error. After the change, the comparison worked cleanly.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20056
---
 .../DifferentialDiffExtractionEngine.php      | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
index d7d7c767e1..861d2ad220 100644
--- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
+++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php
@@ -177,14 +177,21 @@ final class DifferentialDiffExtractionEngine extends Phobject {
             'repository' => $repository,
           ));
 
-        $response = DiffusionQuery::callConduitWithDiffusionRequest(
-          $viewer,
-          $drequest,
-          'diffusion.filecontentquery',
-          array(
-            'commit' => $identifier,
-            'path' => $path,
-          ));
+        try {
+          $response = DiffusionQuery::callConduitWithDiffusionRequest(
+            $viewer,
+            $drequest,
+            'diffusion.filecontentquery',
+            array(
+              'commit' => $identifier,
+              'path' => $path,
+            ));
+        } catch (Exception $ex) {
+          // TODO: See PHI1044. This call may fail if the diff deleted the
+          // file. If the call fails, just detect a change for now. This should
+          // generally be made cleaner in the future.
+          return true;
+        }
 
         $new_file_phid = $response['filePHID'];
         if (!$new_file_phid) {