Opened 6 months ago

Closed 5 months ago

#29170 closed defect (fixed)

Fix R installation errors related to gfortran

Reported by: mkoeppe Owned by:
Priority: blocker Milestone: sage-9.1
Component: packages: standard Keywords:
Cc: dimpase, charpent, fbissey, mjo, gh-timokau Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: Not yet reported upstream; Will do shortly. Work issues:
Branch: 5fbea80 (Commits) Commit: 5fbea8059aff0c6d9994a13788ae71e49387d216
Dependencies: Stopgaps:

Change History (35)

comment:1 Changed 6 months ago by dimpase

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?

comment:2 Changed 5 months ago by mkoeppe

  • Cc mjo added

comment:3 Changed 5 months ago by mjo

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.

comment:4 Changed 5 months ago by mkoeppe

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

comment:5 follow-up: Changed 5 months ago by mkoeppe

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

comment:6 in reply to: ↑ 5 Changed 5 months ago by dimpase

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

comment:7 Changed 5 months ago by mkoeppe

  • Description modified (diff)
  • Priority changed from major to blocker
  • Summary changed from Fix R installation errors related to gfortran to Fix R installation errors related to gfortran, or downgrade R to "experimental"

comment:8 Changed 5 months ago by gh-timokau

  • Cc gh-timokau added

comment:9 Changed 5 months ago by mkoeppe

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)

comment:10 Changed 5 months ago by mkoeppe

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: 

comment:11 Changed 5 months ago by dimpase

bug in R's ./configure ?

comment:12 Changed 5 months ago by mkoeppe

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

comment:13 Changed 5 months ago by mkoeppe

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

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:14 Changed 5 months ago by mkoeppe

  • Branch set to u/mkoeppe/fix_r_installation_errors_related_to_gfortran__or_downgrade_r_to__experimental_

comment:15 Changed 5 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 5fbea8059aff0c6d9994a13788ae71e49387d216

New commits:

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

comment:17 Changed 5 months ago by dimpase

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

comment:18 Changed 5 months ago by mkoeppe

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

comment:19 Changed 5 months ago by mkoeppe

Also in R trunk there is no error handling.

comment:20 Changed 5 months ago by mkoeppe

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.

comment:21 Changed 5 months ago by dimpase

  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.

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

comment:22 Changed 5 months ago by mkoeppe

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.

comment:23 Changed 5 months ago by dimpase

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?

comment:24 Changed 5 months ago by jhpalmieri

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

comment:25 Changed 5 months ago by dimpase

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.

comment:26 Changed 5 months ago by mkoeppe

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.

comment:27 Changed 5 months ago by mkoeppe

  • Status changed from new to needs_review

comment:29 Changed 5 months ago by mkoeppe

  • Description modified (diff)
  • Summary changed from Fix R installation errors related to gfortran, or downgrade R to "experimental" to Fix R installation errors related to gfortran

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

Needs review

comment:30 Changed 5 months ago by dimpase

  • Description modified (diff)
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review
  • Summary changed from Fix R installation errors related to gfortran to Fix R installation errors related to gfortran, or downgrade R to "experimental"

ok, it works.

comment:31 Changed 5 months ago by dimpase

  • Summary changed from Fix R installation errors related to gfortran, or downgrade R to "experimental" to Fix R installation errors related to gfortran

comment:32 Changed 5 months ago by mkoeppe

Thanks!

comment:33 follow-up: Changed 5 months ago by 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.

comment:34 in reply to: ↑ 33 Changed 5 months ago by mjo

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.

comment:35 Changed 5 months ago by vbraun

  • Branch changed from u/mkoeppe/fix_r_installation_errors_related_to_gfortran__or_downgrade_r_to__experimental_ to 5fbea8059aff0c6d9994a13788ae71e49387d216
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.