Opened 19 months ago
Closed 7 months ago
#30887 closed enhancement (fixed)
Add spkgconfigure.m4 for 4ti2, remove direct use of SAGE_LOCAL
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.5 
Component:  packages: optional  Keywords:  4ti2, spkgconfigure.m4 
Cc:  dimpase, mjo, slelievre, arojas, fbissey, ghjengelh, klee, slabbe  Merged in:  
Authors:  Samuel Lelièvre, Matthias Koeppe  Reviewers:  Dima Pasechnik, Antonio Rojas 
Report Upstream:  N/A  Work issues:  
Branch:  646e182 (Commits, GitHub, GitLab)  Commit:  646e182e65fc247d0d2da9aabcad5f892a471a89 
Dependencies:  Stopgaps: 
Description (last modified by )
Part of #27330.
In addition to adjustments to src/sage/interfaces/four_ti_2.py
, also the following needs changing:
src/sage/sandpiles/sandpile.py:path_to_zsolve = os.path.join(SAGE_LOCAL, 'bin', 'zsolve')
Change History (52)
comment:1 Changed 14 months ago by
 Branch set to public/30887
 Cc slelievre added
 Commit set to d79e1c15b832bb3f28fc37c0fcb8fa312feb1136
 Description modified (diff)
 Keywords 4ti2 spkgconfigure.m4 added
 Summary changed from spkgconfigure.m4 for 4ti2 to Add spkgconfigure.m4 for 4ti2
comment:2 Changed 14 months ago by
latte_int
uses 4ti2
as a library. The test is here: https://github.com/latteint/latte/blob/master/m4/4ti2check.m4#L48
comment:3 Changed 14 months ago by
Also Py4ti2
(to be added in #31363) uses 4ti2
as a library  https://github.com/alfsan/Py4ti2/blob/master/setup.py
comment:4 Changed 14 months ago by
singular
is able to use 4ti2
via the commandline interface. Checks are done in https://github.com/Singular/Singular/blob/spielwiese/configure.ac#L352
configure.ac:AC_CHECK_PROGS([FOURTITWO_HILBERT], [hilbert 4ti2hilbert]) configure.ac:AC_CHECK_PROGS([FOURTITWO_MARKOV], [markov 4ti2markov]) configure.ac:AC_CHECK_PROGS([FOURTITWO_GRAVER], [graver 4ti2graver])
comment:5 Changed 14 months ago by
src/sage/interfaces/four_ti_2.py
calls the executables
$ grep call src/sage/interfaces/four_ti_2.py def call(self, command, project, verbose=True): sage: four_ti_2.call("groebner", "test_file", False) # optional  4ti2 subprocess.call(cmd, shell=True, cwd=self.directory()) self.call('zsolve q', project) self.call('qsolve q parbitrary', project) self.call('rays q parbitrary', project) self.call('hilbert q', project) self.call('graver q', project) self.call('ppi 2> /dev/null', n) self.call('circuits q parbitrary', project) self.call('groebner q parbitrary', project)
and does not know that Debian installs them with an executable prefix.
comment:6 Changed 14 months ago by
polymake
calls the executables groebner
, zsolve
, circuits
, which are configurable (https://github.com/polymake/polymake/blob/c2a326f240b11a06e1b1d5fd1c2ca7278bbe60a2/apps/polytope/rules/_4ti2.rules) but it does not know that Debian installs them with an executable prefix
comment:7 Changed 14 months ago by
 Dependencies set to #31534
comment:8 Changed 14 months ago by
 Commit changed from d79e1c15b832bb3f28fc37c0fcb8fa312feb1136 to 602ffb02fed3dd90118f53fc4aa2ceb44d9e4690
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
7c272ec  Add MacPorts package info

8c9f53c  30504: Add _recommended package info for MacPorts

bf0a4d2  30504: Use zmqdevel in MacPorts for zeromq

e424489  30504: Fix gcc gfortran openblas info for MacPorts

b8b7092  build/pkgs/{gcc,gfortran}/distros/macports.txt: Fixup

4acfc5c  Merge #30504

857b7d5  tox.ini: In maximal environments, use IGNORE_MISSING_SYSTEM_PACKAGES=yes for all noncurrent distributions

4e561ac  tox.ini: Use IGNORE_MISSING_SYSTEM_PACKAGES=no for rolling distributions, but IGNORE_MISSING_SYSTEM_PACKAGES=yes for unstable distributions

30d0e8a  Merge #31534

602ffb0  build/pkgs/4ti2/spkgconfigure.m4: New

comment:9 Changed 14 months ago by
 Commit changed from 602ffb02fed3dd90118f53fc4aa2ceb44d9e4690 to ff25e8fabac0115d37c218b8a92f721a649d9789
Branch pushed to git repo; I updated commit sha1. New commits:
ff25e8f  build/pkgs/4ti2/spkgconfigure.m4: Fix quoting

comment:10 Changed 14 months ago by
 Milestone changed from sage9.3 to sage9.4
comment:11 Changed 11 months ago by
 Commit changed from ff25e8fabac0115d37c218b8a92f721a649d9789 to dd364a566b4d3381436cc2bc3d230d551ddfb35a
comment:12 Changed 11 months ago by
 Dependencies #31534 deleted
comment:13 Changed 11 months ago by
 Commit changed from dd364a566b4d3381436cc2bc3d230d551ddfb35a to 390c0d2984b40db48b9b0021e78d6e7e04893455
comment:14 Changed 11 months ago by
On ubuntufocalmaximal
:
Checking whether SageMath should install SPKG 4ti2... checking whether any of gmp mpir glpk zlib is installed as or will be installed as SPKG... no checking for hilbert... no checking for 4ti2hilbert... 4ti2hilbert checking for markov... no checking for 4ti2markov... 4ti2markov checking for graver... no checking for 4ti2graver... 4ti2graver checking for zsolve... no checking for 4ti2zsolve... 4ti2zsolve checking for qsolve... no checking for 4ti2qsolve... 4ti2qsolve checking for rays... no checking for 4ti2rays... 4ti2rays checking for ppi... no checking for 4ti2ppi... 4ti2ppi checking for circuits... no checking for 4ti2circuits... 4ti2circuits checking for groebner... no checking for 4ti2groebner... 4ti2groebner checking for library 4ti2gmp... no configure: no suitable system package found for SPKG 4ti2
comment:15 Changed 11 months ago by
Ubuntu packaging does not have the headers and has the shared libs in a nonstandard place  can't use
comment:16 Changed 11 months ago by
 Commit changed from 390c0d2984b40db48b9b0021e78d6e7e04893455 to fd029453d6c58c17b627159d8ec6d97bc76cc487
comment:17 Changed 11 months ago by
 Cc arojas fbissey added
 Status changed from new to needs_review
comment:18 Changed 11 months ago by
 Summary changed from Add spkgconfigure.m4 for 4ti2 to Add spkgconfigure.m4 for 4ti2, remove direct use of SAGE_LOCAL
comment:19 Changed 11 months ago by
I didn't realize that zsolve
is a part of 4ti2
. Gentoo isn't following debian naming (for now). It all look OK from a Gentoo point of view. I'll give it a proper run through later tonight.
comment:20 Changed 11 months ago by
Works and doctest with system 4ti2 on Gentoo.
comment:21 Changed 11 months ago by
 Cc ghjengelh added
comment:22 Changed 11 months ago by
The 4ti2 binaries in openSUSE, since the package's inception in 2012 (so way before the Debian package by the looks of it), are available as e.g. /usr/bin/4ti2_zsolve, i.e. with a underscore rather than a dash. Alternatively, one can extend $PATH to include /usr/libexec/4ti2 and get them by their original name that way.
comment:23 Changed 11 months ago by
OK, we can check for the prefix 4ti2_
here too
comment:24 Changed 11 months ago by
You may want to send a pull request to Singular and Polymake so that these packages accept system 4ti2 on SuSE
comment:25 Changed 11 months ago by
 Commit changed from fd029453d6c58c17b627159d8ec6d97bc76cc487 to 7d74ad641624e1c24a4e8695529b4416f4dd0678
Branch pushed to git repo; I updated commit sha1. New commits:
7d74ad6  build/pkgs/4ti2/spkgconfigure.m4: Check for executable's with prefix 4ti2_ too

comment:26 Changed 11 months ago by
Let's get this in please
comment:27 followup: ↓ 29 Changed 11 months ago by
AC_TRY_LINK
is obsolete. We spent some time removing these in #30668. autoupdate
produces

build/pkgs/4ti2/spkgconfigure.m4
diff git a/build/pkgs/4ti2/spkgconfigure.m4 b/build/pkgs/4ti2/spkgconfigure.m4 index e01562c50d..a16da8fc4b 100644
a b SAGE_SPKG_CONFIGURE([4ti2], [ 17 17 FORTYTWO_LIBS="l4ti2gmp lzsolve" 18 18 CXXFLAGS="${BACKUP_CXXFLAGS} ${FORTYTWO_CXXFLAGS} ${GMP_CFLAGS}" 19 19 LIBS="${BACKUP_LIBS} ${FORTYTWO_LIBS} ${GMP_LIBS}" 20 AC_ TRY_LINK([20 AC_LINK_IFELSE([AC_LANG_PROGRAM([[ 21 21 #include "4ti2/4ti2.h" 22 ], 23 [ _4ti2_rays_create_state(_4ti2_PREC_INT_ARB); 24 ], [ 22 ]], [[ _4ti2_rays_create_state(_4ti2_PREC_INT_ARB); 23 ]])],[ 25 24 AC_MSG_RESULT([yes]) 26 ], 25 ],[ 27 26 AC_MSG_RESULT([no]) 28 27 sage_spkg_install_4ti2=yes 29 28 ])
and it appears to work.
comment:28 followup: ↓ 30 Changed 11 months ago by
by the way, with system 4ti2
, I have not found a way to tell Sage it is "installed" (./configure does pick it up all right); I tried ./configure withsystem4ti2=force enable4ti2=yes
and still 4ti2
does not appear in the list of optional packages while doing ./sage t ...
.
Surely I can add it explicitly to optional
, but this is very suboptimal.
comment:29 in reply to: ↑ 27 Changed 11 months ago by
comment:30 in reply to: ↑ 28 Changed 11 months ago by
Replying to dimpase:
by the way, with system
4ti2
, I have not found a way to tell Sage it is "installed" (./configure does pick it up all right); I tried./configure withsystem4ti2=force enable4ti2=yes
and still4ti2
does not appear in the list of optional packages while doing./sage t ...
.Surely I can add it explicitly to
optional
, but this is very suboptimal.
Right, that's an important point. For optional packages with spkgconfigure.m4
we don't have a mechanism yet to put them on the default "optional" list. I ran into this trap also recently on the lrslib ticket. That's basically #30746 (sage.doctest.control
: Replace use of sage.misc.package.list_packages
)
comment:31 Changed 11 months ago by
 Commit changed from 7d74ad641624e1c24a4e8695529b4416f4dd0678 to 6c54d00610d16fdc46edad67cfe43d50a3198c87
Branch pushed to git repo; I updated commit sha1. New commits:
6c54d00  use AC_LINK_IFELSE instead of obsolete AC_TRY_LINK

comment:32 Changed 11 months ago by
 Commit changed from 6c54d00610d16fdc46edad67cfe43d50a3198c87 to 709ab878f2a24594d71481c91d5d7d5eb02300eb
Branch pushed to git repo; I updated commit sha1. New commits:
709ab87  sage.feature.join_feature: New, factored out from LatteFeature; use it to implement FourTi2Feature

comment:33 Changed 11 months ago by
 Commit changed from 709ab878f2a24594d71481c91d5d7d5eb02300eb to 2e3c2100f641d2a4e556528848fc34c81aeaca29
comment:34 followup: ↓ 35 Changed 11 months ago by
 Cc klee slabbe added
I have now added a has_4ti2
to sage.doctest.external
, so that the 4ti2 tests are at least run if ./sage t optional=sage,external
is used.
I get that internet
shouldn't be probed without an additional option, but I think that at least for some "safe" features such as imagemagick
, rubiks
, 4ti2
, pandoc
, it really shouldn't require the use of external
to check for these.
comment:35 in reply to: ↑ 34 Changed 11 months ago by
Replying to mkoeppe:
I get that
internet
shouldn't be probed without an additional option, but I think that at least for some "safe" features such asimagemagick
,rubiks
,4ti2
,pandoc
, it really shouldn't require the use ofexternal
to check for these.
(that's for another ticket.)
comment:36 Changed 10 months ago by
This is now #32174 (doctests: Detect "safe" external features even if optional=external
is not used).
comment:37 Changed 10 months ago by
needs a rebase
comment:38 Changed 10 months ago by
 Commit changed from 2e3c2100f641d2a4e556528848fc34c81aeaca29 to 1b8634de22051a749b8e54f3684dc2930a713f39
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
8c49201  build/pkgs/4ti2/spkgconfigure.m4: Check for executables with prefix '4ti2' too

641f687  build/pkgs/sage_conf/src/sage_conf.py.in, src/sage/env.py: Add FOURTITWO... variables

fccb973  build/pkgs/4ti2/spkgconfigure.m4: Add AC_SUBST

3c32540  src/sage/env.py: No fallbacks for FOURTITWO... here

ea548d7  sage.features.four_ti_2: New, use it in sage.interfaces.four_ti_2, sage.sandpiles

f826ded  build/pkgs/4ti2/spkgconfigure.m4: Check for executable's with prefix 4ti2_ too

56016ce  use AC_LINK_IFELSE instead of obsolete AC_TRY_LINK

2b45b77  sage.feature.join_feature: New, factored out from LatteFeature; use it to implement FourTi2Feature

5c23cc9  DocTestReporter: Fix 'sage t optional=all'

1b8634d  sage.doctest.external: Add 4ti2

comment:39 Changed 10 months ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
lgtm
comment:40 Changed 10 months ago by
Thanks!
comment:42 Changed 10 months ago by
Volker, there are dozens of different ways external packages, vendored or not, with dependencies, may fail. More info please.
comment:43 Changed 10 months ago by
 Commit changed from 1b8634de22051a749b8e54f3684dc2930a713f39 to d9d4f999789bcc5477cde60bb26af45dd6b31585
Branch pushed to git repo; I updated commit sha1. New commits:
d9d4f99  Merge tag '9.4.beta6' into t/30887/public/30887

comment:44 Changed 10 months ago by
 Status changed from needs_work to positive_review
Works fine with and without 4ti2 installed. If there is a failure on some machine, we obviously need more information than "tests fail".
comment:45 Changed 10 months ago by
 Status changed from positive_review to needs_work
sage t long warnlong 43.6 randomseed=0 src/sage/misc/dev_tools.py ********************************************************************** File "src/sage/misc/dev_tools.py", line 487, in sage.misc.dev_tools.import_statements Failed example: import_statements('SAGE_ENV') Expected: from sage.env import SAGE_ENV Got: # ** Warning **: several modules for the object {'UNAME': 'Linux', 'HOSTNAME': 'zen', 'LOCAL_IDENTIFIER': 'zen.1421426', 'SAGE_VERSION': '9.4.beta6', 'SAGE_DATE': '20210724', 'SAGE_VERSION_BANNER': 'SageMath version 9.4.beta6, Release Date: 20210724', 'SAGE_VENV': '/home/release/Sage/local', 'SAGE_LIB': '/home/release/Sage/local/lib64/python3.9/sitepackages', 'SAGE_EXTCODE': '/home/release/Sage/local/lib64/python3.9/sitepackages/sage/ext_data', 'SAGE_VENV_SPKG_INST': '/home/release/Sage/local/var/lib/sage/installed', 'SAGE_LOCAL': '/home/release/Sage/local', 'SAGE_SHARE': '/home/release/Sage/local/share', 'SAGE_DOC': '/home/release/Sage/local/share/doc/sage', 'SAGE_SPKG_INST': '/home/release/Sage/local/var/lib/sage/installed', 'SAGE_ROOT': '/home/release/Sage', 'SAGE_SRC': '/home/release/Sage/src', 'SAGE_DOC_SRC': '/home/release/Sage/src/doc', 'SAGE_PKGS': '/home/release/Sage/build/pkgs', 'SAGE_ROOT_GIT': '/home/release/Sage/.git', 'DOT_SAGE': '/home/release/.sage/', 'SAGE_STARTUP_FILE': '/home/release/.sage/initdoctests.sage', 'SAGE_ARCHFLAGS': 'unset', 'SAGE_PKG_CONFIG_PATH': '/home/release/Sage/local/lib/pkgconfig', 'CONWAY_POLYNOMIALS_DATA_DIR': '/home/release/Sage/local/share/conway_polynomials', 'GRAPHS_DATA_DIR': '/home/release/Sage/local/share/graphs', 'ELLCURVE_DATA_DIR': '/home/release/Sage/local/share/ellcurves', 'POLYTOPE_DATA_DIR': '/home/release/Sage/local/share/reflexive_polytopes', 'GAP_ROOT_DIR': '/home/release/Sage/local/share/gap', 'THEBE_DIR': '/home/release/Sage/local/share/thebe', 'COMBINATORIAL_DESIGN_DATA_DIR': '/home/release/Sage/local/share/combinatorial_designs', 'CREMONA_MINI_DATA_DIR': '/home/release/Sage/local/share/cremona', 'CREMONA_LARGE_DATA_DIR': '/home/release/Sage/local/share/cremona', 'JMOL_DIR': '/home/release/Sage/local/share/jmol', 'MATHJAX_DIR': '/home/release/Sage/local/share/mathjax', 'MTXLIB': '/home/release/Sage/local/share/meataxe', 'THREEJS_DIR': '/home/release/Sage/local/share/threejssage', 'PPLPY_DOCS': '/home/release/Sage/local/share/doc/pplpy', 'MAXIMA': '/home/release/Sage/local/bin/maxima', 'MAXIMA_FAS': '/home/release/Sage/local/lib/ecl/maxima.fas', 'KENZO_FAS': '/home/release/Sage/local/lib/ecl/kenzo.fas', 'SAGE_NAUTY_BINS_PREFIX': '', 'FOURTITWO_HILBERT': '', 'FOURTITWO_MARKOV': '', 'FOURTITWO_GRAVER': '', 'FOURTITWO_ZSOLVE': '', 'FOURTITWO_QSOLVE': '', 'FOURTITWO_RAYS': '', 'FOURTITWO_PPI': '', 'FOURTITWO_CIRCUITS': '', 'FOURTITWO_GROEBNER': '', 'ARB_LIBRARY': 'arb', 'CBLAS_PC_MODULES': 'cblas', 'ECL_CONFIG': '/home/release/Sage/local/bin/eclconfig', 'NTL_INCDIR': '', 'NTL_LIBDIR': '', 'LIE_INFO_DIR': '/home/release/Sage/local/lib/LiE', 'OPENMP_CFLAGS': 'fopenmp', 'OPENMP_CXXFLAGS': 'fopenmp', 'SAGE_BANNER': '', 'SAGE_IMPORTALL': 'yes', 'SINGULAR_SO': '/home/release/Sage/local/lib/libSingular4.2.0.so', 'GAP_SO': '/home/release/Sage/local/lib/libgap.so.0.0.0'}: sage.all, sage.all_cmdline, sage.env, sage.features.four_ti_2, sage.repl.ipython_kernel.all_jupyter from sage.env import SAGE_ENV ********************************************************************** 1 item had failures: 1 of 37 in sage.misc.dev_tools.import_statements [60 tests, 1 failure, 0.22 s]  sage t long warnlong 43.6 randomseed=0 src/sage/misc/dev_tools.py # 1 doctest failed 
comment:46 Changed 10 months ago by
 Commit changed from d9d4f999789bcc5477cde60bb26af45dd6b31585 to 646e182e65fc247d0d2da9aabcad5f892a471a89
Branch pushed to git repo; I updated commit sha1. New commits:
646e182  src/sage/features/four_ti_2.py: Move import of SAGE_ENV inside the __init__ method, to remove confusion of sage.misc.dev_tools

comment:48 Changed 9 months ago by
 Milestone changed from sage9.4 to sage9.5
comment:49 Changed 8 months ago by
Let's get this in please.
comment:50 Changed 8 months ago by
 Reviewers changed from Dima Pasechnik to Dima Pasechnik, Antonio Rojas
 Status changed from needs_review to positive_review
All good here with both distro packaging and building from source against system 4ti2
comment:51 Changed 8 months ago by
Thank you!
comment:52 Changed 7 months ago by
 Branch changed from public/30887 to 646e182e65fc247d0d2da9aabcad5f892a471a89
 Resolution set to fixed
 Status changed from positive_review to closed
Here is distro information to get things started.
New commits:
30887: Add distro info for 4ti2