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

Fix OSX Homebrew build issues and made build script more consistent #90

Merged
merged 4 commits into from
May 31, 2017
Merged

Fix OSX Homebrew build issues and made build script more consistent #90

merged 4 commits into from
May 31, 2017

Conversation

Allardvm
Copy link
Contributor

@Allardvm Allardvm commented May 31, 2017

For a while now, we've had issues with OSX builds breaking, sometimes even resulting in a segfault:

It's likely that these issues have different root causes, but at least one of them has to do with the way Homebrew.jl handles the translation from homebrew/core to staticfloat/juliadeps.

In some cases, the ImageMagick.jl build script would ask for the Homebrew prefix to the homebrew/core/imagemagick@6 library, but the Julia homebrew library would still have the staticfloat/juliadeps version installed. Rebuilding the library would not replace the installed version (despite calling for "homebrew/core/imagemagick@6"), but it would update deps.jl to point to the new version—resulting in errors. After testing and merging of the PR, we should probably tag a new version of the package to fix this for the users that filed issues.

This PR includes three sets of changes:

  1. The package always calls for the staticfloat/juliadeps version—both upon the initial installation and upon rebuilding later on. This should prevent inconsistencies between the installed library and deps.jl.
  2. The PR also removed the automatic detection of a system-wide Homebrew installed ImageMagick library, since the detect call (success('brew list imagemagick@6') isn't consistent. After loading the Homebrew package in Julia, the detect call would call the Julia Homebrew, rather than the system-wide Homebrew—which in rare cases would lead to further inconsistencies. The PR now uses the MAGICK_HOME environment variable to allow users to specify their own installation of ImageMagick—similar to the Unix version of the package.
  3. General formatting and consistency fixes. Each platform now uses a similar build script to make maintaining the package a bit easier.

Note that some users that have tried to fix the issue manually in the past by taking control of Julia's Homebrew might have to manually remove the library installed by Homebrew.jl, prune the remaining symlinks, and rebuild the ImageMagick package:

using Homebrew
run(`brew remove imagemagick@6`)
run(`brew prune`)
Pkg.build("ImageMagick")

@Allardvm
Copy link
Contributor Author

The failing nightly builds are due to an issue in ZipFile.jl that seems to have been fixed in fhs/ZipFile.jl@b90db13. A release with this fix hasn't been tagged yet. Let me know how you'd like me to handle that.

@timholy
Copy link
Member

timholy commented May 31, 2017

Awesome. First thing: can you add 0.6 to the testing matrices? You surely know this already, but here and here (two lines worth in the latter case). On 0.6, ZipFile shouldn't fail because the alignment issue hasn't been merged. Assuming 0.6 passes, I'm fine with nightly failing until ZipFile gets tagged.

@timholy
Copy link
Member

timholy commented May 31, 2017

Puzzling. Exit code 56 is "Failure with receiving network data," https://curl.haxx.se/libcurl/c/libcurl-errors.html. I'm going to restart in case it was just a temporary outage.

@Allardvm
Copy link
Contributor Author

This looks unrelated to the changes, perhaps it's a DNS issue again, somewhere? The OSX 0.5 version works this time, perhaps you could restart OSX 0.6 as well to make sure it passes?

@timholy timholy merged commit 9b0d4e3 into JuliaIO:master May 31, 2017
@timholy
Copy link
Member

timholy commented Jun 1, 2017

This is one of those PRs where there aren't enough happy emojis to go around. Many thanks, @Allardvm!

@Allardvm
Copy link
Contributor Author

Allardvm commented Jun 1, 2017

You're welcome! Thanks for the work to get this merged so quickly 👍

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.

2 participants