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

Bump SuiteSparse BinaryBuilder version to v5.4.0+4 #34441

Closed
wants to merge 1 commit into from
Closed

Conversation

ararslan
Copy link
Member

This fixes an issue on FreeBSD with a misidentified OS/ABI value in libcholmod.so, so ldd couldn't read it.

This fixes an issue on FreeBSD with a misidentified OS/ABI value in
libcholmod.so, so `ldd` couldn't read it.
@ararslan ararslan requested a review from staticfloat January 19, 2020 23:08
@ararslan
Copy link
Member Author

Well, this is failing on all platforms, but at least it doesn't have trailing whitespace.

Excerpt from Linux x86:

SparseArrays  ��������� 12.100981 seconds
error during bootstrap:
LoadError("sysimg.jl", 16, LoadError("/buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.5/SuiteSparse/src/SuiteSparse.jl", 24, LoadError("/buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.5/SuiteSparse/src/umfpack.jl", 67, ErrorException("could not load library \"libsuitesparse_wrapper\"\nlibmetis.so: cannot open shared object file: No such file or directory"))))
jl_errorf at /buildworker/worker/package_linux32/build/src/rtutils.c:77
jl_load_dynamic_library at /buildworker/worker/package_linux32/build/src/dlload.c:233
jl_get_library_ at /buildworker/worker/package_linux32/build/src/runtime_ccall.cpp:50
jl_get_library_ at /buildworker/worker/package_linux32/build/src/runtime_ccall.cpp:39 [inlined]
jl_load_and_lookup at /buildworker/worker/package_linux32/build/src/runtime_ccall.cpp:61
unknown function (ip: 0xebdd1a8b)

@ViralBShah ViralBShah added the sparse Sparse arrays label Jan 20, 2020
@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

Need to install the Metis_jll package before SuiteSparse_jll. And the Metis license will need to be added to LICENSE.md.

@fredrikekre
Copy link
Member

Why ship metis when it's not used?

@Keno
Copy link
Member

Keno commented Jan 20, 2020

We can't just add a dependency on the jll in julia base - we guarantee that you can build from source, so somebody would have to write these makefiles. My preference would be to drop the metis dependency until we can move sparse out of base.

@Keno Keno closed this Jan 20, 2020
@Keno Keno reopened this Jan 20, 2020
Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

👎 to metis dependency

@ararslan
Copy link
Member Author

I agree, I'd like to avoid the dependency on METIS for Julia itself. Can we compile a version of SuiteSparse in BinaryBuilder without METIS?

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

It is straightforward to add metis support to the Julia source build. I can provide that.

Metis is used automatically by suitesparse when it is compiled in and present. It is part of suitesparse source - so it is no different than enabling a bunch of other suitesparse libs which we have done over time. This is so that packages can use these libraries because they otherwise have to ship their own suitesparse.

I would prefer not to maintain two versions of suitesparse in BB.

@ararslan
Copy link
Member Author

Okay, if we're going to add METIS then this PR is irrelevant anyway, since we'll need changes to BinaryBuilder's SuiteSparse and Julia's build system, so I'll just close it. It seems in practice that the misidentified OS/ABI in libcholmod on FreeBSD doesn't cause any problems for Julia anyway, AFAICT.

@ararslan ararslan closed this Jan 20, 2020
@ararslan ararslan deleted the aa/bump-bb branch January 20, 2020 21:48
@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

Ok, that's good to know that there isn't an immediate problem on freebsd. I can work on syncing the source build with BB over the next few days.

@Keno
Copy link
Member

Keno commented Jan 20, 2020

Ok, that's good to know that there isn't an immediate problem on freebsd. I can with on syncing the source build with BB over the next few days.

While that's certainly necessary, I think adding an extra dependency to the default distribution warrants some further discussion, and I suspect you'll be unlikely to find people in favor. Can we instead work on moving sparse out of base?

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

Unfortunately this particular one cannot be shipped separately because it patches cholmod. We already have it enabled in BB. Other dependencies like GraphBLAS, for example are not as tightly coupled, and hence are being built as separate BB packages.

On the larger point of moving sparse out, I thought we couldn't do it until 2.0. I have always been in favour for it, even in 1.0, but that is history.

@JeffBezanson
Copy link
Member

I think we should try to do whatever hacks are necessary to be able to move stdlibs out and automatically transition Manifests and Project files. Hopefully it can be done in a non-breaking way.

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

Fwiw, it would be nice to simplify this and not maintain two copies of build scripts. Any solution that makes it possible to have a consistent set of suitesparse libraries without sacrificing functionality would be nice to have.

The correct solution is to separate out from base. But until then, I think the simplest solution is to ship all the libraries that sparse linalg needs.

@staticfloat
Copy link
Member

What stops us from just removing the entire Sparse module from base Julia? Why does it need to be an stdlib?

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

Even if the hacks to move stdlibs out of the Julia repo were in place, wouldn't we need to still ship them as part of the default Julia distribution?

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

What stops us from just removing the entire Sparse module from base Julia? Why does it need to be an stdlib?

I was always told we can't remove it until 2.0.

@JeffBezanson
Copy link
Member

Even if the hacks to move stdlibs out were in place, wouldn't we need to still ship them as part of the default Julia distribution?

I don't believe so; you would just need to Pkg.add them like everything else, which is already the case since a new 1.x version creates a new environment.

@JeffBezanson
Copy link
Member

I suppose it could be breaking for secure environments though, where somebody might depend on the contents of the julia download since Pkg operations aren't possible.

@ViralBShah
Copy link
Member

ViralBShah commented Jan 20, 2020

I guess the question is whether requiring the additional step of installation of a package in a 1.x release is considered breaking.

I think it is, because you would now have programs that worked before with a default Julia install, but would not now without a modification. Unless of course it is orchestrated automatically during the creation of the environment.

For secure environments, are the contents of the default Julia download considered part of the 1.0 API?

@ViralBShah
Copy link
Member

This is perhaps continued elsewhere, but if the bundling of libraries is a concern, we could do the stdlibs related hacks, and build two sets of binaries - one that keeps things as they are (so that network-less systems continue to work as before) and another one that is the way going forward. At 2.0, we can drop the older method.

@StefanKarpinski
Copy link
Member

I guess the question is whether requiring the additional step of installation of a package in a 1.x release is considered breaking.

The point @JeffBezanson is making is that you have to Pkg.add() either way to make the package usable from a given environment, so there's no difference. The only difference is whether Pkg.add() is guaranteed not to download anything or not. We could potentially have a Julia+ download that ships all the things that were once stdlibs which we could recommend for users who need to be able to run their code on disconnected systems.

@fredrikekre
Copy link
Member

You don't need to add stdlibs, using SparseArrays works out of the box.

@StefanKarpinski
Copy link
Member

True in the REPL but not true in projects. We could do the following:

  • ship Julia with a "standard depot" that includes former stdlibs
  • put the standard depot in the default depot path
  • have a "standard environment" that includes all the former stdlibs
  • put the standard environment in the default load path

So something like this:

julia> DEPOT_PATH
3-element Array{String,1}:
 "/Users/stefan/.julia"
 "/Users/stefan/dev/julia/usr/local/share/julia"
 "/Users/stefan/dev/julia/usr/share/julia"
 # standard depot goes here

julia> LOAD_PATH
3-element Array{String,1}:
 "@"
 "@v#.#"
 # former stdlibs environment goes here
 "@stdlib"

It might be possible to turn the existing @stdlib into a real environment and have it include current and former stdlibs. The versions of the former stdlibs can be overwritten by adding them in one of the earlier load path environments. Since the "standard depot" would ship with Julia, you wouldn't have to install them. Since they're in the default load path, you wouldn't have to add them in the REPL. You'd still have to add them in projects, but that's always been the case.

@ViralBShah
Copy link
Member

I have prepared a new SuiteSparse_jll with METIS disabled (to get a few other things working while we figure out how to deal with this). So, it should be possible to update this PR to 5.4.0+6 and hopefully it should also fix the FreeBSD issue.

@ViralBShah ViralBShah added the system:freebsd Affects only FreeBSD label Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sparse Sparse arrays system:freebsd Affects only FreeBSD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants