Opened 11 years ago

Closed 11 years ago

#11107 closed defect (fixed)

bzip2 only considers the SAGE64 environment variable on Solaris and OS X.

Reported by: drkirkby Owned by: GeorgSWeber
Priority: major Milestone: sage-4.7
Component: build Keywords:
Cc: jhpalmieri Merged in: sage-4.7.rc0
Authors: David Kirkby Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jhpalmieri)

I discovered this after trying to debug a Linux installation.

The bzip2 package (in particular the file $SAGE_ROOT/spkg/base/bzip2-1.0.5-install) handles SAGE64 in a different way to the all other packages. It has:

if [ `uname` = "Darwin" -a "$SAGE64" = "yes" ]; then
   echo "Building 64 bit OSX release"
   CFLAGS="-O2 -g -m64 " && export CFLAGS
fi

if [ `uname` = "SunOS" -a "$SAGE64" = "yes" ]; then
   echo "Building 64 bit Solaris release"
   CFLAGS="-O2 -g -m64 " && export CFLAGS
fi

Faster, more flexible and more portable would be:

if [ "x$CFLAG64" = x ] ; then
   CFLAG64=-m64
fi

if [ "x$SAGE64" = xyes ]; then
   echo "Building a 64 bit version of bzip2"
   CFLAGS="-O2 -g $CFLAG64 $CFLAGS"
   export CFLAGS
fi

This would

  • Be consistent with all other packages.
  • Work on any platform, which might be useful on some like AIX
  • Could help debug some Linux issues if Linux is building 32-bit. (I found this useful to help someone on sage-support)
  • Avoids two calls to uname
  • Allows any flag to be used for building 64-bit executable, not just the one use by Sun/Oracle? compilers and GCC. Commercial compilers for both AIX and HP-UX use different flags.
  • Would allow any flags set by the user in CFLAGS to override those here. However, currently setting CFLAGS globally in Sage can not be considered a good idea, but that's a more general bug and well outside the scope of this ticket.

Dave


Apply 11107-bzip2-to-handle-SAGE64-better.patch to the spkg/base repo.

Attachments (1)

11107-bzip2-to-handle-SAGE64-better.patch (1.3 KB) - added by drkirkby 11 years ago.
Mercurial patch to clear up the bzip2 problem so SAGE64 works consistently on any platform, not just Solaris and OS X

Download all attachments as: .zip

Change History (6)

comment:1 Changed 11 years ago by drkirkby

  • Description modified (diff)

Changed 11 years ago by drkirkby

Mercurial patch to clear up the bzip2 problem so SAGE64 works consistently on any platform, not just Solaris and OS X

comment:2 Changed 11 years ago by drkirkby

  • Status changed from new to needs_review

comment:3 Changed 11 years ago by drkirkby

  • Description modified (diff)

I checked and found bzip2 is the only package with this bug, so I have updated the description to reflect this.

Dave

comment:4 Changed 11 years ago by jhpalmieri

  • Authors set to David Kirkby
  • Description modified (diff)
  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

The patch looks good to me.

comment:5 Changed 11 years ago by jdemeyer

  • Merged in set to sage-4.7.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.