Upgrade R to 3.2.0 #18229

EmmanuelCharpentier · 2015-04-16T19:39:27Z

It's that time of the year again (and I'm late...).

Renamed source tarball available from:

http://www.labri.fr/perso/vdelecro/r-3.2.0.tar.gz

We add a new patch to fix a bug in the R_PCRE autoconf macro which leads to configure losing -lz and -lbz2 from LIBS under certain circumstances, when using Sage's versions of these libraries (as opposed to letting R build its own, which we don't want). (Upstream report)

Depends on #18254
Depends on #15642

Upstream: Reported upstream. No feedback yet.

Component: packages: standard

Keywords: r-project

Author: Emmanuel Charpentier, Leif Leonhardy

Branch/Commit: f451e9a

Reviewer: Vincent Delecroix, François Bissey, Leif Leonhardy

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

The text was updated successfully, but these errors were encountered:

EmmanuelCharpentier · 2015-04-16T20:29:37Z

Branch: u/charpent/upgrade_r_to_3_1_3

EmmanuelCharpentier · 2015-04-16T20:36:34Z

Commit: b0f4e43

EmmanuelCharpentier · 2015-04-16T20:37:48Z

comment:3

I forgot : passes ptestlong.

kcrisman · 2015-04-17T01:30:50Z

comment:4

Does plotting and so forth still work properly?

EmmanuelCharpentier · 2015-04-17T05:10:37Z

comment:5

I'll check that.

BTW : R-project released (of course !) 3.2.0 hours after I uploaded this patch ==> needs_work. I amend the ticket.

kiwifb · 2015-04-17T06:12:02Z

comment:6

Actually can you check if you really need to rename upstream's tarball? We can deal with archive with a capital in their name (Pillow and Sphinx for example). You'll have to change checksums.ini to try that.

sagetrac-git · 2015-04-17T06:35:19Z

Changed commit from b0f4e43 to ab208a4

sagetrac-git · 2015-04-17T06:35:19Z

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

`ab208a4`	`Upgrade R to 3.2.0. passes ptestlong (All tests passed\!).`

EmmanuelCharpentier · 2015-04-17T06:42:06Z

comment:8

Upgrade to 3.2.0 done. Passes ptestlong ==> needs_review

R graphics : they work in the sage notebook, not on command line or ipython notebook (same as before...). This would deserve another ticket.

François : the build system may support names with a capital, but sage-fix-pkg-checksums do not. I won't fix that in this ticket. Should I (should you ?) open a new ticket ?

kiwifb · 2015-04-17T07:09:45Z

comment:9

I am a bit surprised since we have other packages with capital in their names and they have to be checksumed too. In fact I just tried the script on Pillow and it worked (as in I removed checksum.ini for pillow and regenerated it). In fact I renamed the current R tarball from r-3.1.2.tar.gz to R-3.1.2.tar.gz and it worked too. Have you even tried it?

EmmanuelCharpentier · 2015-04-17T17:59:52Z

comment:10

Important rectifications :

kcrisman : I was able to get R graphics in the ipython notebook ... but I had to %load_ext rmagic before, and work in a %%R cell. This is far from obvious after reading the docs. I understand that the ipython is work in progress, so this is not (yet) a big problem. But it should eventually be documented in a more obvious way. I am beginnig to think that some kind of guide to ipython notebook for (repentant ?) Sage notebook users would be useful. Maybe ditto for Cloud users (Sagemath cloud is really a different beast ...).

Note that you have to %load_ext rmagic, nor rpy2.ipython (error message : ImportError: No module named ipython"). Does this deserve a ticket ?

fbissey : I tried your way : I moved $SAGE_ROOT/build/pkgs/r/checksums.ini out of the way, renamed $SAGE_ROOT/upstream/r-3.2.0.tar.gz to $SAGE_ROOT/upstream/R-3.2.0.tar.gz and ran sage -sh sage-fix-pkg-checksums : neither r-3.2.0.tar.gz nor R-3.2.0.tar.gz appear in the list of recomputed checksums (nor does pillow or Pillow, by the way), and $SAGE_ROOT/build/pkgs/r/checksums.ini is NOT recreated.

That was on 6.7beta1 + the present patch. Don't you have other patches ?

Status : still "needs_review", IMHO.

HTH,

kcrisman · 2015-04-17T18:23:43Z

comment:11

I wouldn't be concerned about ipynb graphics since that is new (I mean, I'm not, others might be). Has plotting from the command line ever worked without setting a graphics viewer (there is a name for this in R, I just forget it now).

kcrisman · 2015-04-17T18:24:18Z

comment:12

Note that you have to %load_ext rmagic, nor rpy2.ipython (error message : ImportError: No module named ipython"). Does this deserve a ticket ?

I don't know anything about that, but rpy2 shouldn't be necessary for all this.

EmmanuelCharpentier · 2015-04-17T19:05:07Z

comment:13

Replying to @kcrisman:

I wouldn't be concerned about ipynb graphics since that is new (I mean, I'm not, others might be). Has plotting from the command line ever worked without setting a graphics viewer (there is a name for this in R, I just forget it now).

It's known as a "graphic device" in R docs.

To be clear,

r("plot(rnorm(1000),rnorm(1000))")

does nothing (on command line and in ipynb). Returns NULL

r("X11();plot(rnorm(1000),rnorm(1000))")

opens an X11 window (the R graphic device) and displays the cloud in this window (again on command line and in ipynb). Returns NULL, nothing displayed in the notebook.

Interestingly, neither r.plot(r.rnorm(1000),r.rnorm(1000)) nor r.X11();r.plot(r.rnorm(1000),r.rnorm(1000)) do anything on the command line (prints NULL, then the named vector (c("null device"=1). or in the notebook (ditto except that NULL is not printed).

I don't remember having ever used R from sage on the command line (my favorite interface was emacs and ess, for obvious reasons, but The Sage notebook was sometimes handy, and ipynb appears more and more attractive, since %%R seems to work).

HTH,

EmmanuelCharpentier · 2015-04-17T19:26:06Z

comment:14

Replying to @kcrisman:

Note that you have to %load_ext rmagic, nor rpy2.ipython (error message : ImportError: No module named ipython"). Does this deserve a ticket ?

I don't know anything about that, but rpy2 shouldn't be necessary for all this.

Currently, it seems to be, at least in ipynb. In the Sage notebook, the only way I found to get graphs reliably in the notebook itself was to open a device, plot on it (several graphs are possible) and close the device in the same %r cell. The same is possible in an rpy2 %%R cell vhithout to have to open/close the device.

HTH,

kiwifb · 2015-04-17T19:48:52Z

comment:15

Replying to @EmmanuelCharpentier:

fbissey : I tried your way : I moved $SAGE_ROOT/build/pkgs/r/checksums.ini out of the way, renamed $SAGE_ROOT/upstream/r-3.2.0.tar.gz to $SAGE_ROOT/upstream/R-3.2.0.tar.gz and ran sage -sh sage-fix-pkg-checksums : neither r-3.2.0.tar.gz nor R-3.2.0.tar.gz appear in the list of recomputed checksums (nor does pillow or Pillow, by the way), and $SAGE_ROOT/build/pkgs/r/checksums.ini is NOT recreated.

That's very strange I was on 6.6 on a mac when I did that. Can you go in upstream and try ../sage -sh sage-fix-pkg-checksums R-3.2.0.tar.gz explicitly. It may be that it doesn't work with all shells.

EmmanuelCharpentier · 2015-04-17T20:09:34Z

comment:16

Replying to @kiwifb:

Replying to @EmmanuelCharpentier:

fbissey : I tried your way : I moved $SAGE_ROOT/build/pkgs/r/checksums.ini out of the way, renamed $SAGE_ROOT/upstream/r-3.2.0.tar.gz to $SAGE_ROOT/upstream/R-3.2.0.tar.gz and ran sage -sh sage-fix-pkg-checksums : neither r-3.2.0.tar.gz nor R-3.2.0.tar.gz appear in the list of recomputed checksums (nor does pillow or Pillow, by the way), and $SAGE_ROOT/build/pkgs/r/checksums.ini is NOT recreated.

That's very strange I was on 6.6 on a mac when I did that. Can you go in upstream and try ../sage -sh sage-fix-pkg-checksums R-3.2.0.tar.gz explicitly. It may be that it doesn't work with all shells.

I just did that exactly, with the same result : nothing is printed by the script, and the checksums.ini file is not recreated.

Yet another Appleism ? What shell do you use ?

HTH,

kiwifb · 2015-04-17T20:27:08Z

comment:17

It is bash 3.2 as far as I can tell. I am guessing you have 4.x.

EmmanuelCharpentier · 2015-05-04T21:51:01Z

comment:80

It should be prudent to check the (forthcoming) patch by jhpalmieri on a 32-bit (virtual) machine (which I don't have) before accepting it.

Replying to @vbraun:

Also fails on Linux 32-bit:

gcc -std=gnu99 -Wl,--export-dynamic -fopenmp  -L../../lib -L/home/buildslave-sage/slave/sage_git/build/local/lib/  -o R.bin Rmain.o  -lR 
../../lib/libR.so: undefined reference to `BZ2_bzReadGetUnused'
../../lib/libR.so: undefined reference to `BZ2_bzRead'
../../lib/libR.so: undefined reference to `BZ2_bzBuffToBuffCompress'
../../lib/libR.so: undefined reference to `BZ2_bzBuffToBuffDecompress'
../../lib/libR.so: undefined reference to `BZ2_bzWriteClose'
../../lib/libR.so: undefined reference to `BZ2_bzWriteOpen'
../../lib/libR.so: undefined reference to `BZ2_bzReadOpen'
../../lib/libR.so: undefined reference to `BZ2_bzDecompress'
../../lib/libR.so: undefined reference to `BZ2_bzDecompressEnd'
../../lib/libR.so: undefined reference to `BZ2_bzDecompressInit'
../../lib/libR.so: undefined reference to `BZ2_bzWrite'
../../lib/libR.so: undefined reference to `BZ2_bzlibVersion'
../../lib/libR.so: undefined reference to `BZ2_bzReadClose'
collect2: error: ld returned 1 exit status

Full log: http://build.sagedev.org/release/builders/%20%20fast%20Oxford%20arando%20%28Ubuntu%2013.04%20i686%29%20incremental/builds/259/steps/compile/logs/r-project

EmmanuelCharpentier · 2015-05-04T21:53:41Z

comment:81

Doesn't that means that R and Sage do different, possibly incompatible, changes to DYLD_LIBRARY_PATH ?

Replying to @nexttime:

Before I fix the "typo", could someone translate (i.e., rephrase) the following such that it makes sense?
    # Indeed Sage modifies DYLD_LIBRARY_PATH whereas R build process
    # modifies DYLD_LIBRARY_PATH, so that it is Sage's R and not the freshly
    # built one which will get loaded during the build process and if they are
    # different versions then the build process might fail.
Maybe I'm blind or just dumb, but I twice read DYLD_LIBRARY_PATH -- the second occurrence should probably read LD_LIBRARY_PATH, otherwise "whereas" doesn't make sense to me.

kiwifb · 2015-05-04T21:57:05Z

comment:82

Possibly {DY}LD_LIBRARY_PATH plain LD_LIBRARY_PATH is linux but DYLD_LIBRARY_PATH is OS X's version.

sage does load the appropriate one so its libraries are found first but of course during the building process R does its own overloading to find the stuff from the just built R when building the standard R packages. In principle this should be done by augmentation but I have seen libtool doing broken things before (in octave it inserts /usr/lib64 which completely defeat the purpose of the variable).

jhpalmieri · 2015-05-04T22:00:26Z

comment:83

Replying to @nexttime:

Before I fix the "typo", could someone translate (i.e., rephrase) the following such that it makes sense?
    # Indeed Sage modifies DYLD_LIBRARY_PATH whereas R build process
    # modifies DYLD_LIBRARY_PATH, so that it is Sage's R and not the freshly
    # built one which will get loaded during the build process and if they are
    # different versions then the build process might fail.
Maybe I'm blind or just dumb, but I twice read DYLD_LIBRARY_PATH -- the second occurrence should probably read LD_LIBRARY_PATH, otherwise "whereas" doesn't make sense to me.

I'm guessing that this comes from #14706 comment:101, in which case, the second version should say DYLD_FALLBACK_LIBRARY_PATH.

kiwifb · 2015-05-04T22:02:06Z

comment:84

John makes a lot of sense here, that fits the kind of things we want to achieve.

jhpalmieri · 2015-05-04T22:09:16Z

comment:85

Replying to @EmmanuelCharpentier:

It should be prudent to check the (forthcoming) patch by jhpalmieri on a 32-bit (virtual) machine (which I don't have) before accepting it.

Replying to @vbraun:

Also fails on Linux 32-bit:
[snip]

My suggestion should only have an effect on OS X: it's in a block starting

if [ "$UNAME" = "Darwin" ]; then

And it is just a mistake in the shell script, yielding a (non-fatal) error

./spkg-install: line 164: RINSTALL_MOVED: command not found

every time you try to install R on an OS X machine.

nexttime · 2015-05-04T23:01:29Z

comment:86

Replying to @EmmanuelCharpentier:

Replying to @nexttime:

? Fixing the bug in R_PCRE certainly belongs to upgrading R to 3.2.0, since the new version introduced the bug, and as you have seen, it hits at least some of us (depending on the system configuration).

This version introduced the bug : you're right, as far as I can tell. Your case seems to be a corner case.

Did you read the upstream report?

It's the same problem : no.

Same as what?

This is, as far as I understand, a problem with the build infrastructure of Sage.

Nope. The same happens outside of Sage.

The new R version just revealed a latent bug in a very loosely related part of this infrastructure.

Bug in Sage's infrastructure? I don't get the point.

If I'd found this bug, I'd have opened a distinct ticket (T2) and made the present ticket (T1) depend on the new ticket (T2).

Well, technically that doesn't really make sense, since I cannot fix something that hasn't yet been included. (#18254 is slightly different as it modifies an existing file, namely spkg-install.) My changes are based on your branch.

I would agree if we could make this ticket depend on upstream, but then it wouldn't be possible to (positively) review it before upstream decided on a solution.

That way, each different problem would have been identified in the tickets database. As should have been done when a previous version of R opened a Cygwin-related saga...

We at least have different commits with IMHO meaningful commit messages... (which unfortunately often isn't the case).

You probably know my opinion on (not) bumping patch levels and discarding the package changelog we used to have for years.

Sometimes it is better to also fix "unrelated" minor issues (such as the typo) on a ticket dealing with a package (nowadays probably less than it was with "legacy" spkgs), since otherwise they'll simply never get fixed, either because nobody opens individual tickets for them, or the tickets just rotten.

The prerequisite for inclusion of a new (or upgraded) package is that it builds/works on all supported platforms, so fixing build issues (and probably fixing doctests, cf. the PARI upgrade at #18340, which was motivated by an initially seemingly unrelated problem with GNU make) belongs to the ticket that introduces (or upgrades) a package.

There are other issues where it perfectly makes sense to open (a metaticket and) separate (sub-)tickets, e.g. to let Sage build with GCC 5.x (four tickets for four different packages which need fixing), or to let Sage build with clang (dozens of tickets).

[page overflow]

nexttime · 2015-05-04T23:42:47Z

comment:87

Last chance for further changes: ;-)

diff --git a/build/pkgs/r/spkg-install b/build/pkgs/r/spkg-install
index 06a340d..f02bea9 100755
--- a/build/pkgs/r/spkg-install
+++ b/build/pkgs/r/spkg-install
@@ -153,15 +153,15 @@ if [ $? -ne 0 ]; then
 fi
 
 if [ "$UNAME" = "Darwin" ]; then
-    # We have to move old installs of R when upgrading R in Sage on OS X.
-    # Indeed Sage modifies DYLD_LIBRARY_PATH whereas R build process
-    # modifies DYLD_LIBRARY_PATH, so that it is Sage's R and not the freshly
-    # built one which will get loaded during the build process and if they are
-    # different versions then the build process might fail.
+    # We have to move (i.e., hide) old installs of R when upgrading R in Sage on OS X.
+    # Indeed Sage modifies DYLD_LIBRARY_PATH whereas R's build process
+    # modifies DYLD_FALLBACK_LIBRARY_PATH, so that it is Sage's R and not the freshly
+    # built one which will get loaded during the build process; in case the versions
+    # differ, the build process might fail.
     # On systems using LD_LIBRARY_PATH, both Sage and R modify it and R wins,
     # so no problem should occur.
     if [ -d "$SAGE_LOCAL"/lib/R ]; then
-        RINSTALL_MOVED = yes
+        RINSTALL_MOVED=yes
         mv "$SAGE_LOCAL"/lib/R "$SAGE_LOCAL"/lib/R.old
     fi
 fi

jhpalmieri · 2015-05-05T00:04:15Z

comment:88

Looks good to me. I would be happy if you went ahead with the change.

kiwifb · 2015-05-05T00:12:31Z

comment:89

No further changes here. Let's gets this to review now.

sagetrac-git · 2015-05-05T01:17:53Z

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

`f451e9a`	`Fix typo in R's spkg-install (potentially causing a syntax error on MacOS X) (#18229)`

sagetrac-git · 2015-05-05T01:17:53Z

Changed commit from bc2269e to f451e9a

jdemeyer · 2015-05-05T06:02:37Z

comment:91

Replying to @sagetrac-git:

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


f451e9a	`Fix typo in R's spkg-install (potentially causing a syntax error on MacOS X) (#18229)`
It's not a syntax error, it's the shell running the command `RINSTALL_MOVED` with 2 arguments `=` and `yes`.

nexttime · 2015-05-05T06:14:45Z

comment:92

Replying to @jdemeyer:

It's not a syntax error, it's the shell running the command RINSTALL_MOVED with 2 arguments = and yes.

It's a potential syntax error, depending on how R_INSTALL_MOVED is defined ... ;-)

$ R_INSTALL_MOVED(){ expr $@; } 
$ R_INSTALL_MOVED = yes
expr: syntax error

nexttime · 2015-05-05T06:18:56Z

comment:93

Typo. s/R_INSTALL_MOVED/RINSTALL_MOVED/g

EmmanuelCharpentier · 2015-05-05T06:31:23Z

comment:94

6.7beta3+f451e9a builds and passes make ptestlong on Debian testing 64 bits.

I'd rather have corroborating evidence on Mac OS X and Linux 32 bits before setting "positive_review" (and I won't set it since I am one of the authors...).

nexttime · 2015-05-06T06:28:41Z

comment:95

One day^TM we should also really

rm -rf "${TMPDIR:-/tmp}"/Rtmp*

after building (or installing) R.

EmmanuelCharpentier · 2015-05-08T18:35:30Z

comment:96

6.7beta4+f451e9a builds and passes "make ptestlong" on Debian testing amd64, albeit with one failure :
sage -t --long --warn-long 66.8 src/sage/interfaces/expect.py # 1 doctest failed
This doctest succeeds when presented alone:

charpent@asus16-ec:/usr/local/sage-6.7$ sage -t --long --warn-long 66.8 src/sage/interfaces/expect.py
Running doctests with ID 2015-05-08-20-24-57-6dd95266.
Git branch: mabranche
Doctesting 1 file.
sage -t --long --warn-long 66.8 src/sage/interfaces/expect.py
    [77 tests, 5.90 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 5.9 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 5.9 seconds

...and is known to be triggered by a loaded machine. To I'd give it a "positive_review" if

I wasn't one of the authors, and
I' had corroboration on x86 and Mac.

Any takers ? François, any hope in seeing this tested on your Mac ? Volker, any news on the x86 front ?

kiwifb · 2015-05-08T23:33:21Z

comment:97

I am looking at it right now from scratch with 6.7.beta4. Silly stuff, because this doesn't depend on #18156 switching to this branch removes the fix to gcc from 6.7.beta4. So I lost a couple of hours of building while I was doing the week end shopping.

kiwifb · 2015-05-09T04:08:24Z

comment:98

Send it to the bots...

EmmanuelCharpentier · 2015-05-09T05:43:53Z

comment:99

Replying to @kiwifb:

Send it to the bots...

Thank you, François ! Any news from the x86 front ?

kiwifb · 2015-05-09T06:15:31Z

Changed reviewer from Vincent Delecroix to Vincent Delecroix, François Bissey, Leif Leonhardy

kiwifb · 2015-05-09T06:15:31Z

comment:100

Not as such, but I am fairly sure leif solved the bzip2 problem that was cropping up randomly. I can confirm the fix works on several platform. I am personally satisfied that this as been vetted beyond the call of duty. If there are any more issues the bots will tell us. Fill free to edit the authors and reviewer fields as you see fit.

vbraun · 2015-05-12T20:25:36Z

Changed dependencies from #18254 to #18254, #15642

vbraun · 2015-05-14T22:40:13Z

Changed branch from u/leif/R_upgrade_with_new_patch to f451e9a

EmmanuelCharpentier mannequin added this to the sage-6.7 milestone Apr 16, 2015

EmmanuelCharpentier mannequin added c: packages: standard labels Apr 16, 2015

This comment has been minimized.

Sign in to view

EmmanuelCharpentier mannequin added the s: needs review label Apr 16, 2015

EmmanuelCharpentier mannequin self-assigned this Apr 16, 2015

EmmanuelCharpentier mannequin changed the title ~~Upgrade R to 3.1.3~~ Upgrade R to 3.2.0 Apr 17, 2015

EmmanuelCharpentier mannequin added s: needs work and removed s: needs review labels Apr 17, 2015

EmmanuelCharpentier mannequin added s: needs review and removed s: needs work labels Apr 17, 2015

kiwifb added s: positive review and removed s: needs review labels May 9, 2015

vbraun removed the s: positive review label May 14, 2015

vbraun closed this as completed in 79bea43 May 14, 2015

EmmanuelCharpentier mannequin mentioned this issue Apr 30, 2015

Sphinx tarball needs renaming #18341

Closed

Upgrade R to 3.2.0 #18229

Upgrade R to 3.2.0 #18229

Comments

EmmanuelCharpentier mannequin commented Apr 16, 2015

EmmanuelCharpentier mannequin commented Apr 16, 2015

This comment has been minimized.

EmmanuelCharpentier mannequin commented Apr 16, 2015

EmmanuelCharpentier mannequin commented Apr 16, 2015

kcrisman commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

kiwifb commented Apr 17, 2015

sagetrac-git mannequin commented Apr 17, 2015

sagetrac-git mannequin commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

kiwifb commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

kcrisman commented Apr 17, 2015

kcrisman commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

kiwifb commented Apr 17, 2015

EmmanuelCharpentier mannequin commented Apr 17, 2015

kiwifb commented Apr 17, 2015

EmmanuelCharpentier mannequin commented May 4, 2015

EmmanuelCharpentier mannequin commented May 4, 2015

kiwifb commented May 4, 2015

jhpalmieri commented May 4, 2015

kiwifb commented May 4, 2015

jhpalmieri commented May 4, 2015

nexttime mannequin commented May 4, 2015

nexttime mannequin commented May 4, 2015

jhpalmieri commented May 5, 2015

kiwifb commented May 5, 2015

sagetrac-git mannequin commented May 5, 2015

sagetrac-git mannequin commented May 5, 2015

jdemeyer commented May 5, 2015

nexttime mannequin commented May 5, 2015

nexttime mannequin commented May 5, 2015

EmmanuelCharpentier mannequin commented May 5, 2015

nexttime mannequin commented May 6, 2015

EmmanuelCharpentier mannequin commented May 8, 2015

kiwifb commented May 8, 2015

kiwifb commented May 9, 2015

EmmanuelCharpentier mannequin commented May 9, 2015

kiwifb commented May 9, 2015

kiwifb commented May 9, 2015

vbraun commented May 12, 2015

vbraun commented May 14, 2015