spkg-configure.m4 for NTL

[dimpase]

implemented a direct version check using AC_RUN_IFELSE.
The rest is standard, as in #27212, basically.

to be merged as a part of #27822

Depends on #27212
Depends on #27641
Depends on #27751
Depends on #27259

CC: @embray @kiwifb @jdemeyer

Component: build: configure

Keywords: spkg-configure ntl

Author: Dima Pasechnik, Erik Bray

Branch/Commit: public/packages/ntlconf @ 892143a

Reviewer: Erik Bray, Dima Pasechnik

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

[dimpase]

Dependencies: #27212

[dimpase]

Changed dependencies from #27212 to #27212, #27238

[dimpase]

comment:4

here we might want to check that NTL is built with gf2x. However, it appears that it's only question of better performance, not an exta API, if it gf2x is linked.

[dimpase]

Branch: u/dimpase/packages/ntlconf

[dimpase]

Commit: af9ab90

[dimpase]

Last 10 new commits:

b80bf72	Reworked this a bit more
0ca3f56	fix typo
06798ca	added a bit more explanation
b592d77	correct logic for SAGE_GMP_PREFIX etc
5057680	add the AX_ABSOLUTE_HEADER macro
101537b	iml in particular is very picky about being given an absolute path to the
862ca6a	Merge remote-tracking branch 'trac/develop' into HEAD
03dc987	Merged trac #27215 in
c9873ae	Merge remote-tracking branch 'trac/develop' into mpf
af9ab90	deal with --with-ntl, proper version check

[dimpase]

Author: Dima Pasechnik

[dimpase]

Changed dependencies from #27212, #27238 to #27212

[dimpase]

comment:7

nesting of AC_... tests is wrong.

[embray]

comment:8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

[dimpase]

comment:9

also needs

+++ b/build/pkgs/flint/spkg-install
@@ -17,7 +17,7 @@ echo "Configuring FLINT."
     --disable-static \
     --prefix="$SAGE_LOCAL" \
     $SAGE_CONFIGURE_GMP \
-    $SAGE_CONFIGURE_NTL \
+    --with-ntl="$SAGE_NTL_PREFIX" \
     $SAGE_CONFIGURE_MPFR \
     $FLINT_CONFIGURE || sdh_die "Error: Failed to configure FLINT."

as completely omitting --with-ntl results in no NTL linked into flint.

[sagetrac-git]

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

299e367	Replace various --with-gmp=$SAGE_LOCAL flags in spkg-installs with a
058d515	Reworked this a bit more
7beadaa	fix typo
b930874	added a bit more explanation
6d2b496	correct logic for SAGE_GMP_PREFIX etc
43be546	add the AX_ABSOLUTE_HEADER macro
e343418	iml in particular is very picky about being given an absolute path to the
e735b4b	configure version bump
cefb350	deal with --with-ntl, proper version check
d4378cb	flint needs --with-ntl= always

[sagetrac-git]

Changed commit from af9ab90 to d4378cb

[dimpase]

comment:11

rebased over the latest #27212 and fixed comment:9 issue.

[sagetrac-git]

Changed commit from d4378cb to 8fe9503

[sagetrac-git]

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

8fe9503

check for deps of ntl

[dimpase]

comment:13

This is ready for review now. Note that often external NTL packages are built without gf2x, but gf2x is merely an optimisation.

Perhaps, to be completely on the safe side, spkg-configure should check whether gf2x is in, and if it is, only use system's NTL if gf2x will come from the system, too.
(But I don't know how to check whether gf2x is used in NTL, at least not in an OS-independent way).

[kiwifb]

comment:15

Minor remark (it all looks ok) why keep --with-ntl="$SAGE_NTL_PREFIX" instead of using $SAGE_CONFIGURE_NTL like everywhere else in flint?

[dimpase]

comment:16

Replying to @kiwifb:

Minor remark (it all looks ok) why keep --with-ntl="$SAGE_NTL_PREFIX" instead of using $SAGE_CONFIGURE_NTL like everywhere else in flint?

Because $SAGE_CONFIGURE_NTL is empty if NTL comes from the system, but flint must have --with-ntl to build NTL interface.

[kiwifb]

comment:17

Replying to @dimpase:

Replying to @kiwifb:

Minor remark (it all looks ok) why keep --with-ntl="$SAGE_NTL_PREFIX" instead of using $SAGE_CONFIGURE_NTL like everywhere else in flint?

Because $SAGE_CONFIGURE_NTL is empty if NTL comes from the system, but flint must have --with-ntl to build NTL interface.

Thank you for your answer. Once we get some bots reports I think we can move to a positive review.

[dimpase]

comment:18

bots will need need a configure tarball. I'll add it.

[dimpase]

comment:72

Missing quotes here:

--- a/build/pkgs/ntl/spkg-configure.m4
+++ b/build/pkgs/ntl/spkg-configure.m4
@@ -19,7 +19,7 @@ SAGE_SPKG_CONFIGURE([ntl], [
         AC_LINK_IFELSE([
             AC_LANG_PROGRAM([[#include <NTL/ZZ.h>]],
                             [[NTL::ZZ a;]]
-            )], [LIBS=$LIBS -lntl]
+            )], [LIBS="$LIBS -lntl"]
                 [AC_MSG_RESULT([yes])], [
             AC_MSG_RESULT([no]); sage_spkg_install_ntl=yes
         ])

so it works if LIBS was empty, but breaks if it's already not...

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

892143a

add missing quotes

[sagetrac-git]

Changed commit from 4b3caa7 to 892143a

[dimpase]

comment:74

stumble upon this while trying this with more spkg-configs merged.

[vbraun]

comment:75

Please only set this ticket to positive review after you have personally verified that a clean sage build works

[dimpase]

comment:76

what makes you think I did not do this?

[dimpase]

comment:77

A clean build worked great, it's a "dirty" build that revealed the problem of comment:72

[dimpase]

comment:79

with clang on OSX, I just saw a failure to recognise NTL, due to c++ in the test called without -std=gnu++11.

Adding AC_REQUIRE([SAGE_SPKG_CONFIGURE_GCC]) just above AC_REQUIRE([SAGE_SPKG_CONFIGURE_GMP]) in NTL's spkg-configure.m4 fixes this.

But this seems to be a not very consistent way to do thing, as SAGE_SPKG_CONFIGURE_GCC ought to be called before the any other library tests. Do we have a way to force this?

[embray]

comment:80

It might work to just manually call AC_REQUIRE([SAGE_SPKG_CONFIGURE_GCC]) right at the beginning of SAGE_SPKG_COLLECT.

[dimpase]

comment:81

Replying to @embray:

It might work to just manually call AC_REQUIRE([SAGE_SPKG_CONFIGURE_GCC]) right at the beginning of SAGE_SPKG_COLLECT.

hmm, no, this does not work, as at this moment SAGE_SPKG_CONFIGURE_GCC is not defined.

I am thinking of putting it into SAGE_SPKG_CONFIGURE, but this means it's a recursive call, so the only way I can think of would be avoid the recursion:
to define essentially a copy of
SAGE_SPKG_CONFIGURE, without this call, and then define SAGE_SPKG_CONFIGURE_GCC using this copy. After this AC_REQUIRE(...) would not involve a recursion.

[dimpase]

comment:82

Replying to @dimpase:

Replying to @embray:

It might work to just manually call AC_REQUIRE([SAGE_SPKG_CONFIGURE_GCC]) right at the beginning of SAGE_SPKG_COLLECT.

hmm, no, this does not work, as at this moment SAGE_SPKG_CONFIGURE_GCC is not defined.

I am thinking of putting it into SAGE_SPKG_CONFIGURE, but this means it's a recursive call, so the only way I can think of would be avoid the recursion:
to define essentially a copy of
SAGE_SPKG_CONFIGURE, without this call, and then define SAGE_SPKG_CONFIGURE_GCC using this copy. After this AC_REQUIRE(...) would not involve a recursion.

I have opened #28005 to deal with this.