Opened 5 years ago

Closed 5 years ago

#18229 closed enhancement (fixed)

Upgrade R to 3.2.0

Reported by: charpent Owned by: charpent
Priority: minor Milestone: sage-6.7
Component: packages: standard Keywords: r-project
Cc: Merged in:
Authors: Emmanuel Charpentier, Leif Leonhardy Reviewers: Vincent Delecroix, François Bissey, Leif Leonhardy
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: f451e9a (Commits) Commit: f451e9a5227d73fc5ab8719857a4d17aae129eb8
Dependencies: #18254, #15642 Stopgaps:

Description (last modified by leif)

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)

Attachments (1)

dont_lose_libz_libbz2_in_configure.patch (1.2 KB) - added by leif 5 years ago.
Tentative patch (to be put into build/pkgs/r/patches/)

Download all attachments as: .zip

Change History (103)

comment:1 Changed 5 years ago by charpent

  • Branch set to u/charpent/upgrade_r_to_3_1_3

comment:2 Changed 5 years ago by charpent

  • Commit set to b0f4e43a2f36cf349387a645cb6bd9bd6bc7d571
  • Description modified (diff)
  • Status changed from new to needs_review

comment:3 Changed 5 years ago by charpent

  • Owner changed from (none) to charpent

I forgot : passes ptestlong.

comment:4 Changed 5 years ago by kcrisman

Does plotting and so forth still work properly?

comment:5 Changed 5 years ago by charpent

  • Status changed from needs_review to needs_work
  • Summary changed from Upgrade R to 3.1.3 to Upgrade R to 3.2.0

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.

comment:6 Changed 5 years ago by fbissey

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.

comment:7 Changed 5 years ago by git

  • Commit changed from b0f4e43a2f36cf349387a645cb6bd9bd6bc7d571 to ab208a4e69834c786a2f0d4ff5a2626776db4d62

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

ab208a4Upgrade R to 3.2.0. passes ptestlong (All tests passed\!).

comment:8 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

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 ?

comment:9 Changed 5 years ago by fbissey

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?

comment:10 follow-ups: Changed 5 years ago by charpent

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,

comment:11 follow-up: Changed 5 years ago by 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).

comment:12 in reply to: ↑ 10 ; follow-up: Changed 5 years ago by 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.

comment:13 in reply to: ↑ 11 Changed 5 years ago by charpent

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,

comment:14 in reply to: ↑ 12 Changed 5 years ago by charpent

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,

comment:15 in reply to: ↑ 10 ; follow-up: Changed 5 years ago by fbissey

Replying to charpent:

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.

comment:16 in reply to: ↑ 15 ; follow-up: Changed 5 years ago by charpent

Replying to fbissey:

Replying to charpent:

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,

comment:17 Changed 5 years ago by fbissey

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

comment:18 in reply to: ↑ 16 Changed 5 years ago by fbissey

Replying to charpent:

Replying to fbissey:

Replying to charpent:

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 ?

Thinking more about it, I don't think it is strictly an appleism - not of the shell at any rate. I think the underlying file system may be case insensitive and just look case sensitive by hackery (not unlike FAT32 in some ways). In any case someone created checksums for Pillow and Sphinx...

comment:19 Changed 5 years ago by fbissey

I should have run this simple check before the last post. Fairly convincing

Mirage:upstream fbissey$ ls
Pillow-2.2.2.tar.gz			gd-2.0.35.tar.bz2			mpfi-1.5.1.tar.bz2			r-3.1.2.tar.gz
Sphinx-1.2.2.tar.gz			gdmodule-0.56.tar.bz2			mpfr-3.1.2.tar.bz2			ratpoints-2.1.3.tar.bz2
atlas-3.10.2.tar.bz2			gf2x-1.1.tar.bz2			mpir-2.6.0.tar.bz2			readline-6.3.008.tar.gz
boehm_gc-7.2d.tar.bz2			gfan-0.5.tar.bz2			mpmath-0.18.tar.gz			rpy2-2.3.8.tar.gz
boost_cropped-1.52.0.tar.bz2		git-2.1.2.tar.gz			ncurses-5.9.20131221.tar.bz2		rubiks-20070912.tar.bz2
bzip2-1.0.6.20140317.tar.gz		givaro-3.7.1.tar.bz2			networkx-1.8.1.tar.gz			sagenb-0.11.1.tar
cddlib-094g.tar.bz2			glpk-4.55.tar.gz			ntl-6.2.1.tar.bz2			sagetex-2.3.4.tar.bz2
cephes-2.8.tar.bz2			graphs-20120404.tar.bz2			numpy-1.8.1.tar.gz			scipy-0.14.0.tar.gz
cliquer-1.21.tar.bz2			gsl-1.16.tar.gz				palp-2.1.tar.bz2			scons-1.2.0.tar.bz2
combinatorial_designs-20140630.tar.bz2	iconv-1.14.tar.bz2			pari-2.7.1.tar.gz			setuptools-3.6.tar.gz
conway_polynomials-0.4.tar.bz2		iml-1.0.4.tar.bz2			pari_galdata-20080411.tar.bz2		singular-3.1.7p1.tar.bz2
cvxopt-1.1.7.tar.gz			ipython-2.3.0.tar.gz			pari_seadata_small-20090618.tar.bz2	six-1.4.1.tar.gz
cython-0.21.1.tar.gz			jinja2-2.5.5.tar.bz2			patch-2.7.1.tar.gz			sqlalchemy-0.5.8.tar.bz2
dateutil-2.2.tar.gz			jmol-14.2.4_2014.08.03.tar.bz2		pexpect-2.0.tar.bz2			sqlite-3.8.4.3.tar.bz2
docutils-0.12.tar.gz			lcalc-1.23.tar.bz2			pkgconf-0.9.7.tar.gz			symmetrica-2.0.tar.bz2
ecl-13.5.1.tar.bz2			libfplll-4.0.4.tar.bz2			pkgconfig-1.1.0.tar.gz			sympow-1.018.1.tar.bz2
eclib-20140921.tar.bz2			libgap-4.7.5.1.tar.gz			polybori-0.8.3.tar.bz2			sympy-0.7.5.tar.gz
ecm-6.4.4.tar.bz2			libpng-1.2.51.tar.gz			polytopes_db-20120220.tar.bz2		tachyon-0.98.9.tar.bz2
elliptic_curves-0.7.tar.bz2		linbox-1.3.2.tar.bz2			ppl-1.1.tar.bz2				tornado-3.1.1.tar.gz
fflas_ffpack-1.6.0.tar.bz2		lrcalc-1.1.7.tar.gz			pycrypto-2.6.1.tar.gz			zeromq-4.0.4.tar.gz
flint-2.4.4.tar.gz			m4ri-20140914.tar.gz			pygments-1.3.1.tar.bz2			zlib-1.2.8.tar.bz2
flintqs-20070817.tar.bz2		m4rie-20140914.tar.gz			pynac-0.3.2.tar.bz2			zn_poly-0.9.tar.bz2
freetype-2.5.2.tar.bz2			matplotlib-1.3.1.tar.gz			pyparsing-2.0.1.tar.gz
gap-4.7.5.tar.bz2			maxima-5.34.1.tar.bz2			python-2.7.8.tar.gz
gcc-4.9.2.tar.bz2			mpc-1.0.2.tar.gz			pyzmq-14.3.0.tar.gz
Mirage:upstream fbissey$ ll pillow-2.2.2.tar.gz
-rw-r--r--  1 fbissey  staff   2.0M 13 Oct  2014 pillow-2.2.2.tar.gz

comment:20 follow-up: Changed 5 years ago by charpent

François : If I follow you correctly, you conclude that Pillow, Sphinx and Mirage checksums can only be recreated on a Mac with a pseudo-case-sensitive filesystem ? If so I'd be tempted to call this a (serious) bug and file tickets against them.

OTOH, I wonder what has caused the "lower case only in source tarball names" rule to be added to the Sage byzantine build system. In other words, what would be broken if MixedCase? nAmEs were allowed on tarball names ? In yet other words, should we hack up sage-fix-pkg-checksums to allow it to work with mixed-case-named tarballs ?

HTH,

comment:21 in reply to: ↑ 20 ; follow-ups: Changed 5 years ago by fbissey

Replying to charpent:

François : If I follow you correctly, you conclude that Pillow, Sphinx and Mirage checksums can only be recreated on a Mac with a pseudo-case-sensitive filesystem ? If so I'd be tempted to call this a (serious) bug and file tickets against them.

That's what it looks like yes.

OTOH, I wonder what has caused the "lower case only in source tarball names" rule to be added to the Sage byzantine build system. In other words, what would be broken if MixedCase? nAmEs were allowed on tarball names ? In yet other words, should we hack up sage-fix-pkg-checksums to allow it to work with mixed-case-named tarballs ?

The build system obviously works fine, otherwise we would have tons of reports about Sphinx and Pillow by now. But sage-fix-pkg-checksums should be dealt with it is the most obvious piece that cannot cope with mixed/uppercase characters as far as we can see here.

comment:22 in reply to: ↑ 21 Changed 5 years ago by jdemeyer

Replying to fbissey:

But sage-fix-pkg-checksums should be dealt with it is the most obvious piece that cannot cope with mixed/uppercase characters as far as we can see here.

Obviously, sage-fix-pkg-checksums can deal with mixed case characters. It's just that the name in build/pkgs should match the tarball name. The tarball for build/pkgs/r must be named r-something, not R-something. It seems to be an unwritten(?) Sage convention that build/pkgs always uses a lower case name, so I recommend to use lower case also for the tarball name.

comment:23 follow-up: Changed 5 years ago by charpent

OK. It seems now that, whereas at least two Sage developers are eager to qibble over the case of tarballs' names, no one has (yet) opened a ticket for this issue, which is different from the issue at hand, which is R-3.2.0.

Since it is a different issue, I do not think it is wise to try to fix it incidently. I will not touch it in this ticket.

1) Should I open a ticket over the tarball case question ?

2) Could some kind soul review my proposed fix ?

3) Should I open a ticket over the issue of sex of angels in Sage ?

Status<-needs-no-dithering-over-side-issues.

comment:24 in reply to: ↑ 23 Changed 5 years ago by jdemeyer

Replying to charpent:

1) Should I open a ticket over the tarball case question ?

For me, there is no issue, so there shouldn't be a ticket.

comment:25 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

The download link points to r-3.1.3.tar.gz

The Author name should be filled in.

comment:26 Changed 5 years ago by jdemeyer

About the tarball link: I'm pretty sure that Volker will want a direct download link, not "open this webpage, then click on this" instruction.

comment:27 follow-up: Changed 5 years ago by charpent

  • Authors set to Emmanuel Charpentier
  • Description modified (diff)
  • Status changed from needs_work to needs_review

Sorry, I still don't have a server publicly available. Google driwe will have to serve...

comment:28 follow-up: Changed 5 years ago by vdelecroix

  • Description modified (diff)

I put the tarball on my webpage (and checked that the sha1 matches the upstream one).

Vincent

comment:29 in reply to: ↑ 28 Changed 5 years ago by vdelecroix

Replying to vdelecroix:

I put the tarball on my webpage (and checked that the sha1 matches the upstream one).

And it builds fine on my computer (Debian jessie 64 bits)

comment:30 Changed 5 years ago by charpent

FWIW, 6.7beta2 + this patch passes ptestlong with "All tests passed!".

comment:31 Changed 5 years ago by vdelecroix

  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

Let it go.

comment:32 Changed 5 years ago by jhpalmieri

For what it's worth, this fails on OS X 10.10.3. (So does the old version of R, but this one fails in a different way.) See #18254.

comment:33 follow-up: Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

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

comment:34 Changed 5 years ago by fbissey

Hum, it looks like libR.so has been underlinked

gcc -std=gnu99 -shared -fopenmp -L/home/buildslave-sage/slave/sage_git/build/local/lib/   -o libR.so CommandLineArgs.o Rdynload.o Renviron.o RNG.o agrep.o apply.o arithmetic.o array.o attrib.o bind.o builtin.o character.o coerce.o colors.o complex.o connections.o context.o cum.o dcf.o datetime.o debug.o deparse.o devices.o dotcode.o dounzip.o dstruct.o duplicate.o edit.o engine.o envir.o errors.o eval.o format.o gevents.o gram.o gram-ex.o graphics.o grep.o identical.o inlined.o inspect.o internet.o iosupport.o lapack.o list.o localecharset.o logic.o main.o mapply.o match.o memory.o names.o objects.o options.o paste.o platform.o plot.o plot3d.o plotmath.o print.o printarray.o printvector.o printutils.o qsort.o random.o raw.o registration.o relop.o rlocale.o saveload.o scan.o seq.o serialize.o sort.o source.o split.o sprintf.o startup.o subassign.o subscript.o subset.o summary.o sysutils.o times.o unique.o util.o version.o g_alab_her.o g_cntrlify.o g_fontdb.o g_her_glyph.o xxxpr.o   `ls ../unix/*.o ../appl/*.o ../nmath/*.o`   ../extra/pcre/libpcre.a ../extra/tre/libtre.a  ../extra/xz/liblzma.a   -lf77blas -latlas -lgfortran -lm -lquadmath   -lreadline  -lz -lrt -ldl -lm  

which would go unnoticed if R.bin was compiled with -lbzlib. It looks like bzip2 was properly detected at configuration so we should be using the one shipped with sage. I'd ask for the config.log but I am fairly certain I won't see anything abnormal there. Still there could be something in there that wasn't printed.

comment:35 Changed 5 years ago by jhpalmieri

My OSX config.log is here.

comment:36 Changed 5 years ago by fbissey

Nope, nothing as expected. Will have to try to debug the config logic later this evening on my own mac.

comment:37 in reply to: ↑ 27 Changed 5 years ago by leif

Replying to charpent:

Sorry, I still don't have a server publicly available. Google driwe will have to serve...

<OFFTOPIC>

Ads by Google

There's still https://spkg-upload.googlecode.com, but I don't know whether

"Use your Gmail account to email Minh Van Nguyen <mvngu putdothere name thefunnyat gmail putdothere com> to request uploading credentials."

still applies, i.e., whether Minh will respond [in reasonable time].

</OFFTOPIC>

comment:38 Changed 5 years ago by vbraun

Google's code hosting will close in 2016 so its not a good strategy...

comment:39 follow-up: Changed 5 years ago by fbissey

I was going to comment on that. Otherwise I can just upload it there, I got access. Come to that I probably can put it on the lmona.de server for you and be done with it.

comment:40 in reply to: ↑ 39 ; follow-up: Changed 5 years ago by buck

Moved from #18254:

Changed 16 hours ago by fbissey

--without-system-bzlib did the trick. I also noticed that with r-3.1.2 bzip2 was not properly detected during configuration on my mac and therefore the internal copy was used in that case too.

Changed 8 hours ago by jhpalmieri

That works for me, too. All tests pass.

comment:41 Changed 5 years ago by buck

  • Dependencies set to 18254

comment:42 Changed 5 years ago by leif

  • Dependencies changed from 18254 to #18254

comment:43 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

Note : https://spkg-upload.googlecode.com/ does not accept new uploads (as of last year).

Since #18254 is closed, this patch can be reviewed can be reviewed. Changing status accordingly.

comment:44 in reply to: ↑ 21 Changed 5 years ago by charpent

Replying to fbissey:

Replying to charpent:

François : If I follow you correctly, you conclude that Pillow, Sphinx and Mirage checksums can only be recreated on a Mac with a pseudo-case-sensitive filesystem ? If so I'd be tempted to call this a (serious) bug and file tickets against them.

That's what it looks like yes.

  • Pillow : contributed to #16759, which might be related.
  • Sphinx : created #18341.

HTH,

comment:45 in reply to: ↑ 40 Changed 5 years ago by leif

Replying to buck:

Moved from #18254:

fbissey:

--without-system-bzlib did the trick. I also noticed that with r-3.1.2 bzip2 was not properly detected during configuration on my mac and therefore the internal copy was used in that case too.

jhpalmieri:

That works for me, too. All tests pass.

So this means for MacOS X / Xcode (and only that?) we currently (a moving target anyway) would have to configure with --without-system-bzlib, or fix the linking issue in another way, on this ticket here, right?

Not using Sage's bzlib would be a temporary solution anyway:

"Building R using the included versions of zlib, bzip2, xz and PCRE is deprecated: these are frozen (bar essential bug-fixes) and will be removed for R 3.3.0."

(So Sage will finally include xz as well, such that we can use better compression for the upstream tarballs... :P )

comment:46 follow-up: Changed 5 years ago by fbissey

Speaking of detection of bzlib in R, I have another annoyance to signal. The test to check the version of bzlib in R's configure script is broken with gcc 5.1 - I noticed it in R-3.2.0 so this is current. A header is missing in the test program they compile. If upstream is serious about the removal, they will notice quickly.

comment:47 in reply to: ↑ 46 Changed 5 years ago by leif

  • Status changed from needs_review to needs_work
  • Work issues set to Fix detection of bzlib

Replying to fbissey:

Speaking of detection of bzlib in R, I have another annoyance to signal. The test to check the version of bzlib in R's configure script is broken with gcc 5.1 - I noticed it in R-3.2.0 so this is current.

You mean this? 8D

make[6]: Entering directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
gcc-5.1   -I../../src/extra/pcre -I../../src/extra  -I../../src/extra/xz/api -I. -I../../src/include -I../../src/include   -I../../src/nmath -DHAVE_CONFIG_H   -fopenmp -fpic  -g -O2   -c Rmain.c -o Rmain.o
gcc-5.1 -Wl,--export-dynamic -fopenmp  -L../../lib -L/data/Sage/release/devel/sage-6.7.beta3-gcc-5.1.0/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
Makefile:153: recipe for target 'R.bin' failed
make[6]: *** [R.bin] Error 1
make[6]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
Makefile:143: recipe for target 'R' failed
make[5]: *** [R] Error 2
make[5]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
Makefile:28: recipe for target 'R' failed
make[4]: *** [R] Error 1
make[4]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src'
Makefile:59: recipe for target 'R' failed
make[3]: *** [R] Error 1
make[3]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src'
Error building R.

comment:48 Changed 5 years ago by leif

Although I get:

...
checking for BZ2_bzlibVersion in -lbz2... yes
checking bzlib.h usability... yes
checking bzlib.h presence... yes
checking for bzlib.h... yes
checking if bzip2 version >= 1.0.6... yes
checking whether bzip2 support needs to be compiled... no
...
R is now configured for x86_64-unknown-linux-gnu
  ...
  External libraries:        readline, BLAS(ATLAS), LAPACK(generic), zlib, bzlib
  ...

comment:49 Changed 5 years ago by leif

And in config.log I have:

...
configure:34211: checking if bzip2 version >= 1.0.6
configure:34239: gcc-5.1 -o conftest -g -O2  -fpic    -L/data/Sage/release/devel/sage-6.7.beta3-gcc-5.1.0/local/lib/  conftest.c -lbz2 -lz -lrt -ldl -lm  >&5
conftest.c: In function 'main':
conftest.c:254:17: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     char *ver = BZ2_bzlibVersion();
                 ^
conftest.c:255:5: warning: implicit declaration of function 'exit' [-Wimplicit-function-declaration]
     exit(strcmp(ver, "1.0.6") < 0);
     ^
conftest.c:255:5: warning: incompatible implicit declaration of built-in function 'exit'
conftest.c:255:5: note: include '<stdlib.h>' or provide a declaration of 'exit'
conftest.c:255:10: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
     exit(strcmp(ver, "1.0.6") < 0);
          ^
configure:34239: $? = 0
configure:34239: ./conftest
configure:34239: $? = 0
configure:34256: result: yes
configure:34262: checking whether bzip2 support needs to be compiled
configure:34265: result: no
...

So detection apparently works for me.

comment:50 Changed 5 years ago by fbissey

I am surprised on my power7 machine I get

checking zlib.h usability... yes
checking zlib.h presence... yes
checking for zlib.h... yes
checking if zlib version >= 1.2.5... yes
checking whether zlib support needs to be compiled... no
checking for BZ2_bzlibVersion in -lbz2... yes
checking bzlib.h usability... yes
checking bzlib.h presence... yes
checking for bzlib.h... yes
checking if bzip2 version >= 1.0.6... no
checking whether bzip2 support needs to be compiled... yes

But looking closer I may have misdiagnosed it. I think on that system the wrong library may have been used in the test. That's a problem that has plagued the install of gcc 5.1 itself on that system. It doesn't happen with other version of gcc.

comment:51 Changed 5 years ago by leif

With R_CONFIGURE=--without-system-bzlib, I'm getting

gcc-5.1 -shared -fopenmp -L/data/Sage/release/devel/sage-6.7.beta3-gcc-5.1.0/local/lib/   -o libR.so CommandLineArgs.o Rdynload.o Renviron.o RNG.o agrep.o apply.o arithmetic.o array.o attrib.o bind.o builtin.o character.o coerce.o colors.o complex.o connections.o context.o cum.o dcf.o datetime.o debug.o deparse.o devices.o dotcode.o dounzip.o dstruct.o duplicate.o edit.o engine.o envir.o errors.o eval.o format.o gevents.o gram.o gram-ex.o graphics.o grep.o identical.o inlined.o inspect.o internet.o iosupport.o lapack.o list.o localecharset.o logic.o main.o mapply.o match.o memory.o names.o objects.o options.o paste.o platform.o plot.o plot3d.o plotmath.o print.o printarray.o printvector.o printutils.o qsort.o random.o raw.o registration.o relop.o rlocale.o saveload.o scan.o seq.o serialize.o sort.o source.o split.o sprintf.o startup.o subassign.o subscript.o subset.o summary.o sysutils.o times.o unique.o util.o version.o g_alab_her.o g_cntrlify.o g_fontdb.o g_her_glyph.o xxxpr.o   `ls ../unix/*.o ../appl/*.o ../nmath/*.o`  ../extra/bzip2/libbz2.a ../extra/pcre/libpcre.a ../extra/tre/libtre.a  ../extra/xz/liblzma.a   -lf77blas -latlas -lgfortran -lm -lquadmath   -lreadline  -lrt -ldl -lm  
make[4]: Entering directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
mkdir -p -- /tmp/sage-build-tmp/r-3.2.0.p0/src/bin/exec
mkdir -p -- /tmp/sage-build-tmp/r-3.2.0.p0/src/lib
make[4]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
make[3]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
make[3]: Entering directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
gcc-5.1  -I../../src/extra/bzip2 -I../../src/extra/pcre -I../../src/extra  -I../../src/extra/xz/api -I. -I../../src/include -I../../src/include   -I../../src/nmath -DHAVE_CONFIG_H   -fopenmp -fpic  -g -O2   -c Rmain.c -o Rmain.o
gcc-5.1 -Wl,--export-dynamic -fopenmp  -L../../lib -L/data/Sage/release/devel/sage-6.7.beta3-gcc-5.1.0/local/lib/  -o R.bin Rmain.o  -lR 
../../lib/libR.so: undefined reference to `inflateReset'
../../lib/libR.so: undefined reference to `inflateEnd'
../../lib/libR.so: undefined reference to `compress'
../../lib/libR.so: undefined reference to `deflate'
../../lib/libR.so: undefined reference to `inflateInit2_'
../../lib/libR.so: undefined reference to `inflate'
../../lib/libR.so: undefined reference to `crc32'
../../lib/libR.so: undefined reference to `deflateEnd'
../../lib/libR.so: undefined reference to `zlibVersion'
../../lib/libR.so: undefined reference to `deflateInit2_'
../../lib/libR.so: undefined reference to `uncompress'
collect2: error: ld returned 1 exit status
Makefile:153: recipe for target 'R.bin' failed
make[3]: *** [R.bin] Error 1
make[3]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
Makefile:143: recipe for target 'R' failed
make[2]: *** [R] Error 2
make[2]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src/main'
Makefile:28: recipe for target 'R' failed
make[1]: *** [R] Error 1
make[1]: Leaving directory '/tmp/sage-build-tmp/r-3.2.0.p0/src/src'
Makefile:59: recipe for target 'R' failed
make: *** [R] Error 1
Error building R.

comment:52 Changed 5 years ago by leif

(So there, just -lz was missing.)

Configuring with --without-system-bzlib --without-system-zlib, the build succeeds (still with GCC 5.1.0).

comment:53 follow-up: Changed 5 years ago by fbissey

Yes, wrong library and I now understand the root cause of some of my problems on that system courtesy of IBM and nothing to do with R upstream. Anyway there is something strange going on with bzlib but I have no problem using the internal copy here. That being said I may uncover another surprise from the same source, it would take me a bit of time to disentangle the bits that causes gcc to prefer static libraries on my system and rebuild it.

Very strange both zlib and bzlib, does bzlib depend on zlib somehow?

comment:54 follow-up: Changed 5 years ago by fbissey

Now that I have removed the interfering library from the system I can compile R with an external bzlib library and gcc 5.1.0 and I don't experience any of these troubles.

Still there are problems linking an external bzlib on OS X and gcc 5.1.0 - on some systems. This is very strange. Does the failed libR.so have an entry for libbz2 at all?

Last edited 5 years ago by fbissey (previous) (diff)

comment:55 in reply to: ↑ 53 Changed 5 years ago by leif

Replying to fbissey:

Very strange both zlib and bzlib, does bzlib depend on zlib somehow?

No.

I've so far not found anything suspicious in configure; the libs get added to LIBS if the "system" versions are used, so the failing magic must be somewhere in the Makefiles (or some R config thingy these use).

comment:56 in reply to: ↑ 54 Changed 5 years ago by leif

Replying to fbissey:

Still there are problems linking an external bzlib on OS X and gcc 5.1.0 - on some systems. This is very strange. Does the failed libR.so have an entry for libbz2 at all?

No, but for libz when I configure without anything special (using system libraries for both libz and libbz2):

Dynamic section at offset 0x3363f8 contains 36 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libf77blas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libatlas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libgfortran.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libquadmath.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libreadline.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libz.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [librt.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libgomp.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]

comment:57 Changed 5 years ago by leif

When configuring with just --without-system-bzlib libz vanishes as expected (regarding the missing symbols reported):

Dynamic section at offset 0x343978 contains 35 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libf77blas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libatlas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libgfortran.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libquadmath.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libreadline.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [librt.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libgomp.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]

comment:58 Changed 5 years ago by leif

(And they're missing in the command building libR.so, cf. the very long lines some posts above.)

comment:59 Changed 5 years ago by leif

In the Makefile (in src/src/main) of the last attempt, we have:

## use an explicit library: there might be an unsatisfactory -lz around
R_ZLIBS = # ../extra/zlib/libz.a
R_BZLIBS =  ../extra/bzip2/libbz2.a
R_PCRE =  ../extra/pcre/libpcre.a
R_TRE =  ../extra/tre/libtre.a
R_XDR = # ../extra/xdr/libxdr.a
R_XZ =   ../extra/xz/liblzma.a
R_LIBINTL = # ../extra/intl/libintl.a
R_TZONE = # ../extra/tzone/libtz.a

MAIN_LIBS = ../unix/libunix.a ../appl/libappl.a ../nmath/libnmath.a
MAIN_OBJS = `ls ../unix/*.o ../appl/*.o ../nmath/*.o`
EXTRA_STATIC_LIBS = \
  $(R_ZLIBS) $(R_BZLIBS) $(R_PCRE) $(R_TRE) $(R_XDR) $(R_XZ) $(R_LIBINTL) $(R_TZONE)
STATIC_LIBS = $(MAIN_LIBS) $(EXTRA_STATIC_LIBS)

EXTRA_LIBS = $(BLAS_LIBS) $(FLIBS) $(R_XTRA_LIBS)  $(READLINE_LIBS) $(LIBS)

R_binary = R.bin
R_bin_OBJECTS = Rmain.o #$(OBJECTS)
#R_bin_LDADD = $(MAIN_OBJS) $(EXTRA_STATIC_LIBS) $(EXTRA_LIBS)
## Linked against -lRblas becasue -lR is and otherwise ld complains.
R_bin_LDADD = -lR #-lRblas
R_bin_DEPENDENCIES =# libR.a # $(top_builddir)/etc/R.exp

libR_la = libR$(DYLIB_EXT)
libR_la_OBJECTS = $(OBJECTS)
libR_la_LIBADD =  $(MAIN_OBJS) $(EXTRA_STATIC_LIBS) $(EXTRA_LIBS)  # -Wl,-bE:$(top_builddir)/etc/R.exp
libR_la_DEPENDENCIES = $(STATIC_LIBS) $(R_TZONE)  # $(top_builddir)/etc/R.exp

so somehow LIBS got messed up (libz dropped) somewhere on the way.

comment:60 Changed 5 years ago by leif

In src/Makeconf, we already have just

## These are the libs which the final R.bin/libR is linked against.
## Many of these used to be optional: ICU libs still are.
## There may be platform-specific ones, e.g.
## -lrt -ldl on Linux, -lnsl -lsocket -lrt -lsunmath -ldl -liconv on Solaris.
LIBS =  -lrt -ldl -lm

comment:61 Changed 5 years ago by fbissey

You could check what you get for LIBS in config.log and config.status, I'll look at what is going on on OS X.

comment:62 Changed 5 years ago by leif

Yep, just found the "truncated" one in config.status, so it's apparentĺy a configure problem:

S["LIBS"]=" -lrt -ldl -lm"

comment:63 follow-up: Changed 5 years ago by fbissey

Final results goes in the top Makefile. Why does configure behaves that way on some system and not some other. Dissecting on OS X.

comment:64 in reply to: ↑ 63 Changed 5 years ago by leif

Replying to fbissey:

Why does configure behaves that way on some system and not some other. Dissecting on OS X.

Well, the behaviour presumably depends on the outcome of some other tests in configure, such that sometimes the accumulated LIBS get clobbered.

Btw., it's not GCC 5.1; same with GCC 4.9 here.

comment:65 follow-up: Changed 5 years ago by leif

Getting closer...

In the PCRE checks, LIBS gets restored from r_save_LIBS, but that's (in my case) an obsolete value, because r_save_LIBS didn't get set in that check (i.e., the conditions for saving and restoring aren't the same).

comment:66 in reply to: ↑ 65 Changed 5 years ago by leif

Replying to leif:

Getting closer...

In the PCRE checks, LIBS gets restored from r_save_LIBS, but that's (in my case) an obsolete value, because r_save_LIBS didn't get set in that check (i.e., the conditions for saving and restoring aren't the same).

I think it's this:

  • m4/R.m4

    old new  
    31073107#endif
    31083108}
    31093109]])], [r_cv_have_pcre810=yes], [r_cv_have_pcre810=no], [r_cv_have_pcre810=no])])
    3110 fi
    31113110if test "x${r_cv_have_pcre810}" != xyes; then
    31123111  have_pcre=no
    31133112  LIBS="${r_save_LIBS}"
    31143113fi
     3114fi
    31153115AC_MSG_CHECKING([whether PCRE support needs to be compiled])
    31163116if test "x${r_cv_have_pcre810}" = xyes; then
    31173117  AC_MSG_RESULT([no])

comment:67 Changed 5 years ago by leif

Corresponding patch to configure:

  • R-3.2.0/configure

    old new  
    3454334543fi
    3454434544{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $r_cv_have_pcre810" >&5
    3454534545$as_echo "$r_cv_have_pcre810" >&6; }
    34546 fi
    3454734546if test "x${r_cv_have_pcre810}" != xyes; then
    3454834547  have_pcre=no
    3454934548  LIBS="${r_save_LIBS}"
    3455034549fi
     34550fi
    3455134551{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether PCRE support needs to be compiled" >&5
    3455234552$as_echo_n "checking whether PCRE support needs to be compiled... " >&6; }
    3455334553if test "x${r_cv_have_pcre810}" = xyes; then

Works for me (so far outside of Sage). May work on MacOS X as well (omitting --without-system-*), but probably there'll be further problems when Sage's libs are used.

Changed 5 years ago by leif

Tentative patch (to be put into build/pkgs/r/patches/)

comment:68 Changed 5 years ago by leif

Attached patch of course works outside of Sage as well.

comment:69 Changed 5 years ago by leif

...
Successfully installed r-3.2.0.p0
...
$ readelf -d local/lib/R/lib/libR.so 

Dynamic section at offset 0x336418 contains 37 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libf77blas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libatlas.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libgfortran.so.3]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libquadmath.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libreadline.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libbz2.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libz.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [librt.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libgomp.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [ld-linux-x86-64.so.2]
...

comment:70 Changed 5 years ago by leif

  • Authors changed from Emmanuel Charpentier to Emmanuel Charpentier, Leif Leonhardy
  • Branch changed from u/charpent/upgrade_r_to_3_1_3 to u/leif/R_upgrade_with_new_patch
  • Commit changed from ab208a4e69834c786a2f0d4ff5a2626776db4d62 to bc2269e64ccb2f5fcc83cf62085eeeb53d7e1dcf
  • Status changed from needs_work to needs_review
  • Work issues Fix detection of bzlib deleted

New commits:

b0f4e43Upgrade R to 3.1.3. Passes ptestlong (All tests passed\!).
ab208a4Upgrade R to 3.2.0. passes ptestlong (All tests passed\!).
6685dddMerge branch 'u/charpent/upgrade_r_to_3_1_3' of trac.sagemath.org:sage into R_upgrade_18229
bc2269eAdd patch to avoid R 3.2.0's 'configure' losing libraries from LIBS (#18229)

comment:71 Changed 5 years ago by leif

  • Description modified (diff)

comment:72 Changed 5 years ago by leif

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

Patch now submitted upstream.

comment:73 Changed 5 years ago by jhpalmieri

On two different OS X 10.10 machines, one with Xcode 6.1, one with 6.3.1: R builds and passes its self tests, and Sage passes all tests with make ptestlong.

comment:74 follow-up: Changed 5 years ago by jhpalmieri

By the way, here is a typo in spkg-install. Can we fix it on this ticket?

  • build/pkgs/r/spkg-install

    diff --git a/build/pkgs/r/spkg-install b/build/pkgs/r/spkg-install
    index 06a340d..fb2f35a 100755
    a b if [ "$UNAME" = "Darwin" ]; then 
    161161    # On systems using LD_LIBRARY_PATH, both Sage and R modify it and R wins,
    162162    # so no problem should occur.
    163163    if [ -d "$SAGE_LOCAL"/lib/R ]; then
    164         RINSTALL_MOVED = yes
     164        RINSTALL_MOVED=yes
    165165        mv "$SAGE_LOCAL"/lib/R "$SAGE_LOCAL"/lib/R.old
    166166    fi
    167167fi
Last edited 5 years ago by jhpalmieri (previous) (diff)

comment:75 in reply to: ↑ 74 Changed 5 years ago by charpent

Replying to jhpalmieri:

By the way, here is a typo in spkg-install. Can we fix it on this ticket?

Go ahead if you care. I'll review it.

But I'm a little frightened : this is the third time that the port of a new version of R, which is a mindless affair, is hijacked for an unrelated bug (which, by the way, never gets repertoried under a proper heading !). Is filing a *different* bug that hard ?

[ Bandwidth savings ... ]

comment:76 Changed 5 years ago by fbissey

No it is not hard. I am guessing some people find it terribly inconvenient. I am guilty in indulging leif in continuing on this ticket.

comment:77 follow-up: Changed 5 years ago by leif

? 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).

comment:78 in reply to: ↑ 77 ; follow-up: Changed 5 years ago by charpent

Replying to leif:

? 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).

  1. This version introduced the bug : you're right, as far as I can tell. Your case seems to be a corner case.
  2. It's the same problem : no. This is, as far as I understand, a problem with the build infrastructure of Sage. The new R version just revealed a latent bug in a very loosely related part of this infrastructure.

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). 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...

Am I clearer ?

comment:79 follow-up: Changed 5 years ago by leif

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.

comment:80 in reply to: ↑ 33 ; follow-up: Changed 5 years ago by charpent

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

comment:81 in reply to: ↑ 79 Changed 5 years ago by charpent

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

Replying to leif:

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.

comment:82 Changed 5 years ago by fbissey

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).

comment:83 Changed 5 years ago by jhpalmieri

Replying to leif:

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 http://trac.sagemath.org/ticket/14706#comment:101, in which case, the second version should say DYLD_FALLBACK_LIBRARY_PATH.

comment:84 Changed 5 years ago by fbissey

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

comment:85 in reply to: ↑ 80 Changed 5 years ago by jhpalmieri

Replying to charpent:

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.

comment:86 in reply to: ↑ 78 Changed 5 years ago by leif

Replying to charpent:

Replying to leif:

? 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).

  1. 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?


  1. 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]

comment:87 Changed 5 years ago by leif

Last chance for further changes: ;-)

  • build/pkgs/r/spkg-install

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

comment:88 Changed 5 years ago by jhpalmieri

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

comment:89 Changed 5 years ago by fbissey

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

comment:90 follow-up: Changed 5 years ago by git

  • Commit changed from bc2269e64ccb2f5fcc83cf62085eeeb53d7e1dcf to f451e9a5227d73fc5ab8719857a4d17aae129eb8

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

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

comment:91 in reply to: ↑ 90 ; follow-up: Changed 5 years ago by jdemeyer

Replying to git:

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

f451e9aFix 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.

comment:92 in reply to: ↑ 91 Changed 5 years ago by leif

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

comment:93 Changed 5 years ago by leif

Typo. s/R_INSTALL_MOVED/RINSTALL_MOVED/g

comment:94 Changed 5 years ago by charpent

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...).

comment:95 Changed 5 years ago by leif

One dayTM we should also really

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

after building (or installing) R.

comment:96 Changed 5 years ago by charpent

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 1) I wasn't one of the authors, and 2) 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 ?

comment:97 Changed 5 years ago by fbissey

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.

comment:98 follow-up: Changed 5 years ago by fbissey

  • Status changed from needs_review to positive_review

Send it to the bots...

comment:99 in reply to: ↑ 98 Changed 5 years ago by charpent

Replying to fbissey:

Send it to the bots...

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

comment:100 Changed 5 years ago by fbissey

  • Reviewers changed from Vincent Delecroix to Vincent Delecroix, François Bissey, Leif Leonhardy

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.

comment:101 Changed 5 years ago by vbraun

  • Dependencies changed from #18254 to #18254, #15642

comment:102 Changed 5 years ago by vbraun

  • Branch changed from u/leif/R_upgrade_with_new_patch to f451e9a5227d73fc5ab8719857a4d17aae129eb8
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.