Opened 6 years ago
Last modified 4 years ago
#20523 closed enhancement
Upgrade R to 3.3.2 — at Version 49
Reported by: | charpent | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-7.6 |
Component: | packages: standard | Keywords: | r-project, days85 |
Cc: | tscrim, embray, gouezel, jpflori | Merged in: | |
Authors: | Jean-Pierre Flori, Emmanuel Charpentier | Reviewers: | Emmanuel Charpentier |
Report Upstream: | N/A | Work issues: | curl openssl |
Branch: | public/r311 (Commits, GitHub, GitLab) | Commit: | 3b1175391e19a01d40d2c114b706f0fe1751ac84 |
Dependencies: | #21744 #22189 | Stopgaps: |
Description (last modified by )
Change History (49)
comment:1 Changed 6 years ago by
- Cc tscrim embray gouezel jpflori added
comment:2 Changed 6 years ago by
What is the best process for testing a new dependency update?
comment:3 Changed 6 years ago by
I have to forfeit on this one, or at least delay for one eek minimum (RealLife? overflow + ill health).
BTW, it seems that some of our current patches were created only for cygwin environment : I am unable to retrieve their necessity. Could their authors mtell me if they encountered their use in a Unix (Linux) environment ?
Sorry...
comment:4 Changed 6 years ago by
It seems that only the large_address_aware
patch is cygwin-only, upstream report:
The other patches look general purpose.
comment:5 Changed 6 years ago by
- Description modified (diff)
- Summary changed from Upgrade R to 3.3.0 to Upgrade R to 3.3.1
I've got some nonmaskable RealLife? interruptions, seriously interfering with my plans. Sorry for the delay.
I updated the description to reflect the current plans.
comment:6 Changed 6 years ago by
- Branch set to u/charpent/upgrade_r_to_3_3_1
comment:7 Changed 6 years ago by
- Commit set to 336ee16dba727bc77fe76ef924f085ecc3a8a2c3
- Status changed from new to needs_review
The proposed patch seems valid on Linux : it passes ptestlong with errors in two doctests :
---------------------------------------------------------------------- sage -t --long --warn-long 109.1 src/sage/homology/simplicial_complex.py # 1 doctest failed sage -t --long --warn-long 109.1 src/sage/misc/trace.py # 1 doctest failed ----------------------------------------------------------------------
which both pass when run standalone :
charpent@asus16-ec:/usr/local/sage-7$ sage -t --long --warn-long 109.1 src/sage/misc/trace.py Running doctests with ID 2016-08-20-12-45-47-583c0f71. Git branch: t/20523/upgrade_r_to_3_3_1 Using --optional=database_gap,mpir,python2,sage,sage_mode Doctesting 1 file. sage -t --long --warn-long 109.1 src/sage/misc/trace.py [9 tests, 1.79 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 1.8 seconds cpu time: 0.3 seconds cumulative wall time: 1.8 seconds charpent@asus16-ec:/usr/local/sage-7$ sage -t --long --warn-long 109.1 src/sage/homology/simplicial_complex.py Running doctests with ID 2016-08-20-12-46-08-790dac8d. Git branch: t/20523/upgrade_r_to_3_3_1 Using --optional=database_gap,mpir,python2,sage,sage_mode Doctesting 1 file. sage -t --long --warn-long 109.1 src/sage/homology/simplicial_complex.py [588 tests, 6.39 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 6.5 seconds cpu time: 3.0 seconds cumulative wall time: 6.4 seconds
I have had this kind of hiccups since about 7.2 ; reports (on sage-release, mainly) did not alarm the release maintainers ; one of them has generated a tick.
Note that the large_address_aware
has been lost : R 3.3.1 no longer has cygwin-specific sections in configure.ac or Makefile.in (and therefore nothing to patch) ; maybe the cygwin-relevant problems have been fixed upstream ?
I don't have a Windows machine to port Sage to ; I leave this problem to people better(?) equipped than me.
New commits:
336ee16 | Upgrade R to 3.3.1 (correct on Linux).
|
comment:8 Changed 6 years ago by
On the other hand, I found this in the current R Installation and Administration Manual :
C.8 Cygwin
The 32-bit version has never worked well enough to pass R’s make check, and residual support from earlier experiments was removed in R 3.3.0.
The 64-bit version is completely unsupported.
Maybe we should consider to have rather an interface to system's R and make it an optional package ?
I'll broach this subject open on sage-devel (not r-devel, as I wrote incorrectly the first time...).
comment:9 follow-up: ↓ 10 Changed 6 years ago by
I get a failure trying to install this on Cygwin32 with
checking for lzma_version_number in -llzma... no configure: error: "liblzma library and headers are required" Error configuring R.
which is the same failure I had on #20190.
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to tscrim:
I get a failure trying to install this on Cygwin32 with
checking for lzma_version_number in -llzma... no configure: error: "liblzma library and headers are required" Error configuring R.which is the same failure I had on #20190.
From the R 3.3.0 release notes :
The previously included versions of zlib, bzip2, xz and PCRE have been removed, so suitable external (usually system) versions are required (see the ‘R Installation and Administration’ manual).
So it's either :
- add xz-tools (né lzma-utils) to the list of prerequisites for Sage (i. e. relying on system's liblzma) ;
- adding xz-tools (at least the lzma library) to sage (cluttering it with yet one more library to maintain) ;
- or relying on system's R through an interface that remains to be written...
Pick your poison...
NB : I also post this on sage-devel in the recent R thread...
comment:11 follow-up: ↓ 12 Changed 6 years ago by
On Cygwin I needed to apt-cyg install liblzma-devel
(if you have apt-cyg
, otherwise use setup as normal). Without the -devel
package it doesn't install the import library, so -llzma
doesn't work (-llzma.dll
should though).
comment:12 in reply to: ↑ 11 Changed 6 years ago by
Replying to embray:
On Cygwin I needed to
apt-cyg install liblzma-devel
(if you haveapt-cyg
, otherwise use setup as normal). Without the-devel
package it doesn't install the import library, so-llzma
doesn't work (-llzma.dll
should though).
Okay. I just checked on my (Linux) system : xz is not installed in the $(SAGE_ROOT) tree, probably because I have it installed at system level and the build script detects that somehow. (BTW : it is an *optional* package.
Therefore, what is needed is :
- ensure that xz and its development files are installed, either in the system or in the Sage package) ;
- that this is done before the R installation begins ; and
- that R installation depends on xz being somehow available.
This can be simplified, at the expense of making xz a standard package, just by ensuring that xz is installed before R compilation.
I have little idea on how to proceed, since the Sage build system is still a (large) bit obscure to me. What I need is a way to check in the Makefile that xz and its development files are available, install xz if not (can an optional package be installed before the compilation of standard packages is finished ?), and make the R compilation dependent on this target.
Any ideas ?
(BTW : Cc on the sage-devel thread for advice...)
comment:13 Changed 6 years ago by
You IMHO shouldn't change patches which haven't changed at all, that's just confusing. I wouldn't even touch patches where just the offset changed, not the context.
comment:14 Changed 6 years ago by
I'm working on this, especially on Cygwin as I resetuped a Cygwin install for other tickets at SD75.
comment:15 Changed 6 years ago by
Please see this post on sage-devel
's discussion of this topic. I need comments from people not as concerned as we are by Sage's R's future...
Jean-Pierre : did you consider repackaging (bnew style) the old pcre package ?
comment:16 Changed 6 years ago by
Nope I did not yet. I mostly made a clean set of patches.
comment:17 Changed 6 years ago by
- Branch changed from u/charpent/upgrade_r_to_3_3_1 to public/r311
- Commit changed from 336ee16dba727bc77fe76ef924f085ecc3a8a2c3 to 2b7f6f41301b48d54d80975f617c9154d649070d
I updated the set of patches.
I also slightly modiifed the options passed to configure
in spkg-install
.
We still need to deal with the new dependencies.
It seemed to me that on top of libpcre
, R
also depends on libcurl
...
New commits:
2b7f6f4 | Update patches set for R upgrade.
|
comment:18 Changed 6 years ago by
libcurl
is needed according to:
comment:19 follow-ups: ↓ 20 ↓ 22 Changed 6 years ago by
The Cygwin patches to main/Makefile.in
as well as nmath/standalone/Makefile.in
, and etc/Makeconf.in
have effects on non-Cygwin as well.
In the first two, the folder is changed unconditionally, the third may lead to overlinking on non-Cygwin.
I also don't like removing the documentation from SPKG.txt
; at the very least, the patch descriptions should get copied into the patches. Renaming them IMHO isn't necessary either.
The patches have funny permissions; while you chmoded some a-x, the new ones again have u=rwx and go=rx.
In spkg-install
, the lines with options to configure aren't consistently indented (in the diff at least => tabs vs. spaces used).
comment:20 in reply to: ↑ 19 Changed 6 years ago by
Replying to leif:
In
spkg-install
, the lines with options to configure aren't consistently indented.
... and --enable-R-shlib
is redundant, as it has already been added to R_CONFIGURE
a couple of lines above.
comment:21 Changed 6 years ago by
I'll take your comments into account or gve some justification here.
comment:22 in reply to: ↑ 19 Changed 6 years ago by
Replying to leif:
The Cygwin patches to
main/Makefile.in
as well asnmath/standalone/Makefile.in
, andetc/Makeconf.in
have effects on non-Cygwin as well.
In the first two, the folder is changed unconditionally, the third may lead to overlinking on non-Cygwin.
Well spotted, I did not pay attention on that part.
From what I saw there was underlinking in all cases, I don't see how Cygwin or non-Cygwin can change anything but I'll double check.
Anyway I'll only apply this two only coniditonally.
I also don't like removing the documentation from
SPKG.txt
; at the very least, the patch descriptions should get copied into the patches. Renaming them IMHO isn't necessary either.
From what I remember there seemed to be a consensus on putting doc in patches themselves to modify a minimum set of files when adding/removing patches. Although each patch now has some info, I'll add more from what was removed from SPKG.txt.
The patches have funny permissions; while you chmoded some a-x, the new ones again have u=rwx and go=rx.
Windows funky choice, I'll fix it.
In
spkg-install
, the lines with options to configure aren't consistently indented (in the diff at least => tabs vs. spaces used).
I removed all tabs.
comment:23 Changed 6 years ago by
- Commit changed from 2b7f6f41301b48d54d80975f617c9154d649070d to c7f359ee01eee93900a8e309197bfcb65ae19b68
Branch pushed to git repo; I updated commit sha1. New commits:
893d3cb | Remove duplicated "--enable-R-shlib".
|
a79ccf1 | Replace tabs by spaces in R spkg-install.
|
f37aedc | Apply some R patches only on Cygwin.
|
0b79739 | Fix permission of R patches.
|
11d338e | Make description of patches more verbose.
|
c7f359e | Fix SAGE_BUILDING_R patch.
|
comment:24 follow-up: ↓ 25 Changed 6 years ago by
On top of 6.4beta6+#21622 (now necessary to build Sage on Debian), this patch passes ptestlong
with two failures :
---------------------------------------------------------------------- sage -t --long --warn-long 111.2 src/sage/homology/simplicial_complex.py # 1 doctest failed sage -t --long --warn-long 111.2 src/sage/libs/gap/assigned_names.py # 1 doctest failed ----------------------------------------------------------------------
Both of them pass standalone. This kind of failure is a known, iterative (and annoying) problem, probably pertaining to the test infrastructure rather than to your patch.
I am sorely tempted to set this to positive_review
, but I think that thios must be tested by someone testing it on Cygwin, given the changes in the R toolset on Windows.
Let me know if you think this is sufficient.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 6 years ago by
Replying to charpent:
On top of 6.4beta6+#21622 (now necessary to build Sage on Debian), this patch passes
ptestlong
with two failures :---------------------------------------------------------------------- sage -t --long --warn-long 111.2 src/sage/homology/simplicial_complex.py # 1 doctest failed sage -t --long --warn-long 111.2 src/sage/libs/gap/assigned_names.py # 1 doctest failed ----------------------------------------------------------------------Both of them pass standalone. This kind of failure is a known, iterative (and annoying) problem, probably pertaining to the test infrastructure rather than to your patch.
I am sorely tempted to set this to
positive_review
, but I think that thios must be tested by someone testing it on Cygwin, given the changes in the R toolset on Windows.
I tested that R with the patches shipped here buids on Cygwin, though not from a Sage env.
Let me know if you think this is sufficient.
IMHO this is largely sufficient as far as Cygwin is concerned. If by any chance there are some residual side effects on Cygwin they be fixed on follow up tickets.
More of a concern is the dependency on new packages... Can we list them here and I can try to package them:
- libpcre
- libcurl
- anything else?
comment:26 in reply to: ↑ 25 Changed 6 years ago by
- Reviewers set to Emmanuel Charpentier
- Status changed from needs_review to positive_review
Replying to jpflori:
[ ... ]
IMHO this is largely sufficient as far as Cygwin is concerned. If by any chance there are some residual side effects on Cygwin they be fixed on follow up tickets.
Okay : positive review
. BTW, I think that you are the main (first) author...
More of a concern is the dependency on new packages... Can we list them here and I can try to package them:
- libpcre
- libcurl
- anything else?
- xz-tools.
Possible strategy :
1) For now, list them as supplementary dependencies in the main README.TXT.
2) Create a set of tickets to include them in Sage. These packages shuld test for existim system functionality and, if found, do nothong (how to achieve this in a Makefile ?)
3) When those tickets are tested and positively reviewed, make them standard packages, and prerequisites for R. This pulls them off the list of prerequisites.
This way, we can buy time...
BTW : Curl and xz-tools are tools of sufficient "general" interest that we should package the whole things (i. e. binaries and docs (but maybe not test utilities ?)), not just the libraries. And add the binaries to the list of candidates for small_scripts
making them accessible systemwide.
comment:27 Changed 6 years ago by
- Status changed from positive_review to needs_work
Hard dependency on lzma
checking for lzma_version_number in -llzma... no configure: error: "liblzma library and headers are required" Error configuring R.
comment:28 Changed 6 years ago by
- Dependencies set to #21744
lzma
is already available through the optional xz
package. I opened #21744 to this order.
Trivial patch to follow...
comment:29 Changed 6 years ago by
- Commit changed from c7f359ee01eee93900a8e309197bfcb65ae19b68 to 2875b8b62a813dc25b6ea23cd7af1012bd5e4300
Branch pushed to git repo; I updated commit sha1. New commits:
2875b8b | Three-keystroke patch ; depend on xz.
|
comment:30 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:31 Changed 6 years ago by
- Status changed from needs_review to needs_work
- Work issues set to curl openssl
We need to clarify the curl and openssl issues.
comment:32 follow-up: ↓ 33 Changed 6 years ago by
The good news is that R compiles against an ssl-free libcurl when configure is hacked. And it seems that nothing that fails in the test suite is related to this lacking feature.
Not sure how it affects usability though.
comment:33 in reply to: ↑ 32 Changed 6 years ago by
Replying to jpflori:
Not sure how it affects usability though.
Hah ! Most R repositories are switching to https:... addresses (this shown where trying to install a package without having defined a repository first : the graphical interface propose firs a list of repositories with https: addresses ; the only way to reach an http: repository is to click on the last item in that menu, which gives another list of http: repositories.
The move to such repositories parallels a similar move from various distros (starting with Debian...). I doubt that this will get reversed, and I think that, one way or another, Sage's R must keep the ability to use https:
This is true (with aggravation) of other uses of networking in applied statistics : access to remote databases is more and more frequent, and I do not see these uses accept insecure channels for a long time...
comment:34 Changed 6 years ago by
- Description modified (diff)
- Milestone changed from sage-7.2 to sage-7.4
- Summary changed from Upgrade R to 3.3.1 to Upgrade R to 3.3.2
Changing target to 3.3.2. Meanwhile, experimenting.
Still waiting for advice (see sage-devel's sagas)...
comment:35 follow-up: ↓ 36 Changed 6 years ago by
- Commit changed from 2875b8b62a813dc25b6ea23cd7af1012bd5e4300 to 4c896a9920225e37c5aff5f12beb67d809df21d1
comment:36 in reply to: ↑ 35 Changed 6 years ago by
Minimal adjustments (package version + checksum) + fixed a couple of failing doctests due to R's change of representation of function source code.
These doctests might be usefully augmented by a doctest of a real R function. This doesn't seem to have ever been doctested. Advice ?
The resulting Sage (after merging with 7.5beta4) passes ptestlong with two transient failures (which pass with no errors when ran standalone).
The question of the new dependencies is still open (no answer to sage-devel's questions).
comment:37 Changed 5 years ago by
- Commit changed from 4c896a9920225e37c5aff5f12beb67d809df21d1 to 8a05b10b95ddf9a9e7edbf5ab45f77e1eecdf8c5
Branch pushed to git repo; I updated commit sha1. New commits:
8a05b10 | Depend on PCRE
|
comment:38 Changed 5 years ago by
- Commit changed from 8a05b10b95ddf9a9e7edbf5ab45f77e1eecdf8c5 to 2568ce5a909b6eb8784277538f7e32f9ab5ccc34
Branch pushed to git repo; I updated commit sha1. New commits:
2568ce5 | Update spkg-install to conform to #20692.
|
comment:39 Changed 5 years ago by
- Commit changed from 2568ce5a909b6eb8784277538f7e32f9ab5ccc34 to a06fc11b9ea201d9b057e4ae9e840f77ce906087
Branch pushed to git repo; I updated commit sha1. New commits:
a06fc11 | R-shlib is needed in configurefor the (standard) rpy2 package.
|
comment:40 Changed 5 years ago by
- Commit changed from a06fc11b9ea201d9b057e4ae9e840f77ce906087 to f0ab39e3ec0b615e2298abfd5dc08c774d532132
Branch pushed to git repo; I updated commit sha1. New commits:
f0ab39e | Added curl to dependencies.
|
comment:41 Changed 5 years ago by
- Dependencies changed from #21744 to #21744 #21767 #21770 #22189
- Status changed from needs_work to needs_review
Sage-7.6.beta4 + #20523 (this patch) + #21767 + #21770 passes ptestlong witht two transient errors :
---------------------------------------------------------------------- sage -t --long src/sage/homology/simplicial_set_morphism.py # Bad exit: 2 sage -t --long src/sage/homology/simplicial_complex.py # 1 doctest failed ----------------------------------------------------------------------
Both tests pass when run standalone.
This should close the R saga (for now) by allowing the current interfaces to work with R >= 3.3.x.
==>needs_review
comment:42 Changed 5 years ago by
the branch is red - you need a rebase or so...
comment:43 Changed 5 years ago by
- Commit changed from f0ab39e3ec0b615e2298abfd5dc08c774d532132 to d8736709ee4e7e914e4c289a885ad7abefa829b7
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
c1aba6f | Fix permission of R patches.
|
b05ec4b | Make description of patches more verbose.
|
3b58ad0 | Fix SAGE_BUILDING_R patch.
|
fecbfc0 | Three-keystroke patch ; depend on xz.
|
3a4eb9c | Updating R source to 3.3.2.
|
55a8aa4 | R's strig representation of source code has changed. Updated docstrings accordingly.
|
00fcc86 | Depend on PCRE
|
23f9471 | Update spkg-install to conform to #20692.
|
22974b8 | R-shlib is needed in configurefor the (standard) rpy2 package.
|
d873670 | Added curl to dependencies.
|
comment:44 Changed 5 years ago by
Rebased on develop at 7.6.beta4
comment:45 follow-up: ↓ 46 Changed 5 years ago by
A branch dependent on unmerged/under review tickets ought to include all of them, otherwise you leave it to the reviewer to do merges that might potentially be nontrivial.
comment:46 in reply to: ↑ 45 Changed 5 years ago by
Replying to dimpase:
A branch dependent on unmerged/under review tickets ought to include all of them, otherwise you leave it to the reviewer to do merges that might potentially be nontrivial.
If I understand you correctly,we should either :
- merge #21767 and #21770 into develop, then merge #20523 ; OR
- add the contents of #21767 an #21770 into #20523
- Is this correct ?
- What do you recommend ?
comment:47 Changed 5 years ago by
I don't see a difference. Essentially it is as follows.
git checkout -b newrrr develop git pull trac public/curl7510 git pull trac u/charpent/package_libpcre_as_a_standard_package git pull trac public/r311 git push trac --set-upstream HEAD:public/r311
(not sure if the last push
needs -f
or not).
It's also a question whether these 3 branches commute, so that these 3 pull
all go through without manual intervention.
comment:48 Changed 5 years ago by
- Commit changed from d8736709ee4e7e914e4c289a885ad7abefa829b7 to 3b1175391e19a01d40d2c114b706f0fe1751ac84
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
1588558 | Packaging polish.
|
81e3341 | Removed patches handling.
|
c02cb61 | Update Curl to 7.51.0.
|
d31c560 | Add a fat mode to curl.
|
e4e5665 | Upgraded curl to 7.52.1
|
43030c3 | Merge branch 'public/curl7510' of git://trac.sagemath.org/sage into newrrr
|
a8d77b5 | Initial packaging of pcre : unconditionnal installation.
|
21ce3e7 | Removed patches handling.
|
fb088b8 | Merge branch 'u/charpent/package_libpcre_as_a_standard_package' of git://trac.sagemath.org/sage into newrrr
|
3b11753 | Merge branch 'public/r311' of git://trac.sagemath.org/sage into newrrr
|
comment:49 Changed 5 years ago by
- Dependencies changed from #21744 #21767 #21770 #22189 to #21744 #22189
- Description modified (diff)
Done. Added tarballs for curl and pcre, dropped the corresponding ticket dependencies.
ptestlong
running.
Last 10 new commits:
1588558 | Packaging polish.
|
81e3341 | Removed patches handling.
|
c02cb61 | Update Curl to 7.51.0.
|
d31c560 | Add a fat mode to curl.
|
e4e5665 | Upgraded curl to 7.52.1
|
43030c3 | Merge branch 'public/curl7510' of git://trac.sagemath.org/sage into newrrr
|
a8d77b5 | Initial packaging of pcre : unconditionnal installation.
|
21ce3e7 | Removed patches handling.
|
fb088b8 | Merge branch 'u/charpent/package_libpcre_as_a_standard_package' of git://trac.sagemath.org/sage into newrrr
|
3b11753 | Merge branch 'public/r311' of git://trac.sagemath.org/sage into newrrr
|
cc-ing the Windows/Cygwin people.