Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nexttime mannequin opened this issue Mar 17, 2012 · 11 comments
Closed

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

nexttime mannequin opened this issue Mar 17, 2012 · 11 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Mar 17, 2012

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)

  • Fix hardcoded 'g++' in Lcalc 1.23 [p9] #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'...)

CC: @rishikesha @jdemeyer @ohanar @orlitzky

Component: packages: standard

Keywords: rd2 spkg CC CXX C++ compiler hard-coded

Author: Leif Leonhardy

Reviewer: R. Andrew Ohana

Merged: sage-5.0.beta9

Issue created by migration from https://trac.sagemath.org/ticket/12681

@nexttime nexttime mannequin added this to the sage-5.0 milestone Mar 17, 2012
@nexttime nexttime mannequin self-assigned this Mar 17, 2012
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 17, 2012

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

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 17, 2012

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

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

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 17, 2012

Author: Leif Leonhardy

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 17, 2012

comment:1

Attachment: Lcalc-1.23-Makefile.p9-p10.diff.gz

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).

@nexttime nexttime mannequin added the s: needs review label Mar 17, 2012
@orlitzky
Copy link
Contributor

comment:2

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.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Mar 17, 2012

comment:3

Replying to @orlitzky:

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.

@ohanar
Copy link
Member

ohanar commented Mar 18, 2012

Reviewer: R. Andrew Ohana

@ohanar
Copy link
Member

ohanar commented Mar 18, 2012

comment:4

works well and looks good!

@ohanar
Copy link
Member

ohanar commented Mar 18, 2012

Changed keywords from spkg CC CXX C++ compiler hard-coded to rd2 spkg CC CXX C++ compiler hard-coded

@jdemeyer
Copy link

Merged: sage-5.0.beta9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants