Opened 12 years ago
Closed 10 years ago
#9544 closed enhancement (fixed)
Fix flintqs on FreeBSD
Reported by: | pjeremy | Owned by: | pjeremy |
---|---|---|---|
Priority: | major | Milestone: | sage-5.1 |
Component: | porting: BSD | Keywords: | |
Cc: | Merged in: | sage-5.1.rc0 | |
Authors: | Peter Jeremy, Jeroen Demeyer | Reviewers: | Stephen Montgomery-Smith, Jeroen Demeyer, Karl-Dieter Crisman |
Report Upstream: | None of the above - read trac for reasoning. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12855 | Stopgaps: |
Description (last modified by )
TonelliShanks.h references int32_t
but does not directly include <stdint.h> (where it is defined). On FreeBSD using gcc45 (but not the base gcc), this causes compilation to fail.
spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/flintqs-20070817.p8.spkg
Attachments (1)
Change History (18)
comment:1 Changed 12 years ago by
- Description modified (diff)
- Milestone set to sage-4.5.1
comment:2 Changed 12 years ago by
- Description modified (diff)
comment:3 Changed 11 years ago by
comment:4 Changed 10 years ago by
Stephen Montgomery-Smith reports that this patch is absolutely necessary for building under FreeBSD.
comment:5 Changed 10 years ago by
Just for the record: There'll be a new FLINTQS spkg at #12855 soon...
comment:6 Changed 10 years ago by
- Reviewers set to Stephen Montgomery-Smith
The current spkg-install change being used in the port is
-
flintqs-20070817.p6/
old new 7 7 fi 8 8 9 9 cp patches/lanczos.h src/ 10 patch -p0 < patches/TonelliShanks.h.patch 10 11 11 12 cd src
but we'll have to check the exit status on that. Or just add this into #12855 or something.
comment:7 Changed 10 years ago by
- Dependencies set to #12855
- Description modified (diff)
- Milestone changed from sage-5.1 to sage-5.2
- Status changed from new to needs_review
comment:8 Changed 10 years ago by
Rebased to #12855.
comment:9 follow-up: ↓ 10 Changed 10 years ago by
- Report Upstream changed from Not yet reported upstream; Will do shortly. to None of the above - read trac for reasoning.
This looks good, but since it's applied universally and not just on FreeBSD I am hesitant to give it positive review. Jeroen, did you try this out on some of the buildbots?
Also, is there even an "upstream"? This is a five-year-old program, and some searching indicates that maybe a newer version is included in flint itself? Anyway, I figure that the age makes reporting upstream inappropriate.
comment:10 in reply to: ↑ 9 ; follow-up: ↓ 11 Changed 10 years ago by
Replying to kcrisman:
Jeroen, did you try this out on some of the buildbots?
No, but I don't expect problems. What harm could an additional header file do?
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 10 years ago by
- Reviewers changed from Stephen Montgomery-Smith to Stephen Montgomery-Smith, Jeroen Demeyer, Karl-Dieter Crisman
Jeroen, did you try this out on some of the buildbots?
No, but I don't expect problems. What harm could an additional header file do?
I don't know, that's why I asked! Extra imports in Python sometimes cause all kinds of circularity issues or slowdowns, and I don't know enough about C to say either way.
But since you just repackaged this, I think it's appropriate for you to be a reviewer as well. And I checked that the package is well-formed and installs on OS X.
I also notice that you included stdint after stdlib, while Stephen does the opposite in his patch here. I assume this also doesn't matter? Sorry for the very stupid questions.
See also #11792?
comment:12 in reply to: ↑ 11 Changed 10 years ago by
Replying to kcrisman:
Jeroen, did you try this out on some of the buildbots?
No, but I don't expect problems. What harm could an additional header file do?
I don't know, that's why I asked! Extra imports in Python sometimes cause all kinds of circularity issues or slowdowns, and I don't know enough about C to say either way.
In theory this could cause problems, but presumably the C headers are well-designed such that these problems don't occur.
I also notice that you included stdint after stdlib, while Stephen does the opposite in his patch
It also shouldn't matter. The way I did it looked more natural, but there's really no reason.
comment:13 follow-up: ↓ 14 Changed 10 years ago by
It also shouldn't matter. The way I did it looked more natural, but there's really no reason.
Ok.
I'm even having trouble finding where this is used. sage -grep
with various upper/lowercase of flintqs does nothing, it's not in module_list.py
, in spkg/standard/deps
it only has dependencies... and I'm going to need CPU firepower in a few minutes, can't do a full doctest right now. Weird.
comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 10 years ago by
Replying to kcrisman:
I'm even having trouble finding where this is used.
sage -grep
with various upper/lowercase of flintqs does nothing, it's not inmodule_list.py
, inspkg/standard/deps
it only has dependencies... and I'm going to need CPU firepower in a few minutes, can't do a full doctest right now. Weird.
The interface is sage/interfaces/qsieve.py
.
comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 10 years ago by
- Milestone changed from sage-5.2 to sage-5.1
- Status changed from needs_review to positive_review
I'm even having trouble finding where this is used.
sage -grep
with various upper/lowercase of flintqs does nothing, it's not inmodule_list.py
, inspkg/standard/deps
it only has dependencies... and I'm going to need CPU firepower in a few minutes, can't do a full doctest right now. Weird.The interface is
sage/interfaces/qsieve.py
.
Great, thanks. That passes long tests, so since there really isn't any reason anything should be bad, we are good to go. (Yes, I did sage -b
.)
Since p7 is also 5.1, tentatively putting it there, but feel free to do whatever is convenient.
comment:16 in reply to: ↑ 15 Changed 10 years ago by
Replying to kcrisman:
Since p7 is also 5.1, tentatively putting it there, but feel free to do whatever is convenient.
.p7 still needs review...
comment:17 Changed 10 years ago by
- Merged in set to sage-5.1.rc0
- Resolution set to fixed
- Status changed from positive_review to closed
Apparently at least once Stephen Montgomery-Smith compiled Sage on a "port" successfully on FreeBSD without this.