Opened 13 months ago

Closed 13 months ago

Last modified 10 months ago

#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) Commit:
Dependencies: Stopgaps:

Description (last modified by dimpase)

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)

pgfortran.patch (2.0 KB) - added by fbissey 13 months ago.
patch to compile openblas properly with pgfortran

Download all attachments as: .zip

Change History (38)

comment:1 Changed 13 months ago by dimpase

  • Description modified (diff)

comment:2 Changed 13 months ago by dimpase

It appears that all what's needed is

  • build/pkgs/gfortran/spkg-configure.m4

    a b SAGE_SPKG_CONFIGURE([gfortran], [ 
    1414        # compiler.
    1515        if test -z "$FC"; then
    1616            sage_spkg_install_gfortran=yes
    17             SAGE_MUST_INSTALL_GCC([a Fortran compiler is missing])
    1817        fi
    1918
    2019        # see http://www.gnu.org/software/hello/manual/autoconf/Fortran-Compiler.html

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...).

comment:3 follow-up: Changed 13 months ago by 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?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 13 months ago by 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.

comment:5 in reply to: ↑ 4 Changed 13 months ago by fbissey

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 13 months ago by dimpase

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: Changed 13 months ago by 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?

comment:8 Changed 13 months ago by jhpalmieri

  • Cc jhpalmieri added; jpalmieri removed

comment:9 in reply to: ↑ 7 Changed 13 months ago by embray

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 13 months ago by embray

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: Changed 13 months ago by 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...).

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: Changed 13 months ago by 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.

Last edited 13 months ago by embray (previous) (diff)

comment:13 in reply to: ↑ 12 Changed 13 months ago by dimpase

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 13 months ago by jhpalmieri

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 13 months ago by dimpase

flang does not run on OSX yet. It does run on Linux and FreeBSD, though.

comment:16 Changed 13 months ago by fbissey

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 13 months ago by dimpase

imho Sage should not be in business of building compilers.

comment:18 Changed 13 months ago by fbissey

  • Authors set to François Bissey
  • 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:

a666ff9Do not build gcc if only a fortran compiler is missing. More care about check logic. Don't restrict to gfortran.

comment:19 Changed 13 months ago by git

  • Commit changed from a666ff9ca7eaff96d8546b633391ab955b38e486 to c35c6ec1cc26a843a4cfe79a807e5f324afe8dd4

Branch pushed to git repo; I updated commit sha1. New commits:

c35c6ecCorrect spacing

comment:20 Changed 13 months ago by fbissey

  • Status changed from needs_review to needs_work

Stuff is broken, trying to figure out why.


New commits:

c35c6ecCorrect spacing

comment:21 Changed 13 months ago by fbissey

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 13 months ago by fbissey

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 13 months ago by git

  • Commit changed from c35c6ec1cc26a843a4cfe79a807e5f324afe8dd4 to 3185e91e9da7ce92bb90b69a4ef55dd98432b4dd

Branch pushed to git repo; I updated commit sha1. New commits:

2449240Do not check for a fortran compiler in the C/C++ detection macro. Leave it to the fortran macro.
3185e91Extreme streamlining of fortran package macro.

comment:24 Changed 13 months ago by fbissey

  • 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 13 months ago by fbissey

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: Changed 13 months ago by 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.

comment:27 Changed 13 months ago by embray

  • 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: Changed 13 months ago by 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.

comment:29 in reply to: ↑ 28 Changed 13 months ago by jhpalmieri

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: Changed 13 months ago by 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 :(

comment:31 in reply to: ↑ 30 Changed 13 months ago by fbissey

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 13 months ago by fbissey

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 R
    FC="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.

Changed 13 months ago by fbissey

patch to compile openblas properly with pgfortran

comment:33 follow-up: Changed 13 months ago by dimpase

Does non-Apple clang pretend to be GCC? How? Just wondering.

comment:34 in reply to: ↑ 33 Changed 13 months ago by fbissey

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 13 months ago by vbraun

  • Branch changed from u/fbissey/fortran to 3185e91e9da7ce92bb90b69a4ef55dd98432b4dd
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:36 Changed 13 months ago by fbissey

  • 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.

Last edited 13 months ago by fbissey (previous) (diff)

comment:37 Changed 10 months ago by dimpase

  • Keywords spkg-configure added
Note: See TracTickets for help on using tickets.