Opened 12 years ago

Closed 12 years ago

#9592 closed defect (fixed)

Upgrade lcalc to work with Pari svn snapshot 12577 - a pre-release of Pari 2.4.3

Reported by: Jeroen Demeyer Owned by: Jeroen Demeyer
Priority: blocker Milestone: sage-4.6
Component: packages: standard Keywords:
Cc: John Cremona Merged in: sage-4.6.alpha0
Authors: John Cremona, Jeroen Demeyer, Rishikesh Reviewers: Jeroen Demeyer, Leif Leonhardy, Mitesh Patel
Report Upstream: Reported upstream. Little or no feedback. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

After upgrading PARI/GP to a snapshot based on a pre-release of 2.4.3 (#9343), lcalc no longer compiles properly.

See http://wiki.sagemath.org/NewPARI for more information and links.

Attachments (3)

lcalc-newpari.patch (818 bytes) - added by Jeroen Demeyer 12 years ago.
spkg-install.diff (769 bytes) - added by Rishikesh 12 years ago.
lcalc-spkg.patch (12.2 KB) - added by Jeroen Demeyer 12 years ago.
Complete spkg patch (for reference)

Download all attachments as: .zip

Change History (40)

Changed 12 years ago by Jeroen Demeyer

Attachment: lcalc-newpari.patch added

comment:1 Changed 12 years ago by Jeroen Demeyer

Owner: changed from tbd to Jeroen Demeyer
Report Upstream: N/AReported upstream. Little or no feedback.

New version which works with PARI 2.4.3: http://cage.ugent.be/~jdemeyer/sage/lcalc-20100428-1.23.p1.spkg

I have also notified the upstream contact Michael Rubinstein and sent him the patch lcalc-newpari.patch

comment:2 Changed 12 years ago by Jeroen Demeyer

Milestone: sage-4.5.2sage-4.6

comment:3 Changed 12 years ago by Jeroen Demeyer

Authors: Jeroen Demeyer

comment:4 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

comment:5 Changed 12 years ago by Mitesh Patel

The patch level for the new spkg should be 2, since we used p1 for #9665. This should fix the "already installed" problem reported by John Cremona in comment 180 at #9343.

comment:6 in reply to:  5 ; Changed 12 years ago by Leif Leonhardy

Replying to mpatel:

The patch level for the new spkg should be 2, since we used p1 for #9665. This should fix the "already installed" problem reported by John Cremona in comment 180 at #9343.

Just noticed that, too. :)

There are also post-merge comments at #9665, I don't know if they should be included here or if there's even a new ticket for these.

comment:7 Changed 12 years ago by Leif Leonhardy

Jeroen, could you describe in more detail which failure(s) this ticket is intended to fix (including Sage version, operating system, processor etc.)?

John Cremona has successfully installed and tested #9343 (unintentionally) without this new spkg, and I've also successfully installed the other to spkgs and applied the patches from #9343 on top of Sage 4.5.3.alpha0 + #9475 and #9717 on Fedora 13 x86 (Pentium 4 Prescott, gcc 4.4.4).

Of course lcalc wasn't (re)built in the above tests. (I'll try that later, currently running ptestlong without the lcalc package from here.)

comment:8 Changed 12 years ago by Leif Leonhardy

Just for the record: SPKG.txt currently lacks a Dependencies section, the package has no spkg-check, and spkg-install uses make, not $MAKE...

comment:9 Changed 12 years ago by Leif Leonhardy

Cc: John Cremona added

In addition, it contains at least two or three files that should be deleted: some *.bak file, a *.swap.crap file and some static library (*.a) I think we don't need (correct me if I'm wrong here).

(In case Jeroen's patch here is now obsolete due to a meanwhile newer PARI version at #9343, which John Cremona is currently investigating, we could recycle this ticket to address the above mentioned issues.)

comment:10 Changed 12 years ago by John Cremona

Updated spkg: http://www.warwick.ac.uk/staff/J.E.Cremona/lcalc-20100428-1.23.p2.spkg

(This version is based on lcalc-20100428-1.23.p1.spkg as merged in 4.5.2)

I made this before seeing the recent comments here. Feel free to add the Dependencies section etc -- I will not have time to do that for at least a day.

comment:11 in reply to:  6 Changed 12 years ago by Mitesh Patel

Replying to leif:

There are also post-merge comments at #9665, I don't know if they should be included here or if there's even a new ticket for these.

I've added a note at #9665 about this ticket.

comment:12 Changed 12 years ago by David Kirkby

Description: modified (diff)
Summary: Upgrade lcalc to pari 2.4.3Upgrade lcalc to work with Pari svn snapshot 12577 - a pre-release of Pari 2.4.3

I've changed the title and description a bit, to reflect the fact that #9343 is not Pari 2.4.3.

Not even Pari 2.4.2 has ever been released - there is only an alpha of that available.

Dave

comment:13 Changed 12 years ago by David Kirkby

Description: modified (diff)

comment:14 in reply to:  12 ; Changed 12 years ago by Leif Leonhardy

Replying to drkirkby:

I've changed the title and description a bit, to reflect the fact that #9343 is not Pari 2.4.3.

You should perhaps have updated the spkg link to point to John Cremona's new p2 spkg, too. ;-)

comment:15 in reply to:  14 ; Changed 12 years ago by David Kirkby

Replying to leif:

Replying to drkirkby:

I've changed the title and description a bit, to reflect the fact that #9343 is not Pari 2.4.3.

You should perhaps have updated the spkg link to point to John Cremona's new p2 spkg, too. ;-)

I don't know where it is. In any case, it should not be a .p2, since the current one in Sage is lcalc-20100428-1.23.p0.spkg, to a revision should be called lcalc-20100428-1.23.p1.spkg.

I hope the

gcc -Wa,-W

(to suppress warnings from the assembler), has not got back in, as -W is not recognised by the Sun assembler and it creates an error.

Dave

comment:16 in reply to:  15 Changed 12 years ago by Leif Leonhardy

Replying to drkirkby:

Replying to leif:

Replying to drkirkby:

I've changed the title and description a bit, to reflect the fact that #9343 is not Pari 2.4.3.

You should perhaps have updated the spkg link to point to John Cremona's new p2 spkg, too. ;-)

I don't know where it is. In any case, it should not be a .p2, since the current one in Sage is lcalc-20100428-1.23.p0.spkg, to a revision should be called lcalc-20100428-1.23.p1.spkg.

Browser cache issue?

See http://trac.sagemath.org/sage_trac/ticket/9592#comment:10 (where)

and http://trac.sagemath.org/sage_trac/ticket/9592#comment:5 (why).

(lcalc-20100428-1.23.p1.spkg from #9665 was merged into Sage 4.5.2.rc1)

comment:17 Changed 12 years ago by Jeroen Demeyer

I removed some .DS_Store and ._.DS_Store files from John's spkg and uploaded the result to http://cage.ugent.be/~jdemeyer/sage/lcalc-20100428-1.23.p2.spkg

comment:18 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

comment:19 Changed 12 years ago by Rishikesh

Status: newneeds_review

I can see that this spkg does not depend on the upgrade to the new pari. This can be included before the latest version of pari is accepted. In couple of month, I will try to get Mike to use autotools for building. This will eliminate a lot of problems with spkg as of now. I am changing the status to needs review if it is ok with you.

comment:20 Changed 12 years ago by Rishikesh

Following suggestion of Mitesh, I have small some small cleaning up of unnecessary files in patches and few lines in spkg-install over the changes of jdemeyer.

http://sage.math.washington.edu/home/rishikesh/lcalc/lcalc-20100428-1.23.p2.spkg

comment:21 Changed 12 years ago by Leif Leonhardy

Replying to rishi:

I can see that this spkg does not depend on the upgrade to the new pari. This can be included before the latest version of pari is accepted. In couple of month, I will try to get Mike to use autotools for building. This will eliminate a lot of problems with spkg as of now. I am changing the status to needs review if it is ok with you.

Perhaps do some of the clean-up I suggested above?

There are further minor things (like the date/version at the top of spkg-install; SAGE_DEBUG=yes usually disables optimization, unquoted environment variables, etc.).

I wonder if we should add (a) further patch(es) to get rid of some of the annoying warnings (cf. http://trac.sagemath.org/sage_trac/ticket/9343#comment:191 ff.), but we probably shouldn't do too much at this ticket.

I'm not sure if Cygwin support is required yet... ;-)

comment:22 in reply to:  20 Changed 12 years ago by Leif Leonhardy

Replying to rishi:

Following suggestion of Mitesh, I have small some small cleaning up of unnecessary files in patches and few lines in spkg-install over the changes of jdemeyer.

http://sage.math.washington.edu/home/rishikesh/lcalc/lcalc-20100428-1.23.p2.spkg

Could you upload an spkg patch for your changes (except file deletions) here?

It's a bit more convenient for reviewing and adding further changes...

comment:23 in reply to:  20 Changed 12 years ago by Jeroen Demeyer

Replying to rishi:

Following suggestion of Mitesh, I have small some small cleaning up of unnecessary files in patches and few lines in spkg-install over the changes of jdemeyer.

http://sage.math.washington.edu/home/rishikesh/lcalc/lcalc-20100428-1.23.p2.spkg

I copied your spkg to http://cage.ugent.be/~jdemeyer/sage/lcalc-20100428-1.23.p2.spkg (like this, I don't have to update the descriptions of #9343 and #9592).

Changed 12 years ago by Rishikesh

Attachment: spkg-install.diff added

comment:24 in reply to:  21 Changed 12 years ago by Rishikesh

I am not sure what you want to be done. Can you make the changes and attach it here.

Replying to leif:

Replying to rishi:

I can see that this spkg does not depend on the upgrade to the new pari. This can be included before the latest version of pari is accepted. In couple of month, I will try to get Mike to use autotools for building. This will eliminate a lot of problems with spkg as of now. I am changing the status to needs review if it is ok with you.

Perhaps do some of the clean-up I suggested above?

There are further minor things (like the date/version at the top of spkg-install; SAGE_DEBUG=yes usually disables optimization, unquoted environment variables, etc.).

I wonder if we should add (a) further patch(es) to get rid of some of the annoying warnings (cf. http://trac.sagemath.org/sage_trac/ticket/9343#comment:191 ff.), but we probably shouldn't do too much at this ticket.

I'm not sure if Cygwin support is required yet... ;-)

comment:25 Changed 12 years ago by Mitesh Patel

Priority: majorblocker

Changed 12 years ago by Jeroen Demeyer

Attachment: lcalc-spkg.patch added

Complete spkg patch (for reference)

comment:26 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

comment:27 Changed 12 years ago by Mike Hansen

I've made an spkg that builds on Cygwin at #9775 based on the one here. It might make more sense to make any additional changes to the SPKG there.

comment:28 in reply to:  27 Changed 12 years ago by Mitesh Patel

Replying to mhansen:

I've made an spkg that builds on Cygwin at #9775 based on the one here. It might make more sense to make any additional changes to the SPKG there.

See #9845, too.

comment:29 Changed 12 years ago by Mitesh Patel

Authors: Jeroen DemeyerJeroen Demeyer, Rishi

Rishi, do Jeroen's changes look good to you? If they are, I suggest that we leave further changes for other tickets.

comment:30 Changed 12 years ago by Mitesh Patel

Authors: Jeroen Demeyer, RishiJeroen Demeyer, Rishikesh

comment:31 Changed 12 years ago by Leif Leonhardy

Also, the dist/ (Debian) directory should be removed, cf. #5903.

comment:32 in reply to:  31 ; Changed 12 years ago by Mitesh Patel

Replying to leif:

Also, the dist/ (Debian) directory should be removed, cf. #5903.

I suggest that we do this at #9845, unless we otherwise need to update the package here.

comment:33 in reply to:  32 Changed 12 years ago by Leif Leonhardy

Replying to mpatel:

Replying to leif:

Also, the dist/ (Debian) directory should be removed, cf. #5903.

I suggest that we do this at #9845, unless we otherwise need to update the package here.

Yes; hopefully #9845 gets reviewed soon s.t. this ticket won't get merged at all (positively reviewed though), since the former contains all changes from here.

comment:34 Changed 12 years ago by Mitesh Patel

Authors: Jeroen Demeyer, RishikeshJohn Cremona, Jeroen Demeyer, Rishikesh

Do we have a positive review here?

comment:35 Changed 12 years ago by Leif Leonhardy

Doesn't bother me, but should we keep

# disable Cygwin build for now
if [ "$UNAME" = "CYGWIN" ]; then
#    cp ../../patches/Lcommandline_elliptic.cc .
    echo "Sorry, the lcalc build is currently broken"
    echo 1
fi

? (In case I by luck have looked at the current .p2 / its current patches/diffs.)

Fortunately there's a follow-up ticket to address the rest...

comment:36 Changed 12 years ago by Mitesh Patel

Reviewers: Jeroen Demeyer, Leif Leonhardy, Mitesh Patel
Status: needs_reviewpositive_review

Is patches/Lcommandline_elliptic.cc.cygwin.diff up to date? Do we still use patches/Lcommandline_elliptic.cc.cygwin.*. Let's do any necessary updates at #9845.

comment:37 Changed 12 years ago by Mitesh Patel

Merged in: sage-4.6.alpha0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.