Opened 19 months ago

Closed 7 months ago

#30887 closed enhancement (fixed)

Add spkg-configure.m4 for 4ti2, remove direct use of SAGE_LOCAL

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: packages: optional Keywords: 4ti2, spkg-configure.m4
Cc: dimpase, mjo, slelievre, arojas, fbissey, gh-jengelh, 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:

Status badges

Description (last modified by slelievre)

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')

https://repology.org/project/4ti2/versions

Change History (52)

comment:1 Changed 14 months ago by slelievre

  • Authors set to Samuel Lelièvre, ...
  • Branch set to public/30887
  • Cc slelievre added
  • Commit set to d79e1c15b832bb3f28fc37c0fcb8fa312feb1136
  • Description modified (diff)
  • Keywords 4ti2 spkg-configure.m4 added
  • Summary changed from spkg-configure.m4 for 4ti2 to Add spkg-configure.m4 for 4ti2

Here is distro information to get things started.


New commits:

d79e1c130887: Add distro info for 4ti2

comment:2 Changed 14 months ago by mkoeppe

latte_int uses 4ti2 as a library. The test is here: https://github.com/latte-int/latte/blob/master/m4/4ti2-check.m4#L48

comment:3 Changed 14 months ago by mkoeppe

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 mkoeppe

singular is able to use 4ti2 via the command-line interface. Checks are done in https://github.com/Singular/Singular/blob/spielwiese/configure.ac#L352

configure.ac:AC_CHECK_PROGS([FOURTITWO_HILBERT], [hilbert 4ti2-hilbert])
configure.ac:AC_CHECK_PROGS([FOURTITWO_MARKOV], [markov 4ti2-markov])
configure.ac:AC_CHECK_PROGS([FOURTITWO_GRAVER], [graver 4ti2-graver])

comment:5 Changed 14 months ago by mkoeppe

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 mkoeppe

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 mkoeppe

  • Dependencies set to #31534

comment:8 Changed 14 months ago by git

  • Commit changed from d79e1c15b832bb3f28fc37c0fcb8fa312feb1136 to 602ffb02fed3dd90118f53fc4aa2ceb44d9e4690

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

7c272ecAdd MacPorts package info
8c9f53c30504: Add _recommended package info for MacPorts
bf0a4d230504: Use zmq-devel in MacPorts for zeromq
e42448930504: Fix gcc gfortran openblas info for MacPorts
b8b7092build/pkgs/{gcc,gfortran}/distros/macports.txt: Fixup
4acfc5cMerge #30504
857b7d5tox.ini: In -maximal environments, use IGNORE_MISSING_SYSTEM_PACKAGES=yes for all non-current distributions
4e561actox.ini: Use IGNORE_MISSING_SYSTEM_PACKAGES=no for rolling distributions, but IGNORE_MISSING_SYSTEM_PACKAGES=yes for unstable distributions
30d0e8aMerge #31534
602ffb0build/pkgs/4ti2/spkg-configure.m4: New

comment:9 Changed 14 months ago by git

  • Commit changed from 602ffb02fed3dd90118f53fc4aa2ceb44d9e4690 to ff25e8fabac0115d37c218b8a92f721a649d9789

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

ff25e8fbuild/pkgs/4ti2/spkg-configure.m4: Fix quoting

comment:10 Changed 14 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

comment:11 Changed 11 months ago by git

  • Commit changed from ff25e8fabac0115d37c218b8a92f721a649d9789 to dd364a566b4d3381436cc2bc3d230d551ddfb35a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

0f1317330887: Add distro info for 4ti2
1c1c1c8build/pkgs/4ti2/spkg-configure.m4: New
dd364a5build/pkgs/4ti2/spkg-configure.m4: Fix quoting

comment:12 Changed 11 months ago by mkoeppe

  • Dependencies #31534 deleted

comment:13 Changed 11 months ago by git

  • Commit changed from dd364a566b4d3381436cc2bc3d230d551ddfb35a to 390c0d2984b40db48b9b0021e78d6e7e04893455

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

45f8e59build/pkgs/4ti2/spkg-configure.m4: Check for executables with prefix '4ti2-' too
390c0d2build/pkgs/sage_conf/src/sage_conf.py.in, src/sage/env.py: Add FOURTITWO... variables

comment:14 Changed 11 months ago by mkoeppe

On ubuntu-focal-maximal:

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 4ti2-hilbert... 4ti2-hilbert
checking for markov... no
checking for 4ti2-markov... 4ti2-markov
checking for graver... no
checking for 4ti2-graver... 4ti2-graver
checking for zsolve... no
checking for 4ti2-zsolve... 4ti2-zsolve
checking for qsolve... no
checking for 4ti2-qsolve... 4ti2-qsolve
checking for rays... no
checking for 4ti2-rays... 4ti2-rays
checking for ppi... no
checking for 4ti2-ppi... 4ti2-ppi
checking for circuits... no
checking for 4ti2-circuits... 4ti2-circuits
checking for groebner... no
checking for 4ti2-groebner... 4ti2-groebner
checking for library 4ti2gmp... no
configure: no suitable system package found for SPKG 4ti2

comment:15 Changed 11 months ago by mkoeppe

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 git

  • Commit changed from 390c0d2984b40db48b9b0021e78d6e7e04893455 to fd029453d6c58c17b627159d8ec6d97bc76cc487

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

f2d97bfbuild/pkgs/4ti2/spkg-configure.m4: Add AC_SUBST
b5a4aa3src/sage/env.py: No fallbacks for FOURTITWO... here
fd02945sage.features.four_ti_2: New, use it in sage.interfaces.four_ti_2, sage.sandpiles

comment:17 Changed 11 months ago by mkoeppe

  • Authors changed from Samuel Lelièvre, ... to Samuel Lelièvre, Matthias Koeppe
  • Cc arojas fbissey added
  • Status changed from new to needs_review

comment:18 Changed 11 months ago by mkoeppe

  • Summary changed from Add spkg-configure.m4 for 4ti2 to Add spkg-configure.m4 for 4ti2, remove direct use of SAGE_LOCAL

comment:19 Changed 11 months ago by fbissey

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 fbissey

Works and doctest with system 4ti2 on Gentoo.

comment:21 Changed 11 months ago by mkoeppe

  • Cc gh-jengelh added

comment:22 Changed 11 months ago by gh-jengelh

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.

Last edited 11 months ago by gh-jengelh (previous) (diff)

comment:23 Changed 11 months ago by mkoeppe

OK, we can check for the prefix 4ti2_ here too

comment:24 Changed 11 months ago by mkoeppe

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 git

  • Commit changed from fd029453d6c58c17b627159d8ec6d97bc76cc487 to 7d74ad641624e1c24a4e8695529b4416f4dd0678

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

7d74ad6build/pkgs/4ti2/spkg-configure.m4: Check for executable's with prefix 4ti2_ too

comment:26 Changed 11 months ago by mkoeppe

Let's get this in please

comment:27 follow-up: Changed 11 months ago by dimpase

AC_TRY_LINK is obsolete. We spent some time removing these in #30668. autoupdate produces

  • build/pkgs/4ti2/spkg-configure.m4

    diff --git a/build/pkgs/4ti2/spkg-configure.m4 b/build/pkgs/4ti2/spkg-configure.m4
    index e01562c50d..a16da8fc4b 100644
    a b SAGE_SPKG_CONFIGURE([4ti2], [ 
    1717        FORTYTWO_LIBS="-l4ti2gmp -lzsolve"
    1818        CXXFLAGS="${BACKUP_CXXFLAGS} ${FORTYTWO_CXXFLAGS} ${GMP_CFLAGS}"
    1919        LIBS="${BACKUP_LIBS} ${FORTYTWO_LIBS} ${GMP_LIBS}"
    20         AC_TRY_LINK([
     20        AC_LINK_IFELSE([AC_LANG_PROGRAM([[
    2121#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]])],[
    2524        AC_MSG_RESULT([yes])
    26 ], [
     25],[
    2726        AC_MSG_RESULT([no])
    2827        sage_spkg_install_4ti2=yes
    2928])

and it appears to work.

comment:28 follow-up: Changed 11 months ago by 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 --with-system-4ti2=force --enable-4ti2=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 mkoeppe

Replying to dimpase:

AC_TRY_LINK is obsolete. We spent some time removing these in #30668. autoupdate produces

Can you push this to the ticket please?

comment:30 in reply to: ↑ 28 Changed 11 months ago by mkoeppe

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 --with-system-4ti2=force --enable-4ti2=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.

Right, that's an important point. For optional packages with spkg-configure.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 git

  • Commit changed from 7d74ad641624e1c24a4e8695529b4416f4dd0678 to 6c54d00610d16fdc46edad67cfe43d50a3198c87

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

6c54d00use AC_LINK_IFELSE instead of obsolete AC_TRY_LINK

comment:32 Changed 11 months ago by git

  • Commit changed from 6c54d00610d16fdc46edad67cfe43d50a3198c87 to 709ab878f2a24594d71481c91d5d7d5eb02300eb

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

709ab87sage.feature.join_feature: New, factored out from LatteFeature; use it to implement FourTi2Feature

comment:33 Changed 11 months ago by git

  • Commit changed from 709ab878f2a24594d71481c91d5d7d5eb02300eb to 2e3c2100f641d2a4e556528848fc34c81aeaca29

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

dcbbea2DocTestReporter: Fix 'sage -t --optional=all'
2e3c210sage.doctest.external: Add 4ti2

comment:34 follow-up: Changed 11 months ago by mkoeppe

  • 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 mkoeppe

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 as imagemagick, rubiks, 4ti2, pandoc, it really shouldn't require the use of external to check for these.

(that's for another ticket.)

comment:36 Changed 10 months ago by mkoeppe

This is now #32174 (doctests: Detect "safe" external features even if --optional=external is not used).

comment:37 Changed 10 months ago by dimpase

needs a rebase

comment:38 Changed 10 months ago by git

  • Commit changed from 2e3c2100f641d2a4e556528848fc34c81aeaca29 to 1b8634de22051a749b8e54f3684dc2930a713f39

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

8c49201build/pkgs/4ti2/spkg-configure.m4: Check for executables with prefix '4ti2-' too
641f687build/pkgs/sage_conf/src/sage_conf.py.in, src/sage/env.py: Add FOURTITWO... variables
fccb973build/pkgs/4ti2/spkg-configure.m4: Add AC_SUBST
3c32540src/sage/env.py: No fallbacks for FOURTITWO... here
ea548d7sage.features.four_ti_2: New, use it in sage.interfaces.four_ti_2, sage.sandpiles
f826dedbuild/pkgs/4ti2/spkg-configure.m4: Check for executable's with prefix 4ti2_ too
56016ceuse AC_LINK_IFELSE instead of obsolete AC_TRY_LINK
2b45b77sage.feature.join_feature: New, factored out from LatteFeature; use it to implement FourTi2Feature
5c23cc9DocTestReporter: Fix 'sage -t --optional=all'
1b8634dsage.doctest.external: Add 4ti2

comment:39 Changed 10 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:40 Changed 10 months ago by mkoeppe

Thanks!

comment:41 Changed 10 months ago by vbraun

  • Status changed from positive_review to needs_work

Tests fail

comment:42 Changed 10 months ago by dimpase

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 git

  • Commit changed from 1b8634de22051a749b8e54f3684dc2930a713f39 to d9d4f999789bcc5477cde60bb26af45dd6b31585

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

d9d4f99Merge tag '9.4.beta6' into t/30887/public/30887

comment:44 Changed 10 months ago by mkoeppe

  • 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 vbraun

  • Status changed from positive_review to needs_work
sage -t --long --warn-long 43.6 --random-seed=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': '2021-07-24', 'SAGE_VERSION_BANNER': 'SageMath version 9.4.beta6, Release Date: 2021-07-24', 'SAGE_VENV': '/home/release/Sage/local', 'SAGE_LIB': '/home/release/Sage/local/lib64/python3.9/site-packages', 'SAGE_EXTCODE': '/home/release/Sage/local/lib64/python3.9/site-packages/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/init-doctests.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/threejs-sage', '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/ecl-config', '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/libSingular-4.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 --warn-long 43.6 --random-seed=0 src/sage/misc/dev_tools.py  # 1 doctest failed
----------------------------------------------------------------------

comment:46 Changed 10 months ago by git

  • Commit changed from d9d4f999789bcc5477cde60bb26af45dd6b31585 to 646e182e65fc247d0d2da9aabcad5f892a471a89

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

646e182src/sage/features/four_ti_2.py: Move import of SAGE_ENV inside the __init__ method, to remove confusion of sage.misc.dev_tools

comment:47 Changed 10 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Thanks, fixed.

comment:48 Changed 9 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:49 Changed 8 months ago by mkoeppe

Let's get this in please.

comment:50 Changed 8 months ago by arojas

  • 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 mkoeppe

Thank you!

comment:52 Changed 7 months ago by vbraun

  • Branch changed from public/30887 to 646e182e65fc247d0d2da9aabcad5f892a471a89
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.