Opened 9 years ago

Closed 2 weeks ago

#12890 closed enhancement (invalid)

Setup general CFLAGS

Reported by: jdemeyer Owned by: GeorgSWeber
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: kini Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The current way how Sage handles CFLAGS is quite bad. There are several issues:

  1. Too many spkg-install files are trying to do clever stuff to determine CFLAGS. This should be done just once instead of in every spkg. This would imply we don't need to handle SAGE64 and SAGE_DEBUG in every spkg.
  1. Sometimes, the spkg determines CFLAGS which result in a failed build, for example when the gcc issues instructions which the assembler doesn't know about.
  1. Some packages don't do anything clever and just build with simple, suboptimal CFLAGS.

My impression is that MPIR is very good at determining the right CFLAGS. So we should not try to reinvent the wheel, just take MPIR's CFLAGS.

Change History (10)

comment:1 Changed 9 years ago by leif

There are a lot of aspects to this, i.e., one could write a whole essay on it, that's why I haven't [yet] replied on #11616...


The introduction of SAGE64 (including its name) was IMHO a poor idea, or at least badly implemented.

Michael Abshoff had some ideas regarding "standard" variables like CC, CFLAGS etc.; see his old wiki page.

The Sage library (i.e., devel/sage/setup.py) should by itself make sure -fno-strict-aliasing is used to compile Cython-generated code when needed and supported.

Sage's configure doesn't configure anything, nor does it take a --target option. (It could for example also set things like EGREP.)

There's in principle no way to determine in an spkg-install whether the settings of CC, LD, CXXFLAGS etc. originate from Sage (currently mostly sage-env) or the user.

Spkgs are intended to make Sage modular (i.e., they're not supposed to be tied to a specific Sage version); they can [currently] rely on a few basic environment variable settings, but not more, and there's currently no framework for an spkg to require a minimal or specific version of Sage or other spkgs.

The assembler errors ("unknown instruction") are almost exclusively caused by the GCC spkg, since any reasonable operating system or distribution ships a toolchain where parts "match". (A user could of course also install a newer compiler without updating binutils etc., but then it's mainly his fault, and we certainly cannot catch and work around all possible issues.) Note that one usually also needs current versions of gdb and valgrind to debug code which exploits newer CPU features.

MPIR implements much more in assembly than other spkgs do, hence it's less likely that GCC is tempted to emit fancy instructions for MPIR's (high-level) C code. (And MPIR uses its own assembler for its "hand-written" assembly files. We might install yasm into Sage if we also build GCC.)

Still a couple of spkgs (i.e., mostly upstream) ignore CC, CXX, CFLAGS etc., or use some non-standard variables for similar or the same purposes, or even abuse CPP and/or CPPFLAGS.

...

comment:2 Changed 9 years ago by leif

P.S.: The situation with Fortran is even worse. We have a useless sage_fortran script and SAGE_FORTRAN* variables, instead of using FC (or F77), FFLAGS, FCFLAGS and LDFLAGS etc.

comment:3 Changed 9 years ago by kini

  • Cc kini added

comment:4 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:5 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:6 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:7 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:8 Changed 4 weeks ago by mkoeppe

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

outdated, should close

comment:9 Changed 3 weeks ago by chapoton

  • Status changed from needs_review to positive_review

indeed

comment:10 Changed 2 weeks ago by mkoeppe

  • Resolution set to invalid
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.