#11605 closed defect (fixed)
Typos in PARI's spkg-install (2.4.3.alpha.p5)
Reported by: | leif | Owned by: | tbd |
---|---|---|---|
Priority: | blocker | Milestone: | sage-4.7.1 |
Component: | packages: standard | Keywords: | PARI spkg Cygwin |
Cc: | jdemeyer, dimpase, kcrisman, mhansen | Merged in: | sage-4.7.1.rc1 |
Authors: | Dmitrii Pasechnik, Leif Leonhardy | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Someone should have noticed this:
./spkg-install: line 185: [: missing `]' Installing PARI/GP...
Here's the culprit:
build() { ... $MAKE $PARI_MAKEFLAGS gp if [ $? -ne 0]; then echo >&2 "Error: building PARI/GP failed." fi }
Note that also an exit 1
(!) is missing there.
(Two patches apply with fuzz 2 btw.)
New spkg: http://spkg-upload.googlecode.com/files/pari-2.4.3.alpha.p7.spkg
md5sum: fd153c3ee354402bb6fc835b9e8ecd9a pari-2.4.3.alpha.p7.spkg
(This spkg is based on / includes the never merged p6 from #10240, see comments below and there.)
Changelog
pari-2.4.3.alpha.p7 (Leif Leonhardy, July 16th, 2011)
- #11605: Fix typo in spkg-install and add
exit 1
(again) in case the build fails. - Quote some more variables and filenames in messages.
pari-2.4.3.alpha.p6 (Dima Pasechnik, 22 April, 2011)
- made a proper check for libpari.dll on Cygwin, as described in trac #10240
Attachments (3)
Change History (13)
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 8 years ago by
- Cc dimpase kcrisman mhansen added
Changed 8 years ago by
SPKG patch. For review only. (Based on a p6 by me with Dima's patch applied.)
Changed 8 years ago by
SPKG patch. Contains both the changes from here and #10240. Apply to pari-2.4.3.alpha.p5 if at all.
comment:3 in reply to: ↑ 2 Changed 8 years ago by
comment:4 Changed 8 years ago by
- Description modified (diff)
- Keywords PARI spkg Cygwin added
comment:5 Changed 8 years ago by
New spkg is up. Please review such that we can get this into the upcoming Sage 4.7.1.
comment:6 follow-up: ↓ 7 Changed 8 years ago by
I cannot actually test this package on Cygwin for at least two weeks (in fact, I can't on any computer right now), but the change in question for Cygwin looks right still (see #10240, as mentioned), and #10240 does in fact work (I've used it numerous times, and !RegB on the sage-windows list also was able to use it properly). That's as close as I can get to positive review for now, I'm sorry - is that enough? I would say that #10240 would have positive review regardless of whether it is "duplicate" or not.
As for the rest, it looks right, but I have not actually tested any of it, including the quoting etc. I'm sure that someone even a little more advanced in shell script could give it an immediate positive review (or needs work if there is something obvious missed).
Thanks so much for merging the #10240 stuff, by the way!
comment:7 in reply to: ↑ 6 Changed 8 years ago by
comment:8 Changed 8 years ago by
- Priority changed from critical to blocker
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to positive_review
Good! Never mind the fuzz 2
, it is harmless.
comment:9 Changed 8 years ago by
- Merged in set to sage-4.7.1.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
Patch is up. New spkg is on the way...