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

Don't use SAGE_CYTHONIZED in sage_include_directories(sources=True) #23744

Closed
jdemeyer opened this issue Aug 29, 2017 · 7 comments
Closed

Don't use SAGE_CYTHONIZED in sage_include_directories(sources=True) #23744

jdemeyer opened this issue Aug 29, 2017 · 7 comments

Comments

@jdemeyer
Copy link

Currently, SAGE_CYTHONIZED is used by src/sage/env.py to determine some include directories while building Sage.

Since we want to get rid of SAGE_CYTHONIZED (#21535), a first step would be to move this bit to setup.py in order to concentrate all uses of SAGE_CYTHONIZED in one place.

CC: @mkoeppe @embray

Component: build

Author: Jeroen Demeyer

Branch/Commit: 33746a2

Reviewer: Erik Bray

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

@jdemeyer jdemeyer added this to the sage-8.1 milestone Aug 29, 2017
@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

Commit: 33746a2

@jdemeyer
Copy link
Author

New commits:

33746a2Don't use SAGE_CYTHONIZED in sage_include_directories()

@embray
Copy link
Contributor

embray commented Aug 31, 2017

comment:3

I wonder what would happen if we also went ahead and deleted line 110 of sage.env:

_add_variable_or_fallback('SAGE_CYTHONIZED', opj('$SAGE_ROOT', 'src', 'build', 'cythonized'))

That could wait 'til a later change though. Looks good to me otherwise. I agree this is a good first step to removing it entirely.

@embray
Copy link
Contributor

embray commented Aug 31, 2017

Reviewer: Erik Bray

@jdemeyer
Copy link
Author

comment:4

Replying to @embray:

I wonder what would happen if we also went ahead and deleted line 110 of sage.env:

_add_variable_or_fallback('SAGE_CYTHONIZED', opj('$SAGE_ROOT', 'src', 'build', 'cythonized'))

Some doctests in src/sage_setup use that.

@vbraun
Copy link
Member

vbraun commented Sep 10, 2017

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