From 16caf63d98f9b65b75a036dd45e1089f29a16da1 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 9 Nov 2011 19:44:53 -0800 Subject: [PATCH] make arc issue a warning if file upload fails during diff creation Summary: just a little try / catch action in ArcanistDiffWorkflow. "Ideally, it would be good to flag these changes somehow so that "arc patch" can issue the inverse warning (e.g., "This patch could not be completely applied because some binary data was not uploaded.") but that hasn't come up in a real use case yet." I think a change w/ filetype of FILE_IMAGE || FILE_BINARY *and* no phid means that the upload failed so there's no additional flag needed. (True?) however, it would be easy enough to store metadata that explicilty stated whether or not the file upload succeeded. (also / related - i looked through the arc patch workflow a bit and i don't understand how the svn codepath loads up the actual binary files... for git, toGitPatch => buildBinaryChange => getBlob is the right path ) Test Plan: - set 'storage.mysql-engine.max-size' to 0 in my conf, uploaded diffs with files - noted in differential that it correctly detected images versus binary despite file upload failing - noted in differential that images had some empty UI - reverted conf change, uploaded diffs with files - noted in differential file showed up - ran arc patch with DX, where DX had broken files - noted "Downloading binary data... Exception: ERR-BAD-PHID: No such file exists." Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Differential Revision: 1100 --- src/workflow/diff/ArcanistDiffWorkflow.php | 47 +++++++++++++--------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/workflow/diff/ArcanistDiffWorkflow.php b/src/workflow/diff/ArcanistDiffWorkflow.php index e26286c5..fc573492 100644 --- a/src/workflow/diff/ArcanistDiffWorkflow.php +++ b/src/workflow/diff/ArcanistDiffWorkflow.php @@ -844,22 +844,22 @@ EOTEXT } $path = $change->getCurrentPath(); + $name = basename($path); + $old_file = $repository_api->getOriginalFileData($path); - $new_file = $repository_api->getCurrentFileData($path); - - $old_dict = $this->uploadFile($old_file, basename($path), 'old binary'); - $new_dict = $this->uploadFile($new_file, basename($path), 'new binary'); - + $old_dict = $this->uploadFile($old_file, $name, 'old binary'); if ($old_dict['guid']) { $change->setMetadata('old:binary-phid', $old_dict['guid']); } + $change->setMetadata('old:file:size', $old_dict['size']); + $change->setMetadata('old:file:mime-type', $old_dict['mime']); + + $new_file = $repository_api->getCurrentFileData($path); + $new_dict = $this->uploadFile($new_file, $name, 'new binary'); if ($new_dict['guid']) { $change->setMetadata('new:binary-phid', $new_dict['guid']); } - - $change->setMetadata('old:file:size', strlen($old_file)); - $change->setMetadata('new:file:size', strlen($new_file)); - $change->setMetadata('old:file:mime-type', $old_dict['mime']); + $change->setMetadata('new:file:size', $new_dict['size']); $change->setMetadata('new:file:mime-type', $new_dict['mime']); if (preg_match('@^image/@', $new_dict['mime'])) { @@ -867,7 +867,6 @@ EOTEXT } } - return $changes; } @@ -875,9 +874,11 @@ EOTEXT $result = array( 'guid' => null, 'mime' => null, + 'size' => null ); - if (!strlen($data)) { + $result['size'] = $size = strlen($data); + if (!$size) { return $result; } @@ -888,17 +889,25 @@ EOTEXT $mime_type = trim($mime_type); $result['mime'] = $mime_type; - $bytes = strlen($data); - echo "Uploading {$desc} '{$name}' ({$mime_type}, {$bytes} bytes)...\n"; + echo "Uploading {$desc} '{$name}' ({$mime_type}, {$size} bytes)...\n"; - $guid = $this->getConduit()->callMethodSynchronous( - 'file.upload', - array( - 'data_base64' => base64_encode($data), - 'name' => $name, + try { + $guid = $this->getConduit()->callMethodSynchronous( + 'file.upload', + array( + 'data_base64' => base64_encode($data), + 'name' => $name, )); - $result['guid'] = $guid; + $result['guid'] = $guid; + } catch (ConduitClientException $e) { + $message = "Failed to upload {$desc} '{$name}'. Continue?"; + if (!phutil_console_confirm($message, $default_no = false)) { + throw new ArcanistUsageException( + 'Aborted due to file upload failure.' + ); + } + } return $result; }