Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eclib ignores SAGE64 if OS is not Darwin #7814

Closed
sagetrac-drkirkby mannequin opened this issue Jan 2, 2010 · 16 comments
Closed

eclib ignores SAGE64 if OS is not Darwin #7814

sagetrac-drkirkby mannequin opened this issue Jan 2, 2010 · 16 comments

Comments

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 2, 2010

eclib-20080310.p7 suffered the usual problem of many packages - SAGE64 was ignored unless the operating system was OS X. This trivial patch simply ensure SAGE64 is not ignored on any platform.

I've checked in the changes with 'hg'

See:
http://boxen.math.washington.edu/home/kirkby/portability/eclib-20080310.p8/

Component: porting

Author: David Kirkby

Reviewer: Jaap Spies

Merged: sage-4.3.1.alpha2

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

@sagetrac-drkirkby sagetrac-drkirkby mannequin added this to the sage-4.3.1 milestone Jan 2, 2010
@sagetrac-drkirkby sagetrac-drkirkby mannequin self-assigned this Jan 2, 2010
@jaapspies
Copy link
Member

Reviewer: Jaap Spies

@jaapspies
Copy link
Member

comment:1

I think the wrong spkg is in this link.

Jaap

@jaapspies
Copy link
Member

comment:2

I see:

if [ "$SAGE64" = "yes" ]; then
    echo "64 bit MacIntel build"
    DYN_FLAGS="-m64"; export DYN_FLAGS
    PICFLAG="-m64 -fPIC"
fi
export PICFLAG

in spkg-install

Jaap

@sagetrac-drkirkby
Copy link
Mannequin Author

sagetrac-drkirkby mannequin commented Jan 2, 2010

comment:3

yes, I should have removed that comment about the MacIntel. I think you will find it does build, but I will remove that command and make a new package.

@jaapspies
Copy link
Member

comment:4

If there is a new spkg (see above) I can give it a positive review. Tested on Fedora 12 and Open Solaris 32 bit

real	4m15.073s
user	3m35.053s
sys	0m24.029s
Successfully installed eclib-20080310.p8
You can safely delete the temporary build directory
/export/home/jaap/Downloads/sage-4.3/spkg/build/eclib-20080310.p8
Making Sage/Python scripts relocatable...
Making script relocatable
Finished installing eclib-20080310.p8.spkg
jaap@opensolaris:~/Downloads/sage-4.3$ 

After a successful install of ntl and pari.

Jaap

@sagetrac-drkirkby
Copy link
Mannequin Author

sagetrac-drkirkby mannequin commented Jan 3, 2010

comment:5

I've updated the package. It is now missing the comment that its a MacIntel.

Please double check the package again though please - just in case I've messed up.

dave

@jaapspies
Copy link
Member

comment:6

Replying to @sagetrac-drkirkby:

I've updated the package. It is now missing the comment that its a MacIntel.

Please double check the package again though please - just in case I've messed up.

dave

Sure,

Jaap

@jaapspies
Copy link
Member

comment:7

On Open Solaris:

real	4m8.443s
user	3m36.556s
sys	0m24.188s
Successfully installed eclib-20080310.p8

Looks good, build tested on Fedora 12 and Fedora 11.

Positive review.

Jaap

@jaapspies
Copy link
Member

comment:8

Couldn't change to positive review. Will do now.

Jaap

@sagetrac-drkirkby
Copy link
Mannequin Author

sagetrac-drkirkby mannequin commented Jan 3, 2010

comment:9

Cheers.

I'm just going to remove the 'needs review from the title. I'm not sure if we are still supposed to do that or not. I find it easier to find me own sometime if its in the title. But anyway, I'm removing it now.

@jaapspies
Copy link
Member

comment:10

Hi,

It already had a positive review, so ...?

Cheers,

Jaap

@rlmill
Copy link
Mannequin

rlmill mannequin commented Jan 13, 2010

Merged: 4.3.1.alpha2

@rlmill rlmill mannequin removed the s: positive review label Jan 13, 2010
@rlmill rlmill mannequin closed this as completed Jan 13, 2010
@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 13, 2010

Changed merged from 4.3.1.alpha2 to sage-4.3.1.alpha2

@JohnCremona
Copy link
Member

comment:14

Dave, I only just noticed this ticket (from the Release Notes). I think you should have incereased the patch level from p8 to p9 - there now exist two different version s of the eclib spkg both called eclib-20080310.p8 which is rather confusing.

John

@sagetrac-drkirkby
Copy link
Mannequin Author

sagetrac-drkirkby mannequin commented Jan 24, 2010

comment:15

Looking at the diff file I made of the SPKG.txt

http://boxen.math.washington.edu/home/kirkby/portability/eclib-20080310.p8/SPKG.txt.diff

the last revision made was in

=== eclib-20080310.p7 (Michael Abshoff, October 12th, 2008) ===

then I added

eclib-20080310.p8 (David Kirkby, 2nd January 2010)

  • Allow SAGE64 to work on all platforms, not just OS X.

Are you sure the previous one was patch level 8 and not 7? If it was, then SPKG.txt was not updated when it moved to 8. Sorry about that, if I did overlook this. I agree it is confusing, if this is so.

For it to also be merged, and the release manager not notice, seems a bit strange.

Dave

@JohnCremona
Copy link
Member

comment:16

What I found was this. On my own computer I have a p8 with the following changelog entry:

=== eclib-20080310.p8 (John Cremona, January 6th, 2009) ===
 * Change to debugging output in procs/p2points.cc (not relevant for Sage)
 * Change to pdivs() in procs/marith.cc (not relevant for Sage)

Now, whatever that was about, it was not relevant for Sage (either referred to functions not used by anything wrapped in Sage, or under compiler options which Sage does not use), and presumably for that reason I did not make a ticket for it to replace the (then) standard p7 in Sage.

I guess the thing for me to do now is to make a p9 which has both the changes I made in my p8 and the ones you made, and get it into Sage. I have to keep the version of the source files which are used by Sage in sync with the versions I have, otherwise I'll go mad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants