sagemath/sage

Add NTL to cython_aliases and sage.misc.cython library search dirs

mkoeppe opened this issue · 25 comments

(from #31348)

... using SAGE_NTL_PREFIX via sage_conf.

This is for macOS with configurations in which Python extensions no longer have access to /usr/local due to the use of -isysroot in the compiler configuration from sysconfig.

In particular, we add handling for ntl to the .homebrew-build-env script -- so that after brew install ntl; brew unlink ntl, our ./configure still finds NTL.

Depends on #30770
Depends on #31344

CC: @zlscherr @dimpase @orlitzky @videlec @kliem @jhpalmieri @kiwifb

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: dbcbf79

Reviewer: Jonathan Kliem

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

Dependencies: #30770

Author: Matthias Koeppe

Changed dependencies from #30770 to #30770, #31344

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

32576b4sage.misc.cython: Add NTL aliases, cache result of cython_aliases
comment:6

Actually spkg-configure.m4 needs some work too - need to use AX_ABSOLUTE_HEADER to set SAGE_NTL_PREFIX

Changed commit from 32576b4 to 7b1e27b

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

6e30e3aUse variables NTL_INCDIR, NTL_LIBDIR in sage_conf, separate from NTL_PREFIX used in sage-build-env-config; set -std=c++11 in NTL_CFLAGS
7b1e27bMerge distutils directives for Cython files using ntl

Description changed:

--- 
+++ 
@@ -2,4 +2,6 @@
 
 ... using `SAGE_NTL_PREFIX` via `sage_conf`.
 
+This is for macOS with configurations in which Python extensions no longer have access to `/usr/local` due to the use of `-isysroot` in the compiler configuration from sysconfig.
 
+
comment:9

NTL_INCDIR, NTL_LIBDIR needs more work.

NTL_INCDIR = ".
.
///usr/include/NTL"

Changed commit from 7b1e27b to 993c35c

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

4d2828dSwitch Cython files that use NTL to language = c++
993c35cbuild/pkgs/ntl/spkg-configure.m4: Fix up

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

dbcbf79.homebrew-build-env: Add ntl dirs

Changed commit from 993c35c to dbcbf79

Description changed:

--- 
+++ 
@@ -4,4 +4,8 @@
 
 This is for macOS with configurations in which Python extensions no longer have access to `/usr/local` due to the use of `-isysroot` in the compiler configuration from sysconfig.
 
+In particular, we add handling for `ntl` to the `.homebrew-build-env` script -- so that after `brew install ntl; brew unlink ntl`, our `./configure` still finds NTL.
 
+
+
+
kliem commented
comment:16

You changed src/sage/rings/rational.pyx and src/sage/matrix/matrix_rational_dense.pyx to language c++. Is there a reason for this?

kliem commented
comment:17

I'm a bit confused:

In src/sage/misc/cython.py we add NTL_LIBDIR to standard_libdirs and likewise we add NTL_INCDIR to standard_incdirs. Why do we need to add this to every file again that uses ntl?

kliem commented

Reviewer: Jonathan Kliem

kliem commented
comment:19

Replying to @kliem:

You changed src/sage/rings/rational.pyx and src/sage/matrix/matrix_rational_dense.pyx to language c++. Is there a reason for this?

Ok, I see:

[sagelib-9.3.beta7] cc1: warning: command line option ‘-std=c++11’ is valid for C++/ObjC++ but not for C
[sagelib-9.3.beta7] cc1: warning: command line option ‘-std=c++11’ is valid for C++/ObjC++ but not for C

Which raises a bit the question, why its called NTL_CFLAGS and not NTL_CXXFLAGS.

comment:20

Replying to @kliem:

In src/sage/misc/cython.py we add NTL_LIBDIR to standard_libdirs and likewise we add NTL_INCDIR to standard_incdirs. Why do we need to add this to every file again that uses ntl?

sage.misc.cython is only for runtime use of Cython; it is not used by the sagelib build system.

comment:21

Replying to @kliem:

Which raises a bit the question, why its called NTL_CFLAGS and not NTL_CXXFLAGS.

The names follow the conventions of pkg-config - which does not make this distinction.

kliem commented
comment:22

Ok. LGTM.

comment:23

Thank you!