Ticket #12137 (closed defect: fixed)
Fix parallel building of Singular
| Reported by: | jdemeyer | Owned by: | tbd |
|---|---|---|---|
| Priority: | critical | Milestone: | sage-4.8 |
| Component: | packages: standard | Keywords: | |
| Cc: | jhpalmieri, malb | Work issues: | |
| Report Upstream: | Reported upstream. Developers acknowledge bug. | Reviewers: | Volker Braun |
| Authors: | Jeroen Demeyer | Merged in: | sage-4.8.alpha4 |
| Dependencies: | Stopgaps: |
Description (last modified by jdemeyer) (diff)
Attachments
Change History
comment:3 Changed 19 months ago by jdemeyer
- Summary changed from Fix parallel building Singular to Disable parallel building certain parts of Singular
Fixed one issue, hit another. It seems Singular's Makefiles are very broken. Looks like a "$MAKE -j1" is in order.
comment:4 Changed 19 months ago by jdemeyer
- Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
comment:6 follow-up: ↓ 8 Changed 19 months ago by malb
At some Sage/Singular? days there was a push to make Singular compile in parallel and it worked. Perhaps it got broken recently, but the Singular developers do fix these bugs. Hence, they should be reported upstream and fixed,not worked around by "make -j1".
comment:7 Changed 19 months ago by jdemeyer
- Summary changed from Disable parallel building certain parts of Singular to Fix parallel building of Singular
comment:8 in reply to: ↑ 6 Changed 19 months ago by jdemeyer
- Status changed from new to needs_review
Replying to malb:
At some Sage/Singular? days there was a push to make Singular compile in parallel and it worked. Perhaps it got broken recently, but the Singular developers do fix these bugs. Hence, they should be reported upstream and fixed,not worked around by "make -j1".
Fair enough. I found two problems and fixed them. I am now continuously building Singular in a loop, so far it seems to work. So needs_review.
comment:11 Changed 19 months ago by jdemeyer
- Description modified (diff)
- Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.
Changed 19 months ago by jdemeyer
-
attachment
singular-3-1-3-3.p1-p2.diff
added
diff for the new Singular spkg, for review only
comment:12 follow-up: ↓ 13 Changed 19 months ago by vbraun
Is there a particular reason why you are adding the + to some commands in the makefile?
- $(recurse) + +$(recurse)
I'm fine with it as it helps debugging, but it shouldn't change whether or not the Singular spkg builds.
comment:13 in reply to: ↑ 12 Changed 19 months ago by jdemeyer
Replying to vbraun:
Is there a particular reason why you are adding the + to some commands in the makefile?
To fix the warning
make[1]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule.
The problem is that make doesn't recognize it as a recursive rule because the command is the result of a variable expansion.
I'm fine with it as it helps debugging
Debugging, how?
but it shouldn't change whether or not the Singular spkg builds.
True. But it does fit with the theme of fixing parallel building.
comment:14 Changed 19 months ago by vbraun
- Status changed from needs_review to positive_review
- Reviewers set to Volker Braun
I guess the usual use for + rules is to execute the command even if you run make --just-print. I hadn't thought about the make job server, this is indeed modified as well.
comment:15 Changed 19 months ago by jhpalmieri
This also works for me. I could force the old version to fail when building in parallel (on sage.math: MAKEFLAGS='j -l8' ./sage -f ..., and similar on OS X). With the new spkg, it has built every time on sage.math (using both -l30 and -l2). On OS X, I had one failure with the new spkg using -l8, although I've had 4 successful builds that way. The machine only has 2 cores, so -l8 is probably a bad setting anyway. With -l2 on that machine, no problems.
comment:16 Changed 19 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-4.8.alpha4
comment:17 Changed 18 months ago by jdemeyer
- Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.
