Skip to content

JPT for Windows#324

Merged
rbuchberger merged 3 commits intorbuchberger:masterfrom
MUmarShahbaz:master
Jun 10, 2025
Merged

JPT for Windows#324
rbuchberger merged 3 commits intorbuchberger:masterfrom
MUmarShahbaz:master

Conversation

@MUmarShahbaz
Copy link
Copy Markdown
Contributor

@MUmarShahbaz MUmarShahbaz commented Jun 3, 2025

Contents

Continuation of #322

I tried to rebase, not really a pro at git either so I ended up nerfing the previous PR. This is the new PR, it doesn't have the Standalone Imagemagick edits and is ready to merge.

I haven't tested just yet, I'm installing VIPS and will test it soon. Logically speaking the code should work as I have only removed some edits and not made any new ones.

@rbuchberger
Copy link
Copy Markdown
Owner

Weird, the tests pass on my machine. I swear I have had more trouble with the freaking CI than anything else

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

The tests suggest magick is not detected, maybe you're installing the older version of Imagemagick which supports convert and not magick

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

I just checked, Imagemagick 6 is installed by default on Ubuntu which doesn't support magick. I'll make some changes to the workflow to install the Imagemagick 7

@MUmarShahbaz MUmarShahbaz force-pushed the master branch 2 times, most recently from 37f6213 to 0ca7b56 Compare June 4, 2025 14:50
@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

MUmarShahbaz commented Jun 4, 2025

@rbuchberger I managed to fix the workflow, you can check my run here

Had to use a bit of AI to install Imagemagick 7, and it turned out to be a hefty process so I think it might be worth it to take a look at it before merging.

Other than that the repo is working perfectly and passing all tests

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

The installation of Imagemagick 7 alone takes 5-6 minutes.

From what I am seeing on the internet, the best way to speed it up is to either:

  1. Create a chache inside this repo
  2. Upload a pre-processed .deb on a site somewhere.

Right now the workflow is literally building Imagemagick 7 during the run.

Solution 1 is a simple edit to the workflow, and I think it'll be useful for others as an example to use on their own GitHub Workflow, on the other hand Solution 2 is more robust and faster.

Solution 2 will require you to create a new branch and host it via GitHub pages to specifically hold the package. If you make the link public everyone else can use it as well.

I think Solution 2 is the best.

@rbuchberger which do you think is better?

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

MUmarShahbaz commented Jun 4, 2025

If you think Solution 2 is the best, then I'll need you to create and host the branch before continuing.

Note that right now we're only discussing optimization, the PR is already working with 0 failures.

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

@rbuchberger? This PR is ready for merging!

@rbuchberger
Copy link
Copy Markdown
Owner

rbuchberger commented Jun 8, 2025

I just checked, Imagemagick 6 is installed by default on Ubuntu which doesn't support magick. I'll make some changes to the workflow to install the Imagemagick 7

Ahhh damn, this ugy thing is rearing its head again. Here's the problem: a huge portion of CI runners are based on Ubuntu. Like, most of them. For some godforsaken reason (they probably hate me personally), Canonical refuses to ship ImageMagick V7, which means if we don't support V6 we'll break a bunch of websites that use this project. They probably won't notice until they try to deploy, and I really don't want to drop a "you now need to compile IM7 from source for your builds to work" turd in the punchbowl.

So we have a situation: magick and only magick works on windows, convert is broken. On Ubuntu, the opposite is true: convert and only convert works, magick is broken. I don't see any other way than to programatically detect which one to use:

  • On windows, go straight for magick, never try convert.
  • On linux, go for magick and if it's not found then try convert.
    Should probably write a helper to accomplish this and then use it everywhere we're calling convert or magick instead.

Sorry for the delay in answering, crunch time at work and we have a 4 month old at home. Also thanks for troubleshooting the issue and especially for working out the CI. Dealing with CI is the worst, I hate it so much. If you're up for writing it this way go for it, otherwise I can whip it up in a week or two.

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

They probably won't notice until they try to deploy, and I really don't want to drop a "you now need to compile IM7 from source for your builds to work" turd in the punchbowl.

How about a fallback? I can try to make it work using magick and if magick fails, then it'll use convert. That seems to solve all issues.

ubuntu uses IM6 by default which doesn't support magick
@MUmarShahbaz MUmarShahbaz force-pushed the master branch 2 times, most recently from 8d00834 to 23ffdd4 Compare June 9, 2025 06:59
@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

@rbuchberger I've added a fallback and it is now working on both IM6 and IM7.

You can view the workflow runs here.

magick : IM7
convert : IM6

This time, the PR is definitely ready for merging.

@rbuchberger rbuchberger merged commit b0f6563 into rbuchberger:master Jun 10, 2025
4 checks passed
@rbuchberger
Copy link
Copy Markdown
Owner

Outstanding work, thank you so much! I'll cut a release, let me know if anything else can be improved.

@MUmarShahbaz
Copy link
Copy Markdown
Contributor Author

Outstanding work, thank you so much! I'll cut a release, let me know if anything else can be improved.

No problem, I was looking for an excuse to learn ruby anyways. Glad to be able to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Imagemagick command conflict on Windows Under Windows: Libvips is not installed. Imagemagick is not installed

2 participants