Make it possible to disable build of r, rpy2 using ./configure --disable-r

[mkoeppe]

Prompted by a build failure of r on cygwin-standard in https://github.com/mkoeppe/sage/runs/1915093157

  [r-3.6.3]   blas.o: in function `dsymm_':
  [r-3.6.3]   /opt/sage-1e693c9ae51a53dad0fe099efc41439dc898f02f/var/tmp/sage/build/r-3.6.3/src/src/extra/blas/blas.f:2986:(.text+0x4352): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `xerbla_'

we make it possible to disable R using a configure options so that one can get a working Sage installation in this way.

(The original build failure has hopefully resolved been resolved by #29537.)

Previous tickets and discussions:

• 29486

• 29441

• 29170

• ​https://groups.google.com/g/sage-devel/c/IX3niEdmCEM/m/LovmF6gmIwAJ
• https://wiki.sagemath.org/ReleaseTours/sage-9.2#rpy2_and_R
• https://groups.google.com/g/sage-devel/c/alkG4veIsBk/m/432cxzpYAQAJ

Depends on #29537 Depends on #30383

CC: @embray @dimpase @orlitzky @jhpalmieri @kiwifb @novoselt @EmmanuelCharpentier @videlec @kliem

Component: porting: Cygwin

Author: Matthias Koeppe

Branch/Commit: f04c134

Reviewer: François Bissey, John Palmieri

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

[mkoeppe]

comment:1

Again we have a problem with r. As cygwin is one of our supported platforms and r is a standard package, this is a blocker.

(More problems with R are around the corner, for wheel builds of Sage - #31396.)

This justifies another round of discussion for the proposal to downgrade the r and rpy2 to "optional" packages - after #29486, #29170, https://groups.google.com/g/sage-devel/c/IX3niEdmCEM/m/LovmF6gmIwAJ

In #29170, @novoselt pointed out that supporting R is important for SageMathCell; and @kiwifb mentioned that "Suddenly dropping R/rpy2 feels rough. However, we don't have usage data to know how rough it would be."

I think that we do need a bit of substantiation here given to make the case of keeping R supported as a standard package - and its cost of maintenance. Note that our R is also nontrivially patched and an update to the 4.x series is overdue, and nobody has stepped up to do this upgrade.

[jhpalmieri]

comment:2

Maybe this is asking for a new type of package: standard on some platforms, optional on others.

I have been "using" the system's R for a while now ("using" mean that Sage doesn't build it, not that I actually use it for anything). Can we distribute binaries that will use a system R installation? (Maybe this is a bad question, because at least on OS X, can we distribute functioning binaries in the first place?)

[mkoeppe]

Branch: u/mkoeppe/cygwin_standardr_build_fails_____downgrade_rrpy2_to_optional

[mkoeppe]

Commit: b2d4ab8

[mkoeppe]

comment:4

rpy2 3.4.x has wheels for macOS - https://pypi.org/project/rpy2/#files

We should find out with what installation of R these wheels are supposed to work - hopefully R's official binary packages that are on CRAN - see https://mirror.las.iastate.edu/CRAN/

---

New commits:

b2d4ab8

build/pkgs/{r,rpy2}: Downgrade to optional

[mkoeppe]

comment:5

Replying to @jhpalmieri:

(Maybe this is a bad question, because at least on OS X, can we distribute functioning binaries in the first place?)

Binaries have been broken for a long time. I would be in favor of just following Dima's current practice of directing people to just use conda.

[mkoeppe]

Author: Matthias Koeppe

[kiwifb]

comment:7

I am fine with that. But at the very least most of the doctests in sage/interface/r.py have to be marked optional. I haven't checked for import of rpy at this stage.

[kiwifb]

comment:8

sage/stats/r.py relies on R being present. All doctests in there should be optional.

[kiwifb]

comment:9

There are two doctests in sage/repl/ipython_tests.py that run R.

Tests for _r_init_ in sage/structure/sage_object.pyx, I note interfaces for other optional packages (octave, mathematica, polymake...) do not have tests.

That's all that I can see and think of right now.

[mkoeppe]

comment:10

Instead of adding an optional tag to all doctests in these files, perhaps we should go through #30778.

[kiwifb]

comment:11

Yes, that would be the right way to go. In the spirit of splitting, r interface stuff should be splitted in its own package eventually. I guess it is now tagged "early candidate".

[mkoeppe]

comment:12

I have reduced #30778 to a more modest version that just handles "optional" tags as file directives instead of using "sage_setup: distribution" tags.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b2d4ab8 to 8732076

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

8732076

src/sage/repl/ipython_tests.py: Mark R interface tests # optional - rpy2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

dbdf084

src/sage/structure/sage_object.pyx: Mark R interface test # optional - rpy2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 8732076 to dbdf084

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

737b21c

src/sage/stats/r.py: Mark all 2 doctests in this file as # optional - rpy2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from dbdf084 to 737b21c

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

4403924

src/sage/interfaces/r.py: Mark all tests # optional - rpy2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 737b21c to 4403924

[mkoeppe]

comment:17

Because #30778 appears to be controversial, for the present ticket I have just added the line by line optional tags. Ready for review.

[kiwifb]

comment:18

Replying to @mkoeppe:

Because #30778 appears to be controversial, for the present ticket I have just added the line by line optional tags. Ready for review.

I feel your pain. Having to do that is kind of terrible. Further checking will have to be with the bots. Let's go with the positive review now.

[kiwifb]

Reviewer: François Bissey

[mkoeppe]

comment:19

Thank you!

[mkoeppe]

comment:20

Looks like there are a few more files with R tests (https://github.com/mkoeppe/sage/runs/1942659778):

• src/doc/en/prep/Quickstarts/Statistics-and-Distributions.rst
• src/sage/interfaces/interface.py
• src/sage/misc/sageinspect.py

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

405ebb9

More # optional - rpy2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 4403924 to 405ebb9

[kiwifb]

comment:23

Right, those were more difficult to spot since they rely on the fact that the R interface is automatically imported in sage/interfaces/all.py. That run should have got all of them.

[mkoeppe]

comment:24

Thanks! I also just ran make ptestlong-nodoc and there are no more errors.

[vbraun]

comment:25

If addition of standard packages requires a mention on sage-devel then IMHO removal should, too...

Also, doesn't pass the testsuite:

**********************************************************************
File "src/sage/repl/interpreter.py", line 242, in sage.repl.interpreter.SageShellOverride.system_raw
Failed example:
    shell.system_raw('R --version')
Expected:
    R version ...
Got:
    /bin/sh: 1: R: not found
**********************************************************************
File "src/sage/repl/interpreter.py", line 244, in sage.repl.interpreter.SageShellOverride.system_raw
Failed example:
    shell.user_ns['_exit_code']
Expected:
    0
Got:
    127
**********************************************************************
1 item had failures:
   2 of  10 in sage.repl.interpreter.SageShellOverride.system_raw
    [141 tests, 2 failures, 3.57 s]
**********************************************************************
File "src/sage/tests/cmdline.py", line 578, in sage.tests.cmdline.test_executable
Failed example:
    out.find("R version ") >= 0
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/tests/cmdline.py", line 580, in sage.tests.cmdline.test_executable
Failed example:
    err
Expected:
    ''
Got:
    '/home/buildbot/slave/sage_git/build/src/bin/sage: line 627: exec: R: not found\n'
**********************************************************************
File "src/sage/tests/cmdline.py", line 582, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    127
**********************************************************************
1 item had failures:
   3 of 231 in sage.tests.cmdline.test_executable
    [230 tests, 3 failures, 29.96 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/repl/interpreter.py  # 2 doctests failed
sage -t --long --random-seed=0 src/sage/tests/cmdline.py  # 3 doctests failed
----------------------------------------------------------------------

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 405ebb9 to ce3b35f

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

b7c1576	Merge tag '9.3.beta8' into t/31409/cygwin_standard__r_build_fails_____downgrade_r__rpy2_to_optional
ce3b35f	More # optional - r

[jhpalmieri]

comment:28

Replying to @vbraun:

If addition of standard packages requires a mention on sage-devel then IMHO removal should, too...

I think this is a good point.

[mkoeppe]

comment:29

There's nothing to discuss; it's trivial to make the packages standard again if someone steps up and fixes the packages.

[mkoeppe]

comment:30

The purpose of asking for a sage-devel vote when making a package standard, to my understanding, is that the additional future maintenance burden (and bloat) coming from a standard package should receive more scrutiny than one developer's "positive review". None of this applies to the present situation.

[jhpalmieri]

comment:31

You really don't think that people should be made aware that R is being downgraded?

[mkoeppe]

comment:32

There will be a line in the list of merged tickets for the beta version that merges the ticket. I don't think this has greater urgency.

[mkoeppe]

Description changed:

--- 
+++ 
@@ -5,3 +5,8 @@
   [r-3.6.3]   /opt/sage-1e693c9ae51a53dad0fe099efc41439dc898f02f/var/tmp/sage/build/r-3.6.3/src/src/extra/blas/blas.f:2986:(.text+0x4352): relocation truncated to fit: R_X86_64_PC32 against undefined symbol `xerbla_'

+Discussions: +- #29486 +- #29170 +- ​https://groups.google.com/g/sage-devel/c/IX3niEdmCEM/m/LovmF6gmIwAJ +- https://groups.google.com/g/sage-devel/c/alkG4veIsBk/m/432cxzpYAQAJ

[vbraun]

comment:34

I agree with the ticket, but I also think its common courtesy to give other developers a heads-up here.

[mkoeppe]

comment:35

Blocking 9.3, not 9.4

[kcrisman]

comment:36

There will be a line in the list of merged tickets for the beta version that merges the ticket. I don't think this has greater urgency.

Yeah, that will definitely catch everyone's attention.

---

R is one of the oldest wheels of the car (not the current Python-style wheels, figurative wheels from days of yore). We all know that once it's not supported, it will be more than broken. At the very least, before this is merged there should be some kind of automated message when R is invoked via Sage (via the innumerable examples out in the wild now, including in various books about "how to use Sage") that says "here is a Sage website that will help you install an R that works with our interface, and here is what to do to get Sage to recognize it". For non-POSIX geeks, but for an ordinary user. And then these now-optional doctests should be tested.

In particular, this is another piece of mission creep from "viable competitor to M, M, M..." I do get the desire to decouple and ease upgrade processes for external software. But for such an important piece of software (not least because of the innumerable packages) there should at the very least be an easy on-ramp to nudging people to install it when needed, and then some kind of testing. Kind of like how OS X "reminds" one if you try to use git and don't yet have command line tools.

[mkoeppe]

comment:37

Replying to @kcrisman:

In particular, this is another piece of mission creep from "viable competitor to M, M, M..." I do get the desire to decouple and ease upgrade processes for external software.

This ticket is about the very concrete problem that R does not build on one of our standard platforms, Cygwin, and we are not able to use the system R either.

Because failing standard packages block the entire build, we either need to fix it, or downgrade it from standard before the 9.3 release.

[mkoeppe]

comment:38

Replying to @kcrisman:

... when R is invoked via Sage (via the innumerable examples out in the wild now, including in various books about "how to use Sage")

Do you have any pointers to such examples?

[mkoeppe]

Description changed:

--- 
+++ 
@@ -7,6 +7,8 @@

 Discussions:
 - #29486
+- #29441
 - #29170
 - ​https://groups.google.com/g/sage-devel/c/IX3niEdmCEM/m/LovmF6gmIwAJ
+- https://wiki.sagemath.org/ReleaseTours/sage-9.2#rpy2_and_R
 - https://groups.google.com/g/sage-devel/c/alkG4veIsBk/m/432cxzpYAQAJ

[dimpase]

comment:40

IIRC correctly, R is not mentioned in http://sagebook.gforge.inria.fr/english.html

[embray]

comment:41

As a point of reference, I've twice been in workshops with R experts where both Sage and R were being taught to students. When I told the R people that since students have already installed Sage, they can use R through Sage. But this did not interest them as they wanted to have students install RStudio anyways.

Do we know about more than one person who uses R primarily through Sage?

[embray]

comment:42

That said, I haven't really kept up with this issue. What's the problem with R on cygwin? Maybe I can just fix it.

[mkoeppe]

comment:43

Replying to @embray:

What's the problem with R on cygwin?

It does not build; somehow BLAS-related. See ticket description.

More generally, we have been unable to use system BLAS and system R. #30163

[embray]

comment:44

When did the problem start? Did we upgrade R at some point?

I'm not against the current proposal to make R optional, though I'm inclined to agree with William's point about having a deprecation period first. I'll look into the build issue with R.

Alternatively, if the R version was upgraded, resulting in this problem, could we not just revert the upgrade until a later version / when this is able to be fixed?