From 3ad72195bfc187c8d5e573f9ef0659b286aec327 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 27 Aug 2013 09:34:30 -0700
Subject: [PATCH] When converting a file to a binary, populate the binary's
 data

Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.

A simple reproduction case is to build a test file (I used one with bytes from 1..255):

  $ # Don't include \0, since Git treats that specially.
  $ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt

Then add it:

  $ git add example.txt
  $ git commit -a -m derp
  $ arc diff --only HEAD^

You'll be prompted to convert the file to binary:

  Do you want to mark this file as binary and continue? [Y/n] y

Before this patch, that would be followed by:

  Uploading 0 files...

...which is incorrect; we need to upload the new data. After this patch, this shows:

  Uploading 1 files...

...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.

Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.

Reviewers: zeeg, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6815
---
 src/parser/diff/ArcanistDiffChange.php | 13 ++++++++++++-
 src/workflow/ArcanistDiffWorkflow.php  |  9 +++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/parser/diff/ArcanistDiffChange.php b/src/parser/diff/ArcanistDiffChange.php
index 770ecc86..26d187b9 100644
--- a/src/parser/diff/ArcanistDiffChange.php
+++ b/src/parser/diff/ArcanistDiffChange.php
@@ -234,7 +234,18 @@ final class ArcanistDiffChange {
     return $line_map;
   }
 
-  public function convertToBinaryChange() {
+  public function convertToBinaryChange(ArcanistRepositoryAPI $api) {
+
+    // Fill in the binary data from the working copy.
+
+    $this->setOriginalFileData(
+      $api->getOriginalFileData(
+        $this->getOldPath()));
+
+    $this->setCurrentFileData(
+      $api->getCurrentFileData(
+        $this->getCurrentPath()));
+
     $this->hunks = array();
     $this->setFileType(ArcanistDiffChangeType::FILE_BINARY);
     return $this;
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
index 7129a49b..1187fd9f 100644
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -994,7 +994,7 @@ EOTEXT
             "{$byte_warning} If the file is not a text file, you can ".
             "mark it 'binary'. Mark this file as 'binary' and continue?";
           if (phutil_console_confirm($confirm)) {
-            $change->convertToBinaryChange();
+            $change->convertToBinaryChange($repository_api);
           } else {
             throw new ArcanistUsageException(
               "Aborted generation of gigantic diff.");
@@ -1080,7 +1080,7 @@ EOTEXT
         throw new ArcanistUsageException("Aborted workflow to fix UTF-8.");
       } else {
         foreach ($utf8_problems as $change) {
-          $change->convertToBinaryChange();
+          $change->convertToBinaryChange($repository_api);
         }
       }
     }
@@ -2477,8 +2477,9 @@ EOTEXT
       $change->setMetadata("{$type}:file:size", $size);
       if ($spec['data'] === null) {
         // This covers the case where a file was added or removed; we don't
-        // need to upload it. (This is distinct from an empty file, which we
-        // do upload.)
+        // need to upload the other half of it (e.g., the old file data for
+        // a file which was just added). This is distinct from an empty
+        // file, which we do upload.
         unset($need_upload[$key]);
         continue;
       }