Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#30227 closed defect (fixed)

Use both SINGULAR_INCDIR and SINGULAR_CFLAGS

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: build Keywords:
Cc: slelievre, fbissey, dimpase Merged in:
Authors: Matthias Koeppe Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 25c60d0 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by 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)

Change History (13)

comment:1 Changed 4 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Summary changed from Use SINGULAR_CFLAGS correctly to Use both SINGULAR_INCDIR and SINGULAR_CFLAGS

comment:2 Changed 4 months ago by mkoeppe

  • Branch set to u/mkoeppe/use_singular_cflags_correctly

comment:3 Changed 4 months ago by mkoeppe

  • Cc fbissey dimpase added
  • Commit set to 309ee081dca793e4772c0a4dc441950106a70090
  • Status changed from new to needs_review

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


New commits:

309ee08Do not rely on extra_compile_args to pass include dirs

comment:4 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:5 Changed 4 months ago by fbissey

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

comment:6 Changed 4 months ago by git

  • Commit changed from 309ee081dca793e4772c0a4dc441950106a70090 to 25c60d0894cb2e76e0f4d0b480fb59ea93a4aaa2

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

25c60d0Fixup

comment:7 Changed 4 months ago by mkoeppe

Right, thanks

comment:8 Changed 4 months ago by mkoeppe

Needs review.

comment:9 follow-up: Changed 4 months ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

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.

comment:10 Changed 4 months ago by mkoeppe

Thanks!

comment:11 Changed 4 months ago by vbraun

  • Branch changed from u/mkoeppe/use_singular_cflags_correctly to 25c60d0894cb2e76e0f4d0b480fb59ea93a4aaa2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:12 in reply to: ↑ 9 Changed 4 months ago by slelievre

  • Commit 25c60d0894cb2e76e0f4d0b480fb59ea93a4aaa2 deleted

Replying to fbissey:

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!

comment:13 Changed 4 months ago by mkoeppe

Great!

Note: See TracTickets for help on using tickets.