revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-23 05:50:54 +01:00
Code Issues Releases Wiki Activity

When converting a file to a binary, populate the binary's data

Browse source 
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
This commit is contained in:
epriestley 2013-08-27 09:34:30 -07:00
parent a28d4ff3e4
commit 3ad72195bf
2 changed files with 17 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/parser/diff/ArcanistDiffChange.php
View file

@ -234,7 +234,18 @@ final class ArcanistDiffChange {
return $line_map; 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->hunks = array();
$this->setFileType(ArcanistDiffChangeType::FILE_BINARY); $this->setFileType(ArcanistDiffChangeType::FILE_BINARY);
return $this; return $this;

9
src/workflow/ArcanistDiffWorkflow.php
View file

@ -994,7 +994,7 @@ EOTEXT
"{$byte_warning} If the file is not a text file, you can ". "{$byte_warning} If the file is not a text file, you can ".
"mark it 'binary'. Mark this file as 'binary' and continue?"; "mark it 'binary'. Mark this file as 'binary' and continue?";
if (phutil_console_confirm($confirm)) { if (phutil_console_confirm($confirm)) {
$change->convertToBinaryChange(); $change->convertToBinaryChange($repository_api);
} else { } else {
throw new ArcanistUsageException( throw new ArcanistUsageException(
"Aborted generation of gigantic diff."); "Aborted generation of gigantic diff.");
@ -1080,7 +1080,7 @@ EOTEXT
throw new ArcanistUsageException("Aborted workflow to fix UTF-8."); throw new ArcanistUsageException("Aborted workflow to fix UTF-8.");
} else { } else {
foreach ($utf8_problems as $change) { foreach ($utf8_problems as $change) {
$change->convertToBinaryChange(); $change->convertToBinaryChange($repository_api);
} }
} }
} }
@ -2477,8 +2477,9 @@ EOTEXT
$change->setMetadata("{$type}:file:size", $size); $change->setMetadata("{$type}:file:size", $size);
if ($spec['data'] === null) { if ($spec['data'] === null) {
// This covers the case where a file was added or removed; we don't // 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 // need to upload the other half of it (e.g., the old file data for
// do upload.) // a file which was just added). This is distinct from an empty
// file, which we do upload.
unset($need_upload[$key]); unset($need_upload[$key]);
continue; continue;
} }