Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix R installation errors related to gfortran #29170

Closed
mkoeppe opened this issue Feb 9, 2020 · 37 comments
Closed

Fix R installation errors related to gfortran #29170

mkoeppe opened this issue Feb 9, 2020 · 37 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Feb 9, 2020

As reported

https://developer.r-project.org/Blog/public/2019/05/15/gfortran-issues-with-lapack/index.html

Upstream: Not yet reported upstream; Will do shortly.

CC: @dimpase @EmmanuelCharpentier @kiwifb @orlitzky @timokau

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 5fbea80

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/29170

@mkoeppe mkoeppe added this to the sage-9.1 milestone Feb 9, 2020
@dimpase
Copy link
Member

dimpase commented Feb 9, 2020

comment:1

gcc fix here (linked from the R link) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90329

not clear what they actually did - new option(s) added to gfortran?

@orlitzky
Copy link
Contributor

comment:3

Ok, I read that whole thread so no one else has to. Ultimately, this is due to broken code that needs to be updated. The GCC developers have done two things:

  • Added -fc-prototypes-external, to help people fix that broken code.
  • In the meantime, they have added a workaround in the form of the (on by default) -ftail-call-workaround flag (https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html). That keeps the broken code working for now, but the option will eventually be turned off and go away.

The workaround was backported all the way back to gcc-7.x, so really what we should be doing is telling everyone to use a new-enough gfortran. The workaround is in gcc-9.2, and based on the release dates, I presume it's also in 7.5, 8.4, and everything newer than that.

We can probably just update gfortran's spkg-configure.m4 to ensure a new-enough version. Maybe for now it would suffice to check if the -ftail-call-workaround flag is supported. That would rule out older compilers from before things were broken, but is easy to implement.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 20, 2020

comment:4

See also: #29379 Upgrade R to 3.6.3 (which does not fix these errors)

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 21, 2020

comment:5

See also: #29378 Update OpenBLAS to 0.3.9 (which also does not fix these errors)

@dimpase
Copy link
Member

dimpase commented Mar 21, 2020

comment:6

Replying to @mkoeppe:

See also: #29378 Update OpenBLAS to 0.3.9 (which also does not fix these errors)

IIRC, the bugs in question (related to passing strings) are in "canonical" Fortran Lapack routines, and OpenBLAS does not amend Fortran code

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Fix R installation errors related to gfortran Fix R installation errors related to gfortran, or downgrade R to "experimental" Mar 21, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:9

Our R 3.6.2 already contains checks in configure that are intended to address the problem with the Fortran ABI. On one of the failing platforms, however, I see:

[r-3.6.2.p0] checking if need -fno-optimize-sibling-calls for gfortran... yes
[r-3.6.2.p0] checking for type of 'hidden' Fortran character lengths... 
[r-3.6.2.p0] checking for xmkmf... no

which looks suspicious (note - no result shown for the type)

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:10

from config.log there:

configure:20320: checking for type of 'hidden' Fortran character lengths
/usr/bin/ld: conftestf.o: relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIC
/usr/bin/ld: final link failed: nonrepresentable section on output
collect2: error: ld returned 1 exit status
configure:20384: result: 

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:11

bug in R's ./configure ?

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:12

In any case missing error handling. I'm on it

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:13

Adding -fPIC to CFLAGS & FCFLAGS seems to do the trick.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

Commit: 5fbea80

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

New commits:

5fbea80build/pkgs/r/spkg-install.in: Work around a failing R configure check for hidden Fortran character lengths

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:16

Tests run at https://github.com/mkoeppe/sage/actions/runs/60562504

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:17

One needs to check whether the latest R still has this issue, and notify upstream, if it does.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:18

see above - upgrading to the latest release 3.6.3 did not fix the problem. I'll check for unreleased fixes

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:19

Also in R trunk there is no error handling.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:20

https://github.com/wch/r-source/blob/trunk/configure.ac#L1108
In this situation, the macro R_PROG_FC_CHAR_LEN_T (from m4/R.m4) produces an empty $r_cv_prog_fc_char_len_t, which causes the compilation errors. Error checking should be added either in the macro or in the macro call.

Upstream does not have an open bug reporting system. Perhaps an R user can communicate this problem to upstream.

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

Upstream: Not yet reported upstream; Will do shortly.

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:21

Shouldn't R also handle the need to add -fPIC in its ./configure ?

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:22

I don't really understand what's happening there. It may have to do with the way we compile the gfortran spkg. Note all of the platforms where the failure occured use system gcc but a gfortran from our spkg.

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:23

maybe our gfortran doesn't have the needed workaround in, or perhaps there is a gfortran's default that needs to be turned on at gfortran's build time?

@jhpalmieri
Copy link
Member

comment:24

Maybe we should get rid of our gfortran instead of R, at least on the various linux distributions where there are other sane alternatives.

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:25

there are unfortunate souls that need Sage running on HPC systems, which often have quite outdated software and non-responsive or just absent sysadmins, and so they need to build Sage's toolchain.

I'd spin out the toolchain in a separate package.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:26

I think this is a working fix. I would not want to get rid of the sage distribution's ability to install gfortran. Lots of poorly maintained Linux boxes in universities everywhere.

And we already advertise the distribution packages for gfortran and R.

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:28

Tests run at https://github.com/mkoeppe/sage/actions/runs/60588138

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:29

This seems to fix the problem. See for example for ubuntu-bionic-minimal at https://github.com/mkoeppe/sage/runs/524868489

Needs review

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Fix R installation errors related to gfortran, or downgrade R to "experimental" Fix R installation errors related to gfortran Mar 22, 2020
@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

Reviewer: Dima Pasechnik

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:30

ok, it works.

@dimpase dimpase changed the title Fix R installation errors related to gfortran Fix R installation errors related to gfortran, or downgrade R to "experimental" Mar 22, 2020
@dimpase dimpase changed the title Fix R installation errors related to gfortran, or downgrade R to "experimental" Fix R installation errors related to gfortran Mar 22, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 22, 2020

comment:32

Thanks!

@dimpase
Copy link
Member

dimpase commented Mar 22, 2020

comment:33

R_PROG_FC_CHAR_LEN_T might be buggy, it does not use any "normal" autoconf macros for test programs, doing instead pure shell.

@orlitzky
Copy link
Contributor

comment:34

Replying to @dimpase:

R_PROG_FC_CHAR_LEN_T might be buggy, it does not use any "normal" autoconf macros for test programs, doing instead pure shell.

I think you're right and we would wind up with -fPIC added for this one check if they had used the autotools magic. Adding -fPIC globally works around the issue, but isn't a good long-term solution (Case 3 in https://wiki.gentoo.org/wiki/Project:AMD64/Fixing_-fPIC_Errors_Guide).

Since the compile/link command is hard-coded into R_PROG_FC_CHAR_LEN_T, can we append -fPIC for just the one check? Then upstream should be able to either do the same thing, or use some autotools stuff to fix it in the future.

@vbraun
Copy link
Member

vbraun commented Mar 25, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants