Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#20523 closed enhancement (fixed)

Upgrade R to 3.3.3

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, Dima Pasechnik
Report Upstream: N/A Work issues: curl openssl
Branch: f029f66 (Commits, GitHub, GitLab) Commit:
Dependencies: #21767, #21770 Stopgaps:

Status badges

Description (last modified by charpent)

Usual reason...

This one is a bit more complicated than usual : the Windows toolchain has changed ; however, I do not have a Windows system to test this.

Original tarballs :

Attachments (1)

libcurl_https_support.patch (3.0 KB) - added by jpflori 5 years ago.
Remove check for https support in curl.

Download all attachments as: .zip

Change History (126)

comment:1 Changed 5 years ago by tscrim

  • Cc tscrim embray gouezel jpflori added

cc-ing the Windows/Cygwin people.

comment:2 Changed 5 years ago by embray

What is the best process for testing a new dependency update?

comment:3 Changed 5 years ago by charpent

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 5 years ago by jpflori

It seems that only the large_address_aware patch is cygwin-only, upstream report:

The other patches look general purpose.

comment:5 Changed 5 years ago by charpent

  • 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 5 years ago by charpent

  • Branch set to u/charpent/upgrade_r_to_3_3_1

comment:7 Changed 5 years ago by charpent

  • 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:

336ee16Upgrade R to 3.3.1 (correct on Linux).

comment:8 Changed 5 years ago by charpent

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

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

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

comment:10 in reply to: ↑ 9 Changed 5 years ago by charpent

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: Changed 5 years ago by embray

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 5 years ago by charpent

Replying to embray:

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

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 :

  1. ensure that xz and its development files are installed, either in the system or in the Sage package) ;
  2. that this is done before the R installation begins ; and
  3. 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 5 years ago by leif

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 5 years ago by jpflori

I'm working on this, especially on Cygwin as I resetuped a Cygwin install for other tickets at SD75.

comment:15 Changed 5 years ago by charpent

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 5 years ago by jpflori

Nope I did not yet. I mostly made a clean set of patches.

comment:17 Changed 5 years ago by jpflori

  • 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:

2b7f6f4Update patches set for R upgrade.

comment:19 follow-ups: Changed 5 years ago by leif

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

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

comment:20 in reply to: ↑ 19 Changed 5 years ago by leif

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 5 years ago by jpflori

I'll take your comments into account or gve some justification here.

comment:22 in reply to: ↑ 19 Changed 5 years ago by jpflori

Replying to leif:

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.

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 5 years ago by git

  • Commit changed from 2b7f6f41301b48d54d80975f617c9154d649070d to c7f359ee01eee93900a8e309197bfcb65ae19b68

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

893d3cbRemove duplicated "--enable-R-shlib".
a79ccf1Replace tabs by spaces in R spkg-install.
f37aedcApply some R patches only on Cygwin.
0b79739Fix permission of R patches.
11d338eMake description of patches more verbose.
c7f359eFix SAGE_BUILDING_R patch.

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

Let me know if you think this is sufficient.

comment:25 in reply to: ↑ 24 ; follow-up: Changed 5 years ago by jpflori

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 5 years ago by charpent

  • Authors changed from Emmanuel Charpentier to Jean-Pierre Flori, Emmanuel Charpentier
  • 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 5 years ago by vbraun

  • 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 5 years ago by charpent

  • 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 5 years ago by git

  • Commit changed from c7f359ee01eee93900a8e309197bfcb65ae19b68 to 2875b8b62a813dc25b6ea23cd7af1012bd5e4300

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

2875b8bThree-keystroke patch ; depend on xz.

comment:30 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

#20523 (present patch) + #21744 pass ptestlong successfully over both 7.4 and 7.5beta0.

==>needs_review

Note : I suspect that we may have to do likewise for pcre and curl. I shall package them, and open tickets to get them included as standard packages as the need arises.

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

comment:31 Changed 5 years ago by jpflori

  • 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: Changed 5 years ago by jpflori

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 5 years ago by charpent

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 5 years ago by charpent

  • 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: Changed 5 years ago by git

  • Commit changed from 2875b8b62a813dc25b6ea23cd7af1012bd5e4300 to 4c896a9920225e37c5aff5f12beb67d809df21d1

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

6c197f6Updating R source to 3.3.2.
4c896a9R's strig representation of source code has changed. Updated docstrings accordingly.

comment:36 in reply to: ↑ 35 Changed 5 years ago by charpent

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 git

  • Commit changed from 4c896a9920225e37c5aff5f12beb67d809df21d1 to 8a05b10b95ddf9a9e7edbf5ab45f77e1eecdf8c5

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

8a05b10Depend on PCRE

comment:38 Changed 5 years ago by git

  • Commit changed from 8a05b10b95ddf9a9e7edbf5ab45f77e1eecdf8c5 to 2568ce5a909b6eb8784277538f7e32f9ab5ccc34

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

2568ce5Update spkg-install to conform to #20692.

comment:39 Changed 5 years ago by git

  • Commit changed from 2568ce5a909b6eb8784277538f7e32f9ab5ccc34 to a06fc11b9ea201d9b057e4ae9e840f77ce906087

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

a06fc11R-shlib is needed in configurefor the (standard) rpy2 package.

comment:40 Changed 5 years ago by git

  • Commit changed from a06fc11b9ea201d9b057e4ae9e840f77ce906087 to f0ab39e3ec0b615e2298abfd5dc08c774d532132

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

f0ab39eAdded curl to dependencies.

comment:41 Changed 5 years ago by charpent

  • 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 dimpase

the branch is red - you need a rebase or so...

comment:43 Changed 5 years ago by git

  • Commit changed from f0ab39e3ec0b615e2298abfd5dc08c774d532132 to d8736709ee4e7e914e4c289a885ad7abefa829b7

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

c1aba6fFix permission of R patches.
b05ec4bMake description of patches more verbose.
3b58ad0Fix SAGE_BUILDING_R patch.
fecbfc0Three-keystroke patch ; depend on xz.
3a4eb9cUpdating R source to 3.3.2.
55a8aa4R's strig representation of source code has changed. Updated docstrings accordingly.
00fcc86Depend on PCRE
23f9471Update spkg-install to conform to #20692.
22974b8R-shlib is needed in configurefor the (standard) rpy2 package.
d873670Added curl to dependencies.

comment:44 Changed 5 years ago by charpent

Rebased on develop at 7.6.beta4

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

comment:46 in reply to: ↑ 45 Changed 5 years ago by charpent

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 :

  1. merge #21767 and #21770 into develop, then merge #20523 ; OR
  2. add the contents of #21767 an #21770 into #20523
  1. Is this correct ?
  2. What do you recommend ?

comment:47 Changed 5 years ago by dimpase

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 git

  • Commit changed from d8736709ee4e7e914e4c289a885ad7abefa829b7 to 3b1175391e19a01d40d2c114b706f0fe1751ac84

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

1588558Packaging polish.
81e3341Removed patches handling.
c02cb61Update Curl to 7.51.0.
d31c560Add a fat mode to curl.
e4e5665Upgraded curl to 7.52.1
43030c3Merge branch 'public/curl7510' of git://trac.sagemath.org/sage into newrrr
a8d77b5Initial packaging of pcre : unconditionnal installation.
21ce3e7Removed patches handling.
fb088b8Merge branch 'u/charpent/package_libpcre_as_a_standard_package' of git://trac.sagemath.org/sage into newrrr
3b11753Merge branch 'public/r311' of git://trac.sagemath.org/sage into newrrr

comment:49 Changed 5 years ago by charpent

  • 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:

1588558Packaging polish.
81e3341Removed patches handling.
c02cb61Update Curl to 7.51.0.
d31c560Add a fat mode to curl.
e4e5665Upgraded curl to 7.52.1
43030c3Merge branch 'public/curl7510' of git://trac.sagemath.org/sage into newrrr
a8d77b5Initial packaging of pcre : unconditionnal installation.
21ce3e7Removed patches handling.
fb088b8Merge branch 'u/charpent/package_libpcre_as_a_standard_package' of git://trac.sagemath.org/sage into newrrr
3b11753Merge branch 'public/r311' of git://trac.sagemath.org/sage into newrrr

comment:50 Changed 5 years ago by charpent

Passes ptestlong with the usual :

----------------------------------------------------------------------
sage -t --long src/sage/homology/simplicial_complex.py  # 1 doctest failed
----------------------------------------------------------------------

which passes standalone.

==> Still needs_review

comment:51 follow-up: Changed 5 years ago by dimpase

Why have you removed #21767 and #21770 from dependencies? You have merged parts of them here, thus, certainly, they are dependencies, and should be reviewed before this one!

comment:52 in reply to: ↑ 51 Changed 5 years ago by charpent

Replying to dimpase:

Why have you removed #21767 and #21770 from dependencies?

Because any modification they bringed is made here.

You have merged parts

Parts ? I do not understand. Unless I'l seriously mistaken abut what git does, any modification resulting from these branches will be effected here.

of them here, thus, certainly, they are dependencies, and should be reviewed before this one!

Again, I do not understand : if any part of them is defective, it should be fixed, and this fix re-merged here. What is the point ?

These questions are not rhetorical : I really do not understand your point.

comment:53 Changed 5 years ago by charpent

I have asked for invalidation of #21767 and #21770 n their respective tickets (can't do that myself...).

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

They can (and should) be considered separate/independent issues deserving their own tickets. So I would reinstate them as dependencies here because they need to be handled first.

(PS: While I agree with Dima that it is a very good practice to merge in your dependencies, it is not strictly necessary.)

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

Replying to tscrim:

They can (and should) be considered separate/independent issues deserving their own tickets. So I would reinstate them as dependencies here because they need to be handled first.

<Sigh> Why didn't you say so first ... </Sigh>

(PS: While I agree with Dima that it is a very good practice to merge in your dependencies, it is not strictly necessary.)

I tried to follow the guidelines of the Developer's guide that aim to break a large issue in smaller, independent ones. Merging one ticket into another implies review of the same patches *twice*.

I didn't see Dima's point but understood the aim of consistency. Now, I am lost. Do you really want two reviews of the same code ?

I'll wait for your answer before cancelling my invalidation requests and re-adding the dependencies. After all, these tickets were ready for reviews only four and three months ago...

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

comment:56 in reply to: ↑ 55 ; follow-up: Changed 5 years ago by tscrim

Replying to charpent:

Replying to tscrim:

They can (and should) be considered separate/independent issues deserving their own tickets. So I would reinstate them as dependencies here because they need to be handled first.

<Sigh> Why didn't you say so first ... </Sigh>

Sorry I didn't chime in sooner.

(PS: While I agree with Dima that it is a very good practice to merge in your dependencies, it is not strictly necessary.)

I tried to follow the guidelines of the Developer's guide that aim to break a large issue in smaller, independent ones. Merging one ticket into another implies review of the same patches *twice*.

I didn't see Dima's point but understood the aim of consistency. Now, I am lost. Do you really want two reviews of the same code ?

It won't be two reviews of the same code. Only the difference from this code and the dependencies will need to be reviewed.

comment:57 in reply to: ↑ 56 Changed 5 years ago by charpent

  • Status changed from needs_review to needs_work

Replying to tscrim:

[ Snip... ]

I didn't see Dima's point but understood the aim of consistency. Now, I am lost. Do you really want two reviews of the same code ?

It won't be two reviews of the same code. Only the difference from this code and the dependencies will need to be reviewed.

I'm not sure to understand this either....

Okay. I'll reinstate dependencies, unmark my invalidation requests.

And fix a possible issue I just spotted (patches on Cygwin).

Marking this as needs_work for now.

comment:58 Changed 5 years ago by git

  • Commit changed from 3b1175391e19a01d40d2c114b706f0fe1751ac84 to 8e0761131da60b81249e559694659c78ccfcf6cc

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

8e07611Reinsert Cygwin-specific patches (lost in the mist of battle).

comment:59 Changed 5 years ago by charpent

  • Dependencies changed from #21744 #22189 to #21744 #21767 #21770 #22189
  • Status changed from needs_work to needs_review

needs_review again.


New commits:

8e07611Reinsert Cygwin-specific patches (lost in the mist of battle).

comment:60 Changed 5 years ago by charpent

  • Status changed from needs_review to needs_work

R 3.3.3 is out (and has been for one week...). I intend to update this ticket.

This shouldn't refrain potential reviewers to review #21767 an #21770, which do not seem to need an update.

comment:61 Changed 5 years ago by git

  • Commit changed from 8e0761131da60b81249e559694659c78ccfcf6cc to 516f0291f093af9415003b6b99c7d0ecb1ee8ac8

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

516f029Upgraded to R 3.3.3

comment:62 Changed 5 years ago by charpent

  • Description modified (diff)
  • Summary changed from Upgrade R to 3.3.2 to Upgrade R to 3.3.3

Compiles cleanly. Updated ticket title and tarball pointer.

Formal tests to come (on a decent machine). Tonight ?

comment:63 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

This version :

  • Compiles cleanly.
  • passes ptestlong with :
    ----------------------------------------------------------------------
    sage -t --long src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed
    sage -t --long src/sage/combinat/rigged_configurations/tensor_product_kr_tableaux_element.py  # Bad exit: 2
    sage -t --long src/sage/modular/arithgroup/arithgroup_generic.py  # Bad exit: 2
    ----------------------------------------------------------------------
    

The first one is a well-known difference of expression of the ideal of a multivariable polynopial, already subject of a ticket.

The last two are transient errors : these tests pass when ran standalone. Race conditions ?

  • Passes its own test suite.

==>needs_review.

comment:64 follow-up: Changed 5 years ago by dimpase

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

comment:65 follow-ups: Changed 5 years ago by dimpase

something funny is going on on OSX here, it builds with clang (OSX's "native" compiler) rather than with gcc. Is this a bug in our package, or this is on purpose?

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

Replying to dimpase:

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

as far as I understand, it's managed in R's main .configure script.

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

Replying to dimpase:

something funny is going on on OSX here, it builds with clang (OSX's "native" compiler) rather than with gcc. Is this a bug in our package, or this is on purpose?

I dunno. Being Macless, I can't test it. That may be under governance of R's configure.

As far as I understand, one might force CC to be Sage's gcc rather than the default clang (I do not know Macs enough to know if this is mandatory or even useful to run with Sage). If so, a Mac-specific patch to spkg-install should be (relatively) trivial.

ISTR that R has some Fortran libraries. What happens on Macs when clang is used ?

HTH,

comment:68 in reply to: ↑ 66 ; follow-up: Changed 5 years ago by dimpase

Replying to charpent:

Replying to dimpase:

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

as far as I understand, it's managed in R's main .configure script.

my question is rather how do I test that they work?

comment:69 in reply to: ↑ 67 Changed 5 years ago by dimpase

Replying to charpent:

Replying to dimpase:

something funny is going on on OSX here, it builds with clang (OSX's "native" compiler) rather than with gcc. Is this a bug in our package, or this is on purpose?

I dunno. Being Macless, I can't test it. That may be under governance of R's configure.

As far as I understand, one might force CC to be Sage's gcc rather than the default clang (I do not know Macs enough to know if this is mandatory or even useful to run with Sage). If so, a Mac-specific patch to spkg-install should be (relatively) trivial.

I think this should be fixed.

it might be problematic to use C++ libraries compiled with gcc with anything compiled with clang. While R itself does not need C++ for building, one can have an R extension that does, and this might lead to troubles...

ISTR that R has some Fortran libraries. What happens on Macs when clang is used ?

It uses gfortran for Fortran, still.

comment:70 in reply to: ↑ 65 ; follow-up: Changed 5 years ago by jhpalmieri

Replying to dimpase:

something funny is going on on OSX here, it builds with clang (OSX's "native" compiler) rather than with gcc. Is this a bug in our package, or this is on purpose?

From the spkg-install file:

    if [ $MACOSX_VERSION -ge 16 ]; then
        echo "OS X 10.$[$MACOSX_VERSION-4] Building with clang."
        CC=clang
    fi

This is from #21567.

comment:71 in reply to: ↑ 70 Changed 5 years ago by dimpase

Replying to jhpalmieri:

Replying to dimpase:

something funny is going on on OSX here, it builds with clang (OSX's "native" compiler) rather than with gcc. Is this a bug in our package, or this is on purpose?

This is from #21567.

OK, I see. I didn't follow this closely enough. No issue then here.

comment:72 in reply to: ↑ 68 ; follow-ups: Changed 5 years ago by charpent

Replying to dimpase:

Replying to charpent:

Replying to dimpase:

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

as far as I understand, it's managed in R's main .configure script.

my question is rather how do I test that they work?

From R, trivially :

This might be docscripted. Problems :

  • Find a file whose existence is certain
  • Mark the curl doctest as "internet", on order not to execute it in automated tests (that might run on an unconnected achine).
  • Find some nice pattern where the Perl-compatible matching *differs* from the standard one...

HTH,

comment:73 in reply to: ↑ 72 Changed 5 years ago by dimpase

Replying to charpent:

Replying to dimpase:

Replying to charpent:

Replying to dimpase:

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

as far as I understand, it's managed in R's main .configure script.

my question is rather how do I test that they work?

From R, trivially :

  • file.download("http://Url-of-soime-file-you-are-sure-in-exists", method="curl") ## libcurl

OK, I'll check this.

  • grep("Some.*funny.*pattern", SomeCharVector, perl=TRUE) ## libpcre

I don't know R, so this one look too cryptic. Could you provide a concrete example?

This might be docscripted. Problems :

  • Find a file whose existence is certain

how about http://www.sagemath.org/index.html ?

  • Mark the curl doctest as "internet", on order not to execute it in automated tests (that might run on an unconnected achine).
  • Find some nice pattern where the Perl-compatible matching *differs* from the standard one...

really no idea what you mean here exactly...

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

I forgot the obvious :

> extSoftVersion()["PCRE"]
             PCRE 
"8.39 2016-06-14" 
> capabilities()["libcurl"]
libcurl 
   TRUE 

Not knowing Perl-compatible regular expressin, I'll have to think a bit about a good example...

comment:75 in reply to: ↑ 74 ; follow-up: Changed 5 years ago by dimpase

Replying to charpent:

I forgot the obvious :

> extSoftVersion()["PCRE"]
             PCRE 
"8.39 2016-06-14" 
> capabilities()["libcurl"]
libcurl 
   TRUE 

aren't these just perfect to make doctests?

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

Replying to charpent:

I forgot the obvious :

> extSoftVersion()["PCRE"]
             PCRE 
"8.39 2016-06-14" 
> capabilities()["libcurl"]
libcurl 
   TRUE 

Not knowing Perl-compatible regular expressin, I'll have to think a bit about a good example...

Try that :

> gsub("abc([[:alpha:]]*)def","\\U\\1","abcxyzdef")
[1] "Uxyz"
> gsub("abc([[:alpha:]]*)def","\\U\\1","abcxyzdef",perl=TRUE)
[1] "XYZ"

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

Replying to dimpase:

Replying to charpent:

I forgot the obvious :

> extSoftVersion()["PCRE"]
             PCRE 
"8.39 2016-06-14" 
> capabilities()["libcurl"]
libcurl 
   TRUE 

aren't these just perfect to make doctests?

Not really : these just test for the presence of the software. Downloading a real file, or the example I just cooked for PCRE, ensure that the software works as expected. Of course, I checked that these packages pass their own testsuite.

Note : I'll have to learn how to make a doctest, somewhere in the Sages's "r" documentation. Later ? Now, I'm just a bit overwhelmed (RealLife?(TM) is sometimes a bitch...).

comment:78 Changed 5 years ago by charpent

  • Passes its own testsuite.
  • (#21567 + #21770 + #20523 remerged with the two previous ones) passes ptestlong with two failures :
    ----------------------------------------------------------------------
    sage -t --long src/sage/homology/simplicial_complex.py  # 1 doctest failed
    sage -t --long src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed
    ----------------------------------------------------------------------
    

The first one is transient (passes when ran standalone) ; the second one is a well-documented, ticketed (#21884) and not yet fixed difference on the expression of the expected result.

Should I upload my local branch for #20523 (this patch) ? I think so, and will do so unless someone stops me in the next half hour...

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

  • Commit changed from 516f0291f093af9415003b6b99c7d0ecb1ee8ac8 to cbe9134ad7aaf1b490ea424164ba1991d8440696

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

2e615c1Merge branch 'public/curl7510' of trac.sagemath.org:sage into rupd
bf54f54bumped up the version
bfc26efWups. chmod +x spkg-install...
ef48f71Upgraded to pcre 8.40
1c88cdcMerge branch 'u/charpent/package_libpcre_as_a_standard_package' of git://trac.sagemath.org/sage into finalr
cbe9134Merge branch 'public/r311' of git://trac.sagemath.org/sage into finalr

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

  • Description modified (diff)

Replying to git:

[ ... ]

  • Uploaded the merged result.
  • Updated tarballs

==> needs_review for good.

comment:81 Changed 5 years ago by jdemeyer

I'm currently testing this on OS X.

comment:82 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work
configure: error: libcurl >= 7.28.0 library and headers are required with support for https
Error configuring R.

comment:83 Changed 5 years ago by jpflori

openssl/OS X drama...

Changed 5 years ago by jpflori

Remove check for https support in curl.

comment:84 Changed 5 years ago by jpflori

Feel free to use the patch.

comment:85 Changed 5 years ago by dimpase

It did build for me on OSX (not the current branch, I didn't try it yet, the one around comment 70).

comment:86 Changed 5 years ago by dimpase

if we build this with clang on OSX anyway, can we build with OpenSSL support, no?

comment:87 follow-up: Changed 5 years ago by jpflori

R does not need OpenSSL directly. It just wants curl to have been built with https support.

comment:88 in reply to: ↑ 87 Changed 5 years ago by dimpase

Replying to jpflori:

R does not need OpenSSL directly. It just wants curl to have been built with https support.

and is this possible to achieve without dirty hacks (i.e. not using ssl headers hidden deeply in Xcode in some location that is not stable) ?

comment:89 follow-ups: Changed 5 years ago by jpflori

Not sure... and I'm not very inclined on working on this right now.

But if we were to include the patch removing the check we'll get a compiling though with less feature solution. If you think that's a good idea, I can inlcude it in the branch.

comment:90 in reply to: ↑ 89 ; follow-ups: Changed 5 years ago by dimpase

Replying to jpflori:

Not sure... and I'm not very inclined on working on this right now.

But if we were to include the patch removing the check we'll get a compiling though with less feature solution. If you think that's a good idea, I can inlcude it in the branch.

Using clang, curl can be built with --with-darwinssl on OSX. This does use Apple's replacement for openssl. (doesn't work with gcc as it needs support for Apple's blocks extension for C). Should we do this?

comment:91 Changed 5 years ago by jpflori

Good idea as well.

But I still think we should include the patch to be able to build Sage without ssl.

comment:92 Changed 5 years ago by jpflori

@dima: can you open another ticket for building curl with clang and the right option on OS X?

comment:93 in reply to: ↑ 90 Changed 5 years ago by charpent

Replying to dimpase:

Replying to jpflori:

Not sure... and I'm not very inclined on working on this right now.

But if we were to include the patch removing the check we'll get a compiling though with less feature solution. If you think that's a good idea, I can inlcude it in the branch.

Using clang, curl can be built with --with-darwinssl on OSX. This does use Apple's replacement for openssl. (doesn't work with gcc as it needs support for Apple's blocks extension for C). Should we do this?

By all means, yes. Updating R is a matter of some urgency...

Possible plan :

  • Dima : patch libcurl (#21767) to add this switch to its spkg-install. Check it. Push it to trac as needs_review.
  • Me : review it on Linux (with probably no effect at all) ==> re-mark #21767 as positive review
  • Me also : re-merge #21767 into #20523, re-test it, push it to trac as needs_review
  • Dima : re-test #20523 on a Mac, and possibly mark it as poitive_review .

What do you think ?

comment:94 follow-up: Changed 5 years ago by jpflori

Just open another ticket for the curl script modifs please. No need to reopen #21767.

comment:95 in reply to: ↑ 89 Changed 5 years ago by charpent

Replying to jpflori:

Not sure... and I'm not very inclined on working on this right now.

But if we were to include the patch removing the check we'll get a compiling though with less feature solution. If you think that's a good idea, I can inlcude it in the branch.

I think that this should be a separate ticket. I have reservations about trestricting the abilities of the software it includes ; in R case, the loss of secure communications would be a big loss :

  • R repositories (> 10000 packages now) migrate towards https access. I doubt that they remain http-accessible for long.
  • Access to Web databases (important for many applied statisticians) often requires secure communications.

So, at the minimum, let it be a choice of the user, and not something imposed. (For similar reasons, I oppose Jeroen Demerey's position, which entails a non-functional pip : too big a loss of functionality to be imposed).

And treat it as an ussue separate fron this gofddam R upgrade, which is 11 months late...

comment:96 in reply to: ↑ 94 ; follow-ups: Changed 5 years ago by charpent

Replying to jpflori:

Just open another ticket for the curl script modifs please. No need to reopen #21767.

OK, Ill do that, but will have to add it to #20523 dependencies. And merge it afterwards...

comment:97 in reply to: ↑ 96 Changed 5 years ago by charpent

  • Dependencies changed from #21744 #21767 #21770 #22189 to #21744 #21767 #21770 #22189 #22618

Replying to charpent:

Replying to jpflori:

Just open another ticket for the curl script modifs please. No need to reopen #21767.

OK, Ill do that, but will have to add it to #20523 dependencies. And merge it afterwards...

That's now #22618 (added to dependencies).

comment:98 in reply to: ↑ 90 Changed 5 years ago by jdemeyer

Replying to dimpase:

Using clang, curl can be built with --with-darwinssl on OSX. This does use Apple's replacement for openssl. (doesn't work with gcc as it needs support for Apple's blocks extension for C). Should we do this?

Perhaps, but that's orthogonal to this ticket. The OpenSSL dependency is not limited to OS X, so the solution to 82 cannot be specific to OS X.

comment:99 follow-up: Changed 5 years ago by jdemeyer

  • Dependencies changed from #21744 #21767 #21770 #22189 #22618 to #21767, #21770

I agree with jpflori. Let's include the R patch and also possibly fix building curl on OS X with OpenSSL.

comment:100 in reply to: ↑ 96 Changed 5 years ago by dimpase

Replying to charpent:

Replying to jpflori:

Just open another ticket for the curl script modifs please. No need to reopen #21767.

why not, I can do this right now on #21767, it will be ready by lunchtime...

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

Replying to jdemeyer:

I agree with jpflori. Let's include the R patch and also possibly fix building curl on OS X with OpenSSL.

With that patch, will R use SSL when compiled on a system where OpenSSL or Gutls exist(s) ?

comment:102 Changed 5 years ago by jpflori

Yes it will.

But it will also allow to build Sage on systems without openssl.

comment:103 in reply to: ↑ 101 Changed 5 years ago by dimpase

Replying to charpent:

Replying to jdemeyer:

I agree with jpflori. Let's include the R patch and also possibly fix building curl on OS X with OpenSSL.

With that patch, will R use SSL when compiled on a system where OpenSSL or Gutls exist(s) ?

it will use Darwin's own ssl on new OSX systems, otherwise it will do the same as before; i.e. it's an OSX-only patch for new OSX systems (only if $MACOS_VERSION -ge 16 holds)

comment:104 in reply to: ↑ 72 ; follow-up: Changed 5 years ago by dimpase

Replying to charpent:

Replying to dimpase:

Replying to charpent:

Replying to dimpase:

how does one check that it can use pcre and curl ? is it in the tests? (I'd like to see this on OSX)

as far as I understand, it's managed in R's main .configure script.

my question is rather how do I test that they work?

From R, trivially :

it is download.file :-)

The following works in R (run from sage -sh prompt) with clang-compiled curl on OSX:

> capabilities()["libcurl"]
libcurl 
   TRUE 
> download.file("http://www.sagemath.org/index.html", method="curl",destfile="/tmp/bla.html")
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 17758    0 17758    0     0  92938      0 --:--:-- --:--:-- --:--:--  110k
> download.file("https://www.sagemath.org/index.html", method="curl",destfile="/tmp/sbla.html")
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 17758    0 17758    0     0  67312      0 --:--:-- --:--:-- --:--:-- 68563
> 

(so both http and https work)

comment:105 in reply to: ↑ 76 Changed 5 years ago by dimpase

Replying to charpent:

Replying to charpent:

I forgot the obvious :

> extSoftVersion()["PCRE"]
             PCRE 
"8.39 2016-06-14" 
> capabilities()["libcurl"]
libcurl 
   TRUE 

Not knowing Perl-compatible regular expressin, I'll have to think a bit about a good example...

Try that :

> gsub("abc([[:alpha:]]*)def","\\U\\1","abcxyzdef")
[1] "Uxyz"
> gsub("abc([[:alpha:]]*)def","\\U\\1","abcxyzdef",perl=TRUE)
[1] "XYZ"

OK, these work on OSX, too.

comment:106 in reply to: ↑ 104 Changed 5 years ago by charpent

Replying to dimpase:

[ Snip... ]

(so both http and https work)

To be fully paranoid, we should check that both downloads give the same file ;-). (I did that, but in a non-portable way).

Probably not really useful.

Let's start another ticket fo doctest it (and the PCRE test) in a future enhancement ?

comment:107 follow-up: Changed 5 years ago by dimpase

as far as OSX is concerned, this looks done to me. Anything else that stops this ticket?

comment:108 follow-up: Changed 5 years ago by jpflori

  • Reviewers changed from Emmanuel Charpentier to Emmanuel Charpentier, Dima Pasechnik
  • Status changed from needs_work to positive_review

LGTM.

We should still open a followup ticket to include the openssl-less patch.

comment:109 Changed 5 years ago by jpflori

Please review #22619.

comment:110 in reply to: ↑ 108 Changed 5 years ago by charpent

Replying to jpflori:

LGTM.

A big "Thank you" to all people having helped to close this 11 months gap !

We should still open a followup ticket to include the openssl-less patch.

Agreed. I think that running an open system which needs communication (I just don't see applied statistics without communication as very practical) without means to have *secure* communication is dangerously silly, but I also agree that there might be an use for "isolated" systems, for which communication is baggage.

But this must be an *option*, not something forced on the user by a deficient implementation.

comment:111 Changed 5 years ago by jpflori

It is an option: if you have openssl installed, you'll get https without any further action.

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

  • Status changed from positive_review to needs_work

82

comment:113 in reply to: ↑ 112 Changed 5 years ago by charpent

Replying to jdemeyer:

82

Compiles for me with #22619 merged in. make ptestlong running (needs about 1 hour). Would depending on #22619 be enough for you ?

comment:114 Changed 5 years ago by git

  • Commit changed from cbe9134ad7aaf1b490ea424164ba1991d8440696 to f029f66bbf131eb7cdd4f8bfbe42d5c560baea1c

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

f029f66This simple patch removes an overkill check in R's configure.

comment:115 Changed 5 years ago by jdemeyer

  • Milestone changed from sage-7.4 to sage-8.0
  • Status changed from needs_work to needs_review

I think it's better to add the patch from #22619 to this ticket, which I just did.

comment:116 follow-up: Changed 5 years ago by dimpase

  • Milestone changed from sage-8.0 to sage-7.4

Jeroen, I presume #comment:82 has been fixed in #21767. Please check if it works for you.

comment:117 Changed 5 years ago by dimpase

also, given how little in Sage actually depends on R, IMHO this can be added in 7.6, no need to wait for 8.0...

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

Replying to dimpase:

Jeroen, I presume #comment:82 has been fixed in #21767.

Depends what you mean. Yes, it's fixed on OS X. But there remains the issue that Sage (and therefore R) should not require OpenSSL.

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

  • Milestone changed from sage-7.4 to sage-7.6

Replying to jdemeyer:

Replying to dimpase:

Jeroen, I presume #comment:82 has been fixed in #21767.

Depends what you mean. Yes, it's fixed on OS X. But there remains the issue that Sage (and therefore R) should not require OpenSSL.

Jeroen,

Since 7.6.beta2 (IIRC), Sagemath depends on some SSL implementation, and we recommend OpenSSL.

BTW #20523 + #22619 passes ptestlong with the usual quirks (one transient error, one well-known, ticketed difference of expression) ; since I have OpenSSL installed systemwide, I can reach an https-accessible R repository.. Is that enough for you ? Or can you check that your patch does what you so dearly desire ?

BTW : setting the milestone at 7.6 : this upgrade is late enough.

comment:120 in reply to: ↑ 119 Changed 5 years ago by jdemeyer

Replying to charpent:

Since 7.6.beta2 (IIRC), Sagemath depends on some SSL implementation, and we recommend OpenSSL.

That's a documentation bug, see #22620.

comment:121 Changed 5 years ago by jpflori

  • Keywords days85 added
  • Status changed from needs_review to positive_review

comment:122 follow-up: Changed 4 years ago by dimpase

the following is, most probably, an R bug:

[r-3.3.3.p0] making tre-match-backtrack.d from tre-match-backtrack.c
[r-3.3.3.p0] regerror.c:25:10: fatal error: 'libintl.h' file not found
[r-3.3.3.p0] #include <libintl.h>
[r-3.3.3.p0]          ^

which pops up if libintl.h is installed in /usr/local/include/. Making a symbolic link to it in SAGE_LOCAL/include allows this to proceed.

It appears that the corresponding place in the R installation ignores CFLAGS, as passing CFLAGS='-I/usr/local/include' on the command line has no effect.

comment:123 Changed 4 years ago by dimpase

There appears to be a platform-dependent bug in how curl dependency is handled. With libcurl not installed in /usr/lib (resp. no libcurl headers in /usr/include) it does not build on a FreeBSD 11.0 system. I had to apply

--- a/build/pkgs/r/spkg-install
+++ b/build/pkgs/r/spkg-install
@@ -115,6 +115,11 @@ if [ "$UNAME" = "CYGWIN" ]; then
     mv patches/cygwin/*.patch patches/
 fi
 
+CURL_CPPFLAGS="-I/$SAGE_LOCAL/include"
+CURL_LIBS="-L/$SAGE_LOCAL/lib -lcurl"
+export CURL_CPPFLAGS
+export CURL_LIBS
+
 config()
 {
     ./configure \

comment:124 Changed 4 years ago by vbraun

  • Branch changed from public/r311 to f029f66bbf131eb7cdd4f8bfbe42d5c560baea1c
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:125 in reply to: ↑ 122 Changed 3 years ago by dimpase

  • Commit f029f66bbf131eb7cdd4f8bfbe42d5c560baea1c deleted

Replying to dimpase:

the following is, most probably, an R bug:

[r-3.3.3.p0] making tre-match-backtrack.d from tre-match-backtrack.c
[r-3.3.3.p0] regerror.c:25:10: fatal error: 'libintl.h' file not found
[r-3.3.3.p0] #include <libintl.h>
[r-3.3.3.p0]          ^

which pops up if libintl.h is installed in /usr/local/include/. Making a symbolic link to it in SAGE_LOCAL/include allows this to proceed.

It appears that the corresponding place in the R installation ignores CFLAGS, as passing CFLAGS='-I/usr/local/include' on the command line has no effect.

As a matter of fact, one needs these -I/... added to CPPFLAGS as there is some pre-processing happening in the build here.

Note: See TracTickets for help on using tickets.