Merge libsingular-julia back and build it on demand

[fingolfin]

Merge libsingular-julia back and build it on demand

This PR resolves issue #600. That is, it merges the libsingular-julia back into this repository (it was removed here in 3a1d92c and moved to https://github.com/oscar-system/libsingular-julia).

As a result of this PR, changes which need to update the C++ code in libsingular-julia in lockstep with the Julia code in Singular.jl can be done with a single PR, and CI tests will be able to test these PRs effectively, because they will end up compiling the C++ code and using it, instead of using the latest libsingular_julia_jll release.

This works by adding code that detects if the libsingular-julia code differs from what was used to build libsingular_julia_jll (this is done by comparing the "git tree hash" of the source directories; for the JLL, we store this hash in a file inside the JLL). If a difference is detected then we compile the C++ code from scratch and load it instead of the JLL. This is essentially imitating what GAP.jl has been doing for quite some time. In my experience it is a major relieve when working on the C++ code and e.g. debugging it.

Some decisions I made which people may wish to debate and change:

• This code is meant for use during development only. It is not intended to be used in releases / by end users.
• if a difference is detected, then a full clean rebuild of the C++ code is triggered every time Singular.jl is loaded. This slows down loading a bit (though for me it's just a few seconds), but has the advantage of being very simple to implement, and also avoiding a ton of potential problems caused by accidentally using outdated compiled code. We could change this, if desired, if someone has a convincing argument for it?
• the compiled code is stored in the package directory and not in a temp dir or scratch space. Again this was the simplest to implement. Of course writing into the package dir is a BAD idea when a package is installed in the regular way (the Julia devs would ideally like all package directories to be read-only). But it should be fine for a dev'ed package. Since the entire code in this PR is not meant to be used in release versions, that seems OK?

Open questions:

1. Should we try to preserve the git history of https://github.com/oscar-system/libsingular-julia in this import?

We don't want released versions to compile the C++ code (because this can fail in so many ways, and produces an overhead). So:

2. can we detect whether the package is installed as an ordinary package versus being dev'ed? (Perhaps just check for the presence of a .git dir in the package directory?)

3. If so, should we either...

   • just always use the JLL in that case, even if the version doesn't match (bad: then different code is executed when a user installs it versus when a user devs it);
   • refuse to proceed (i.e., don't try to compile C++ code, just abort with an error)

4. can we add tooling to stop us (or at least reduce the likelihood) from accidentally making a release of Singular.jl without a matching updating of libsingular_julia_jll?

   • Perhaps one CI test could just report on whether libsingular_julia_jll is "up-to-date" (the version installed by Project.toml, that is), and if not, it fails -- but only on master, not in PRs

TODO:

• for some reason each time we do using Singular, it redoes precompilation (and thus compilers the C++ code twice). Fix this!
• make sure it actually passes CI tests (this PR will tell)
• update libsingular_julia_jll to point here and contain the relevant tree-hash of the source files
• once libsingular_julia_jll is updated, change the dependency on it in Project.toml
• perhaps add a way to insert compile flag overrides, e.g. to build a debug version?
• add code to detect if the Singular_jll version differs and trigger a rebuild then (not urgent, for now one can always force a rebuilding by insert a space into the C++ sources somewhere)
• set up code coverage tracking for the C++ code (?)

[benlorenz]

A few comments:

• I personally prefer to keep git history as much as possible, it shouldn't be too hard to do?
• There is https://pkgdocs.julialang.org/v1/api/#Pkg.dependencies which has is_tracking_path, but in theory that path could point to .julia/packages/Singular/something as well. So I would probably try this combined with having a .git directory (or even .git file if someone is using git worktrees...).
• Maybe we can use a product override via LocalPreferences.toml instead of manually setting the path via some extra variable, see here: https://docs.binarybuilder.org/stable/jll/#Overriding-specific-products .

I don't really like too much automatic re-compilation, maybe this can be put behind an interactive prompt (and disallowed in non-interactive mode) except when allowed via an explicit environment flag that is set in the yml for this repository?
(My opinion might be influenced by the fact that full recompilation for libpolymake-julia takes about 4 minutes even with -j4)

I don't fully understand yet how versioning is supposed work in the end.
Suppose I want to do some breaking change (in the libsingular-julia code) in a PR here, then I would need to run the CI on my branch without adjusting the compat for libsingular_julia_jll first (otherwise Pkg.jl will not find any compatible jll package), using the autobuild discussed here. Maybe then one creates a new libsingular_julia_jll release from a commit on that branch and finally adjusts the compat bound, now CI might run without autobuild which would then allow the merge to master?
This would make it somewhat difficult when there are multiple branches which would bring in new libsingular_julia releases, but otherwise this would ensure that for master there is always a matching libsingular_julia_jll available?

This somewhat depends on the amount of changes expected to libsingular-julia, if we can do a new jll for each change to the library then the above approach might be preferrable.
If we want to collect multiple changes and rarely create new jll release then creating it from master might be better?

[fingolfin]

@benlorenz thanks for the feedback, very useful. I'll ponder it some more, but a quick note: yes, versioning is still a bit shaky. So far my assumption was that we'll do this:

1. develop a new feature in a branch involving Julia and C++ code changes
2. merge the branch into Singular.jl, once it passes CI
3. update libsingular_julia_jll recipe on yggdrasil
4. after libsingular_julia_jll is updated, follow up with a PR on Singular.jl to update Project.toml accordingly

Admittedly this is clunky and also means we'd temporarily have versions of the master branch that don't have a matching libsingular_julia_jll. We can then decide whether to update libsingular_julia_jll immediately, or wait for a release to do it (with the risk that we could "forget" about it unless we take extra precautions).

However I'd still argue that it is overall superior to what we have now, where we have to:

1. coordinate two PRs on two repositories (one with Julia changes, one with C++ changes); the one with Julia changes won't work at all for now
2. merge the C++ branch on the libsingular_julia repository (without any CI, so we just hope it is "ready")
3. update libsingular_julia_jll recipe on yggdrasil
4. after libsingular_julia_jll is updated, update the Singular.jl branch, so that for the first time CI runs on our feature, and hope it'll pass
5. merge the Julia branch into Singular.jl (if it passes; if not, go back to step 1 and repeat)

But of course it would be good to improve this further. Besides, I just encountered at least one other really annoying problem: I wanted to work on a feature that requires the new Singular_jll, but I couldn't because Pkg was unable to find a version of libsingular_julia_jll compatible with the new Singular_jll sigh. Of course I can work around that (just release an update for libsingular_julia_jll built using the existing C++ code, against the new Singular_jll), and this workaround should be absolutely safe, too. But I wonder if there is something we can do about it?

BTW building libsingular_julia_jll only takes a few seconds. And updating the JLL frequently shouldn't be an issue either.

Back to the proposed workflow: you describe an alternative where master will always have a valid matching libsingular_julia_jll. Besides the problem of coordinating multiple PRs that want to update it (which is not a big problem in practice as we don't update it that often), the other is that it is unclear where to point the libsingular_julia_jll recipe then: it wants a commit in this repo. So that would mean the PR must from a branch on this repo, not on a fork, meaning that 3rd parties would not be able to submit such PRs. Also, we then should only merge such PRs, never rebase or squash them (I am not sure if we have a way to enforce this for select PRs, instead of enforcing it globally).

Overall I am therefore sceptical about this approach (but I am certainly not ruling it out yet)

I'd be happy to discuss this further, I certainly don't claim to have a perfect solution yet, only that even this clunky one is IMHO way better than what we have now

[benlorenz]

I also hit some annoying compat issues with polymake_jll at some point and added a workaround to our test-prepare.jl which just pulls the jll manually, kills the compat entries and then develops that directory:
https://github.com/oscar-system/libpolymake-julia/blob/master/test-prepare.jl#L93-L102
(Not really beautiful but it did seem to work)

[fingolfin]

Note to self: New plan for getting this PR ready:

1. make a new PR which just re-imports the libsingular_julia here, including nicely grafted history (branching at the parent of 3a1d92c, which removed deps/src). DONE
2. archive the libsingular_julia repo DONE
3. update the Yggdrasil recipe to point at the new location; also update that recipe to compute the tree hashes and add them into the JLL WIP, see [libsingular_julia] various updates JuliaPackaging/Yggdrasil#6853
4. apply the CMakeLists.txt cleanup from this PR, update Yggdrasil to match
5. update this PR here accordingly

This will relax the process a bit, time wise, as we can pause between each step w/o impeding the development of Singular.jl.

[fingolfin]

@hannes14 @ederc @benlorenz this is now ready to be merged from my point of view. That does not mean the work is 100% done, but it is good enough to be merged and used, and gradually improved. Once merged I'll open an issue with remaining TODOs. From the top of my head:

• update README.maintainer.md further to correctly state how to update both Singular_jll *and* libsingular_julia_jll` (I sketched this in a comment above already; I am not fleshing it out right now because I'd like to do it once, in full, to make sure it works as I envision it)
• add a new CI job for master only which fails if the JLL treehash and the deps/src treehash don't match
  • anyone doing a release of Singular_jll hopefully will first check the CI status for master and thus notice this...
• add code to detect if Singular.jl is a regular release version / not a dev version -- and in that case tree a treehash mismatch as an error

There is probably more but all seem to be things we can deal with gradually.

So unless anyone objects I'll merge this a bit later today. Then will test it with PRs #660 and #661 by @hannes14 and @ederc and (assuming that works) make a fresh release of the JLL and Singular.jl

[hannes14]

Points 1 and 2 appear twice, delete (probably) the second.

[fingolfin]

Indeed, thank you. Fixed now