From 5b6fbf70e05902204ec66f37c01a4ad65fe81a38 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 29 May 2011 10:45:18 -0700
Subject: [PATCH] Disable 'textconv' when diffing

Summary:
Adds "--no-textconv" to all 'git diff' commands so we don't invoke textconv. See
T178 for discussion.

Test Plan:
Added something like this to .gitattributes:

  *.txt diff=uppercase

And then this to .git/config:

  [diff "uppercase"]
    textconv = /path/to/uppercase

...where "uppercase" is a script which takes a file and emits an uppercase
version of it.

Then I added a "wisdom.txt" text file:

  The cow goes "moo".
  The duck goes "quack".

Without this patch, the file appears in uppercase in Differential, i.e. textconv
runs. With this patch, it appears as the original text.

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: elgenie, aran, tuomaspelkonen
Differential Revision: 372
---
 src/repository/api/git/ArcanistGitAPI.php | 31 ++++++++++++++++++-----
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/repository/api/git/ArcanistGitAPI.php b/src/repository/api/git/ArcanistGitAPI.php
index 5b4e2c30..1df02444 100644
--- a/src/repository/api/git/ArcanistGitAPI.php
+++ b/src/repository/api/git/ArcanistGitAPI.php
@@ -60,11 +60,11 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return $this->relativeCommit;
   }
 
-  private function getDiffOptions() {
+  private function getDiffFullOptions() {
     $options = array(
+      self::getDiffBaseOptions(),
       '-M',
       '-C',
-      '--no-ext-diff',
       '--no-color',
       '--src-prefix=a/',
       '--dst-prefix=b/',
@@ -73,8 +73,22 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
     return implode(' ', $options);
   }
 
+  private function getDiffBaseOptions() {
+    $options = array(
+      // Disable external diff drivers, like graphical differs, since Arcanist
+      // needs to capture the diff text.
+      '--no-ext-diff',
+      // Disable textconv so we treat binary files as binary, even if they have
+      // an alternative textual representation. TODO: Ideally, Differential
+      // would ship up the binaries for 'arc patch' but display the textconv
+      // output in the visual diff.
+      '--no-textconv',
+    );
+    return implode(' ', $options);
+  }
+
   public function getFullGitDiff() {
-    $options = $this->getDiffOptions();
+    $options = $this->getDiffFullOptions();
     list($stdout) = execx(
       "(cd %s; git diff {$options} %s --)",
       $this->getPath(),
@@ -84,7 +98,7 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
   public function getRawDiffText($path) {
     $relative_commit = $this->getRelativeCommit();
-    $options = $this->getDiffOptions();
+    $options = $this->getDiffFullOptions();
     list($stdout) = execx(
       "(cd %s; git diff {$options} %s -- %s)",
       $this->getPath(),
@@ -167,16 +181,18 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
   public function getWorkingCopyStatus() {
     if (!isset($this->status)) {
 
+      $options = $this->getDiffBaseOptions();
+
       // Find committed changes.
       list($stdout) = execx(
-        '(cd %s; git diff --no-ext-diff --raw %s --)',
+        "(cd %s; git diff {$options} --raw %s --)",
         $this->getPath(),
         $this->getRelativeCommit());
       $files = $this->parseGitStatus($stdout);
 
       // Find uncommitted changes.
       list($stdout) = execx(
-        '(cd %s; git diff --no-ext-diff --raw HEAD --)',
+        "(cd %s; git diff {$options} --raw HEAD --)",
         $this->getPath());
       $uncommitted_files = $this->parseGitStatus($stdout);
       foreach ($uncommitted_files as $path => $mask) {
@@ -230,8 +246,9 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
   }
 
   public function getPreReceiveHookStatus($old_ref, $new_ref) {
+    $options = $this->getDiffBaseOptions();
     list($stdout) = execx(
-      '(cd %s && git diff --no-ext-diff --raw %s %s --)',
+      "(cd %s && git diff {$options} --raw %s %s --)",
       $this->getPath(),
       $old_ref,
       $new_ref);