#9523: Upgrade the Readline spkg to 6.1
------------------------+---------------------------------------------------
Reporter: cwitty | Owner: tbd
Type: defect | Status: needs_work
Priority: blocker | Milestone: sage-4.6.1
Component: packages | Keywords:
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
------------------------+---------------------------------------------------
Changes (by leif):
* status: needs_review => needs_work
Comment:
A few comments:
* The way {{{CFLAGS}}} (and {{{CXXFLAGS}}}) are set overrides user-
specified settings ({{{-O2}}}, {{{-Wall}}}).
* The handling of {{{SAGE_DEBUG}}} is inconsistent (and does not disable
optimization):
{{{
#!sh
# If SAGE_DEBUG is set either unset (the default), or set to 'yes'
# then add debugging information.
# Since both the Sun and GNU compilers accept -g to give debugging
information,
# there is no need to do anything specific to one compiler or the other.
if [ "x$SAGE_DEBUG" = "x" ] || [ "x$SAGE_DEBUG" = "xyes" ] ; then
echo "Code will be built with debugging information present. Set
'SAGE_DEBUG' to 'no' if you don't want that."
# Actually anything other than 'yes' or '1' will cause
# no debugging information to be added.
CFLAGS="$CFLAGS -g "
CXXFLAGS="$CXXFLAGS -g "
else
echo "No debugging information will be used during the build of this
package."
echo "Unset SAGE_DEBUG if you want debugging information present (-g
added)."
fi
}}}
I would add debugging symbols by default anyway.
* {{{make}}} is used instead of {{{$MAKE}}}. I don't know why we do the
build and install in one call.
* The following is most probably wrong for readline 6.1:
{{{
#!sh
elif [ "$UNAME" = "OpenBSD" ]; then
DYLIB_NAME="$SAGE_LOCAL"/lib/libreadline.so.6.0
}}}
* The following should simply be {{{if grep -q ... ; then ...}}}:
{{{
#!sh
if [ `grep 11.1 /etc/SuSE-release > /dev/null; echo $?` -eq 0 ]; then
}}}
(or at least {{{if grep ... >/dev/null; then ...}}}). The preceding
{{{test -f}}} is also superfluous, but I agree makes it perhaps more
readable.
* The use of {{{set +/-e}}} is quite confusing and error-prone, and its
use is '''definitely wrong here''', since we don't get the exit status of
"copying patches" and, worse, {{{build()}}}:
{{{
#!sh
...
set -e
...
cp patches/shobj-conf src/support/
if [ $? -ne 0 ]; then
echo "Error copying patch over."
exit 1
fi
cd src/
build()
{
./configure --prefix="$SAGE_LOCAL" $CONF_FLAGS
make install
}
build
set +e
if [ $? -ne 0 ]; then
echo "Error building and installing readline."
exit 1
fi
set -e
...
}}}
* It is safer to quote (all instances of) {{{$UNAME}}}.
* The {{{build()}}} functions is almost useless, and in fact does {{{make
install}}} (see above).
I originally wanted to set this to "needs info" (w.r.t. OpenBSD), but now
I think at least ''some of'' the above really needs to be fixed, therefore
"needs work".
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9523#comment:17>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
