Opened 3 years ago

Closed 3 years ago

#22058 closed defect (fixed)

Upgrade git to 2.11.0

Reported by: charpent Owned by:
Priority: major Milestone: sage-7.5
Component: packages: standard Keywords:
Cc: slelievre Merged in:
Authors: Emmanuel Charpentier Reviewers: Jeroen Demeyer, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: bd5a04f (Commits) Commit: bd5a04f3491d6565eb7481b7b0bdb9bec2393b04
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Motivation : recent changes in OpenSSL cause current git (2.6.2) to fail to compile with recent (>=1.1) version of openssl, due to changes in macros defining library function declarations.

This seems to have affected a number of packages. An example is given here, where the solution was a fix to the affected software (R package).

Tarball: https://www.kernel.org/pub/software/scm/git/git-2.11.0.tar.gz

Change History (22)

comment:1 Changed 3 years ago by dimpase

IHMO it's OK to go for 2.11.0.

comment:2 Changed 3 years ago by charpent

  • Description modified (diff)
  • Summary changed from Upgrade git to 2.10.2 (or 2.11 ?) to Upgrade git to 2.11

comment:3 Changed 3 years ago by charpent

  • Branch set to u/charpent/upgrade_git_to_2_10_2__or_2_11___

comment:4 Changed 3 years ago by charpent

  • Authors set to Emmanuel Charpentier
  • Commit set to 4fc4c5ec54c2491d1a1f750536dac34000fdb7d9
  • Status changed from new to needs_review

The proposed patch :

  • build and installs cleanly a functional git 2.11.0
  • sort-of passes its own test suite (fails on items marked as "known breakage" in the log) :
    fixed   0
    success 13952
    failed  0
    broken  192
    total   14340
    
  • results in a sage system that passes ptestlong with one known and unrelated failure (see #21884) :
    ----------------------------------------------------------------------
    sage -t --long --warn-long 88.0 src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed
    ----------------------------------------------------------------------
    

==> needs_review


New commits:

5f3c5f1Update source to 2.11.0, fix checksum.
4fc4c5eThe config file has to be generated.
Last edited 3 years ago by charpent (previous) (diff)

comment:5 Changed 3 years ago by tmonteil

  • Cc slelievre added

I do not understand the urge for a "recent (>=1.1) version of openssl" (note for example that 1.1.0 entered Debian unstable less than 2 month ago, and testing only half a month ago). We are using 1.0.2, which is the current Long Term Support, and will be maintained until 2019-12 (3 years from now). Sticking to it allowed us not to be concerned by the last (2016-11-10) security fixes (which were about 1.1.0 only). It is usually easier to wait for some maturity, especially since we do not need latest openssl features (nor bugs). I am not saying that we have to stick to 1.0.2 until 2019-12, but i do not see why se should deal with the teething problems for free and no benefit (our exprience will not help R or git with that either).

comment:6 follow-up: Changed 3 years ago by charpent

You are indeed right : we (as Sage users) do not need "recent" versions of anything.

But we (as users of other software, corresponding with other users of other software) need reasonably current versions of software more or less in sync with what our "real world" correspondents use. This, of course varies from use to use. For example, to work with other statisticians using R, I need in practice to be able to use R latest released versions (R-help won't answer questions related to bugs in older versions), and in order to submit a new or revised package, I have to check it against the latest (daily !) development version.

We might also need new (real or virtual) machines (e. g. for testing purposes. So we need to accept installation on *reasonably* current machines. Therefore we have to be able to install Sage on these machines.

I agree that nobody in is right mind will install (and work on) debian unstable. But, except for servers purpose, nobody in his right mind will install debian stable either, at least on a personal work machine. Debian testing (or analogues in other distributions) seems to be the default choice, debian stable + backports (or reasonable facsimiles) being a distant second choice. For example, you can probably bet that Ubuntu 17.04 will have openssl 1.1.

Furthermore, this (understandable) reluctance to update can easily become a hindrance (think python2 vs python3). We have already been trapped by this in the past... and another trap is going to close on us : see this thread : it's a nice one... (not yet a ticket, because I'm not sure of the kind of answer to give).

So, all of this boils down to the definition of what is "reasonably current"... If "debian testing" or analogues are too new to be stated, the README.md file should state that.

Feel free to request a discussion on this topic on, e. g. sage-devel ; this might need clarification.

In the meantime, what do you think of the proposed patch ?

comment:7 in reply to: ↑ 6 ; follow-up: Changed 3 years ago by tmonteil

Replying to charpent:

You are indeed right : we (as Sage users) do not need "recent" versions of anything.

Well, for example there have been various updates to latest unreleased PARI master branch, because it was useful.

But we (as users of other software, corresponding with other users of other software) need reasonably current versions of software more or less in sync with what our "real world" correspondents use.

The Long Term Support version should be one of the "reasonably current versions".

This, of course varies from use to use. For example, to work with other statisticians using R, I need in practice to be able to use R latest released versions (R-help won't answer questions related to bugs in older versions), and in order to submit a new or revised package, I have to check it against the latest (daily !) development version.

I have an apparently working way to use a recent (external) version of R in Sage/rpy2 (i did that for SDL), i will open a ticket, but i first have to close some branches because my laptop is old and i can not afford to swith branch and let it burn for recompilations.

We might also need new (real or virtual) machines (e. g. for testing purposes. So we need to accept installation on *reasonably* current machines. Therefore we have to be able to install Sage on these machines.

I agree that nobody in is right mind will install (and work on) debian unstable. But, except for servers purpose, nobody in his right mind will install debian stable either, at least on a personal work machine. Debian testing (or analogues in other distributions) seems to be the default choice, debian stable + backports (or reasonable facsimiles) being a distant second choice. For example, you can probably bet that Ubuntu 17.04 will have openssl 1.1.

My guess is that Debian moved 1.1.0 into testing because "The freeze is coming" (very soon), and of course Ubuntu 17.04 will follow since it is basically a snapshot of unstable (at least it was, i did not follow their recent evoltions).

Note however that Debian struggled with 1.1.0 in experimental for more that 6 months now, an that the current (in preparation) Ubuntu 17.04 still uses 1.0.2:

https://packages.qa.debian.org/o/openssl/news.rss20.xml http://packages.ubuntu.com/zesty/openssl

I just do not understand why we have to enter such a struggle. My guess is that openssl recently changed various things, so that various softwares went broken, and it is prehaps good to just wait that the situation get stablilized, i am not claiming we should stick to 1.0.2.

Furthermore, this (understandable) reluctance to update can easily become a hindrance (think python2 vs python3). We have already been trapped by this in the past... and another trap is going to close on us : see this thread : it's a nice one... (not yet a ticket, because I'm not sure of the kind of answer to give).

This is not a reluctance. Regarding the comparison with Python 2to3, we had to wait that all our dependencies get python3 ready anyway.

So, all of this boils down to the definition of what is "reasonably current"... If "debian testing" or analogues are too new to be stated, the README.md file should state that.

The current testing is a pre-frozen, where maintainers are in a hurry to let things enter earlier because it will soon be forbidden.

Feel free to request a discussion on this topic on, e. g. sage-devel ; this might need clarification.

In the meantime, what do you think of the proposed patch ?

It looks good. Unfortunately, i currently do not have the ressource to test it seriously :/

comment:8 in reply to: ↑ 7 Changed 3 years ago by charpent

A few points...

Replying to tmonteil:

[ Snip... ]

The Long Term Support version should be one of the "reasonably current versions".

You mean Ubuntu LTS ? A bit too old on the "desktop software" and "webware" side, to the taste of my correspondents...

[ Re-Snip... ]

I have an apparently working way to use a recent (external) version of R in Sage/rpy2 (i did that for SDL), i will open a ticket, but i first have to close some branches because my laptop is old and i can not afford to swith branch and let it burn for recompilations.

This is of great interest to me : maintaining an usable R in Sage is one of my goals. However, R is a standard package. Unless a decision is made to expel it, we are bound to maintain it. And cope with its new requirements (xz, https-enabled curl, pcre).

Furthermore, keeping all the functionalities of the current pexpect interface won't be easy. This interface is highly inefficient (as underscored by William Stein, its author...), and ill-documented, but has extremely wide possibilities. Could you Cc me on this ?

[ Re-re-Snip .. ]

I just do not understand why we have to enter such a struggle. My guess is that openssl recently changed various things, so that various softwares went broken, and it is prehaps good to just wait that the situation get stablilized, i am not claiming we should stick to 1.0.2.

Whatever the motivations might be, and unless we advertise a "recent installations need nod apply" policy in the README.md file, there will be installation on (new) machines with 1.1.0 installed. Since we can't ship openssl ourselves, we have to support that.

[ Re-re-re-Snip... ]

The current testing is a pre-frozen, where maintainers are in a hurry to let things enter earlier because it will soon be forbidden.

Whatever the (good or bad) reasons, it's a fact of life we have to cope with. Don't fight the weather, man : it's an ungratifying business...

In the meantime, what do you think of the proposed patch ?

It looks good. Unfortunately, i currently do not have the ressource to test it seriously :/

Any takers ?

comment:9 follow-up: Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work
  • Summary changed from Upgrade git to 2.11 to Upgrade git to 2.11.0

The tarball taken from github is NOT a proper source distribution. There should be no need to run $MAKE configure.

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

Replying to jdemeyer:

The tarball taken from github is NOT a proper source distribution.

I do not understand : this is the address pointed to by the git repository. Would you care to explain ?

There should be no need to run $MAKE configure.

Why ? Again, explanations are welcome.

comment:11 follow-up: Changed 3 years ago by jdemeyer

What I mean is that a git repository is not the same as a source tarball. Take Sage for example: the tarball generated by make dist is not just a packaging of the git repo.

In particular, the actual source tarball of git (see new ticket description) contains the configure script, which the git repo does not contain. So, if you use the actual source tarball of git, then you do not need to run $MAKE configure.

comment:12 Changed 3 years ago by git

  • Commit changed from 4fc4c5ec54c2491d1a1f750536dac34000fdb7d9 to 48efb15401231c3513475a1b008fa1da3463afa4

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

e3c4835Revert "The config file has to be generated."
48efb15Update checksums for git.

comment:13 Changed 3 years ago by charpent

installs cleanly and passes its own test suite.

comment:14 Changed 3 years ago by charpent

  • Status changed from needs_work to needs_review

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

Replying to jdemeyer:

What I mean is that a git repository is not the same as a source tarball. Take Sage for example: the tarball generated by make dist is not just a packaging of the git repo.

In particular, the actual source tarball of git (see new ticket description) contains the configure script, which the git repo does not contain. So, if you use the actual source tarball of git, then you do not need to run $MAKE configure.

Thanks a lot : I just followed the github link to the archives and was not aware of the kernel.org source... For my edification : where is this documented ?

comment:16 in reply to: ↑ 15 Changed 3 years ago by jdemeyer

Replying to charpent:

For my edification : where is this documented ?

Good question, it's hard to find. I found it under the "Older releases" link at https://git-scm.com/downloads

comment:17 Changed 3 years ago by jdemeyer

I would prefer the commits to be squashed (1 commit instead of 4). Can you do that?

comment:18 follow-up: Changed 3 years ago by git

  • Commit changed from 48efb15401231c3513475a1b008fa1da3463afa4 to bd5a04f3491d6565eb7481b7b0bdb9bec2393b04

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

bd5a04fUpdate source to 2.11.0, fix checksum.

comment:19 in reply to: ↑ 18 Changed 3 years ago by charpent

Replying to git:

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

bd5a04fUpdate source to 2.11.0, fix checksum.

1've checked that git builds and passes its own testsuite :

[git-2.11.0] fixed   0
[git-2.11.0] success 13952
[git-2.11.0] failed  0
[git-2.11.0] broken  192
[git-2.11.0] total   14340

I'd rather not make ptestlong on this machine, which is too damned slow for the test (about 4 hours...).

comment:20 Changed 3 years ago by charpent

Against Sage 7.5 built on a pristine Debian (testing) VM + current system's compilers and OpenSSL + development libs (libssl-dev, currently 1.1.0c) :

  • Passes its own testsuite, with similar results :
    fixed   0
    success 13276
    failed  0
    broken  185
    total   13728
    
  • The resulting sage passes ptestlong with no errors.

One notes that the testsuite

  • depends on an utility (msgfmt, in Debian's gettext package), that seems unneeded for anything else in Sage ;
  • that the number of tests seems to depend on other installed software.

Still needs_review.

comment:21 Changed 3 years ago by dimpase

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Dima Pasechnik
  • Status changed from needs_review to positive_review

looks good to me.

comment:22 Changed 3 years ago by vbraun

  • Branch changed from u/charpent/upgrade_git_to_2_10_2__or_2_11___ to bd5a04f3491d6565eb7481b7b0bdb9bec2393b04
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.