Opened 12 years ago
Closed 12 years ago
#10260 closed defect (fixed)
module_list.py: quaternion_algebra_element.pyx wrongly uses -std=c99 option
Reported by: | Jeroen Demeyer | Owned by: | Georg S. Weber |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6.1 |
Component: | build | Keywords: | |
Cc: | Merged in: | sage-4.6.1.alpha2 | |
Authors: | Jeroen Demeyer | Reviewers: | François Bissey |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
It's C++ code...
Attachments (1)
Change History (9)
Changed 12 years ago by
Attachment: | 10260_quaternion_algebra_element_cpp.patch added |
---|
comment:1 Changed 12 years ago by
Status: | new → needs_review |
---|
comment:2 follow-up: 6 Changed 12 years ago by
comment:3 follow-up: 4 Changed 12 years ago by
Could you explain what "D_XPG6" is and why it's needed? (I've seen this elsewhere, and yet to have anyone justify it.)
comment:4 Changed 12 years ago by
Replying to robertwb:
Could you explain what "D_XPG6" is and why it's needed? (I've seen this elsewhere, and yet to have anyone justify it.)
After a quick googling it looks like something ugly. First hit says: XPG was the X/Open Portability Guide, a pre-POSIX consortium to promote interoperability between different UNIX vendors. XPG6 is "X/Open Portability Guide, Issue 6".
But that one is a little bit more verbose about what that flag is supposed to do: http://unix.derkeiler.com/Newsgroups/comp.unix.solaris/2007-01/msg00360.html In short make C99 code work inside c++ code... Which may explain why someone bothered to put it together with -std=c99. Also the post ask how to enable it on solaris which explains why there is a uname_specific command with it.
It seem to be regarded as a bad idea by a number of people.
comment:5 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
I am giving the present patch a positive review. It is fairly simple and as actually been in use in sage-on-gentoo for a while.
We can do the other one later in another ticket.
comment:6 Changed 12 years ago by
Replying to fbissey:
Tell you what. I introduced exactly the same patch in sage-on-gentoo at the end of June. While you are at it can we fix this one as well:
Extension('sage.rings.polynomial.polynomial_rational_flint', sources = ['sage/rings/polynomial/polynomial_rational_flint.pyx', 'sage/libs/flint/fmpq_poly.c'], language = 'c++', extra_compile_args=["-std=c99"] + uname_specific('SunOS', [], ['-D_XPG6']), libraries = ["csage", "flint", "ntl", "gmpxx", "gmp"], include_dirs = [SAGE_ROOT + '/local/include/FLINT/', SAGE_ROOT + '/devel/sage/sage/libs/flint/'], depends = [SAGE_ROOT + "/local/include/FLINT/flint.h"]),
This one is harder, because there are two files, one C99 and one C++. So we cannot simply remove -std=c99
there.
comment:7 Changed 12 years ago by
Reviewers: | → François Bissey |
---|
comment:8 Changed 12 years ago by
Merged in: | → sage-4.6.1.alpha2 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Tell you what. I introduced exactly the same patch in sage-on-gentoo at the end of June. While you are at it can we fix this one as well:
That's the only other with that "QA" problem. I'll happily give it a positive review.