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

Multi-GCC BinaryBuilder Binary Dependencies for Julia v1.0 #48

Merged
merged 38 commits into from
Jan 4, 2019
Merged

Multi-GCC BinaryBuilder Binary Dependencies for Julia v1.0 #48

merged 38 commits into from
Jan 4, 2019

Conversation

juan-pablo-vielma
Copy link
Contributor

Provides binaries with new multi-gcc BinaryProvider that should work with official julia binaries and custom source-builds. In particular, it should fix #38 and jump-dev/Cbc.jl#56

To avoid future issues like jump-dev/Cbc.jl#56 (and jump-dev/SCS.jl#107 (comment)) the binaries are self contained:

  • all dependencies are statically linked (except system libraries like libgfortran)
  • coin-blas and coin-lapack are used instead of Julia provider versions as Arch ships a non-standard build that links to the system's openblas and BP cannot yet deal with this (BB try 2 SCS.jl#107 (comment)).
  • only Clp symbols are exported (symbols from dependencies like coin-lapack and coin-osi are not exported)

using the separate dynamic libraries for dependencies would not provide an advantage until there is Pkg3 support for binary dependencies (right now each COIN solver would download its own copy of each dependency, which could cause unexpected symbol clashes if the solvers are independently updated).

@@ -6,6 +6,7 @@ VERSION < v"0.7.0-beta2.199" && __precompile__()
module Clp

using Compat
using Libdl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tu suppress a warning about Clp not having Libdl in its dependencies (Libdl is used by BinaryProvider), e.g. : JuliaIO/CodecZlib.jl#26

)

# To fix gcc4 bug in Windows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be a link to the issue here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

deps/build.jl Outdated
# trying to install is not itself installed) then load it up!
if unsatisfied || !isinstalled(dl_info...; prefix=prefix)
# Download and install binaries
# no dynamic dependencies until Pkg3 support for binaries
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indent level of the comments. Usually comments start at the indent level of the surrounding code, e.g.,

if foo
    f()
    # This is a comment.
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@mlubin
Copy link
Member

mlubin commented Jan 1, 2019

@juan-pablo-vielma note that I pushed changes to your branch, so you'll need to pull from your own branch before making additional commits.

@blegat
Copy link
Member

blegat commented Jan 2, 2019

Just tested the PR on ArchLinux and it works

deps/build.jl Outdated
LibraryProduct(prefix, String["libOsiClp"], :libOsiClp),
LibraryProduct(prefix, String["libClp"], :libClp),
LibraryProduct(prefix, String["libClpSolver"], :libClpSolver),
#LibraryProduct(prefix, ["libOsiClp"], :libOsiClp),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: Remove commented code or explain why it's commented and when it should be uncomented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted. We may want it back on the BinaryBuilder side eventually, but not on the BinaryProvider side.

@mlubin mlubin merged commit a767718 into jump-dev:master Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Build failed to get libcoinblas
3 participants