Apply the new patch byte size limit to mail patch generation in Differential
Summary: Ref T13137. See PHI592. Depends on D19444. Apply a limit up front to stop patches which are way too big (e.g., 600MB of videos) from generating in the first place. Test Plan: - Configured inline patches in git format. - Created a normal revision, got an inline git patch. - Created a revision with a 10MB video file, got no inline patch. - (Added a bunch of debugging stuff to make sure the internal pathway was working.) Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13137 Differential Revision: https://secure.phabricator.com/D19445
This commit is contained in:
		| @@ -682,8 +682,13 @@ final class DifferentialTransactionEditor | ||||
|       if ($config_inline || $config_attach) { | ||||
|         $body_limit = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); | ||||
|  | ||||
|         $patch = $this->buildPatchForMail($diff); | ||||
|         if ($config_inline) { | ||||
|         try { | ||||
|           $patch = $this->buildPatchForMail($diff, $body_limit); | ||||
|         } catch (ArcanistDiffByteSizeException $ex) { | ||||
|           $patch = null; | ||||
|         } | ||||
|  | ||||
|         if (($patch !== null) && $config_inline) { | ||||
|           $lines = substr_count($patch, "\n"); | ||||
|           $bytes = strlen($patch); | ||||
|  | ||||
| @@ -706,7 +711,7 @@ final class DifferentialTransactionEditor | ||||
|           } | ||||
|         } | ||||
|  | ||||
|         if ($config_attach) { | ||||
|         if (($patch !== null) && $config_attach) { | ||||
|           // See T12033, T11767, and PHI55. This is a crude fix to stop the | ||||
|           // major concrete problems that lackluster email size limits cause. | ||||
|           if (strlen($patch) < $body_limit) { | ||||
| @@ -1411,13 +1416,14 @@ final class DifferentialTransactionEditor | ||||
|       array('style' => 'font-family: monospace;'), $patch); | ||||
|   } | ||||
|  | ||||
|   private function buildPatchForMail(DifferentialDiff $diff) { | ||||
|   private function buildPatchForMail(DifferentialDiff $diff, $byte_limit) { | ||||
|     $format = PhabricatorEnv::getEnvConfig('metamta.differential.patch-format'); | ||||
|  | ||||
|     return id(new DifferentialRawDiffRenderer()) | ||||
|       ->setViewer($this->getActor()) | ||||
|       ->setFormat($format) | ||||
|       ->setChangesets($diff->getChangesets()) | ||||
|       ->setByteLimit($byte_limit) | ||||
|       ->buildPatch(); | ||||
|   } | ||||
|  | ||||
|   | ||||
| @@ -5,6 +5,7 @@ final class DifferentialRawDiffRenderer extends Phobject { | ||||
|   private $changesets; | ||||
|   private $format = 'unified'; | ||||
|   private $viewer; | ||||
|   private $byteLimit; | ||||
|  | ||||
|   public function setFormat($format) { | ||||
|     $this->format = $format; | ||||
| @@ -35,6 +36,15 @@ final class DifferentialRawDiffRenderer extends Phobject { | ||||
|     return $this->viewer; | ||||
|   } | ||||
|  | ||||
|   public function setByteLimit($byte_limit) { | ||||
|     $this->byteLimit = $byte_limit; | ||||
|     return $this; | ||||
|   } | ||||
|  | ||||
|   public function getByteLimit() { | ||||
|     return $this->byteLimit; | ||||
|   } | ||||
|  | ||||
|   public function buildPatch() { | ||||
|     $diff = new DifferentialDiff(); | ||||
|     $diff->attachChangesets($this->getChangesets()); | ||||
| @@ -52,15 +62,18 @@ final class DifferentialRawDiffRenderer extends Phobject { | ||||
|     $bundle = ArcanistBundle::newFromChanges($changes); | ||||
|     $bundle->setLoadFileDataCallback(array($loader, 'loadFileData')); | ||||
|  | ||||
|     $byte_limit = $this->getByteLimit(); | ||||
|     if ($byte_limit) { | ||||
|       $bundle->setByteLimit($byte_limit); | ||||
|     } | ||||
|  | ||||
|     $format = $this->getFormat(); | ||||
|     switch ($format) { | ||||
|       case 'git': | ||||
|         return $bundle->toGitPatch(); | ||||
|         break; | ||||
|       case 'unified': | ||||
|       default: | ||||
|         return $bundle->toUnifiedDiff(); | ||||
|         break; | ||||
|     } | ||||
|   } | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley