Use both SINGULAR_INCDIR and SINGULAR_CFLAGS

[mkoeppe]

... to set include_dirs and extra_compile_args in Cython distutils directives. Do not rely on extra_compile_args for that.

Likewise for other libraries.

Otherwise, the include search order is not correct (see https://groups.google.com/d/msg/sage-release/SdxKEn7CuLM/3ru84S_zAgAJ)

CC: @slel @kiwifb @dimpase

Component: build

Author: Matthias Koeppe

Branch: 25c60d0

Reviewer: François Bissey

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

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Description changed:

--- 
+++ 
@@ -1,7 +1,7 @@
-It needs to be split into `include_dirs` and `extra_compile_args`.
+... to sey `include_dirs` and `extra_compile_args` in Cython distutils directives.
+
+Likewise for other libraries.

 Otherwise, the include search order is not correct (see https://groups.google.com/d/msg/sage-release/SdxKEn7CuLM/3ru84S_zAgAJ)

-
-

[mkoeppe]

Branch: u/mkoeppe/use_singular_cflags_correctly

[mkoeppe]

comment:3

Samuel: please check if this fixes the problem that you reported.

---

New commits:

309ee08

Do not rely on extra_compile_args to pass include dirs

[mkoeppe]

Commit: 309ee08

[mkoeppe]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-... to sey `include_dirs` and `extra_compile_args` in Cython distutils directives.
+... to set `include_dirs` and `extra_compile_args` in Cython distutils directives. Do not rely on `extra_compile_args` for that.

 Likewise for other libraries.

[kiwifb]

comment:5

Did you mean FFLASFFPACK_INCDIR in

diff --git a/src/sage/libs/linbox/fflas.pxd b/src/sage/libs/linbox/fflas.pxd
index fe475eb..5af349a 100644
--- a/src/sage/libs/linbox/fflas.pxd
+++ b/src/sage/libs/linbox/fflas.pxd
@@ -1,4 +1,5 @@
 # distutils: extra_compile_args = FFLASFFPACK_CFLAGS
+# distutils: include_dirs = FFLASFFPACK_CFLAGS
 # distutils: libraries = FFLASFFPACK_LIBRARIES
 # distutils: library_dirs = FFLASFFPACK_LIBDIR
 # distutils: extra_link_args = FFLASFFPACK_LIBEXTRA

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 309ee08 to 25c60d0

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

25c60d0

Fixup

[mkoeppe]

comment:7

Right, thanks

[mkoeppe]

comment:8

Needs review.

[kiwifb]

comment:9

I was hoping one of the reporter would pip in for the review really. At worst the changes here don't hurt anything. Actually they probably fix some corner cases that no one got into as far as we know.

[kiwifb]

Reviewer: François Bissey

[mkoeppe]

comment:10

Thanks!

[vbraun]

Changed branch from u/mkoeppe/use_singular_cflags_correctly to 25c60d0

[slel]

Changed commit from 25c60d0 to none

[slel]

comment:12

Replying to @kiwifb:

I was hoping one of the reporter would pip in for the review really. At worst the changes here don't hurt anything. Actually they probably fix some corner cases that no one got into as far as we know.

Seems to work for me, thanks! See below. Sorry it took me so long.

$ brew install singular
...
$ brew list --versions | grep singular
singular 4.1.3p2_1
$ cd /path/to/sage_root
$ make distclean
...
$ git branch -vv
* develop    83caa4befa [origin/develop] Updated SageMath version to 9.2.beta7
$ git trac try 30227
Fetching most recent beta version...
Fetching remote branch 25c60d0894cb2e76e0f4d0b480fb59ea93a4aaa2...

Merge of the most recent beta and the remote branch successful. When you are
finished, switch back to one of the existing branches. For example:

    git checkout develop

$ git branch -vv
* (HEAD detached from 83caa4befa) af1210c872 Merge commit '25c60d0894cb2e76e0f4d0b480fb59ea93a4aaa2' of git://trac.sagemath.org/sage into HEAD
  develop                         83caa4befa [origin/develop] Updated SageMath version to 9.2.beta7
$ source ./.homebrew-build-env
$ MAKE='make -j1 -s V=0'
$ ./bootstrap
...
$ CC=clang CXX=clang++ OBJCC=clang OBJCXX=clang++ ./configure -q
$ make
...
[zope_interface-none] installing. Log file: /opt/s/sage9/logs/pkgs/zope_interface-none.log
  [zope_interface-none] successfully installed.
[sagelib-9.2.beta7] installing. Log file: /opt/s/sage9/logs/pkgs/sagelib-9.2.beta7.log
  [sagelib-9.2.beta7] successfully installed.

Testing that Sage starts...
[2020-08-10 23:24:06] SageMath version 9.2.beta7, Release Date: 2020-08-02
This looks like the first time you are running Sage.
Cleaning up, do not interrupt this.
Done cleaning.
Yes, Sage starts.

real    211m10.735s
user    200m40.799s
sys 17m40.156s
Sage build/upgrade complete!

[mkoeppe]

comment:13

Great!