#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 )
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:
- to SAGE_ROOT: 13349_deprecate_sage_fortran.patch
- to the Sage library: 13349_doc.patch
Attachments (2)
Change History (27)
comment:1 Changed 8 years ago by
- Description modified (diff)
- Summary changed from Deprecate SAGE_FORTRAN to Deprecate SAGE_FORTRAN, support FC
comment:2 Changed 8 years ago by
- Component changed from PLEASE CHANGE to build
- Description modified (diff)
- Status changed from new to needs_review
comment:3 Changed 8 years ago by
- Cc leif jhpalmieri added
comment:4 follow-ups: ↓ 5 ↓ 9 Changed 8 years ago by
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 11 Changed 8 years ago by
Replying to leif:
I'd also keep the
SAGE_FORTRAN
andSAGE_FORTRAN_LIB
entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference toFC
andFCFLAGS
).
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: ↓ 7 ↓ 10 Changed 8 years ago by
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
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 togfortran
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
I'd also set (or unset) F77
likewise.
comment:9 in reply to: ↑ 4 ; follow-up: ↓ 12 Changed 8 years ago by
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
andSAGE_FORTRAN_LIB
entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference toFC
andFCFLAGS
).
If they shouldn't be used anymore, what's the point?
Not directly related to this ticket, but
CPP
andCPPFLAGS
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: ↓ 13 Changed 8 years ago by
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 togfortran
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
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: ↓ 15 Changed 8 years ago by
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
andSAGE_FORTRAN_LIB
entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference toFC
andFCFLAGS
).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
andCPPFLAGS
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: ↓ 14 Changed 8 years ago by
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 togfortran
as well, since$SAGE_LOCAL/bin/
comes first in the path anyway.If
FC
is unset, it will default to "gfortran" anyway, see how thesage_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
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
Replying to leif:
I'd also keep the
SAGE_FORTRAN
andSAGE_FORTRAN_LIB
entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference toFC
andFCFLAGS
).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
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
- Description modified (diff)
Changed 8 years ago by
comment:18 follow-up: ↓ 19 Changed 8 years ago by
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
comment:19 in reply to: ↑ 18 Changed 8 years ago by
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: ↓ 21 Changed 8 years ago by
- 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
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: ↓ 24 Changed 8 years ago by
There are references to SAGE_FORTRAN
in the following packages:
- atlas (only in a "print" statement)
- cliquer (which doesn't have Fortran code)
- libgcrypt (which doesn't have Fortran code)
- lrcalc (which doesn't have Fortran code)
- rubiks (which doesn't have Fortran code)
- 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
- 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
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
I guess sqlite is not a problem.
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
(andCXX
) [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
andSAGE_FORTRAN_LIB
entries for now, just stating that these are deprecated (or, more precisely, obsolete), and shouldn't be used anymore (with a reference toFC
andFCFLAGS
).Not directly related to this ticket, but
CPP
andCPPFLAGS
should IMHO be mentioned as well.