Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Feb 9, 2021 · 25 comments
Closed

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

mkoeppe opened this issue Feb 9, 2021 · 25 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 9, 2021

(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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Feb 9, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

Dependencies: #30770

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

Changed dependencies from #30770 to #30770, #31344

@mkoeppe mkoeppe changed the title Add NTL to cython_aliases Add NTL to cython_aliases and sage.misc.cython library search dirs Feb 9, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

Branch: u/mkoeppe/add_ntl_to_cython_aliases

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 9, 2021

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 9, 2021

Commit: 32576b4

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 9, 2021

comment:6

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2021

Changed commit from 32576b4 to 7b1e27b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2021

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

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 11, 2021

comment:9

NTL_INCDIR, NTL_LIBDIR needs more work.

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2021

Changed commit from 7b1e27b to 993c35c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2021

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

dbcbf79.homebrew-build-env: Add ntl dirs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 12, 2021

Changed commit from 993c35c to dbcbf79

@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Feb 16, 2021

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
Copy link
Contributor

kliem commented Feb 16, 2021

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
Copy link
Contributor

kliem commented Feb 16, 2021

Reviewer: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Feb 16, 2021

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 16, 2021

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 16, 2021

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
Copy link
Contributor

kliem commented Feb 16, 2021

comment:22

Ok. LGTM.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 16, 2021

comment:23

Thank you!

@vbraun
Copy link
Member

vbraun commented Mar 9, 2021

Changed branch from u/mkoeppe/add_ntl_to_cython_aliases to dbcbf79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants