New build.jl using BinaryBuilder #98

mlubin · 2018-02-16T18:24:16Z

If this works, closes (or at least makes stale) all build-related issues.

Thanks a lot to @staticfloat for developing IpoptBuilder!

Closes #61
Closes #68
Closes #84
Closes #86
Closes #90
Closes #94
Closes #97

tkelman · 2018-02-16T18:26:47Z

deps/build.jl

 else
-    fpu_control = ""
+    error("Your platform $(Sys.MACHINE) is not supported by this package!")


this completely breaks the package on freebsd and alpine linux, this really isn't suitable to be released until there's a fallback so those platforms that are working now aren't left in the cold

At a minimum the build script needs to be changed to support finding a local version of Ipopt in LD_LIBRARY_PATH.

At a minimum the build script needs to be changed to support finding a local version of Ipopt in LD_LIBRARY_PATH.

We explicitly didn't want to support this, because picking up random versions of libraries from the environment is the source of many woes.

The easiest solution right now is probably to replace the error() here with including the old BinDeps build script.

the source of many woes

In this particular case, it's necessary to be able to use a 3-5x faster version of the library that you need to build yourself because it uses non-redistributable academic-license components

Yes, Ipopt is unusual because some serious users will want to compile their own version anyway. Also the API is pretty static so we don't have to worry about version incompatibilities.

In that case, what we should do is tell the users to install the custom binaries (or provide the magic script to build it) into the deps/usr directory within this directory, then invert the two if statements; e.g.

# First, check to see if we're all satisfied if any(!satisfied(p; verbose=verbose) for p in products) if platform_key() in keys(download_info) # Download and install binaries url, tarball_hash = download_info[platform_key()] install(url, tarball_hash; prefix=prefix, force=true, verbose=true) ...

In fact, that should be the default.

This will work just fine as long as the libraries are within deps/usr/lib, etc... I'm not so sure we should just pick up random things on LD_LIBRARY_PATH though; do you think it's too much of a bother to tell the user to install to that prefix in this case? Especially since this is a case where we're expecting the user to compile manually.

The deps/usr/lib solution sounds fine to me.

mlubin · 2018-02-16T18:41:01Z

Close/open to trigger AppVeyor.

mlubin · 2018-02-16T19:49:57Z

@staticfloat note the failures on appveyor

staticfloat · 2018-02-16T21:03:00Z

Okay the Windows errors are because we use GCC 7 in BinaryBuilder.jl which has moved on to libgfortran-4.dll, whereas Julia itself is built with GCC 5.4, which still uses libgfortran-3.dll. We would need to move to GCC 7 on the buildbots for this to work properly, and I don't think we can just ship libgfortran-4.dll along with libipopt-1.dll here, because if we did we'd end up with two libgfortran's linked into Julia's process space.

Perhaps it's time for me to revisit moving the Windows buildbots over to a cross-compile + wine architecture again. The epic yak-shave continues.

staticfloat · 2018-02-17T23:51:30Z

One small note; you should create an ExecutableProduct(prefix, "ampl") and use that instead of this line.

staticfloat · 2018-02-20T18:59:38Z

New build.jl that bundles windows with libgfortran-3.dll. The important part is:

# Download binaries from hosted location
bin_prefix = "https://github.com/staticfloat/IpoptBuilder/releases/download/v3.12.8-6"

# Listing of files generated by BinaryBuilder:
download_info = Dict(
  BinaryProvider.Linux(:x86_64, :glibc)      => ("$bin_prefix/Ipopt.x86_64-linux-gnu.tar.gz",      "2377d4ce1035c059063b955b3d6946b50d2cc61e161d979437e5f6b39f14d95f"),
  BinaryProvider.Linux(:i686, :glibc)        => ("$bin_prefix/Ipopt.i686-linux-gnu.tar.gz",        "b83147200788e285695cf92215df5a2c86084b2a701ce790504617984a05d038"),
  BinaryProvider.Linux(:aarch64, :glibc)     => ("$bin_prefix/Ipopt.aarch64-linux-gnu.tar.gz",     "e3a0535743697e06da11802609817fcf11a35f95481cdf799a63acc8ecc86cce"),
  BinaryProvider.Linux(:armv7l, :glibc)      => ("$bin_prefix/Ipopt.arm-linux-gnueabihf.tar.gz",   "54293ba6800868a7fdfc483f356ca95d6f936b7e44d09d97bcf65b2a4d44ee36"),
  BinaryProvider.MacOS()                     => ("$bin_prefix/Ipopt.x86_64-apple-darwin14.tar.gz", "0160c20de4cf63afd207cf93d2bca206816d65256891b61ede32a55891ecc581"),
  BinaryProvider.Windows(:x86_64)            => ("$bin_prefix/Ipopt.x86_64-w64-mingw32.tar.gz",    "4b178443e18b0e581a88331c62e8f9ab3457b292f94b0a39b66f60ebaa8aa340"),
  BinaryProvider.Windows(:i686)              => ("$bin_prefix/Ipopt.i686-w64-mingw32.tar.gz",      "ec415dcf6559fcce3987473ab270bcb66e14f38d9422ac86d88ef267ea0ab3f0"),
)

staticfloat · 2018-02-20T20:17:12Z

Woohoo! It passes! Also, whoops we’re still using the deprecated syntax. I’ll fix that on my ipoptbuilder repo (you’ll also need to fix it here, pass in a symbol as a variable name for your products, then use the write_deps_file function instead of the macro) and then I think we’re ready to go here.

mlubin · 2018-02-22T09:37:08Z

Awesome :)
Before merging I'd also like to resolve:

Supporting and documenting user-provided binaries
ExecutableProduct(prefix, "ampl") as you mentioned

staticfloat · 2018-02-24T10:46:59Z

Supporting and documenting user-provided binaries

The documentation would basically say "build from source like normal, except say ./configure --prefix=~/.julia/v0.6/Ipopt/deps/usr instead of ./configure --prefix=/usr/local. That should be it. Where would you want that put?

ExecutableProduct(prefix, "ampl") as you mentioned

The newest build.jl has this.

mlubin · 2018-02-24T14:03:40Z

@staticfloat the logic for user-provided binaries is a bit off. write_deps_file needs to be called also when download/install is skipped. When that's working I can add a note to the README and consider this resolved.

mlubin · 2018-02-24T15:23:25Z

Another issue, seems like a tricky one, is that the ipopt binary doesn't work on travis (https://travis-ci.org/JuliaOpt/Ipopt.jl/jobs/345663191#L1584) because
error while loading shared libraries: libgfortran.so.4: cannot open shared object file: No such file or directory

staticfloat · 2018-02-24T18:17:17Z

write_deps_file needs to be called also when download/install is skipped.

I was a little conflicted on this; but then I thought about this again and you're absolutely right. I've updated the build.jl file on the IpoptBuilder repository and made this the default for all new build.jl files in the future, but you can also just move the write_deps_file() call out of the two if statements.

Another issue, seems like a tricky one, is that the ipopt binary doesn't work on travis

Aha, that is a really good bug you just found. The default library search path for executables isn't containing the directory for the libgfortran that comes with Julia. Can you try doing the following immediately after you call check_deps() in your __init__() function:

julia_libdir = dirname(first(filter(x -> contains(x, "libjulia"), Sys.Libdl.dllist())))
@static if Compat.Sys.isapple()
    ENV["DYLD_FALLBACK_LIBRARY_PATH"] = string(get(ENV, "DYD_FALLBACK_LIBRARY_PATH", ""), ":", julia_libdir)
elseif Compat.Sys.islinux()
    ENV["LD_LIBRARY_PATH"] = string(get(ENV, "LD_LIBRARY_PATH", ""), ":", julia_libdir)
end

(By the by, I just noticed you aren't calling check_deps() in your __init__() function, you should do that so that if a library goes mysteriously missing, the user is told to run Pkg.build() again)

If that LD_LIBRARY_PATH thing works, we will probably end up doing that automatically if we detect that you have ExecutableProduct's within your check_deps().

mlubin · 2018-02-24T19:51:54Z

@staticfloat, thanks! Your fix works on Linux but not OS X. Random googling says that DYLD_FALLBACK_LIBRARY_PATH is effectively disabled by default on OS X: https://apple.stackexchange.com/questions/212945/unable-to-set-dyld-fallback-library-path-in-shell-on-osx-10-11-1.

staticfloat · 2018-02-25T04:21:00Z

Hmmm, looks like DYLD_LIBRARY_PATH works on my machine, but DYLD_FALLBACK_LIBRARY_PATH doesn't. Can you confirm?

staticfloat · 2018-02-25T08:15:59Z

Fantastic. I'll go ahead and add this to check_deps() so that it happens automatically.

Ref: JuliaPackaging/BinaryProvider.jl#40

mlubin · 2018-02-25T08:38:44Z

I'd actually prefer for this not to be hidden in check_deps. People are inevitably going to try and use these binaries outside of julia, and putting this code in check_deps would make it one step harder to find out what's going on because check_deps isn't checked into the repository anywhere.

tkelman · 2018-03-01T09:18:49Z

REQUIRE

@@ -1,5 +1,4 @@
-julia 0.6
-BinDeps
+julia 0.6.2


what doesn't work on 0.6.0 or 0.6.1? is this platform specific?

I was only able to run the linux binaries on 0.6.2. Earlier versions have a mismatching version of libgfortran.

if 0.6.0 and 0.6.1 work on non linux then it should be

julia 0.6 @linux julia 0.6.2

though that's also specific to binaries, distro packages of julia aren't necessarily going to be built with gcc 7.

or source builds of julia for that matter

I don't have the ability to test 0.6.0 and 0.6.1 on macOS. It might have the same issue. We'll probably direct source builders of Julia to build their own copy of Ipopt. Don't know about distro packages.

Source builds and distro packages are a design question with BinaryBuilder. We'll be following whichever conventions emerge in the julia ecosystem.

travis/appveyor can easily test old point releases on a branch

sounds like an open design question that hasn't been given a lot of thought. there are big gaps and holes in what it works for that would be regressions for this package if released in its current state.

sounds like an open design question that hasn't been given a lot of thought. there are big gaps and holes in what it works for that would be regressions for this package if released in its current state.

I agree, but we also need to consider that macOS binaries have been broken since November (#94) which is quite a regression. I will at least hold off on releasing before we figure out what's happening with CoinOptServices.

couldn't you use binaryprovider just on mac for now then?

tkelman · 2018-03-01T09:21:44Z

REQUIRE

-@osx Homebrew
-@windows WinRPM
+BinaryProvider
+Compat


0.27 for isapple etc

tkelman reviewed Feb 16, 2018

View reviewed changes

mlubin closed this Feb 16, 2018

mlubin reopened this Feb 16, 2018

mlubin added 2 commits February 20, 2018 14:55

New build.jl using BinaryBuilder

a590486

update build.jl

b6b91a6

mlubin force-pushed the ml/ipoptbuilder branch from e93c58d to b6b91a6 Compare February 20, 2018 19:55

update build.jl to v3.12.8-9

2650294

mlubin added 4 commits February 24, 2018 15:05

amplexe now defined in deps.jl

bf02d99

linux binaries require 0.6.2

b38a015

BinDeps no longer needed

3a2785a

test binary on travis

5521504

fix library paths for the ipopt binary

4684111

remove stale comment

6db9a52

try DYLD_LIBRARY_PATH

a919b09

mlubin mentioned this pull request Feb 25, 2018

Augment *_LIBRARY_PATH on relevant platforms for ExecutableProducts JuliaPackaging/BinaryProvider.jl#40

Merged

document custom installation

b0b0fe0

tkelman reviewed Mar 1, 2018

View reviewed changes

mlubin merged commit b51624e into master Mar 1, 2018

mlubin deleted the ml/ipoptbuilder branch March 1, 2018 09:21

tkelman reviewed Mar 1, 2018

View reviewed changes

REQUIRE

@osx Homebrew

@windows WinRPM

BinaryProvider

Compat

Copy link

Contributor

tkelman Mar 1, 2018

Choose a reason for hiding this comment

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

0.27 for isapple etc

This was referenced Mar 1, 2018

CoinOptServices build broken on macOS Sierra JuliaOpt/CoinOptServices.jl#40

Open

write deps file for user-provided binaries #102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New build.jl using BinaryBuilder #98

New build.jl using BinaryBuilder #98

mlubin commented Feb 16, 2018 •

edited

tkelman Feb 16, 2018

mlubin Feb 16, 2018

staticfloat Feb 16, 2018

tkelman Feb 16, 2018

mlubin Feb 16, 2018

staticfloat Feb 16, 2018 •

edited

staticfloat Feb 16, 2018

mlubin Feb 16, 2018

mlubin commented Feb 16, 2018

mlubin commented Feb 16, 2018

staticfloat commented Feb 16, 2018

staticfloat commented Feb 17, 2018

staticfloat commented Feb 20, 2018

staticfloat commented Feb 20, 2018

mlubin commented Feb 22, 2018

staticfloat commented Feb 24, 2018

mlubin commented Feb 24, 2018 •

edited

mlubin commented Feb 24, 2018

staticfloat commented Feb 24, 2018 •

edited

mlubin commented Feb 24, 2018

staticfloat commented Feb 25, 2018

staticfloat commented Feb 25, 2018 •

edited

mlubin commented Feb 25, 2018

tkelman Mar 1, 2018

mlubin Mar 1, 2018

tkelman Mar 1, 2018

tkelman Mar 1, 2018

mlubin Mar 1, 2018

mlubin Mar 1, 2018

tkelman Mar 1, 2018

mlubin Mar 1, 2018

tkelman Mar 1, 2018

tkelman Mar 1, 2018

New build.jl using BinaryBuilder #98

New build.jl using BinaryBuilder #98

Conversation

mlubin commented Feb 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticfloat Feb 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlubin commented Feb 16, 2018

mlubin commented Feb 16, 2018

staticfloat commented Feb 16, 2018

staticfloat commented Feb 17, 2018

staticfloat commented Feb 20, 2018

staticfloat commented Feb 20, 2018

mlubin commented Feb 22, 2018

staticfloat commented Feb 24, 2018

mlubin commented Feb 24, 2018 • edited

mlubin commented Feb 24, 2018

staticfloat commented Feb 24, 2018 • edited

mlubin commented Feb 24, 2018

staticfloat commented Feb 25, 2018

staticfloat commented Feb 25, 2018 • edited

mlubin commented Feb 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlubin commented Feb 16, 2018 •

edited

staticfloat Feb 16, 2018 •

edited

mlubin commented Feb 24, 2018 •

edited

staticfloat commented Feb 24, 2018 •

edited

staticfloat commented Feb 25, 2018 •

edited