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:

• cygwin: Do not use system R #29486
• upgrade rpy2 package 2.8.2 -> 3.3.5, upgrade R to 3.6.3, add new dependencies #29441
• Fix R installation errors related to gfortran #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_standard__r_build_fails_____downgrade_r__rpy2_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.

[sagetrac-git]

Changed commit from b2d4ab8 to 8732076

[sagetrac-git]

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

8732076

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

[sagetrac-git]

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

dbdf084

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

[sagetrac-git]

Changed commit from 8732076 to dbdf084

[sagetrac-git]

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

[sagetrac-git]

Changed commit from dbdf084 to 737b21c

[sagetrac-git]

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

4403924

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

[sagetrac-git]

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.

[sagetrac-git]

Changed commit from 49c10c1 to f0a5fb5

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5141bc3	build/pkgs/widgetsnbextension: Patch out dependency on notebook (backport from widgetsnbextension-4)
fee8379	Merge #31278
82d8030	m4/sage_spkg_enable.m4: Cosmetic change to help strings
3e92998	configure --disable-notebook: Fix up what packages are disabled
74a83fc	build/pkgs/matplotlib/dependencies: Undo removal of tornado
93d31b9	Remove use of unicode superscripts in configure --help
18fbb85	m4/sage_spkg_collect.m4: Fix description of SAGE_OPTIONAL_CLEANED_PACKAGES
c3e4093	Rename SAGE_OPTIONAL_CLEANED_PACKAGES to SAGE_OPTIONAL_UNINSTALLED_PACKAGES
a67300b	Merge #30383
f0a5fb5	configure.ac: Add option --disable-r

[embray]

comment:87

There seems to be a simpler bug here that I don't think is addressed yet by any of your fixes. I didn't know there was now an --enable-fat-binary flag to configure. I'm glad there is, and it makes sense. But perhaps because of that the SAGE_FAT_BINARY environment variable is being ignored. I added set -x to the splg-install for openblas and can see that its value is getting unset, which is why the CI configuration wasn't picking it up either despite having it in env. I think, if set, SAGE_FAT_BINARY should still override whatever was passed to configure.

[mkoeppe]

comment:88

Replying to @embray:

There seems to be a simpler bug here that I don't think is addressed yet by any of your fixes.

The simple bug was -- as you correctly suspected -- in my CI script. The testing is done through tox, which does not pass through environment variables (except for those explicitly configured to be passed through).

[mkoeppe]

comment:89

(see commit 1092d4b in #29537)

[mkoeppe]

comment:90

Replying to @embray:

I didn't know there was now an --enable-fat-binary flag to configure. I'm glad there is, and it makes sense. But perhaps because of that the SAGE_FAT_BINARY environment variable is being ignored.

This was done in #30375. Both SAGE_FAT_BINARY and the option --enable-fat-binary are respected at configure time and are made persistent for the build through build/bin/sage-build-env-config. Mixing in the environment variable set at make time is not done; I don't think this is a problem.

[sagetrac-git]

Changed commit from f0a5fb5 to f04c134

[sagetrac-git]

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

4916415	Merge tag '9.3.beta9' into t/30383/new_package_type__optional_enabled_by_default
f04c134	Merge branch 't/30383/new_package_type__optional_enabled_by_default' into t/31409/cygwin_standard__r_build_fails_____downgrade_r__rpy2_to_optional

[embray]

comment:92

Replying to @mkoeppe:

Replying to @embray:

I didn't know there was now an --enable-fat-binary flag to configure. I'm glad there is, and it makes sense. But perhaps because of that the SAGE_FAT_BINARY environment variable is being ignored.

This was done in #30375. Both SAGE_FAT_BINARY and the option --enable-fat-binary are respected at configure time and are made persistent for the build through build/bin/sage-build-env-config. Mixing in the environment variable set at make time is not done; I don't think this is a problem.

Still, it used to be possible to set it when running make as well. I guess it's not so important, but it feels like a regression, and was confusing to me.

[mkoeppe]

comment:93

Replying to @embray:

it used to be possible to set it when running make as well. I guess it's not so important, but it feels like a regression, and was confusing to me.

I think the reasoning is that -- in contrast to SAGE_DEBUG and the compiler flags -- that it is not useful to control SAGE_FAT_BINARY for just a few packages -- you get a portable Sage distribution only if all packages are compiled like this. I would have no strong objections though to add it back though (in a separate ticket - it's unrelated to the present ticket).

[mkoeppe]

comment:94

The present ticket needs review - it would be nice to get this into 9.3

[jhpalmieri]

comment:95

./configure --help does not print --enable-r as an option.

[jhpalmieri]

comment:96

Although I see ./configure --disable-r, so maybe it's not important.

[mkoeppe]

comment:97

I think it's common practice to only show one of --enable-... and --disable-... depending on what is the default.

For our optional packages, I think we decided to list both because of the perhaps unusual semantics of --disable-... uninstalling a previously installed optional package.

[mkoeppe]

comment:98

Replying to @mkoeppe:

For our optional packages, I think we decided to list both because of the perhaps unusual semantics of --disable-... uninstalling a previously installed optional package.

And... of course because our default for optional depends on whether the package is already installed. And this information is not known at bootstrap time (when the help text is put together), only at configure time.

[jhpalmieri]

Changed reviewer from François Bissey to François Bissey, John Palmieri

[jhpalmieri]

comment:99

Okay, that makes sense. It works as advertised, so let's merge it.

[mkoeppe]

comment:100

Thanks!

[mkoeppe]

comment:101

Setting priority to blocker to bring this ticket to the attention of the release bot.

[vbraun]

Changed branch from u/mkoeppe/cygwin_standard__r_build_fails_____downgrade_r__rpy2_to_optional to f04c134