Fix an issue where arc diff would loop indefinitely for a property change

Summary:
When we hit a diff which is missing file context, we try to pull it synthetically later. This works for moves and copies, but currently fails for property changes. Since it failed, we didn't have context, so we'd try to pull it again...

The general problem this creates is that when you mark a file "+x" without changing it, we can't show you the content in Differential. Not a huge deal. In some future diff, I'll build the content synthetically.

Adds commits to cover this behavior:

  commit 1830a13adf764b55743f7edc6066451898d8ffa4
  Author: epriestley <git@epriestley.com>
  Date:   Tue Nov 6 17:11:18 2012 -0800

      Mark koan2 +x and edit it.

  commit 8ecc728bcc9b482a9a91527ea471b04fc1a025cf
  Author: epriestley <git@epriestley.com>
  Date:   Tue Nov 6 17:08:44 2012 -0800

      Move 'text' to 'executable' and mark it +x.

  commit 39c8e7dd3914edff087a6214f0cd996ad08e5b3d
  Author: epriestley <git@epriestley.com>
  Date:   Tue Nov 6 16:36:59 2012 -0800

      Mark koan as +x.

Test Plan: Ran unit tests. Previously, they looped indefinitely. Now, they pass.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

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

src/parser/ArcanistBundle.php

@@ -377,7 +377,8 @@ final class ArcanistBundle {
 
       if ($type == ArcanistDiffChangeType::TYPE_COPY_HERE ||
           $type == ArcanistDiffChangeType::TYPE_MOVE_HERE ||
-          $type == ArcanistDiffChangeType::TYPE_COPY_AWAY) {
+          $type == ArcanistDiffChangeType::TYPE_COPY_AWAY ||
+          $type == ArcanistDiffChangeType::TYPE_CHANGE) {
         if ($old_mode !== $new_mode) {
           $result[] = "old mode {$old_mode}".PHP_EOL;
           $result[] = "new mode {$new_mode}".PHP_EOL;

src/parser/ArcanistDiffParser.php

@@ -543,9 +543,15 @@ final class ArcanistDiffParser {
             //
             // ...i.e., there is no associated diff.
 
-            $change->setNeedsSyntheticGitHunks(true);
-            if ($move_source) {
-              $move_source->setNeedsSyntheticGitHunks(true);
+            // This allows us to distinguish between property changes only
+            // and actual moves. For property changes only, we can't currently
+            // build a synthetic diff correctly, so just skip it.
+            // TODO: Build synthetic diffs for property changes, too.
+            if ($change->getType() != ArcanistDiffChangeType::TYPE_CHANGE) {
+              $change->setNeedsSyntheticGitHunks(true);
+              if ($move_source) {
+                $move_source->setNeedsSyntheticGitHunks(true);
+              }
             }
             return;
           }

src/parser/__tests__/ArcanistBundleTestCase.php

@@ -130,6 +130,60 @@ final class ArcanistBundleTestCase extends ArcanistTestCase {
     }
 
     switch ($commit) {
+      case '1830a13adf764b55743f7edc6066451898d8ffa4':
+        // "Mark koan2 as +x and edit it."
+
+        $this->assertEqual(1, count($changes));
+
+        $c = $changes['koan2'];
+        $this->assertEqual(
+          ArcanistDiffChangeType::TYPE_CHANGE,
+          $c->getType());
+
+        $this->assertEqual(
+          '100644',
+          idx($c->getOldProperties(), 'unix:filemode'));
+
+        $this->assertEqual(
+          '100755',
+          idx($c->getNewProperties(), 'unix:filemode'));
+        break;
+      case '8ecc728bcc9b482a9a91527ea471b04fc1a025cf':
+        // "Move 'text' to 'executable' and mark it +x."
+
+        $this->assertEqual(2, count($changes));
+
+        $c = $changes['executable'];
+        $this->assertEqual(
+          ArcanistDiffChangeType::TYPE_MOVE_HERE,
+          $c->getType());
+
+        $this->assertEqual(
+          '100644',
+          idx($c->getOldProperties(), 'unix:filemode'));
+
+        $this->assertEqual(
+          '100755',
+          idx($c->getNewProperties(), 'unix:filemode'));
+        break;
+      case '39c8e7dd3914edff087a6214f0cd996ad08e5b3d':
+        // "Mark koan as +x."
+        // Primarily a test against a recusive synthetic hunk construction bug.
+        $this->assertEqual(1, count($changes));
+
+        $c = $changes['koan'];
+        $this->assertEqual(
+          ArcanistDiffChangeType::TYPE_CHANGE,
+          $c->getType());
+
+        $this->assertEqual(
+          '100644',
+          idx($c->getOldProperties(), 'unix:filemode'));
+
+        $this->assertEqual(
+          '100755',
+          idx($c->getNewProperties(), 'unix:filemode'));
+        break;
       case 'c573c25d1a767d270fed504cd993e78aba936338':
         // "Copy a koan over text, editing the original koan."
         // Git doesn't really do anything meaningful with this.

src/parser/__tests__/patches/1830a13adf764b55743f7edc6066451898d8ffa4.gitpatch

@@ -0,0 +1,12 @@
+diff --git a/koan2 b/koan2
+old mode 100644
+new mode 100755
+--- a/koan2
++++ b/koan2
+@@ -10,3 +10,5 @@
+ as moves if both files
+ are mostly similar. So
+ you need a lot of text.
++
++Execute!
+

src/parser/__tests__/patches/39c8e7dd3914edff087a6214f0cd996ad08e5b3d.gitpatch

@@ -0,0 +1,4 @@
+diff --git a/koan b/koan
+old mode 100644
+new mode 100755
+

src/parser/__tests__/patches/8ecc728bcc9b482a9a91527ea471b04fc1a025cf.gitpatch

@@ -0,0 +1,6 @@
+diff --git a/text b/executable
+old mode 100644
+new mode 100755
+rename from text
+rename to executable
+