Opened 12 years ago

Closed 10 years ago

Last modified 10 years ago

#7128 closed defect (duplicate)

zlib-1.2.3.p4 always builds 32-bit binaries on Solaris.

Reported by: drkirkby Owned by: tbd
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: porting Keywords:
Cc: jsp Merged in:
Authors: Reviewers: Jaap Spies, David Kirkby
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

An inspection of spkg-install for zlib shows that the -m64 flag is only added on OS X, and not on Solaris.

# The -fPIC is needed otherwise builing libpng fails later
# (at least on a Debian 64-bit opteron).

if [ `uname` = "Darwin" -a "$SAGE64" = "yes" ]; then
   CFLAGS=" -m64 $CFLAGS -fPIC -g -I\"$SAGE_LOCAL/include\""
   cp ../patches/configure-OSX-64 configure
else
   CFLAGS="$CFLAGS -fPIC -g -I\"$SAGE_LOCAL/include\""
fi

There are several things wrong with this

  • -fPIC is not a universally used flag. The correct flag to use on Solaris is -KPIC, though -fPIC will be accepted. On other compilers, such as those on AIX or HP-UX, there is no guarantee that -fPIC is the correct flag.
  • The -m64 flag to build 64-bit code is only used on OS X. It is not used on Solaris, despite the fact we are supposed to be supporting Solaris. On some compilers, the correct flag to produce 64-bit code is not -m64. IBM's compiler on AIX uses -q64, and HP's on HP-UX uses +DD64.

Change History (13)

comment:1 Changed 11 years ago by drkirkby

  • Cc david.kirkby@… added
  • Report Upstream set to N/A

comment:2 Changed 11 years ago by drkirkby

  • Authors set to David Kirkby
  • Cc jsp added; david.kirkby@… removed
  • Status changed from new to needs_review

I've sorted this out for Solaris. Currently the option added is always -m64, which is not ideal, as it will break with compilers other than GNU or Sun, but when #7818 get added, sorting this lot out will be a lot easier.

On OS X, not only is -m64 added to the flags, but an altered version of a configure script is copied too. There is no need to change the configure script on Solaris, so OS X is still handled differently.

Previous code:

if [ "x`uname`" = "xDarwin" ] && [ "x$SAGE64" = "xyes" ]; then
   CFLAGS=" -m64 $CFLAGS -fPIC -g -I\"$SAGE_LOCAL/include\""
   cp ../patches/configure-OSX-64 configure
else
   CFLAGS="$CFLAGS $FPIC_FLAG -g -I\"$SAGE_LOCAL/include\""
fi

Revised code:

if [ "x`uname`" = "xDarwin" ] && [ "x$SAGE64" = "xyes" ]; then
   CFLAGS=" -m64 $CFLAGS -fPIC -g -I\"$SAGE_LOCAL/include\""
   cp ../patches/configure-OSX-64 configure
elif [ "x`uname`" != "xDarwin" ] && [ "x$SAGE64" = "xyes" ]; then
   CFLAGS="-m64 $CFLAGS $FPIC_FLAG -g -I\"$SAGE_LOCAL/include\""
else
   CFLAGS="$CFLAGS $FPIC_FLAG -g -I\"$SAGE_LOCAL/include\""
fi

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

Sorry, I forgot to add the location of the code.

http://boxen.math.washington.edu/home/kirkby/portability/zlib-1.2.3.p6/

There's an spkg there. Best tested on Solaris, using a 64-bit build.

comment:4 in reply to: ↑ 3 Changed 11 years ago by jsp

Replying to drkirkby:

Sorry, I forgot to add the location of the code.

http://boxen.math.washington.edu/home/kirkby/portability/zlib-1.2.3.p6/

There's an spkg there. Best tested on Solaris, using a 64-bit build.

I was about to ask :)

Jaap

comment:5 Changed 11 years ago by jsp

Looks good, but SPKG.txt is not according the standards.

e.g. missing Changelog header, new line between the entries.

Jaap

comment:6 Changed 11 years ago by jsp

  • Reviewers set to Jaap Spies

comment:7 Changed 11 years ago by drkirkby

I've correct those two defects.

dave

comment:8 Changed 11 years ago by jsp

  • Status changed from needs_review to positive_review

Looks good. Works ok. So positive review.

comment:9 Changed 11 years ago by rlm

Oops! This link appears to be broken. Where is the spkg?

comment:10 Changed 11 years ago by drkirkby

  • Status changed from positive_review to needs_work

Please ignore this patch. I could have swore I updated the ticket to say so, but seem to have forgotten. I purposely removed the link. Just ignore it. I've set it to needs work.

comment:11 Changed 10 years ago by drkirkby

This can be closed as fixed by #9008 in sage-4.5.alpha0, when zlib was updated.

comment:12 Changed 10 years ago by jdemeyer

  • Authors David Kirkby deleted
  • Resolution set to duplicate
  • Reviewers changed from Jaap Spies to Jaap Spies, David Kirkby
  • Status changed from needs_work to closed

comment:13 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.7 to sage-duplicate/invalid/wontfix
Note: See TracTickets for help on using tickets.