Opened 8 years ago

Closed 8 years ago

#13287 closed defect (fixed)

Override more compiler-related environment variables if Sage's GCC is used

Reported by: leif Owned by: leif
Priority: minor Milestone: sage-5.3
Component: scripts Keywords: GCC spkg CPP FC F77 F95 Fortran compiler preprocessor sage-env
Cc: jdemeyer, drkirkby Merged in: sage-5.3.beta2
Authors: Leif Leonhardy Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

Currently only CC and CXX get modified (in spkg/bin/sage-env) if Sage's GCC spkg is installed (more precisely, if $SAGE_LOCAL/bin/{gcc,g++} are present).

We should also set / override CPP and (at least) also various commonly-used Fortran compiler variables to make sure the versions from Sage's GCC installation get used, since the user may have specified these, and the build might break (due to mixing different compiler versions). (As an example, building ECL currently fails if $CPP finds ffi.h although it is not really installed system-wide, but rather only in e.g. /usr/lib/<ARCH>/<$CPP's version>/include/; see ticket:13150:5 ff.)


Apply

to the Sage root repository.

Attachments (1)

trac_13287-set_CPP_FC_etal_if_GCC_spkg_is_installed.patch (1.3 KB) - added by leif 8 years ago.
Patch to sage-env to also override CPP, FC, F77 and F95 settings when appropriate. Based on Sage 5.2.rc0. Apply to the Sage root repository.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by leif

Not sure whether we have to (or should) handle OBJC, OBJCXX etc. as well.

Although compilers for further languages may get built and installed by the GCC spkg, these aren't used by Sage or its components, at least -- AFAIK -- not currently (with the exception of R using an Objective C compiler, but on MacOS X only).

comment:2 Changed 8 years ago by leif

  • Cc drkirkby added

Dave, thoughts w.r.t. Fortran compiler variables (FC, F77, F95, what else?) and maybe ones for other languages (Java etc.)?

(IIRC, like me you're keen of getting rid of the odd sage_fortran script, and probably SAGE_FORTRAN, which is slightly related.)

comment:3 Changed 8 years ago by leif

(Tentative inline patch to sage-env is currently here.)

Changed 8 years ago by leif

Patch to sage-env to also override CPP, FC, F77 and F95 settings when appropriate. Based on Sage 5.2.rc0. Apply to the Sage root repository.

comment:4 Changed 8 years ago by leif

  • Authors set to Leif Leonhardy
  • Description modified (diff)
  • Status changed from new to needs_review

Initial patch now attached here.

comment:5 Changed 8 years ago by leif

P.S.: Is this at all documented in the Sage Installation Guide? (I.e., that setting CC etc. later has no effect if Sage's GCC is installed, or likewise e.g. some wrapper scripts in $SAGE_LOCAL/bin/.)

comment:6 Changed 8 years ago by jdemeyer

The patch looks good on first sight, but I haven't tested it properly.

comment:7 follow-up: Changed 8 years ago by jdemeyer

exporting CPP when it's not necessarily defined doesn't look like a good idea.

comment:8 in reply to: ↑ 7 Changed 8 years ago by leif

Replying to jdemeyer:

exporting CPP when it's not necessarily defined doesn't look like a good idea.

Hmmm. If it's not already defined, exporting it has no effect. It may be (defined and) empty, but in that case it originates from the "global" environment, i.e., has been set to "" (and already exported) by the user, which is certainly wrong.

Any reason it doesn't get a default value if it's not already set in sage-env (like CC, but also AR, RANLIB etc.)?

I could either add the latter, or maybe only export CPP if we set it there (if [ -x ... ]).

The whole thing looks a bit inconsistent, but spkgs may rely on the other variables being defined, so I'd prefer to also let CPP always have a value.

comment:9 Changed 8 years ago by leif

P.S.: I can't imagine a situation where some script sources sage-env, sets CPP, but does not want it to get exported.

comment:10 follow-up: Changed 8 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

I don't fully agree with all of the patch, but at least it's an improvement.

comment:11 in reply to: ↑ 10 Changed 8 years ago by leif

Replying to jdemeyer:

I don't fully agree with all of the patch, but at least it's an improvement.

Well, I'm open to further changes (although I don't mind merging it as is either)...

comment:12 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.3.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.