Opened 11 years ago

Closed 11 years ago

#12681 closed defect (fixed)

Fix hardcoded 'g++' in Lcalc 1.23 [p9]

Reported by: Leif Leonhardy Owned by: Leif Leonhardy
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords: rd2 spkg CC CXX C++ compiler hard-coded
Cc: Rishikesh, Jeroen Demeyer, R. Andrew Ohana, Michael Orlitzky Merged in: sage-5.0.beta9
Authors: Leif Leonhardy Reviewers: R. Andrew Ohana
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Leif Leonhardy)

The horrible Makefile still uses CC for compiling C++ (as well as C), and hardcodes it to g++.

The updated spkg (p10) fixes this by using $(CXX) for compiling C++, and allows both CC and CXX in the Makefile to get overridden by their respective environment settings.


New spkg: http://boxen.math.washington.edu/home/leif/Sage/spkgs/lcalc-1.23.p10.spkg

md5sum: a37f527cbfeb24eef307574a5665c7a8 lcalc-1.23.p10.spkg

lcalc-1.23.p10 (Leif Leonhardy, March 17th 2012)

  • #12681: Fix hard-coded 'g++'. The (patched) Makefile now uses $(CXX) (which *defaults* to 'g++') for compiling and linking C++, $(CC) (which *defaults* to 'gcc') for compiling C, although the latter is [currently] hardly used. See also "Special Update/Build? Instructions" above. (We could now also set INSTALL_DIR and use 'make install'...)

Attachments (2)

lcalc-1.23.p9-p10.diff (7.8 KB) - added by Leif Leonhardy 11 years ago.
Diff between the p9 and my p10. For reference / review only.
Lcalc-1.23-Makefile.p9-p10.diff (3.9 KB) - added by Leif Leonhardy 11 years ago.
Diff between the (patched) Makefile of the p9 and the p10. For reference / review only.

Download all attachments as: .zip

Change History (8)

Changed 11 years ago by Leif Leonhardy

Attachment: lcalc-1.23.p9-p10.diff added

Diff between the p9 and my p10. For reference / review only.

Changed 11 years ago by Leif Leonhardy

Diff between the (patched) Makefile of the p9 and the p10. For reference / review only.

comment:1 Changed 11 years ago by Leif Leonhardy

Authors: Leif Leonhardy
Cc: Rishikesh Jeroen Demeyer R. Andrew Ohana added
Description: modified (diff)
Status: newneeds_review

I've also attached a diff between the resulting Makefiles (p9 vs. p10), for review only of course.

I apologize in case there are already pending changes to the Lcalc spkg; haven't searched for such (also since the p9 is quite new).

comment:2 Changed 11 years ago by Michael Orlitzky

Cc: Michael Orlitzky added

Same comment as on ratpoints: I think we should avoid adding patches unless absolutely necessary. The conditional assignment of CC is nice, but if we're going to do it, we should try to get it added upstream and override it the standard way in the meantime.

(Using $CXX instead of g++ is obviously correct; although we should report that upstream too.)

comment:3 in reply to:  2 Changed 11 years ago by Leif Leonhardy

Replying to mjo:

Same comment as on ratpoints: I think we should avoid adding patches unless absolutely necessary.

Lcalc is a special case in many ways (cf. my reply on #12682), and we have to patch the Makefile anyway (since there's still no configure, and therefore the Makefile is supposed to get edited).

Michael Rubinstein would certainly be happy if we sent him some generic solution, but so far nobody found the time for such, as mentioned.


(Using $CXX instead of g++ is obviously correct; although we should report that upstream too.)

We already did so, IIRC... Btw., haven't yet looked at the 1.3 (beta) version, which I think is still work in progress. We can certainly contribute to that, although e.g. Jeroen rejected to make patches more upstream-friendly, i.e. generic (w.r.t. supporting different PARI versions). Also took me some time to convince people to use -lgmp instead of -lmpir.

comment:4 Changed 11 years ago by R. Andrew Ohana

Reviewers: R. Andrew Ohana
Status: needs_reviewpositive_review

works well and looks good!

comment:5 Changed 11 years ago by R. Andrew Ohana

Keywords: rd2 added

comment:6 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.0.beta9
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.