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

Use patched Cython for farey_symbol #23022

Closed
jdemeyer opened this issue May 18, 2017 · 15 comments
Closed

Use patched Cython for farey_symbol #23022

jdemeyer opened this issue May 18, 2017 · 15 comments

Comments

@jdemeyer
Copy link
Contributor

This reverts #13336 and fixes the building of farey_symbol on Cygwin using a Cython patch from #23004.

Depends on #23004

CC: @embray

Component: cython

Author: Jeroen Demeyer

Branch: cd4857d

Reviewer: Erik Bray

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

@jdemeyer jdemeyer added this to the sage-8.0 milestone May 18, 2017
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor Author

@jdemeyer
Copy link
Contributor Author

New commits:

57e4e0fAdd Cython patch for Cygwin
cd4857dUse patched Cython for farey_symbol

@jdemeyer
Copy link
Contributor Author

Commit: cd4857d

@embray
Copy link
Contributor

embray commented May 19, 2017

comment:4

Looks good in principle--testing now.

@embray
Copy link
Contributor

embray commented May 19, 2017

comment:5

Okay, works for me!

@embray
Copy link
Contributor

embray commented May 19, 2017

Reviewer: Erik Bray

@kiwifb
Copy link
Member

kiwifb commented May 20, 2017

comment:6

I have a couple of question relevant to sage-on-distro. Ok gentoo because I have a feeling binary distro won't care too much but I do because I make several versions of sage available for building and I need to be sure things will work with at least "current stable" and "current develop" (beta). Therefore I need to know if I need to provide different version of cython depending on the sage version.

@jdemeyer
Copy link
Contributor Author

comment:7

Replying to @kiwifb:

No, only on Cygwin.

Yes.

@kiwifb
Copy link
Member

kiwifb commented May 21, 2017

comment:8

Thank you for your time Jeroen.

@vbraun
Copy link
Member

vbraun commented May 21, 2017

Changed branch from u/jdemeyer/use_patched_cython_for_farey_symbol to cd4857d

@embray
Copy link
Contributor

embray commented May 22, 2017

comment:10

fbissey--you might be interested in commenting on #21459 though. In that case Jeroen is proposing a solution that requires a patch to Cython that would affect affect downstream packaging. All things being equal, I like his solution best and would prefer to use it. But I don't see a way around the fact that it would break building Sage on other platforms without a patched Cython.

@embray
Copy link
Contributor

embray commented May 22, 2017

Changed commit from cd4857d to none

@kiwifb
Copy link
Member

kiwifb commented May 22, 2017

comment:11

Replying to @embray:

fbissey--you might be interested in commenting on #21459 though. In that case Jeroen is proposing a solution that requires a patch to Cython that would affect affect downstream packaging. All things being equal, I like his solution best and would prefer to use it. But I don't see a way around the fact that it would break building Sage on other platforms without a patched Cython.

For the record I am already in the business of providing a patched cython for sage-on-gentoo. I sometimes feel more free than my debian colleagues...

@embray
Copy link
Contributor

embray commented May 22, 2017

comment:12

Okay. Yeah, for Debian it might be more of a problem, and I don't want to make additional pain for that ongoing, and IIUC nearly complete work.

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

4 participants