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

Add search prefix for homebrew on arm in imagemagick.m4 #372

Merged
merged 1 commit into from Jan 4, 2021

Conversation

shivammathur
Copy link
Contributor

This PR adds /opt/homebrew search prefix in imagemagick.m4

Homebrew on new arm machines uses /opt/homebrew as prefix.
So ImageMagick binaries are installed in /opt/homebrew/bin

Ref: https://docs.brew.sh/Installation

Homebrew on new arm machines uses /opt/homebrew as prefix.
So ImageMagick binaries are installed in /opt/homebrew/bin
@Danack Danack merged commit 448c1cd into Imagick:master Jan 4, 2021
@Danack
Copy link
Collaborator

Danack commented Jan 4, 2021

insert witty comment about needing to personally test this on one of those new macs before merging here

@Danack
Copy link
Collaborator

Danack commented Jan 4, 2021

Thanks.

@glensc
Copy link

glensc commented Mar 5, 2021

@shivammathur do you know does it validate the library what it finds for the matching architecture? so this doesn't end up finding amd64.so when building arm.so

@shivammathur
Copy link
Contributor Author

shivammathur commented Mar 5, 2021

@glensc It does not, it just checks sequentially in a list of prefixes, and will break while building on arm macOS if x86 imagemagick is installed in /usr/local.
So, this can be improved by checking for architecture using AC_CANONICAL_HOST and AC_CANONICAL_TARGET. I think pcre extension does this to check JIT support.

That said, providing imagemagick directory is supported and is a better solution if you have a multiarch setup.

@glensc
Copy link

glensc commented Mar 5, 2021

@shivammathur or try linking with the library found. that's probably slower.

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.

None yet

3 participants