#29697 closed enhancement (fixed)
src/setup.py, src/sage/env.py (sage_include_directories): Do not add another copy of SAGE_INC, SAGE_LOCAL/lib to include dirs, library dirs
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  build  Keywords:  
Cc:  dimpase, fbissey, mjo, isuruf  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  9a50cba (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
$SAGE_LOCAL/{include,lib}
are already added to the front of the search paths CPATH
and LIBRARY_PATH
by sageenv
.
We remove code that adds another copy. This removes a dependency on SAGE_LOCAL
during the build of sagelib.
The function sage_include_directories
, apart from its use by setup.py
, is also used by sage.misc.cython.cython
.
Change History (14)
comment:1 Changed 2 years ago by
 Branch set to u/mkoeppe/src_setup_py__src_sage_env_py__sage_include_directories___do_not_add_another_copy_of_sage_inc__sage_local_lib_to_include_dirs__library_dirs
comment:2 followup: ↓ 7 Changed 2 years ago by
 Commit set to 16247ad9675343f1d3e1158cc5e43137aec82645
comment:3 Changed 2 years ago by
 Description modified (diff)
comment:4 Changed 2 years ago by
I plan to add some doctests to make sure that functions such as sage.misc.cython.cython
also work correctly when sage.all
is imported into plain Python (without runnning sageenv
). Things like this are currently not tested at all (and probably not guaranteed by anything); I plan to use #29446 for this.
comment:5 Changed 2 years ago by
 Milestone changed from sage9.1 to sage9.2
 Status changed from new to needs_review
comment:6 Changed 2 years ago by
 Commit changed from 16247ad9675343f1d3e1158cc5e43137aec82645 to 9a50cba23abb1c6ff4b022c62624da800b88e282
Branch pushed to git repo; I updated commit sha1. New commits:
9a50cba  src/sage/env.py (sage_include_directories): Fixup doctest

comment:7 in reply to: ↑ 2 Changed 2 years ago by
comment:8 Changed 2 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
Well, I think you have gone after the only breakage in the last commit. Unless someone can point to a corner case, it is all positive from me.
comment:9 Changed 2 years ago by
Thanks!
comment:10 Changed 2 years ago by
I didn't realise at the time, but that will be one less line of patching for me in sageongentoo.
comment:11 Changed 2 years ago by
 Branch changed from u/mkoeppe/src_setup_py__src_sage_env_py__sage_include_directories___do_not_add_another_copy_of_sage_inc__sage_local_lib_to_include_dirs__library_dirs to 9a50cba23abb1c6ff4b022c62624da800b88e282
 Resolution set to fixed
 Status changed from positive_review to closed
comment:12 Changed 22 months ago by
 Commit 9a50cba23abb1c6ff4b022c62624da800b88e282 deleted
This may have caused some breakage: https://groups.google.com/d/msg/sagerelease/SdxKEn7CuLM/3ru84S_zAgAJ
comment:13 Changed 22 months ago by
... because of incorrect usage of extra_compile_args
in some of our modules  to be fixed in #30227
comment:14 Changed 16 months ago by
Followup: #31335
I don't think it is controversial at all. Is there anyone who think we are forgetting something here?
New commits:
src/sage/env.py (sage_include_directories): Do not put SAGE_INC in front of the sage source include directories
src/setup.py: Do not put SAGE_LOCAL/lib in front of the library directories