Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13349 closed enhancement (fixed)

Deprecate SAGE_FORTRAN, support FC

Reported by: jdemeyer Owned by: tbd
Priority: critical Milestone: sage-5.3
Component: build Keywords:
Cc: leif, jhpalmieri Merged in: sage-5.3.rc0
Authors: Jeroen Demeyer Reviewers: Leif Leonhardy, John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Thanks to the GCC spkg, most uses of the SAGE_FORTRAN and SAGE_FORTRAN_LIB environment variables are not needed anymore. So I propose to continue supporting them for now, but deprecate their usage.

Instead, we should support the standard variable FC as alternative to SAGE_FORTRAN. And I think (LD_)LIBRARY_PATH is a good alternative for SAGE_FORTRAN_LIB.

Apply:

Attachments (2)

13349_deprecate_sage_fortran.patch (3.8 KB) - added by jdemeyer 8 years ago.
13349_doc.patch (6.2 KB) - added by jdemeyer 8 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Deprecate SAGE_FORTRAN to Deprecate SAGE_FORTRAN, support FC

comment:2 Changed 8 years ago by jdemeyer

  • Component changed from PLEASE CHANGE to build
  • Description modified (diff)
  • Status changed from new to needs_review

comment:3 Changed 8 years ago by jdemeyer

  • Cc leif jhpalmieri added

comment:4 follow-ups: Changed 8 years ago by leif

I'd replace "not every Sage package recognizes this" by "not every Sage package recognizes this yet".

Not sure whether that's still true; at least all standard spkgs should respect CC (and CXX) [meanwhile or soon] -- Andrew Ohana and me opened tickets for all spkgs (we were aware of) which didn't a while ago.

I'd also keep the SAGE_FORTRAN and SAGE_FORTRAN_LIB entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference to FC and FCFLAGS).


Not directly related to this ticket, but CPP and CPPFLAGS should IMHO be mentioned as well.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by leif

Replying to leif:

I'd also keep the SAGE_FORTRAN and SAGE_FORTRAN_LIB entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference to FC and FCFLAGS).

P.S.: In addition. Although we may sleep for 5 seconds, I frequently hear "Who reads logs?". And there might even be a few people who actually read the Installation Guide before attempting to build Sage... ;-)

comment:6 follow-ups: Changed 8 years ago by leif

It's not clear to me why you unset FC in case Sage's GCC is (or will be) installed; in that case I'd just set it to gfortran as well, since $SAGE_LOCAL/bin/ comes first in the path anyway.

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

Replying to leif:

It's not clear to me why you unset FC in case Sage's GCC is (or will be) installed; in that case I'd just set it to gfortran as well, since $SAGE_LOCAL/bin/ comes first in the path anyway.

I mean if you don't set FC there, some packages might still pick up some other Fortran compiler.

comment:8 Changed 8 years ago by leif

I'd also set (or unset) F77 likewise.

comment:9 in reply to: ↑ 4 ; follow-up: Changed 8 years ago by jdemeyer

Replying to leif:

I'd replace "not every Sage package recognizes this" by "not every Sage package recognizes this yet".

I don't like that, because it sounds too much like a promise. If in 2 years, still not all packages support $CC, it will look ridiculous. The current sentence is simply an informative statement of fact.

I'd also keep the SAGE_FORTRAN and SAGE_FORTRAN_LIB entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference to FC and FCFLAGS).

If they shouldn't be used anymore, what's the point?

Not directly related to this ticket, but CPP and CPPFLAGS should IMHO be mentioned as well.

I think we should only mention the most important ones, namely CFLAGS, CXXFLAGS and FCFLAGS. Otherwise, there is an essentially endless list of environment variables...

comment:10 in reply to: ↑ 6 ; follow-up: Changed 8 years ago by jdemeyer

Replying to leif:

It's not clear to me why you unset FC in case Sage's GCC is (or will be) installed; in that case I'd just set it to gfortran as well, since $SAGE_LOCAL/bin/ comes first in the path anyway.

If FC is unset, it will default to "gfortran" anyway, see how the sage_fortran script is created.

comment:11 in reply to: ↑ 5 Changed 8 years ago by jdemeyer

Replying to leif:

P.S.: In addition. Although we may sleep for 5 seconds, I frequently hear "Who reads logs?".

At least, it will appear at the very start of the build. Just type "make" and that message will appear immediately, so most users should see it (unless they are building using automated scripts of course...)

comment:12 in reply to: ↑ 9 ; follow-up: Changed 8 years ago by leif

Replying to jdemeyer:

Replying to leif:

I'd replace "not every Sage package recognizes this" by "not every Sage package recognizes this yet".

I don't like that, because it sounds too much like a promise. If in 2 years, still not all packages support $CC, it will look ridiculous. The current sentence is simply an informative statement of fact.

Well, at least all standard spkgs should -- as mentioned -- meanwhile or soon respect CC etc. (So it's IMHO at least a valid promise. ;-) )


I'd also keep the SAGE_FORTRAN and SAGE_FORTRAN_LIB entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference to FC and FCFLAGS).

If they shouldn't be used anymore, what's the point?

Documentation. In a couple of years, we can certainly remove it.


Not directly related to this ticket, but CPP and CPPFLAGS should IMHO be mentioned as well.

I think we should only mention the most important ones, namely CFLAGS, CXXFLAGS and FCFLAGS. Otherwise, there is an essentially endless list of environment variables...

Well, CPP and CPPFLAGS are closely related, and one or two more entries don't make the list endless. In addition, there seem to be quite many people confusing CPP* with CXX*.

comment:13 in reply to: ↑ 10 ; follow-up: Changed 8 years ago by leif

Replying to jdemeyer:

Replying to leif:

It's not clear to me why you unset FC in case Sage's GCC is (or will be) installed; in that case I'd just set it to gfortran as well, since $SAGE_LOCAL/bin/ comes first in the path anyway.

If FC is unset, it will default to "gfortran" anyway, see how the sage_fortran script is created.

At least in the long run, we should get rid of the sage_fortran script as well.

If a package sees FC being set, it'll (hopefully) use that. If it's unset, it'll try to guess the Fortran compiler, which might yield (Sage's) gfortran, but also some other Fortran compiler which happens to be installed on the system. (Explicitly setting FC etc. to sage_fortran in every spkg which uses Fortran is a redundant kludge.)

comment:14 in reply to: ↑ 13 Changed 8 years ago by jdemeyer

Replying to leif:

At least in the long run, we should get rid of the sage_fortran script as well.

Of course we should. I consider this ticket as a first step towards that.

comment:15 in reply to: ↑ 12 Changed 8 years ago by jdemeyer

Replying to leif:

I'd also keep the SAGE_FORTRAN and SAGE_FORTRAN_LIB entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference to FC and FCFLAGS).

If they shouldn't be used anymore, what's the point?

Documentation. In a couple of years, we can certainly remove it.

If it should not be used, why should it be documented? The message when doing make states clearly what should be used instead.

comment:16 Changed 8 years ago by jdemeyer

leif, what's the minimum which should be done for this patch to get positive review from you?

comment:17 Changed 8 years ago by jdemeyer

  • Description modified (diff)

Changed 8 years ago by jdemeyer

comment:18 follow-up: Changed 8 years ago by jhpalmieri

I think I agree with Leif that the old variables should still be documented. I can imagine a user finding out about them by searching through sage-devel, for example, and then wondering why they're not documented or whether they were removed. Maybe a sentence or two: "the following variables have been deprecated, as of Sage 5.3. Instead of SAGE_FORTRAN, use FC, and instead of SAGE_FORTRAN_LIB, ..." Users shouldn't have to find out this sort of thing by reading warning messages.

After a few releases, we could also make the deprecation more obvious, by making the user hit RET to continue building after we print the warning message.

Changed 8 years ago by jdemeyer

comment:19 in reply to: ↑ 18 Changed 8 years ago by jdemeyer

Replying to jhpalmieri:

I think I agree with Leif that the old variables should still be documented. I can imagine a user finding out about them by searching through sage-devel, for example, and then wondering why they're not documented or whether they were removed. Maybe a sentence or two: "the following variables have been deprecated, as of Sage 5.3. Instead of SAGE_FORTRAN, use FC, and instead of SAGE_FORTRAN_LIB, ..." Users shouldn't have to find out this sort of thing by reading warning messages.

Done, needs review.

comment:20 follow-up: Changed 8 years ago by jhpalmieri

  • Reviewers set to Leif Leonhardy, John Palmieri
  • Status changed from needs_review to positive_review

This all looks okay to me. I haven't thoroughly tested that SAGE_FORTRAN_LIB can usually be omitted (although I've tried that on one or two platforms and it's worked), and I don't know how to test that when it cannot be omitted, using LIBRARY_PATH or LD_LIBRARY_PATH is the right solution. But I'm willing to believe it.

Do we need followup tickets to take care of references to these variables in spkgs, or will that work itself out over the months/years?

  • atlas
  • cliquer
  • lcalc

comment:21 in reply to: ↑ 20 Changed 8 years ago by jdemeyer

Replying to jhpalmieri:

I haven't thoroughly tested that SAGE_FORTRAN_LIB can usually be omitted (although I've tried that on one or two platforms and it's worked), and I don't know how to test that when it cannot be omitted, using LIBRARY_PATH or LD_LIBRARY_PATH is the right solution.

Well, I never understood why SAGE_FORTRAN_LIB was ever needed, especially because we do not have a corresponding SAGE_CXX_LIB for the C++ runtime library. So I'm certainly not surprised that it's simply never needed (even less so with the GCC spkg).

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

There are references to SAGE_FORTRAN in the following packages:

  1. atlas (only in a "print" statement)
  2. cliquer (which doesn't have Fortran code)
  3. libgcrypt (which doesn't have Fortran code)
  4. lrcalc (which doesn't have Fortran code)
  5. rubiks (which doesn't have Fortran code)
  6. sqlite (which doesn't have Fortran code)

So, as far as I can see, no package actually directly uses the value of SAGE_FORTRAN.

comment:23 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.3.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 in reply to: ↑ 22 Changed 8 years ago by jhpalmieri

Replying to jdemeyer:

So, as far as I can see, no package actually directly uses the value of SAGE_FORTRAN.

I was mainly concerned about ATLAS, for example, telling people to set SAGE_FORTRAN when that variable has been deprecated. It also looks like sqlite might print "annoying messages" if SAGE_FORTRAN is unset.

comment:25 Changed 8 years ago by jhpalmieri

I guess sqlite is not a problem.

Note: See TracTickets for help on using tickets.