recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Support ImageMagick 7's "magick" command to supersede "convert"

Summary:
Make Phorge also look for the "magick" binary which supersedes "convert" since ImageMagick version 7.

Closes T16474

Test Plan:
1. Rename the commands in the two "Filesystem::binaryExists()" checks to some nonsense
2. As an admin, set http://phorge.localhost/config/edit/files.enable-imagemagick/ to `enabled`
3. See in top bar a new setup warning pointing to http://phorge.localhost/config/issue/files.enable-imagemagick/ about "'magick' or 'convert' binary not found or Imagemagick is not installed."
4. Correct the two commands in the two "Filesystem::binaryExists()" checks, as they are in this patch
5. Upload an animated GIF image file `F1`, go to http://phorge.localhost/file/transforms/1/ and manually trigger a transform by clicking a "Regenerate" button, get a result (optionally insert phlog() statements in PhabricatorFileImageTransform::applyImagemagick() to prove that ImageMagick is used)
6. Probably create some Macro meme? I did not.
7. Check the "Behavioral Changes" section on https://imagemagick.org/script/porting.php and do not spot any commands used in the Phorge codebase

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T16474

Differential Revision: https://we.phorge.it/D26742

Andre Klapper 7fe9bfe8 18b9acba

+47 -21
+32 -15
src/applications/config/check/PhabricatorImagemagickSetupCheck.php
··· 6 6 return self::GROUP_OTHER; 7 7 } 8 8 9 + /** 10 + * Get the name of the ImageMagick binary. Since ImageMagick version 7, the 11 + * "magick" command is replacing the old "convert" command. 12 + * 13 + * @return string|null 14 + */ 15 + public function getImageMagickBinaryName() { 16 + if (Filesystem::binaryExists('magick')) { 17 + return 'magick'; 18 + } else if (Filesystem::binaryExists('convert')) { 19 + return 'convert'; 20 + } else { 21 + return null; 22 + } 23 + } 24 + 9 25 protected function executeChecks() { 10 26 $imagemagick = PhabricatorEnv::getEnvConfig('files.enable-imagemagick'); 11 - if ($imagemagick) { 12 - if (!Filesystem::binaryExists('convert')) { 13 - $message = pht( 14 - "You have enabled Imagemagick in your config, but the '%s' ". 15 - "binary is not in the webserver's %s. Disable imagemagick ". 16 - "or make it available to the webserver.", 17 - 'convert', 18 - '$PATH'); 27 + if ($imagemagick && $this->getImageMagickBinaryName() === null) { 28 + $message = pht( 29 + "You have enabled Imagemagick in your config, but the '%s' or '%s' ". 30 + "binary is not in the webserver's %s. Disable imagemagick ". 31 + "or make it available to the webserver.", 32 + 'magick', 33 + 'convert', 34 + '$PATH'); 19 35 20 - $this->newIssue('files.enable-imagemagick') 21 - ->setName(pht( 22 - "'%s' binary not found or Imagemagick is not installed.", 'convert')) 23 - ->setMessage($message) 24 - ->addRelatedPhabricatorConfig('files.enable-imagemagick') 25 - ->addPhabricatorConfig('environment.append-paths'); 26 - } 36 + $this->newIssue('files.enable-imagemagick') 37 + ->setName(pht( 38 + "'%s' or '%s' binary not found or Imagemagick is not installed.", 39 + 'magick', 40 + 'convert')) 41 + ->setMessage($message) 42 + ->addRelatedPhabricatorConfig('files.enable-imagemagick') 43 + ->addPhabricatorConfig('environment.append-paths'); 27 44 } 28 45 } 29 46 }
+3 -2
src/applications/files/config/PhabricatorFilesConfigOptions.php
··· 215 215 pht( 216 216 'This option will use Imagemagick to rescale images, so animated '. 217 217 'GIFs can be thumbnailed and set as profile pictures. Imagemagick '. 218 - 'must be installed and the "%s" binary must be available to '. 219 - 'the webserver for this to work.', 218 + 'must be installed and the "%s" or "%s" binary must be available '. 219 + 'to the webserver for this to work.', 220 + 'magick', 220 221 'convert')), 221 222 222 223 );
+4 -1
src/applications/files/transform/PhabricatorFileImageTransform.php
··· 118 118 119 119 $out = new TempFile(); 120 120 121 - $future = new ExecFuture('convert %s %Ls %s', $tmp, $argv, $out); 121 + $binary = id(new PhabricatorImagemagickSetupCheck()) 122 + ->getImageMagickBinaryName(); 123 + 124 + $future = new ExecFuture('%s %s %Ls %s', $binary, $tmp, $argv, $out); 122 125 // Don't spend more than 60 seconds resizing; just fail if it takes longer 123 126 // than that. 124 127 $future->setTimeout(60)->resolvex();
+8 -3
src/applications/macro/engine/PhabricatorMemeEngine.php
··· 211 211 return null; 212 212 } 213 213 214 + $binary = id(new PhabricatorImagemagickSetupCheck()) 215 + ->getImageMagickBinaryName(); 216 + 214 217 // Test of the GIF is an animated GIF. If it's a flat GIF, we'll fall 215 218 // back to GD. 216 219 $input = new TempFile(); 217 220 Filesystem::writeFile($input, $template_data); 218 - list($err, $out) = exec_manual('convert %s info:', $input); 221 + list($err, $out) = exec_manual('%s %s info:', $binary, $input); 219 222 if ($err) { 220 223 return null; 221 224 } ··· 231 234 $output = new TempFile(); 232 235 233 236 $future = new ExecFuture( 234 - 'convert %s -coalesce +adjoin %s_%s', 237 + '%s %s -coalesce +adjoin %s_%s', 238 + $binary, 235 239 $input, 236 240 $input, 237 241 '%09d'); ··· 250 254 } 251 255 252 256 $future = new ExecFuture( 253 - 'convert -dispose background -loop 0 %Ls %s', 257 + '%s -dispose background -loop 0 %Ls %s', 258 + $binary, 254 259 $output_files, 255 260 $output); 256 261 $future->setTimeout(10)->resolvex();