#26715 closed defect (fixed)
build/pkgs/gfortran/spkg-configure.m4 works incorrectly if CC and CXX are already there
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-8.5 |
Component: | build: configure | Keywords: | spkg-configure |
Cc: | embray, jhpalmieri, vbraun, fbissey | Merged in: | |
Authors: | François Bissey | Reviewers: | Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | 3185e91 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
As a result of refactoring configure.ac in #24919, absence of (g)fortran now forces installation of full gcc, instead of just gfortran part of it.
This is particularly crucial on OSX, where we don't normally build full gcc any more, but only build gfortran.
To reproduce, move gfortran out of your PATH, and re-run ./configure. You'll see that now gcc
and gfortran
are scheduled for installation...
Attachments (1)
Change History (38)
comment:1 Changed 2 years ago by
- Description modified (diff)
comment:2 Changed 2 years ago by
comment:3 follow-up: ↓ 4 Changed 2 years ago by
This is all a bit tricky. What control logic is there when you don't have gfortran (so that the above check will say sage_spkg_install_gfortran=yes
) but gcc must be installed as well?
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 2 years ago by
Replying to fbissey:
This is all a bit tricky. What control logic is there when you don't have gfortran (so that the above check will say
sage_spkg_install_gfortran=yes
) but gcc must be installed as well?
There is build/pkgs/gcc/spkg-configure.m4
which should take care of this.
comment:5 in reply to: ↑ 4 Changed 2 years ago by
Replying to dimpase:
Replying to fbissey:
This is all a bit tricky. What control logic is there when you don't have gfortran (so that the above check will say
sage_spkg_install_gfortran=yes
) but gcc must be installed as well?There is
build/pkgs/gcc/spkg-configure.m4
which should take care of this.
It doesn't but the the section of code you point too in the diff can only be reached if sage_spkg_install_gcc
is no. Otherwise the variable I was worried about is set to no.
The only real issue to be careful here is that the gcc bit needs to be run before the gfortran bit. If we go by lexical order everything is fine in this particular case. But that may be something to remember if there is ever another package with an "optional split" or a where checking a dependency first is important.
comment:6 Changed 2 years ago by
It seems to be the case for "package Bar provides functionality Foo" kind of stuff that one can dive in :-P
Should this Sage component be called YAPS (Yet Another Packaging System)? And by virtue of name it must be implemented in yaml...
comment:7 follow-up: ↓ 9 Changed 2 years ago by
Amusing. But more seriously your diff certainly point to an issue that needs to be fixed and is logically what we are after. However the line probably should be replaced with a notice. It was probably the intent in the first place. When do you intend to submit a branch for review?
comment:8 Changed 2 years ago by
- Cc jhpalmieri added; jpalmieri removed
comment:9 in reply to: ↑ 7 Changed 2 years ago by
Replying to fbissey:
Amusing. But more seriously your diff certainly point to an issue that needs to be fixed and is logically what we are after. However the line probably should be replaced with a notice. It was probably the intent in the first place. When do you intend to submit a branch for review?
I think having some kind of notice was the intent, though written exactly as I did was probably a mistake on my part. Looking at the diff from #24919, this is the original code I removed from configure.ac
:
-# Check that the Fortran compiler accepts free-format source code
-# (as opposed to the older fixed-format style from Fortran 77).
-# This helps verify the compiler works too, so if some idiot
-# sets FC to /usr/bin/ls, we will at least know it's
-# not a working Fortran compiler.
-AC_LANG(Fortran)
-if test -z "$FC"; then
- need_to_install_gfortran=yes
-else
and this is the equivalent code that was moved to build/pkgs/gfortran/spkg-configure.m4
:
+ # Check that the Fortran compiler accepts free-format source code (as
+ # opposed to the older fixed-format style from Fortran 77).
+ # This helps verify the compiler works too, so if some idiot sets FC to
+ # /usr/bin/ls, we will at least know it's not a working Fortran
+ # compiler.
+ if test -z "$FC"; then
+ sage_spkg_install_gfortran=yes
+ SAGE_MUST_INSTALL_GCC([a Fortran compiler is missing])
+ fi
The use of the SAGE_MUST_INSTALL_GCC
macro here is a mistake. It might be a remnant from before we created a separate "gfortran" package (that actually happened after I first introduced #24919, and while I tried to incorporate that change it looks like this might have been left in).
As Dima suggested it can just be removed and/or replaced with an AC_MSG_NOTICE
.
comment:10 Changed 2 years ago by
Actually, I think there needs to be a bit more logic restructuring too. That fi
should be an else
. There's no point in checking AC_FC_FREEFORM
if $FC
is empty, as in the original code.
comment:11 follow-up: ↓ 14 Changed 2 years ago by
I'd say we're wedded to gfortran too much. As far as I know, flang is in a good state, and a good candidate to replace gfortran (well, yes, it's not that fast yet, but, OK...).
This ticket would be a good occasion to fix this, and check for presence of a meaningful Fortran compiler, not specifically gfortran.
comment:12 follow-up: ↓ 13 Changed 2 years ago by
This really isn't specifically about gfortran. It is, in fact, just checking that a fortran compiler exists, and if not is installing gfortran. Let's not overcomplicate things here: We've identified a simple fix. I'll attach a patch if no one else does.
comment:13 in reply to: ↑ 12 Changed 2 years ago by
Replying to embray:
This really isn't specifically about gfortran. It is, in fact, just checking that a fortran compiler exists, and if not is installing gfortran. Let's not overcomplicate things here: We've identified a simple fix. I'll attach a patch if no one else does.
Please do.
comment:14 in reply to: ↑ 11 Changed 2 years ago by
Replying to dimpase:
I'd say we're wedded to gfortran too much. As far as I know, flang is in a good state, and a good candidate to replace gfortran (well, yes, it's not that fast yet, but, OK...).
If flang is pretty good, then can you open another ticket to add it as an optional package? It would be nice to have a quicker way to build Sage on OS X for users who don't want to install their own gfortran.
comment:15 Changed 2 years ago by
flang does not run on OSX yet. It does run on Linux and FreeBSD, though.
comment:16 Changed 2 years ago by
flang
is not the only option by far. When I started the work on clang
I effectively widened the pool of C/C++ compiler usable - not limited to clang. In fact I compiled sage with the Intel C/C++ compiler (and gfortran) with minimal difficulties. But it's not widely advertised and certainly we cannot say it is supported. So widening fortran compiler is good and may be support a different one.
But building flang
, OS X or not, is still a hardship. I am afraid Intel fortran will do horrible and unhelpful optimisation but should be tried and then there is the community PGI compiler. Those are closed sources but somewhat easily available across some platforms (may be not freeBSD).
comment:17 Changed 2 years ago by
imho Sage should not be in business of building compilers.
comment:18 Changed 2 years ago by
- Branch set to u/fbissey/fortran
- Commit set to a666ff9ca7eaff96d8546b633391ab955b38e486
- Status changed from new to needs_review
OK does this branch satisfy everyone.
New commits:
a666ff9 | Do not build gcc if only a fortran compiler is missing. More care about check logic. Don't restrict to gfortran.
|
comment:19 Changed 2 years ago by
- Commit changed from a666ff9ca7eaff96d8546b633391ab955b38e486 to c35c6ec1cc26a843a4cfe79a807e5f324afe8dd4
Branch pushed to git repo; I updated commit sha1. New commits:
c35c6ec | Correct spacing
|
comment:20 Changed 2 years ago by
- Status changed from needs_review to needs_work
comment:21 Changed 2 years ago by
OK that was weird. After running bootstrap
even after nicely reworking the syntax in the best possible ways and even in M4sh, I still had an empty else...fi
section where I expected AC_FC_FREEFORM
to be expanded.
It seems like that macro doesn't really like to live in a conditional. In fact it looks like it goes next to AC_PROG_FC all by itself. AC_PROG_FC (and also CC and CXX) is currently called in configure.ac
, gcc/spkg-configure.m4
and of course gfortran/spkg-configure.m4
.
I also learned a few things about AC_REQUIRE
in the process.
comment:22 Changed 2 years ago by
I also did some testing with FC=/bin/ls
because it was helpfully suggested in comments
checking whether we are using the GNU Fortran compiler... no checking whether /bin/ls accepts -g... no checking for Fortran flag needed to accept free-form source... unknown configure: Your Fortran compiler does not accept free-format source code configure: which means the compiler is either seriously broken, or configure: is too old to build Sage. configure: === checking whether to install the gfortran SPKG ===
This is also nicely triggered if there is no FC
found so it is enough.
comment:23 Changed 2 years ago by
- Commit changed from c35c6ec1cc26a843a4cfe79a807e5f324afe8dd4 to 3185e91e9da7ce92bb90b69a4ef55dd98432b4dd
comment:24 Changed 2 years ago by
- Status changed from needs_work to needs_review
I left the issue of multiple calls to AC_PROG_*
for later overall. Just touched gcc for cosmetics really.
comment:25 Changed 2 years ago by
For the record I just built and successfully doctested sage with this ticket and the PGI fortran compiler. A couple of issues with doing that though. More about it in the morning before I move to Intel compilers.
comment:26 follow-up: ↓ 28 Changed 2 years ago by
Builds for me on OS X Mojave with my gfortran compiler temporarily removed. The Sage gfortran
package took more than twice as long to build as any other Sage package, which I think is typical, but also reinforces my hope that we can suggest other fortran compilers for use with Sage.
comment:27 Changed 2 years ago by
- Reviewers set to Erik Bray
- Status changed from needs_review to positive_review
The AC_FC_FREEFORM
macro always got expanded in a strange place for me too, and I could never figure out exactly why! I didn't realize it was already called automatically by AC_PROG_FC
, so thank you for figuring that out. This looks good to me.
comment:28 in reply to: ↑ 26 ; follow-up: ↓ 29 Changed 2 years ago by
Replying to jhpalmieri:
Builds for me on OS X Mojave with my gfortran compiler temporarily removed. The Sage
gfortran
package took more than twice as long to build as any other Sage package, which I think is typical, but also reinforces my hope that we can suggest other fortran compilers for use with Sage.
That's because this involves compiling most, if not all, of gcc and only installing the gfortran bits.
comment:29 in reply to: ↑ 28 Changed 2 years ago by
Replying to embray:
Replying to jhpalmieri:
Builds for me on OS X Mojave with my gfortran compiler temporarily removed. The Sage
gfortran
package took more than twice as long to build as any other Sage package, which I think is typical, but also reinforces my hope that we can suggest other fortran compilers for use with Sage.That's because this involves compiling most, if not all, of gcc and only installing the gfortran bits.
I understand that, I just wish we could build a fortran compiler more quickly. (That's why I install /usr/local/bin/gfortran
on every OS X machine I use for Sage.)
comment:30 follow-up: ↓ 31 Changed 2 years ago by
Totally happy if someone wants to look into that. Too bad flang is (according to François) still difficult to install on macOS, which would defeat some of the purpose :(
comment:31 in reply to: ↑ 30 Changed 2 years ago by
Replying to embray:
Totally happy if someone wants to look into that. Too bad flang is (according to François) still difficult to install on macOS, which would defeat some of the purpose :(
It's a pain on all OS full stop. I think you have to have a private patched build of llvm, you cannot reuse clang bits if you have them. I won't recommend PGI fortran (needs hand holding) but ifc may be fine, I can try other suggestions if I have access to them.
comment:32 Changed 2 years ago by
OK wrap up for people interested in using something like pgfortran, you can get it from https://www.pgroup.com/ . Before anyone ask, you cannot currently use pgcc/pgc++ to compile sage because they don't pretend to be gcc unlike clang and Intel. sage still wants a compiler that at least pretend to be gcc.
Set your environment
export PATH="/path_to_compiler_prefix/bin:$PATH" export LDFLAGS="-Wl,-rpath=/path_to_compiler_prefix/lib" export FPICFLAGS="-fPIC" ./configure FC=pgfortran
FPICFLAGS
is specific to R.
- I will send a couple of PR to openblas. openblas's
f_check
script does a pretty good job of finding all the linking flags needed for your compiler. But in the case of PGI, specifically, ruins the results completely - literally WTF? I will attach a patch for now.
- R needs hand holding. Its configure script has section that are hand crafted instead of using autotools standard macros. The only reason I can think of for doing that is ignorance. This is why you have to define
FPICFLAGS
in advance for it. However, it is not sufficient for configure to work out how to mix fortran and C code. So once the build breaks with an error on RFC="pgfortran -fPIC" ./sage -i r
and once done resume the build normally.
Don't define FC=/path_to_compiler/my_fortran_compiler
. numpy's distutils extension get confused if you do that and scipy's compilation will fail because gnu options are passed to the compiler instead of appropriate ones.
comment:33 follow-up: ↓ 34 Changed 2 years ago by
Does non-Apple clang pretend to be GCC? How? Just wondering.
comment:34 in reply to: ↑ 33 Changed 2 years ago by
Replying to dimpase:
Does non-Apple clang pretend to be GCC? How? Just wondering.
It does. Compiler identification is by pragma, and clang on OS X or otherwise has the pragmas saying it is gcc-4.2.1. The intel compiler is a bit different, at installation time it will look at your default gcc install and mimic all the needed pragmas to fake it. Both compiler still have their own vendor pragmas. But by default AC_PROG_CC
will test the gcc pragma to set the variable IS_GCC
(or something like that) and both clang (any OS) and Intel will set it to yes
.
comment:35 Changed 2 years ago by
- Branch changed from u/fbissey/fortran to 3185e91e9da7ce92bb90b69a4ef55dd98432b4dd
- Resolution set to fixed
- Status changed from positive_review to closed
comment:36 Changed 2 years ago by
- Commit 3185e91e9da7ce92bb90b69a4ef55dd98432b4dd deleted
And at last for those still interested I did a build with gcc+ifort (intel fortran compiler). Same kind of set up as for PGI. I had to also provide LD_LIBRARY_PATH
for the compiler's libraries because they don't know how to find each other in the same folder :(
openblas need the upgrade to 0.3.3 because of this https://github.com/xianyi/OpenBLAS/issues/1548
R is still fiddly and you still need to define FPICFLAGS
manually. I think most of the difficulties there would disappear by using the intel compiler for C/C++ as well as fortran.
Anyway everything was sorted and built properly sage works and pass its doctests with flying colours on linux.
Adding intel for C/C++ requires adjustment in pari and possibly sqlite from what I remember. But this is an alternative that could really be made to work.
comment:37 Changed 2 years ago by
- Keywords spkg-configure added
It appears that all what's needed is
build/pkgs/gfortran/spkg-configure.m4
SAGE_MUST_INSTALL_GCC([a Fortran compiler is missing])with this change, and no gfortran in
PATH
, everything looks as it should after running./configure
.Could someone with access to OSX please try this? (Don't forget to run ./bootstrap before ./configure, i.e. you do need autotools for such a check...).