Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#14876 closed enhancement (fixed)

Update NTL to 6.0.0

Reported by: vbraun Owned by: jdemeyer
Priority: major Milestone: sage-6.2
Component: packages: standard Keywords:
Cc: leif, jpflori Merged in:
Authors: Volker Braun, Jean-Pierre Flori Reviewers: Jean-Pierre Flori,François Bissey
Report Upstream: N/A Work issues:
Branch: 8c9d711 (Commits, GitHub, GitLab) Commit:
Dependencies: #14333 Stopgaps:

Status badges

Attachments (1)

trac_14876_ntl_typedef.patch (6.5 KB) - added by vbraun 8 years ago.
Initial patch

Download all attachments as: .zip

Change History (44)

comment:1 Changed 8 years ago by leif

  • Cc leif added

comment:2 Changed 8 years ago by leif

... and keep the $MAKE patches in mind (don't recall the ticket number right now), in case they're still necessary.

comment:3 follow-up: Changed 8 years ago by vbraun

If you mean #14692: I won't included anything from that ticket as long as it is not finished.

comment:4 in reply to: ↑ 3 Changed 8 years ago by leif

Replying to vbraun:

If you mean #14692: I won't included anything from that ticket as long as it is not finished.

Yes, that's what I meant.

It is finished, you just have to include the ($MAKE-related) patches.

I rebased Jean-Pierre's spkg at #2114 on mine, but then Jeroen felt he had to make trivial changes to that again, so that I would now again have to rebase mine on that of #2114, but that doesn't make sense if you're going to create a 6.0.0 anyway.

comment:5 Changed 8 years ago by jpflori

  • Cc jpflori added

Yes please include #14692, there was just a trivial change to .hgignore to do and hg addremove but I completely forgot...

comment:6 Changed 8 years ago by jpflori

If Shoup has some interest on updating NTL again, we should definitely forward him our patches.

Changed 8 years ago by vbraun

Initial patch

comment:7 Changed 8 years ago by vbraun

Please somebody go ahead and forward our patches...

comment:8 follow-up: Changed 8 years ago by jpflori

Ok, I'll take care of forwarding the patches.

comment:9 Changed 8 years ago by vbraun

  • Description modified (diff)

comment:10 Changed 8 years ago by jpflori

Groumpf, you just prevented me from posting that:

Please don't produce any spkg yet if not already done. Before forwarding the patches I have to check they still apply onto 6.0.0, so I can draft an initial spkg on top of the one from #14692 and then you can fix whatever you want on top of it to ease the git transition and so on.

So I'll just check your spkg.

comment:11 Changed 8 years ago by vbraun

Singular doesn't build with NTL-6.0.0.

In the interest of getting this git-ready let's wait with the version bump.

comment:12 Changed 8 years ago by vbraun

  • Description modified (diff)
  • Summary changed from Update NTL to 6.0.0 and track all files to Update NTL to 6.0.0
  • Work issues set to Singular

comment:13 Changed 8 years ago by vbraun

For the record, this is not fixed in Singular-3.1.6 (#14333), but is fixed in the upstream trunk https://groups.google.com/d/msg/libsingular-devel/aiMvEnN8qyg/cIFUsOXxK_MJ

comment:14 in reply to: ↑ 8 ; follow-up: Changed 8 years ago by jpflori

Replying to jpflori:

Ok, I'll take care of forwarding the patches.

Done and taken into account by Shoup. It's on his todo list. Great news.

comment:15 in reply to: ↑ 14 Changed 8 years ago by jpflori

Replying to jpflori:

Replying to jpflori:

Ok, I'll take care of forwarding the patches.

Done and taken into account by Shoup. It's on his todo list. Great news.

For future reference: http://shoup.net/pipermail/ntl_shoup.net/2013-July/000041.html

Note that Victor's answer is not appearing there.

comment:16 Changed 7 years ago by jpflori

  • Authors changed from Volker Braun to Volker Braun, Jean-Pierre Flori
  • Branch set to u/jpflori/ticket/14876
  • Commit set to 649e8890d6894d2a7913fe51989ebaf207e1d23c
  • Dependencies set to 14333
  • Description modified (diff)
  • Reviewers set to Jean-Pierre Flori
  • Status changed from new to needs_review
  • Work issues changed from Singular to segfaults

Singular update is positively reviewed. I simply gitified everything and had to add a definition in the Singular patch for NTL 6.0.0 compat. Let's get this in, I agree with Volker previous changes. The only point left is quickly check that I did not screwed up the singular patch modif; and fix all the horrible segfaults.


New commits:

649e889Update NTL to 6.0.0.
eca3246# Wed Jul 10 11:09:10 2013 -0400

comment:17 Changed 7 years ago by git

  • Commit changed from 649e8890d6894d2a7913fe51989ebaf207e1d23c to 5c6cf3dc40163956e250836c34f782f5eba93547

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

5c6cf3dUpdate Singular's patch for NTL 6.0.0 compatibility.
46c4b36Update NTL to 6.0.0.
e2fafd7# Wed Jul 10 11:09:10 2013 -0400
dd46991Remove description of deleted patches.
536a31fUpdate Singular function interface to API changes in Singular 3.1.6.
1753f06Update Singular to version 3-1-6.

comment:18 Changed 7 years ago by jpflori

  • Dependencies changed from 14333 to #14333

comment:19 Changed 7 years ago by jpflori

  • Status changed from needs_review to needs_work

comment:20 Changed 7 years ago by jpflori

It seems we now have double frees. As a first guess, I'd say that with the previous hacks we explicitely destroyed the hacked C++ objects, and we still do, but Cython might as well now it's aware that these are C++ object, and so boom.

comment:21 Changed 7 years ago by git

  • Commit changed from 5c6cf3dc40163956e250836c34f782f5eba93547 to b6d64b4fef401d71b7a6b573604691a79984fa64

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

b6d64b4Fix double free issues with NTL 6.0.0.

comment:22 Changed 7 years ago by jpflori

  • Status changed from needs_work to needs_review
  • Work issues segfaults deleted

All tests should pass now.

comment:23 Changed 7 years ago by fbissey

Just to be clear. You git branch has both #14333 and this ticket. Which now that I wrote it, seem totally normal. With git, branch depending from another ticket will include change from the depending ticket.

comment:24 Changed 7 years ago by jpflori

I don't really know what should happen here. For sure #14333 is a dependency. But I forgot the # in the dependencies field so maybe that's what confused the git plugin.

comment:25 follow-up: Changed 7 years ago by fbissey

OK so I will try to do some testing of this. Apart from singular the following packages have a dependency to ntl:

  • flint
  • eclib
  • linbox
  • "sage-clib" and sage itself.

So my plan is to start from 6.1.beta2 add #14333, this ticket the flint upgrade and then rebuild the rest.

comment:26 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:27 in reply to: ↑ 25 Changed 7 years ago by jpflori

Replying to fbissey:

OK so I will try to do some testing of this. Apart from singular the following packages have a dependency to ntl:

  • flint
  • eclib
  • linbox
  • "sage-clib" and sage itself.

So my plan is to start from 6.1.beta2 add #14333, this ticket the flint upgrade and then rebuild the rest.

Any result on that? :)

comment:28 Changed 7 years ago by fbissey

I dropped the ball because of other stuff. It will be easier to start from 6.2.beta2 since singular has been merged.

comment:29 Changed 7 years ago by fbissey

Well so far it is extremely painful and not going very far. Switching branch with sage --dev checkout brought me in a state where I ended downloading older versions of sage's dependencies. I have a thing about complaining about documentation, I'll certainly try to write some stuff down if I ever figure out things.

comment:30 Changed 7 years ago by fbissey

Oh and the tarball cannot be found for good measure.

comment:31 follow-up: Changed 7 years ago by jpflori

  • Description modified (diff)

Typo indeed.

comment:32 Changed 7 years ago by jpflori

  • Description modified (diff)

comment:33 in reply to: ↑ 31 Changed 7 years ago by fbissey

Replying to jpflori:

Typo indeed.

Can't believe I didn't notice that.

comment:34 Changed 7 years ago by fbissey

Can you rebase the branch on 6.2.beta2? I have done:

  • ./sage -dev checkout --ticket 14876
  • downloaded ntl-6.0.0.tar.bz2 to upstream
  • tried to rebuild flint...
cat build/pkgs/flint/package-version.txt 
2.3.p1

So that branch is still on the old versions of flint, singular and eclib. I need to be able to rebuild them to fully test the ticket.

comment:35 Changed 7 years ago by git

  • Commit changed from b6d64b4fef401d71b7a6b573604691a79984fa64 to 8c9d71149f0a968fc6f876991c9a73aa37f18990

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

8c9d711Merge remote-tracking branch 'trac/develop' into ticket/14876

comment:36 Changed 7 years ago by fbissey

Do you know how I should checkout the changes? ./sage -dev checkout --ticket 14876 still leaves me with flint 2.3.p1.

comment:37 Changed 7 years ago by jpflori

I'd say

git fetch --all
git checkout -b test_ntl trac/u/jpflori/ticket/14876

Of course, you need to be tracking everything from the track git branch for that to work. I don't remmeber the syntac, but something like

git fetch trac u/jpflori/ticket/14876
git checkout -b test_ntl trac/u/jpflori/ticket/14876

might do the trick if you don't.

I don't know how to do that with the dev scripts, sorry :(

comment:38 Changed 7 years ago by fbissey

Forget about the dev scripts for now.

git fetch --all
git checkout u/jpflori/ticket/14876

worked. pity I didn't think of switching to a test branch earlier. So far I did ./sage -f flint,eclib,singular,linbox and ./sage -b successfully. Running ./sage -tp 12 --all --long now.

comment:39 Changed 7 years ago by fbissey

  • Reviewers changed from Jean-Pierre Flori to Jean-Pierre Flori,François Bissey
  • Status changed from needs_review to positive_review
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 1234.6 seconds
    cpu time: 12669.7 seconds
    cumulative wall time: 13905.8 seconds

Yes!

comment:40 Changed 7 years ago by fbissey

I just had a thought. Will the dependency of ntl need an artificial bump to be rebuilt or is the upgrade system able to cope with that?

comment:41 Changed 7 years ago by vbraun

Dependents of NTL will be rebuilt automatically.

I tried upgrading and it didn't produce a working Sage, but complete rebuild does. Possibly because wrong header files are used at one point, one of the limitations of our build system.

comment:42 Changed 7 years ago by vbraun

  • Branch changed from u/jpflori/ticket/14876 to 8c9d71149f0a968fc6f876991c9a73aa37f18990
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:43 Changed 7 years ago by jpflori

  • Commit 8c9d71149f0a968fc6f876991c9a73aa37f18990 deleted

For those interested, 6.1 has been released: #15938 (nothing to see there yet).

Note: See TracTickets for help on using tickets.