Opened 6 years ago

Closed 6 years ago

#18961 closed enhancement (fixed)

upgrade ECL to 15.3.7

Reported by: dimpase Owned by:
Priority: major Milestone: sage-6.9
Component: packages: standard Keywords:
Cc: fbissey Merged in:
Authors: Dima Pasechnik Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 6bb249b (Commits, GitHub, GitLab) Commit: 6bb249b1c1eca2f794330873ce2497029a4f344e
Dependencies: Stopgaps:

Status badges

Description (last modified by dimpase)

ECL we ship is very old. Here is an update to the current stable version.

  • ECL git here and the tarball can be obtained there, too. But it is not useful without renaming, patching, and running autoconf. This from it we create an updated tarball by running spkg-src script.

ECL tarball to use is here

The initial work on this ticket was done on #18920.

Change History (17)

comment:1 Changed 6 years ago by dimpase

  • Authors set to Dima Pasechnik
  • Cc fbissey added
  • Status changed from new to needs_review

comment:2 follow-up: Changed 6 years ago by fbissey

I thought you were going to update spkg-src to run autotools, have I been mistaken on that point? Other than that I cannot really check the patch for cygwin but I am assuming they would be OK. The other stuff done to the tarball and then as a consequence to the configure script are a bit extreme in my view but I can keep to the status quo.

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

Replying to fbissey:

I thought you were going to update spkg-src to run autotools, have I been mistaken on that point?

Right, thanks for reminding me. I'll update spkg-src.

comment:4 in reply to: ↑ 3 Changed 6 years ago by dimpase

Replying to dimpase:

Replying to fbissey:

I thought you were going to update spkg-src to run autotools, have I been mistaken on that point?

Right, thanks for reminding me. I'll update spkg-src.

But running autoconf in spkg-src would only make sense if almost all patches are applied already. So in effect we are creating a modified tarball, if we are not willing to have autoconf as a dependence.

I don't know, tell me if this is OK, or not...

PS. On the other hand I can just do patching in spkg-src and again in spkg-install.

Last edited 6 years ago by dimpase (previous) (diff)

comment:5 Changed 6 years ago by fbissey

sage-src point is that we ship a modified tarball and it helps generate it. In my opinion running autoreconf is fair game. You have to remember spkg-src is not run by the user building sage only by the packager providing a new version of the package.

That being said the change allowed in are only things that are not trivial:

  • removal of files (making a smaller tarball, gap anyone?)
  • regeneration of script (autoreconf is a case in point if the changes are not easily patched back into configure and al. It is generally cleaner.)

Whether you want to exercise the second bullet point is at your discretion. I didn't for the last singular patch, but it was a trivial change.

comment:6 Changed 6 years ago by git

  • Commit changed from b669a434bdf2f91d21254ccf186bc86396dd4909 to 06082f616b2b23d92db1539869dd05fc16644176

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

06082f6moved patching into spkg-install, and other updates around this

comment:7 Changed 6 years ago by dimpase

OK, done; I have moved all the patching into spkg-src, and further updated it to make creation of the tarball etc as automatic as I can. As a byproduct, spkg-install is simpler.

comment:8 Changed 6 years ago by dimpase

  • Status changed from needs_review to needs_work

oops, wrong tarball naming...

comment:9 Changed 6 years ago by git

  • Commit changed from 06082f616b2b23d92db1539869dd05fc16644176 to c4db16972b72afdc8deff8afae8c6d6d6eb6f771

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

c4db169updated...

comment:10 Changed 6 years ago by fbissey

Sorry to be a pain but I would like you to add a comment in SPKG.txt that from this release all the patches are applied in spkg-src and therefore do not need to be applied in spkg-install. I would go as far as adding a comment in spkg-install signalling that patches are already applied from spkg-src. I believe that's the first package to take a total inclusion course from spkg-src in this way.

comment:11 Changed 6 years ago by git

  • Commit changed from c4db16972b72afdc8deff8afae8c6d6d6eb6f771 to 6bb249b1c1eca2f794330873ce2497029a4f344e

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

6bb249bupdated SPKG.txt, added message to spkg-install, fixed patching

comment:12 Changed 6 years ago by dimpase

  • Description modified (diff)

New commits:

c4db169updated...
6bb249bupdated SPKG.txt, added message to spkg-install, fixed patching

comment:13 Changed 6 years ago by dimpase

  • Description modified (diff)
  • Status changed from needs_work to needs_review

OK, done. Took a while to find and remove an evil "continue" from spkg-src... :-)

comment:14 Changed 6 years ago by fbissey

Thank you. I am a bit behind in term of my sage work. I maintain sage-on-gentoo as a priority but various pressures interfere with my work here. I will try to complete this review today by testing on a Mac where I think the potential for problem is the greatest.

comment:15 Changed 6 years ago by fbissey

All build and basic tests related to maxima and ecl passed, however I want to check what's happening with libffi before giving this a tick. libffi is normally present in some form on system we build sage (otherwise python's ctype wouldn't be built and then numpy wouldn't build I think).

So I think the old ecl was picking the system libffi without help while we now have to explicitly ask for it. I definitely think we should use libffi if available. I am surprised it built properly without actually - may be it got picked even so there was a visible message in configure.

comment:16 Changed 6 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

OK it was always the case that libffi wasn't found on OS X at least.

comment:17 Changed 6 years ago by vbraun

  • Branch changed from u/dimpase/updateecl to 6bb249b1c1eca2f794330873ce2497029a4f344e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.