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

fix flintxx development #20281

Closed
rwst opened this issue Mar 24, 2016 · 14 comments
Closed

fix flintxx development #20281

rwst opened this issue Mar 24, 2016 · 14 comments

Comments

@rwst
Copy link

rwst commented Mar 24, 2016

Current flint-2.5.2 is from last year and lacks a recent fix from git master addressing compile errors of flintxx headers. This ticket patches Sage's flint to enable Pynac development with flintxx, the C++ API of flint.

flintlib/flint@c0768dc

Component: packages: standard

Author: Ralf Stephan

Branch/Commit: 45ebadb

Reviewer: Jeroen Demeyer

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

@rwst rwst added this to the sage-7.2 milestone Mar 24, 2016
@rwst
Copy link
Author

rwst commented Mar 24, 2016

Branch: u/rws/fix_flintxx_development

@rwst
Copy link
Author

rwst commented Mar 24, 2016

New commits:

350d13720281: fix flintxx development

@rwst
Copy link
Author

rwst commented Mar 24, 2016

Commit: 350d137

@rwst
Copy link
Author

rwst commented Mar 24, 2016

Author: Ralf Stephan

@jdemeyer
Copy link

comment:3

pynac doesn't use flint.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 24, 2016

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

45ebadb20281: revert Pynac dependency, add it later

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 24, 2016

Changed commit from 350d137 to 45ebadb

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@rwst
Copy link
Author

rwst commented Mar 24, 2016

comment:7

Thanks.

@kiwifb
Copy link
Member

kiwifb commented Mar 24, 2016

comment:8

I personally don't think the fix you submitted upstream is any good. You are probably configuring with -I${some_prefix}/flint/flintxx when you should use -I${some_prefix}/flint -I${some_prefix}/flint/flintxx. In fact it would be better, I think, to only have ${some_prefx}/flint and have the flintxx headers prefixed with flintxx.

@kiwifb
Copy link
Member

kiwifb commented Mar 24, 2016

comment:9

Possibly, depending on your includes it should have been #include "flint/flint.h" rather than #include "../flint.h". That fells more correct to me.

@rwst
Copy link
Author

rwst commented Mar 24, 2016

comment:10

It is a complete mystery to me how you get the idea that I submitted anything upstream. I merely reported the issue at flintlib/flint#217. Consequently your remarks should be added there.

@kiwifb
Copy link
Member

kiwifb commented Mar 24, 2016

comment:11

Right I saw "reported by Ralph Stephan and fixed by ..." and I have conflated things a little bit. I guess I thought you worked together. But right it is just that I don't think it is a good solution in my opinion.

@vbraun
Copy link
Member

vbraun commented Mar 26, 2016

Changed branch from u/rws/fix_flintxx_development to 45ebadb

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