Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade minimagick 5 #132

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lukeasrodgers
Copy link

Using just the specs as a guide, this seems like a fairly minimal set of changes that would permit support for minimagick 5, addressing all deprecation warnings.

It also introduces distinct gemfiles for minimagick v4 and v5 which could both be run in CI. I have not yet modified the Github actions yaml to do that, but should be pretty easy to do.

A couple specs are still failing, related to the strip command, but I figured it would be worth posting this to see if it seems like the right direction before spending more time on it.

@courtsimas
Copy link

Can we get this merged?

zzak added a commit to zzak/rails that referenced this pull request Jan 9, 2025
```
[MiniMagick] MiniMagick::Image#mime_type has been deprecated, because it wasn't returning correct result for all formats ImageMagick supports. Unfortunately, returning the correct MIME type would be very slow, because it would require ImageMagick to read the whole file. It's better to use Marcel and MimeMagic gems, which are able to determine the MIME type just from the image header.
```

This method is removed in v5 of `mini_magick` gem:

> Removed deprecated Image#mime_type, as it wasn't accurate.
> MIME type from file content should be determined either using Marcel
> or MimeMagic, or mime-types or MiniMime using Image#type.

Since we depend on it through `image_processing` gem
(`mini_magick (>= 4.9.5, < 5)`), while work is being done in
janko/image_processing#132 so sooner or later this test will fail.
@henrahmagix
Copy link

I'd very much like this to be merged ☺️ I've tried running the tests locally but I have vips issues and I can't figure out how to fix them; I have libvips 8.16.0 on arm64 (macOS 15.1.1), but if anyone else can confirm the tests work fine on this, it'd be SO wonderful to merge and release an update pls! 🙌

@janko
Copy link
Owner

janko commented Jan 13, 2025

I'll try to get this merged this week, thanks for the work.

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.

4 participants