Skip to content

Re-fixes issue #12, reintroduced due to 286138d #91

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

Merged
merged 1 commit into from
Jun 2, 2017
Merged

Re-fixes issue #12, reintroduced due to 286138d #91

merged 1 commit into from
Jun 2, 2017

Conversation

oschulz
Copy link
Contributor

@oschulz oschulz commented Jun 2, 2017

Commit 286138d removed the fix for issue #12. Observed on CentOS-7 with system-installed ImageMagick.

@timholy
Copy link
Member

timholy commented Jun 2, 2017

Thanks. Hadn't realized this might still be an issue.

@Allardvm
Copy link
Contributor

Allardvm commented Jun 3, 2017

I'm not a huge fan of the somewhat hacky fix, because we don't fix the root cause. I assume the error is thrown from __init__(), because the preload and unload hooks aren't added to deps.jl when the library is found in a system path. If this is the case, then wouldn't the below also fix the issue?

  1. remove the call to init_deps from __init__().
  2. add ccall((:MagickWandGenesis, libwand), Void, ()) to __init__().
  3. remove the ccall from all the init_deps() functions defined by the BinDeps providers.
  4. move all the init_deps() functions to each provider's preload function, rather than the onload function to ensure the ENV setup is in place when the library is first opened.
    update: no, this doesn't work because the package is precompiled

If you're on-board with this, I'm happy to open a PR that does this. I'll need to open a PR anyway, since #90 didn't turn out to be a final fix for the Homebrew library issue. Even though the package now always calls for Homebrew's staticfloat/juliadeps/imagemagick@6 library, it just installed homebrew/core/imagemagick@6 for me again after calling Pkg.update(). So I'll have to try a different approach.

@oschulz
Copy link
Contributor Author

oschulz commented Jun 4, 2017

@timholy, will v0.3.1 go on METADATA? Would help me and some folks I work with a lot.

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.

3 participants