sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.34k stars 454 forks source link

Make arb config more flexible and find also flint-arb #30936

Closed tobiasdiez closed 1 year ago

tobiasdiez commented 3 years ago

If one has flint-arb installed but not arb as a system package, then compilation currently fails unless one explicitly sets ARB_LIBRARY = flint-arb. In this ticket, we also check for the existence of flint-arb making this manual step obsolete. This is similar in spirit to #30706, which does the same for CBLAS.

The config file build/pgks/arb/spkg-configure.m4 does a similar check (I think) and maybe can/needs to be adapted as well. If that's the case I would like to ask if somebody else can implement the necessary changes, as I've no experience with these config files. Thanks!

Depends on #30901

CC: @mkoeppe

Component: build

Branch/Commit: public/build/arbconfig @ 5d606e5

Issue created by migration from https://trac.sagemath.org/ticket/30936

tobiasdiez commented 3 years ago
comment:2

Merged the latest develop branch and version of #30901.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

0e31204Merge branch 'develop' of git://github.com/sagemath/sage into public/build/arbconfig
3fcaf5fMerge branch 'develop' of git://github.com/sagemath/sage into public/build/multiarchsimple
090e6f1Simplify code
fa4556aRemove _get_sage_local
5d606e5Merge branch 'public/build/multiarchsimple' of git://trac.sagemath.org/sage into public/build/arbconfig
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from c54df32 to 5d606e5

mkoeppe commented 3 years ago
comment:4

Debian renames libarb to libflint-arb because there is already a different library taking the name libarb. So code that only checks the presence of libarb or libflint-arb (in whatever order) cannot be correct.

tobiasdiez commented 3 years ago
comment:5

The code only provides a reasonable fall-back if SAGE_ARB_LIBRARY doesn't exist. Thus users on debian just have to set SAGE_ARB_LIBRARY = "flint-arb" and everything works as before.
Do I miss something?

(The purpose of this ticket was not to reimplement the checks in arb/spkg-configure in the setup.py file.)

mkoeppe commented 3 years ago
comment:6

The ticket tries to support a use case that is not currently supported (installation of sagelib without running ./configure). Instead of putting more complicated defaults into env.py, it would be better to develop a proper solution in sage.features.

tobiasdiez commented 3 years ago
comment:7

Want I want to support is installation of sagelib without the sage_conf package since I don't know how to install the latter with pipenv (I could install it by specifying the relative path in the pipfile, but that really ugly). It would be more practical if sage_conf would be a config file in src instead of a python package, but we discussed this already elsewhere...

So in this ticket I provided a reasonable fallback, similar to what is done for singular and gap. How would this be implemented with sage.features?

mkoeppe commented 3 years ago
comment:8

Replying to @tobiasdiez:

Want I want to support is installation of sagelib without the sage_conf package since I don't know how to install the latter with pipenv (I could install it by specifying the relative path in the pipfile, but that really ugly).

The Sage distribution builds a wheel of it, you would just install from there.

mkoeppe commented 3 years ago
comment:9

Replying to @tobiasdiez:

How would this be implemented with sage.features?

You would write a class similar to CythonFeature (from src/sage/features/__init__.py) with a method revealing the library name. The class would use the information from sage.env.ARB_LIBRARY (get rid of the default); but if unset, it would try to link a test program using acb_mat_eig_simple against the library. In fact, using CythonFeature for doing this test could pretty much work -- except that I'm not sure if sage.misc.cython perhaps pulls in too many modules at the moment.

tobiasdiez commented 3 years ago

Changed author from Tobias Diez to none

tobiasdiez commented 3 years ago
comment:10

That sounds reasonable. But it's also more work than I planned to invest into this, so I'll leave this to somebody else to implement.

mkoeppe commented 1 year ago

Outdated; we have SAGE_ARB_LIBRARY.