Ticket #12799 (closed enhancement: fixed)
Fix PolyBoRi's dependencies in `module_list.py`
| Reported by: | AlexanderDreyer | Owned by: | AlexanderDreyer |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.0 |
| Component: | build | Keywords: | |
| Cc: | leif | Work issues: | |
| Report Upstream: | None of the above - read trac for reasoning. | Reviewers: | Leif Leonhardy |
| Authors: | Alexander Dreyer | Merged in: | sage-5.0.beta13 |
| Dependencies: | Stopgaps: |
Description (last modified by AlexanderDreyer) (diff)
Actually only the pbori module needs to get rebuilt; while touching $SAGE_LOCAL/include/polybori.h doesn't help, touching $SAGE_LOCAL/include/polybori/* does (afterwards running ./sage -b).
The better alternative is to fix the dependency list in module_list.py
Current patch
- Apply polybori_depends.3.patch
Attachments
-
polybori_depends.patch
(1.1 KB) -
added by AlexanderDreyer 14 months ago.
- Patch for the sage library (devel/sage)
-
polybori_depends.2.patch
(930 bytes) -
added by AlexanderDreyer 14 months ago.
- Patch for the sage library (devel/sage)
-
polybori_depends.3.patch
(930 bytes) -
added by AlexanderDreyer 14 months ago.
- patch rebased to sage-5.0 beta11
Change History
Changed 14 months ago by AlexanderDreyer
-
attachment
polybori_depends.patch
added
comment:1 Changed 14 months ago by AlexanderDreyer
- Status changed from new to needs_review
- Description modified (diff)
comment:2 follow-up: ↓ 3 Changed 14 months ago by leif
- Reviewers set to Leif Leonhardy
- Description modified (diff)
- Authors changed from AlexanderDreyer to Alexander Dreyer
As mentioned on #12655, I'd rather make the extension module depend on just $SAGE_INC/polybori.h and/or its libraries, $SAGE_LOCAL/lib/{polybori,polybori_groebner}.so, and touch the former from spkg-install at least in case you don't add the libraries.
AFAIK Cython is smart enough to notice when (at least directly) included headers are newer, but this doesn't help if their timestamps are preserved during PolyBoRi's installation.
Adding all of PolyBoRi's headers explicitly seems a bit overkill to me.
[Also note that we have both $SAGE_INC/polybori.h and $SAGE_INC/polybori/polybori.h, which are not the same files.]
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 14 months ago by AlexanderDreyer
Replying to leif:
As mentioned on #12655, I'd rather make the extension module depend on just $SAGE_INC/polybori.h and/or its libraries, $SAGE_LOCAL/lib/{polybori,polybori_groebner}.so, and touch the former from spkg-install at least in case you don't add the libraries.
The timestamp of include/polybori/config.h also varies, this would also help here.
AFAIK Cython is smart enough to notice when (at least directly) included headers are newer, but this doesn't help if their timestamps are preserved during PolyBoRi's installation.
Adding all of PolyBoRi's headers explicitly seems a bit overkill to me.
[Also note that we have both $SAGE_INC/polybori.h and $SAGE_INC/polybori/polybori.h, which are not the same files.]
The first header is just for convenience.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 14 months ago by leif
Replying to AlexanderDreyer:
The timestamp of include/polybori/config.h also varies, this would also help here.
Then I'd add just that... (cross-posting)
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 7 Changed 14 months ago by AlexanderDreyer
Replying to leif:
Replying to AlexanderDreyer:
The timestamp of include/polybori/config.h also varies, this would also help here.
Then I'd add just that... (cross-posting)
Ok, let's try this one: polybori_depends.2.patch
Changed 14 months ago by AlexanderDreyer
-
attachment
polybori_depends.2.patch
added
Patch for the sage library (devel/sage)
comment:8 follow-ups: ↓ 9 ↓ 10 Changed 14 months ago by leif
- Status changed from needs_review to positive_review
- Component changed from algebra to build
Applies to Sage 5.0.beta11 with three lines offset, but I don't mind.
Changed 14 months ago by AlexanderDreyer
-
attachment
polybori_depends.3.patch
added
patch rebased to sage-5.0 beta11
comment:9 in reply to: ↑ 8 Changed 14 months ago by AlexanderDreyer
- Description modified (diff)
Replying to leif:
Applies to Sage 5.0.beta11 with three lines offset, but I don't mind.
Thanks! I rebased the patch accordingly.
comment:10 in reply to: ↑ 8 ; follow-up: ↓ 11 Changed 14 months ago by jdemeyer
Replying to leif:
Applies to Sage 5.0.beta11 with three lines offset, but I don't mind.
I consider an offset in a patch to be totally harmless; fuzz 1 should be rebased but I can usually live with; fuzz 2 must be rebased.
comment:11 in reply to: ↑ 10 Changed 14 months ago by leif
comment:12 Changed 14 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-5.0.beta13
Patch for the sage library (devel/sage)