search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 478 forks source link

Package libcurl (and curl) as a standard package #21767

Closed 7822f248-ba56-45f1-ab3d-4de7482bdf9f closed 7 years ago

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago

Rationale : R (standard package) requires libcurl (and possibly the curl executable), but stopped to package it with version 3.3.0.

See discussion on sage-devel.

Tarball: https://curl.haxx.se/download/curl-7.53.1.tar.bz2

CC: @jpflori @jdemeyer @jhpalmieri @vbraun

Component: packages: standard

Keywords: r-project, days85

Stopgaps: use clang and --with-darwinssl on OSX

Author: Emmanuel Charpentier, Jean-Pierre Flori

Branch/Commit: 4c7d5eb

Reviewer: Jean-Pierre Flori, Jeroen Demeyer, Dima Pasechnik

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

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:1

Plan :

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago

Branch: u/charpent/package_libcurl__and_curl__as_a_standard_package

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:3

This initial packaging :

The resulting Sage passes testlong with no errors (not even transient, for a marvel...

Marking needs_review. See the thread on sage-devel for discussion of conditional installation.

New commits:

2884837Initial packaging of curl : unconditionnal installation.
7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago

Commit: 2884837

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -2,4 +2,4 @@

 See [discussion](https://groups.google.com/forum/#!topic/sage-devel/9gun2Wo6zGM) on sage-devel.

-Original tarball is [here](https://curl.haxx.se/download/curl-7.50.3.tar.gz)
+**Tarball**: https://curl.haxx.se/download/curl-7.50.3.tar.gz
jdemeyer commented 8 years ago
comment:4

Author name please...

jdemeyer commented 8 years ago
comment:5

I don't like this style of logging:

CC       libcurl_la-curl_gethostname.lo

I like to see the gcc command-line to figure out what is going on.

Usually, this can be changed by passing V=1 to the make command line (see for example build/pkgs/libfplll/spkg-install)

jpflori commented 8 years ago
comment:6

Would be nice to have the standard pieces of code to check we are in a Sage shell and to apply potential patches.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:7

Replying to @jdemeyer:

I don't like this style of logging:

CC       libcurl_la-curl_gethostname.lo

I like to see the gcc command-line to figure out what is going on.

Usually, this can be changed by passing V=1 to the make command line (see for example build/pkgs/libfplll/spkg-install)

Doable. On your head be it...

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago

Author: Emmanuel Charpentier

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:8

Replying to @jdemeyer:

Author name please...

Done

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:9

Replying to @jpflori:

Could you clarify :

Would be nice to have the standard pieces of code to check we are in a Sage shell

??? What"'s that ? A simple hint might be to test ! X$SAGE_ROOT = X, but you seem to have something more sophisticated in head.

and to apply potential patches.

??? Clarification required. What are "potential patches" ?

jpflori commented 8 years ago
comment:10

Replying to @EmmanuelCharpentier:

Replying to @jpflori:

Could you clarify :

Would be nice to have the standard pieces of code to check we are in a Sage shell

??? What"'s that ? A simple hint might be to test ! X$SAGE_ROOT = X, but you seem to have something more sophisticated in head.

https://github.com/sagemath/sage-prod/blob/master/build/pkgs/ecl/spkg-install#L3

and to apply potential patches.

??? Clarification required. What are "potential patches" ?

https://github.com/sagemath/sage-prod/blob/master/build/pkgs/ecl/spkg-install#L13

jdemeyer commented 8 years ago
comment:11

Replying to @EmmanuelCharpentier:

Doable. On your head be it...

What do you mean with this?

jdemeyer commented 8 years ago
comment:12

Replying to @jpflori:

https://github.com/sagemath/sage-prod/blob/master/build/pkgs/ecl/spkg-install#L13

Please don't, see #20692 instead.

jdemeyer commented 8 years ago
comment:13

The package builds fine.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:14

Replying to @jdemeyer:

Replying to @EmmanuelCharpentier:

Doable. On your head be it...

What do you mean with this?

Nothing rude, be assured ! (sorry if that sounded rude...). I just mean that I'm not very fond of miles-log logs, where the relevant information doesn't stand out (common problem with, for example, LaTeX).

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

1588558Packaging polish.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 2884837 to 1588558

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:16

In this branch :

This builds and passes the package's own test suite without errors ; the resulting Sage passes ptestlong with the usual transient error on simplicial_complex.py, which passes succesfully standalone.

Re: #20692 : This ticket has been in the works for 5 months and still depends on two other tickets. I'd rather not wait for it to be merged in Sage before getting the damn curl in Sage : R depends on it, and updating it begins to be a matter of some (slight) urgency : more and more R packages can no longer be installed under 2.4.2... That doesn't mean I won't revise the way to patch things when #20692 is merged.

Anyway, the point is moot for now : there are no patches...

==>needs_review

jdemeyer commented 8 years ago
comment:17

Replying to @EmmanuelCharpentier:

I just mean that I'm not very fond of miles-log logs, where the relevant information doesn't stand out

At least the relevant information exists in the log.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:18

Replying to @jdemeyer:

Replying to @EmmanuelCharpentier:

I just mean that I'm not very fond of miles-log logs, where the relevant information doesn't stand out

At least the relevant information exists in the log.

Indeed. I can see your point : it's mainly a grep vs vgrep (non-)issue ; probably coming from our respective initial curricula (a surgeon (even a dental one) needs to see (literally) a lot of things at once ; details can (usually) wait. Hence my preference for breadth-first searches).

jpflori commented 8 years ago
comment:19

The HTTPS/openssl issue is still an issue.

jdemeyer commented 8 years ago

Work Issues: HTTPS

jpflori commented 8 years ago
comment:21

Without patching I fear the only solution is to make curl and R optional. Groumpf.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:22

Replying to @jdemeyer:

As compiled, curl supports https. You can check this with curl-config --protocols.

The issue raised by Volker is valid, but has little to do with the present issue (nor with R's update...).

Furthermore, I think we shouldn't try to solve locally a systemwide problem.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:23

Replying to @jpflori:

Without patching I fear the only solution is to make curl and R optional. Groumpf.

Care to explain ? I'm a dentist of very little mind, and have trouble following you.

jpflori commented 8 years ago
comment:24

IIRC we cannot automatically install openssl bc of license issues... Hence no https support in curl and then R does not want to build.

jpflori commented 8 years ago
comment:25

I guess you get https support because you have openssl and its headers installed.

jpflori commented 8 years ago
comment:26

And openssl is not a dependency of sage ATM.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:27

Replying to @jpflori:

And openssl is not a dependency of sage ATM.

Therefore, the obvious solution is to add OpenSSL as a prerequisite to Sage. (easy patch : two words in the main README.md...).

Would that be a problem ?

Should we open a discussion on sage-devel ?

jhpalmieri commented 8 years ago
comment:28

Or add curl (with SSL support) as a prerequisite to Sage. How widely installed are curl and OpenSSL? OS X seems to include curl and libcurl, and also libssl (but is that the same as OpenSSL? or is it at least good enough?). What about the various linux distributions?

jdemeyer commented 8 years ago
comment:29

Replying to @jhpalmieri:

Or add curl (with SSL support) as a prerequisite to Sage. How widely installed are curl and OpenSSL? OS X seems to include curl and libcurl, and also libssl (but is that the same as OpenSSL? or is it at least good enough?). What about the various linux distributions?

I would guess that curl itself is widely available, but maybe not with "development headers" that many distros package separately.

jdemeyer commented 8 years ago
comment:30

As I tried to explain [comment:12], please remove this patch block (especially given that there are no patches):

# Patch sources.
for patch in ../patches/*.patch; do
    [ -r "$patch" ] || continue  # Skip non-existing or non-readable patches
    patch -p1 <"$patch"
    if [ $? -ne 0 ]; then
        echo >&2 "Error applying '$patch'"
        exit 1
    fi
done
jdemeyer commented 8 years ago
comment:31

Replying to @EmmanuelCharpentier:

I'd rather not wait for it to be merged in Sage before getting the damn curl in Sage

I never said that you need to depend on that ticket. If you remove the "patching" block, it will work with or without #20692.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 1588558 to 81e3341

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

81e3341Removed patches handling.
7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:33

Replying to @sagetrac-git:

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

81e3341Removed patches handling.

Builds OK, passes its test suite.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:35

Replying to @jdemeyer:

Care to explain ?

jdemeyer commented 8 years ago
comment:36

[comment:19]

jpflori commented 8 years ago
comment:37

Some old discussions that might enlighten you:

jpflori commented 8 years ago
comment:38

Discussion at:

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 8 years ago
comment:39

Replying to @jpflori:

Discussion at:

Thanks for the heads-up. It's important, and I'm afraid that the full size of the problem hasn't yet been considered : the R ecosystem is much larger (by about 3-4 magnitudes) than some seem to think...

jpflori commented 8 years ago

Changed branch from u/charpent/package_libcurl__and_curl__as_a_standard_package to public/curl7510

jpflori commented 8 years ago
comment:40

@Volker: I've added a fat mode to avoid a lot of stuff curl might be tempted to use. I did not touch the SSL/TLS options. Should I?

New commits:

c02cb61Update Curl to 7.51.0.
d31c560Add a fat mode to curl.
jpflori commented 8 years ago

Changed commit from 81e3341 to d31c560

jpflori commented 8 years ago

Changed author from Emmanuel Charpentier to Emmanuel Charpentier, Jean-Pierre Flori

jpflori commented 8 years ago

Changed work issues from HTTPS to none

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:41

I think that the (pseudo-)legal considerations which motivated last month's saga leave us few options.

I looked a bit closer to the idea of using an rpy2-based new interface to an external R. This is doable, but not in the short term.

I am under the impression that the current (pexpect) interface won't work reliably with a system R (especially on "exotic" systems such as cygwin and the Mac). Therefore, depending on a systemwide installation for R is out of the question in the short term. We need an interim solution.

My current plan is :

Only after this interim solution works, envision the much larger task of creating a new interface.

Jean-Pierre : I think that omitting https requirement for R is a very bad idea : I do not know how longer R users will be able to depend on http-only repositories ; and, as I said before, R practical use is highly dependent of access to the 9000+ R packages in existence.

Advice required. Do you think useful to open an new discussion on sage-devel ?

Note 1 : the patchbot test of #21744 is marked "pending". What does that mean ?

Note 2 : the patchbot test of #21770 fails repeatedly due to thye source tarball being unavailable on Sage standard repositories. What should I do ? The (ftp) address of the original tarball is in the ticket's header.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from d31c560 to e4e5665