-
Notifications
You must be signed in to change notification settings - Fork 560
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
Updating JLLs with Dependency("libcxxwrap_julia_jll")
#2160
Comments
I tried the latest
|
@barche Damn! OK, first the good news is that in Julia 1.3 and 1.4, The bad news is that I can reproduce the issue you describe on macOS; and also a crash on Linux with Julia 1.5 (log see below). I'll try to look into both. UPDATE: submitted this also as JuliaInterop/libcxxwrap-julia#75
|
@jstrube so, are the new JLLs at least working correctly for you? |
LCIO.jl tests fine. I also get a CxxWrap segfault on the test...
|
I checked the dylib's in
This makes me (a) suspect that libgmpxx is also affected, and (b) wonder if this is somehow a C++ issue. |
Anyway, this may mean that static linking libosxunwind isn't an option after all... |
I believe the Julia 1.5 linux segfault is orthogonal to the unwind issue, there have been sporadic crashes in this spot in the containers.jl test for a long time, I had hoped this was due to binary incompatibility too, but since that cause has been eliminated, it seems I need to dig deeper. At least it seems somewhat reproducible now :) |
Looking at where it fails in the C++ test, I believe this is related to C++ exceptions. The libunwind code also mentions exceptions in the context of this symbol. I expect this will happen whenever C++ code throws an exception. Libcxxwrap-julia catches all these exceptions and passes the message text to |
OK, it's "good" to hear that this segfault was there before (I mean, a segfault is never good, but you know what I mean...). It crashes in I wonder if this might be a GC issue again. I just submitted PR JuliaInterop/libcxxwrap-julia#74 but that's pure guess work and not based on any actual debugging yet. Actually, I just see that your Travis tests against Julia nightly reproduce the crash, and my PR did nothing about it. So... |
We resolve the libcxxwrap crashes on Julia 1.5 by rebuilding with GCC 8. Since much of the cxxwrap code is in the headers, JLL builders using libcxxwrap should also consider using GCC 8 or newer. @jstrube that might affect your JLLs, too -- but of course if they work fine right now, there is no immediate need to rebuild them. However, I also noticed that your JLLs are still limited in the platforms they support -- just two, in fact. Is that intentional? |
I can rebuild, but both LCIO.jl and FastJet.jl test fine. That could just be a result of lacking coverage, though, so I'll get on that. |
I am confused: both LCIO_jll and FastJet_jll support for more platforms than their |
Oh, you're right! Thanks. Let me try to expand the platforms for the wrappers, then. |
Been a hot minute! I'll soon jump on the bandwagon and take care of doing my best at updating |
Hi @fingolfin , really appreciate your work! The |
It might be a bit too late to discuss the way versioning of jlls when coupled with libjulia should be since several jlls already adopted the For example, extend the version number following the format Just food for thought at this point, probably, but I'd like to stir up some discussion, search for some opinions. Or simple backhanded "no" slap-in-the-face answers, I accept those too, no biggy ^^ |
@rgcv that's what I used initially but it is not possible because such versions are by definition prereleases, and the package registry does not accept prereleases. |
@fingolfin Should've guessed you tried that, silly of me to wonder. Totally fine. In fact, major and minor could replace minor and patch bumps. Instead of having This however seems problematic once 2.0 eventually hits. Granted, by then things might have changed enough for this not to be a problem. Concerns me nonetheless. Be that as it may, right now, your proposal is more than sufficient and I'll gladly adopt it. |
@rgcv not at all silly, it was/is a very reasonable question and idea (I would have greatly preferred the approach you suggested for various reasons). Anyway, I've written some more on potential versioning schemes here: #2327 (comment) |
This is basically done (only z3 remains to be adapted, but there was no feedback on my PR from the original author of the Z3 builder, and they haven't been active on GitHub for ~half a year, so it doesn't seem to be a priority. |
Once we have versions of
libcxxwrap_julia_jll
for all Julia versions, we'll need to update the various JLLs using it to also provide multiple versions. Here I try to describe what's going on and why a change is needed; explain how to make that change; and track the progress of each affected package.What's going on
Julia does not currently provide a stable ABI, meaning that things that were compiled against, say, Julia 1.3, may or may not work when linked at runtime against Julia 1.4/1.5/... -- and this holds of course for any combination of Julia versions. For libcxxwrap and anything using its headers (including all (?) JLLs using
Dependency("libcxxwrap_julia_jll")
), it is indeed a fact that a binary compiled against Julia 1.3 can lead to segfaults during GC when used with any later Julia version, and vice versa. Right now it is not definitely known whether other Julia combinations can lead to issues, but it is not unlikely that this will eventually happen.Thus the only safe way forward is to provide binaries for
libcxxwrap_julia_jll
which are compiled against each of Julia 1.3, 1.4, 1.5, ....In an ideal world, the julia version would simply be part of the
Platform
, and we'd be done. Indeed, @staticfloat added support for that -- alas, it will only work in Julia >= 1.6; older versions will balk when they see such platform identifiers. So as long as one wants to support any older Julia versions (which I'd expect to be everybody right now, as 1.6 has not yet been released, not even branched), extra work is needed.On the upside to all this, cxxwrap now supports many more platforms, which you can potentially benefit from for your own JLLs, too!
How to deal with it
The following is an initial suggestion, and will be revised based on feedback; I suggest that people wait a bit with trying to implement this, until we've ironed out everything
In a nutshell, here's the plan: for each affected JLL, we build it several times, once for each supported Julia minor version (so e.g. one each for Julia 1.3, 1.4 and 1.5+), but built from the same source. For technical reasons, each such build will have to carry a separate version. The convention I propose is to set the patch level of your JLL to the minor version of Julia (so
libcxxwrap_julia_jll
0.8.4 was built against Julia 1.4). Yes, that means you can't have a one-to-one mapping with the actual versions of the code your JLL wraps; but the longterm plan for JLLs is to completely decouple JLL versions from the versions of the wrapped software anyway.There are many ways to deal with this; I'll describe one of them here, which I am using in PR #1798. The core idea is to rename your current
build_tarballs.jl
tocommon.jl
and then add one subdir for each supported Julia version, each with abuild_tarballs.jl
includingcommon.jl
.For example, PR #1798 renames
L/libsingular_julia/build_tarballs.jl
toL/libsingular_julia/common.jl
and addsL/libsingular_julia/libsingular_julia@1.3/build_tarballs.jl
which looks like this:For technical reasons, the
build_tarballs.jl
for Julia 1.4 and 1.5 will be added in later PRs (AFAIK Yggdrasil tooling does not allow adding multiple versions of a JLL in a single PR -- so far that would have been a weird thing to do anyway).In
common.jl
, some changes are made:version
is adjusted to set the patchlevel to match the Julia minor version:version = VersionNumber(0, 2, julia_version.minor)
libcxxwrap_julia_jll
(this could be neater, see also Add a way to "inherit" default platform list from another JLL BinaryBuilder.jl#957)Julia_jll
, we now needlibjulia_jll
in the right version:BuildDependency(PackageSpec(name="libjulia_jll", version=julia_version)),
build_tarballs
about the runtime Julia requirements, by adding this keyword argument:julia_compat = "$(julia_version.major).$(julia_version.minor)"
Note that this is not using
~
as one might expect, simply so that the build for Julia 1.5 can be used with Julia 1.6 until alibjulia_jll
for 1.6 exists (yes, that might not work, due to ABI incompatibilities; but at least it has a chance of working, which is better than what we'd have if we used~
in the version specification. Technically this means one could also use the JLL version made for Julia 1.3 in Julia 1.4; but since the JLL version for Julia 1.4 is strictly newer than that for Julia 1.3, this shouldn't happen unless a user forces that, and well, then they deserve it...UPDATE: we also recommend that all packages using
libcxxwrap_julia_jl
should usepreferred_gcc_version=v"8"
or higher; see #2236 and JuliaInterop/libcxxwrap-julia#75Affected JLLs and their status
Note: I plan to ping the respective authors once we have a stable plan, but for now, it seems premature to bother them. In particular until PR #2158 is ready...
ConnectFourSolver
-- @findmywayFastJet_Julia_Wrapper
-- @jstrubeJulia 1.3Julia 1.4HelFEM
-- @mortenpiLCIO_Julia_Wrapper
-- @jstrubeJulia 1.3Julia 1.4OpenSpiel
-- @findmywaySDPA
-- @ViralBShahjlqml
-- @barchelibcgal_julia
-- @rgcvPR Update libcgal_julia to new libcxxwrap / libjulia #2326libcxxwrap_julia
libpolymake_julia
-- @benlorenzlibsingular_julia
-- @fingolfinz3
-- @ahumenbergerThe text was updated successfully, but these errors were encountered: