app-icon icon indicating copy to clipboard operation
app-icon copied to clipboard

Windows support

Open jtl3d opened this issue 8 years ago • 10 comments

This looks like a fantastic tool, but I get this when trying to run it on windows:

Failed to call 'convert icon.png -resize 167x167 ios\mobile\Images.xcassets\AppIcon.appiconset\ipad-83.5x83.5-2x.png. Error is 'Command failed: convert icon.png -resize 167x167 ios\mobile\Images.xcassets\AppIcon.appiconset\ipad-83.5x83.5-2x.png
Invalid Parameter - -resize

I installed ImageMagick, so I think maybe convert is conflicting with a Windows command?

jtl3d avatar Jun 14 '17 13:06 jtl3d

Hi @jtl3d, looks like an issue indeed. I will see what I can do to diagnose, but I'll need to set up a windows VM so it'll be few days.

Quick question - have you got imagemagic installed in your PATH?

If you are just on a plain command prompt and type:

convert -version

What is the output?

dwmkerr avatar Jun 17 '17 08:06 dwmkerr

Hi @dwmkerr, same error here.

As @jtl3d suggested, it does look like ImageMagick's "convert" conflicts with a Windows command: on my machine, after installing ImageMagick, starting a new console window & verifying that ImageMagick's binary folder is in my PATH, where convert returns C:\Windows\System32\convert.exe.

Complicating factor: after installing ImageMagick's ImageMagick-7.0.6-9-Q16-x64-dll.exe Windows release, there's no app named 'convert' under ImageMagick's install folder anyway, and the installed local help gives example commandline syntax that's always prefixed with "magick", e.g. magick convert rose.jpg rose.png. Seems like the commandline's just different on Windows.

acha11 avatar Aug 27 '17 14:08 acha11

Hi @dwmkerr, quick follow-up: I modified my local npm\node_modules\app-icon\src\imagemagick\call-imagemagick.js and changed line 10 from childProcess.exec(command, (err, stdout, stderr) => { to childProcess.exec('magick ' + command, (err, stdout, stderr) => {

After that, the error which @jtl3d originally reported, and which I also encountered, didn't occur - icons were generated just fine.

acha11 avatar Aug 27 '17 14:08 acha11

Hi @acha11 that's awesome, thanks for sharing! I'll check the same changes work fine on MacOS X and Linux and then will update the codebase. Cheers!

dwmkerr avatar Aug 28 '17 00:08 dwmkerr

No problem, thanks for a great library! Let me know if I can assist by testing on Windows.

acha11 avatar Aug 28 '17 04:08 acha11

Note to self / potential contributors

The challenge here is that if IM 6 is installed, then the code in master won't work, as the convert command is going to call another binary instead of image magick. This means we will need to:

  1. Correctly identify the full path to convert (painful)
  2. OR use IM 7, which uses a single magick binary (e.g. magick convert xxx)

Option 2 is probably better, however we will then need to make sure the linux version still works happily with 6 (as well as 7 ideally). So probably the way to go is:

  1. Extract the imagemagick code into a separate module.
  2. Add more robustness, with the module checking for v6/7 and using the magick command as needed
  3. Use the same logic for app-splash

This has the benefit of extra modularisation, no duplication between app-icon and app-splash, and may help others in the same situation.

dwmkerr avatar Jan 07 '18 09:01 dwmkerr

Just installed IM7 on windows and app-icon but it seems it still not working throw err that Error: ImageMagick must be installed. Try: brew install imagemagick and when i run convert from the command line I get Must specify a file system but when i run magick it actually works fine then.

To tweak it a little bit I checked into @acha11 comment childProcess.exec('magick ' + command, (err, stdout, stderr) => { and went a little bit further to replace the commandExists('convert', (err, imageMagickInstalled) => { to commandExists('magick', (err, imageMagickInstalled) => { inside npm\node_modules\app-icon\src\imagemagick\is-imagemagick-installed.js but still not working

Quadriphobs1 avatar Apr 13 '18 10:04 Quadriphobs1

@Quadriphobs1 can you try with this branch?

feature/windows-build

I think that this actually fixes the windows issues, I just need to get the CI build running properly. If this build fixes your issue I'll release it as a patch and fix the CI issues for it separately

dwmkerr avatar Apr 16 '18 13:04 dwmkerr

Encountered same issues with convert -resize, followed the steps @acha11 mentioned, but as @Quadriphobs1 app-icon generate just keep failing. To solved reinstalled imagemagick an added the checkbox option to support legacy features as convert, and run command with administrator permissions.

image

ps. tried without modifying the childProcess line of code as @jtl3d mentioned, and it worked. So I guess it was solved.

jnosornov avatar Jan 08 '19 21:01 jnosornov

Running CL as administrator under W10 works for me (with legacy utilities)

bodtx avatar Jan 15 '19 14:01 bodtx