Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Use existing binary builders for MPFR? #1

Closed
fingolfin opened this issue Mar 19, 2020 · 14 comments
Closed

Use existing binary builders for MPFR? #1

fingolfin opened this issue Mar 19, 2020 · 14 comments

Comments

@fingolfin
Copy link
Member

Perhaps we can use https://github.com/JuliaBinaryWrappers/MPFR_jll.jl/releases which is based on https://github.com/JuliaPackaging/Yggdrasil/blob/master/M/MPFR/build_tarballs.jl ?

(There is also one for MPIR there, but as I understand, we need a patch release beyond MPIR 3.0.0 ? Ah, if only there were MPIR releases...)

@fingolfin
Copy link
Member Author

MPFR was added as a dependency in ff67e90 but the source build still uses a downloaded MPFR -- do we really want that? Perhaps we can also get rid of arb, ant, mpir in there?

@benlorenz
Copy link
Member

Perhaps we can use https://github.com/JuliaBinaryWrappers/MPFR_jll.jl/releases which is based on https://github.com/JuliaPackaging/Yggdrasil/blob/master/M/MPFR/build_tarballs.jl ?

These releases are not supposed to be used directly but by adding a dependency in the Project.toml via Pkg.add and then using MPFR_jll. See https://julialang.org/blog/2019/11/artifacts/.

I think we should switch to this kind of binary dependencies as soon as possible.
I am currently trying to adapt polymake and for testing purposes and until they are added to Yggdrasil (and the official registry) my binary artifacts are available in our custom registry: https://github.com/oscar-system/OscarRegistry
If a pull request for Yggdrasil is accepted the corresponding binary package something_jll is automatically added to the main julia registry and available for Pkg.add, see

(There is also one for MPIR there, but as I understand, we need a patch release beyond MPIR 3.0.0 ? Ah, if only there were MPIR releases...)

Flint (binary) uses the julia GMP instead of MPIR, so I dont think we need MPIR.

MPFR was added as a dependency in ff67e90 but the source build still uses a downloaded MPFR -- do we really want that? Perhaps we can also get rid of arb, ant, mpir in there?

I think we should get rid of the whole source-build stuff in the transition, I hope to remove most deps/build.jl files.

Also adding dependencies of dependencies should not be necessary anymore with the pkg artifacts system.

PS: For custom library installation (i.e. from source) it is possible to use the override system from Pkg.jl, see https://julialang.github.io/Pkg.jl/dev/artifacts/#Overriding-artifact-locations-1
Something like this might work in ~/.julia/artifacts/Overrides.toml to use flint_jll with a custom flint installation:

[cabf4574-4f87-5802-9a0b-e4b5b01a80fc]
libflint = "/home/me/source-builds/libflint.so"

That uuid comes from here, and might change when this is integrated upstream.

@fingolfin
Copy link
Member Author

fingolfin commented Mar 19, 2020

Just to clarify: I am aware that JLLs should be used like regular Julia packages; the commit I referenced does not abuse a JLL, but rather uses an old-style BinaryBuilder for MPFR.

I agree that switching to JLLs is the way to go, and now that we agreed to require Julia >= 1.3, we are open to do so. But for now, we need to get this working very quickly to resolve conflicts between the latest versions of Oscar.jl and Polymake.jl; this requires Nemo.jl and Polymake.jl to use LoadFlint.jl (PRs upcoming).

Thank you for the pointer to the override system, that's very helpful. Based on this, I am tempted to agree that we can get rid of custom source builds here and probably in others of our packages...

BEGIN RANT:
Finally, I have to say really wish that documentation around BinaryBuilder and co was better (or existent in some cases). Things keep changing drastically and it is difficult to figure out how and when. The whole system strikes me as very complex and underdocumented (aside: IMHO this applies to far too many parts of the Julia ecosystem :-/).
END RANT

@fingolfin
Copy link
Member Author

@benlorenz note that the reason we have this package LoadFlint.jl at all, as opposed to just using a hypothetical Flint_jll.jl is the following: after loading libflint.so, we need to ccall its __flint_set_memory_functions function. That's basically all this package does.

If there was a way to instruct BinaryBuilder to insert some code at the end of the __init__ function of JLLs, we could probably use that, produce a JLL, and discard this package again. I did not find such a way, but that doesn't mean there isn't one; perhaps you know more? Another option would be to copy the code calling __flint_set_memory_functions to both Polymake.jl and Nemo.jl. As long as they set the same memory functions, this is fine. The concern we have with that approach is that then it is possible some other package starts using flint and sets different functions; then crashes may start to happen.

A third option would be to modify flint to directly call jl_malloc etc. . At first I dismissed this, thinking that the issue with this is the same as I have with making a GAP BinaryBuilder a major hassle. But now I think it might actually be doable, since we just need a "fake" julia.h which declares four functions, and that should be it.

So perhaps we can do that and discard this package again...

@fieker
Copy link
Contributor

fieker commented Mar 19, 2020 via email

@fieker
Copy link
Contributor

fieker commented Mar 19, 2020

I'd also like to add that moving to JLL means Nemo abandoning Julia 1.0. I doubt that you will get Bill to agree to that under any circumstances...
The future will do JLL, but now the idea is to make LoadFlint usable and used asap.

@wbhart
Copy link

wbhart commented Mar 19, 2020

Correct. Not until there is a new Julia LTS.

@benlorenz
Copy link
Member

benlorenz commented Mar 19, 2020

If there was a way to instruct BinaryBuilder to insert some code at the end of the __init__ function of JLLs, we could probably use that, produce a JLL, and discard this package again. I did not find such a way, but that doesn't mean there isn't one; perhaps you know more?

No, I don't think there a method to achieve this but there were some related discussions. Some libraries or executables need to have environment variables set to load successfully and it would be nice to have them set in the jll instead of every dependent package. See here.

I'd also like to add that moving to JLL means Nemo abandoning Julia 1.0.

I just realized that these artifacts could still be used in legacy mode by generating a (or several) corresponding build.jl, see https://github.com/JuliaPackaging/Yggdrasil#binaryproviderjl.

This would need some if VERSION > v"1.3.0-rc4" for the using x_jll statements and in the build script. But this could be used to make sure that the same artifacts are used also on julia 1.0.

This might be what you originally wanted but I went somewhat offtopic, sorry about that...

The future will do JLL, but now the idea is to make LoadFlint usable and used asap.

A first try via the custom registry for LoadFlint seems to work for Polymake.jl on travis for julia 1.3:
https://github.com/oscar-system/Polymake.jl/runs/520677843
(1.4 has other issues, 1.0 doesn't support custom registries.)

PS: I agree with your rant about the documentation for BinaryBuilder and related stuff ...

@fieker
Copy link
Contributor

fieker commented Mar 20, 2020 via email

@wbhart
Copy link

wbhart commented Mar 20, 2020

New packages take three days to register. It's to stop people registering nonsense packages and packages that cause security issues, I think. But look at the readout from the bots which analyse the package and tell you of anything you need to fix to get is "auto-accepted", otherwise it will be scheduled for manual review.

@benlorenz
Copy link
Member

What is the 1.4 problem? I am doing most of my playing in 1.4.0.rc2 at the moment, and all is fine.

Locally 1.4rc2 seems fine, but on travis and github actions CxxWrap.jl seems to crash during precompilation:
see for example here: https://travis-ci.com/github/oscar-system/Polymake.jl/jobs/296590429
similar crash for singular.jl here: https://travis-ci.com/github/oscar-system/Singular.jl/jobs/297761006

We have already created a ticket (JuliaInterop/CxxWrap.jl#214) for cxxwrap.

@ViralBShah
Copy link

BTW, you can set things up so that you use artifacts for 1.3+ and the old system for older releases quite easily. See this example in Clp.jl: https://github.com/JuliaOpt/Clp.jl/blob/master/deps/build.jl

@ViralBShah
Copy link

1.6 is expected to be the new LTS. So that is not too far away.

@fingolfin
Copy link
Member Author

This was (essentially) resolved by PR #4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants