From 36de84eeee5119966a532249f32bfbb86f726678 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 14 Jun 2011 21:25:31 -0700 Subject: [PATCH] Make image diffs work again Summary: I looked at this quickly to see what was involved and there were only a couple of issues so here's a patch: - On OSX, the "-i" flag does not mean "--mime". Use "--mime" explicitly instead. This is a minor fix which affects only OS X. - I wasn't able to repro the "crazy executables" behavior and think it might have been me messing something up, so nuke it until we see an issue. - Some guid vs phid shenanigans. Differential reads "phid" but arcanist set "guid". We should move toward "phid"; I started using "guid" before I realized it was an overloaded term that also refers to a specific GUID implementation (Microsoft's UUID). Test Plan: Uploaded an image diff, saw images in Differential. Reviewed By: aran Reviewers: jianfeng, jungejason, aran, tuomaspelkonen CC: aran Differential Revision: 466 --- src/workflow/diff/ArcanistDiffWorkflow.php | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index f4dc2bba..8d2a8418 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -755,10 +755,10 @@ EOTEXT $new_dict = $this->uploadFile($new_file, basename($path), 'new binary'); if ($old_dict['guid']) { - $change->setMetadata('old:binary-guid', $old_dict['guid']); + $change->setMetadata('old:binary-phid', $old_dict['guid']); } if ($new_dict['guid']) { - $change->setMetadata('new:binary-guid', $new_dict['guid']); + $change->setMetadata('new:binary-phid', $new_dict['guid']); } $change->setMetadata('old:file:size', strlen($old_file)); @@ -785,17 +785,11 @@ EOTEXT return $result; } - $future = new ExecFuture('file -ib -'); + $future = new ExecFuture('file -b --mime -'); $future->write($data); list($mime_type) = $future->resolvex(); $mime_type = trim($mime_type); - if (strpos($mime_type, ',') !== false) { - // TODO: This is kind of silly, but 'file -ib' goes crazy on executables. - $mime_type = reset(explode(',', $mime_type)); - } - - $result['mime'] = $mime_type; // TODO: Make this configurable.