An improved spkg-install (for general usage)
Dr. David Kirkby
assembler to suppress warnings. I've seen other packages send warning to
/dev/null.
I decided to change the spkg-install for 'lcalc' somewhat, in a way that
I think can be better for most packages.
The revised spkg-install does the following things, which most
spkg-install's do not do.
1) It uses 'set -e' to exit the spkg-install if any error occurs.
2) It tests for SAGE_LOCAL in a somewhat better way than present
if [ "x$SAGE_LOCAL" = "x" ]
(It's not a good idea to test on "", though I'm sure one could argue it
has not broken yet)
3) It adds -m64 if the variable SAGE64 is set to 'yes'. (If you don't
set SAGE64, the system will do whatever its default happens to be).
4) It builds with debug flags (-g) by default (as agreed the other day)
if SAGE_DEBUG is not set.
5) If SAGE_DEBUG is set to 1, 'yes' or 'TRUE', then again it will build
with -g.
But any other setting of SAGE_DEBUG will result in -g not being in
CFLAGS. Same for CXXFLAGS
6) It checks to determine if the C compiler is Sun or GNU, then adds
appropriate flags to show all warnings, depending on the compiler.
(-Wall for GNU. I'm not sure if my choice of flags with Sun's compiler
is best, but I'll find that out later).
7) Same as (5) above, but for the C++ compiler.
8) Checks if the C and C++ compilers are not a mix of Sun and GNU. In
other words, if you try to use a Sun C compiler with a GNU C++ compiler,
it will exit with an error.
Ideally this needs extending to Fortran too.
It would also be sensible to check that the version of gcc is the same
as the version of g++ and gfortran. It's quite possible the first C++
compiler in the path is version X, someone sets CC to a later version Y,
but does not do it with CXX. So the compilers are mixed. I don't
specifically know if that will break, but it can't be sensible.
The latter part of spkg-install I've not touched. I'm interested in
general thoughts on this. I'd like to see a bit less of hiding of warnings.
I'm attaching the file for general comment - not particularly about its
use in 'lcalc', though that was my motivation, as its attempts to
suppress assembler warnings were causing problems on Solaris, as the
assembler did not accept the GNU flag.
Dave
William Stein
> I noticed one package (lcalc) was passing GNU specific flags to the
> assembler to suppress warnings. I've seen other packages send warning to
> /dev/null.
>
> I decided to change the spkg-install for 'lcalc' somewhat, in a way that
> I think can be better for most packages.
>
>
> The revised spkg-install does the following things, which most
> spkg-install's do not do.
>
> 1) It uses 'set -e' to exit the spkg-install if any error occurs.
> 2) It tests for SAGE_LOCAL in a somewhat better way than present
>
> if [ "x$SAGE_LOCAL" = "x" ]
>
> (It's not a good idea to test on "", though I'm sure one could argue it
> has not broken yet)
better, and to me it looks worse.
> 3) It adds -m64 if the variable SAGE64 is set to 'yes'. (If you don't
> set SAGE64, the system will do whatever its default happens to be).
into Sage, i.e., it's only run via pexpect.
> 4) It builds with debug flags (-g) by default (as agreed the other day)
> if SAGE_DEBUG is not set.
>
> 5) If SAGE_DEBUG is set to 1, 'yes' or 'TRUE', then again it will build
> with -g.
>
> But any other setting of SAGE_DEBUG will result in -g not being in
> CFLAGS. Same for CXXFLAGS
specific list of allowed values or not set, and if not, it immediately
>
> 6) It checks to determine if the C compiler is Sun or GNU, then adds
> appropriate flags to show all warnings, depending on the compiler.
> (-Wall for GNU. I'm not sure if my choice of flags with Sun's compiler
> is best, but I'll find that out later).
>
> 7) Same as (5) above, but for the C++ compiler.
>
> 8) Checks if the C and C++ compilers are not a mix of Sun and GNU. In
> other words, if you try to use a Sun C compiler with a GNU C++ compiler,
> it will exit with an error.
>
> Ideally this needs extending to Fortran too.
>
> It would also be sensible to check that the version of gcc is the same
> as the version of g++ and gfortran. It's quite possible the first C++
> compiler in the path is version X, someone sets CC to a later version Y,
> but does not do it with CXX. So the compilers are mixed. I don't
> specifically know if that will break, but it can't be sensible.
>
> The latter part of spkg-install I've not touched. I'm interested in
> general thoughts on this. I'd like to see a bit less of hiding of warnings.
made generic.
>
>
> I'm attaching the file for general comment - not particularly about its
> use in 'lcalc', though that was my motivation, as its attempts to
> suppress assembler warnings were causing problems on Solaris, as the
> assembler did not accept the GNU flag.
>
> Dave
>
> >
>
> # Test for SAGE_LOCAL in the best possible way.
> if [ "x$SAGE_LOCAL" = "x" ]; then
> echo "SAGE_LOCAL undefined ... exiting";
> echo "Maybe run 'sage -sh'?"
> exit 1
> fi
>
> # Force an exit on all errors. However, not it will be desirable to unset this
> # sometimes, as you might want to test for a particular error and exit with
> # a message. You can't do that with 'set -e'. So use 'set +e' to unset it
> set -e
>
> # Compile for 64-bit if SAGE64 is set to 'yes'. This is documented, so only 'yes' will do.
> if [ "x$SAGE64" = "xyes" ]; then
> CFLAGS="$CFLAGS -m64 "
> fi
>
> # If SAGE_DEBUG is set either unset (the default), set to '1', 'yes', or 'TRUE'
> # then build with debugging information. Otherwise, don't 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" -o "x$SAGE_DEBUG" = "x1" ] ; then
> echo "Code will be built with debugging information present. Set 'SAGE_DEBUG' to '0' if you don't want that."
> CFLAGS="$CFLAGS -g "
> CXXFLAGS="$CXXFLAGS -g "
> elif [ "x$SAGE_DEBUG" = "xyes" -o "x$SAGE_DEBUG" = "xTRUE" ] ; then
> echo "Code will be built with debugging information present. Set 'SAGE_DEBUG' to '0' if you don't want that."
> CFLAGS="$CFLAGS -g "
> CXXFLAGS="$CXXFLAGS -g "
> fi
>
> # Display all warnings from the compilers. Do so in a way that works for
> # both The GNU Compiler Collection (gcc, g++, gfortran), as well as for
> # Sun's compilers. Since the compilers use different methods, the compiler
> # must be tested.
>
> # gcc/g++/gfortran uses -Wall to show this.
> # The best flags for the Sun compilers are not known, but my best guess
> # is below.
>
> if "$CC" -flags 2>&1 | grep -i sun ; then
> CFLAGS="$CFLAGS -xtransition -erroff=none "
> SUN_COMPILER=1
> elif "$CC" --version | grep 'GCC' ; then
> CFLAGS="$CFLAGS -Wall "
> GNU_COMPILER=1
> else
> echo "The spkg-install file can't work out what C compiler you are using"
> echo "No flags will be added to show extra warnings while compiling C"
> fi
>
> # Determine if the C++ compiler is the Sun or GNU compiler
> # Add appropiate flag(s) to show all warnings.
> if "$CXX" -flags 2>&1 | grep -i sun ; then
> CXXFLAGS="$CXXFLAGS -xtransition -erroff=none "
> SUN_COMPILER=1
> elif "$CXX" --version | grep 'GCC' ; then
> CXXFLAGS="$CXXFLAGS -Wall "
> GNU_COMPILER=1
> else
> echo "The spkg-install file can't work out what C++ compiler you are using"
> echo "No flags will be added to show extra warnings while compiling C++"
> fi
>
> # Determine if the Fortran compiler is the Sun or GNU compiler
> # Add appropiate flag(s) to show all warnings.
> # Until I know exactly how a user is supposed to specify that, I'll leave that.
> # In any case, fortran is not used in lcalc.
>
> # Checks that the user is not mixing the Sun and GNU compilers. This problem
> # has been seen on code built with the aid of SCons, but in general could
> # happen with any code if the user has specified a C compiler but not a C++ one.
>
> if [ "x$SUN_COMPILER" = "x1" -a "x$GNU_COMPILER" = "x1" ] ; then
> echo "You are mixing the Sun and GNU C/C++ compilers"
> echo "Such a combination will lead to problems. Check CC and CXX carefully."
> echo "Exiting ..."
> exit 1
> fi
>
> cd src
>
> if [ `uname` = "SunOS" ]; then
> LCALC_LIBS="-lpari -lmpfr -lgmpxx -lgmp -liberty"
> else
> LCALC_LIBS="-lpari -lmpfr -lgmpxx -lgmp"
> fi
>
> export CC CXX CFLAGS CXXFLAGS LCALC_LIBS
>
> echo "Building Rubinstein's lcalc program using CC=$CC"
> echo " CCX=$CXX"
> echo " CFLAGS=$CFLAGS"
> echo " CXXFLAGS=$CXXFLAGS"
> echo " LCALC_LIBS=$LCALC_LIBS"
>
> # disable Cygwin build for now
> if [ `uname` = "CYGWIN" ]; then
> echo "Sorry, the lcalc build is currently broken"
> echo 1
> fi
>
> cd src
>
> set +e
>
> success() {
> if [ $? -ne 0 ]; then
> echo "Error building lcalc '$1'"
> exit 1
> fi
> }
>
> set -e
>
> export DEFINES=""
> cp ../../patches/Makefile.sage Makefile
> make lcalc
> success 'plain'
>
> echo "Now copying over the lcalc binary"
> cp lcalc "$SAGE_LOCAL/bin"
> success 'install'
>
>
>
--
William Stein
Associate Professor of Mathematics
University of Washington
http://wstein.org
Dr. David Kirkby
> On Sun, Aug 2, 2009 at 9:35 AM, Dr. David Kirkby<david....@onetel.net> wrote:
>> I noticed one package (lcalc) was passing GNU specific flags to the
>> assembler to suppress warnings. I've seen other packages send warning to
>> /dev/null.
>>
>> I decided to change the spkg-install for 'lcalc' somewhat, in a way that
>> I think can be better for most packages.
>>
>>
>> The revised spkg-install does the following things, which most
>> spkg-install's do not do.
>>
>> 1) It uses 'set -e' to exit the spkg-install if any error occurs.
>
> +1 we should do that for all spkg-install's
Good.
>> 2) It tests for SAGE_LOCAL in a somewhat better way than present
>>
>> if [ "x$SAGE_LOCAL" = "x" ]
>>
>> (It's not a good idea to test on "", though I'm sure one could argue it
>> has not broken yet)
>
> Why is it not a good idea to test on ""? I'm not convinced this is any
> better, and to me it looks worse.
Testing on "" breaks on some shells, including some early versions of
bash. I agree it looks ugly.
I'm quite happy to leave it as you wrote it, but Chris Johnson, who is
an author of a book on shell scripting,
http://www.amazon.co.uk/Shell-Scripting-Recipes-Problem-Solution-Approach/dp/1590594711
said on comp.unix.shell, that the best test would be:
if [ -n "$SAGE_LOCAL" ] ## true if SAGE_LOCAL is not null
if [ -z "$SAGE_LOCAL" ] ## true if SAGE_LOCAL is null
For problems I've had before on other things, Chris's solutions have
always been spot-on, so I tend to trust what he says is correct. He uses
POSIX commands, not relying on the latest version of bash, so they would
work in any shell.
So perhaps
if [ -z "$SAGE_LOCAL" ] ; then
echo "SAGE_LOCAL undefined ... exiting";
echo "Maybe run 'sage -sh'?"
exit 1
fi
would be the best possible test. From a practical point of view, it
probably makes no difference.
>> 3) It adds -m64 if the variable SAGE64 is set to 'yes'. (If you don't
>> set SAGE64, the system will do whatever its default happens to be).
>
> +1. This only worked on OS X 64-bit because lcalc is not C linked
> into Sage, i.e., it's only run via pexpect.
But does it need an OS X specific test in there? If it will make no
difference on any other platform, it seems pointless putting a test. I
assumed it would be useful on Solaris. It if will cause problems on
other platforms, then of course I can test for OSX, as we done before.
>> 4) It builds with debug flags (-g) by default (as agreed the other day)
>> if SAGE_DEBUG is not set.
>>
>> 5) If SAGE_DEBUG is set to 1, 'yes' or 'TRUE', then again it will build
>> with -g.
>>
>> But any other setting of SAGE_DEBUG will result in -g not being in
>> CFLAGS. Same for CXXFLAGS
>
> Just a thought: sage-env could check that SAGE_DEBUG is one of a
> specific list of allowed values or not set, and if not, it immediately
True. And perhaps that should be the place to check if the C, C++ and
Fortran compilers are all the same versions, and not a mix of the lot.
Certainly if one uses gcc,
BTW, I don't see any way to specify a Fortran compiler in Sage - that
sis something else we should do. Then it will allow one to test if the
C, C++ and Fortran compilers are all the same.
>> 6) It checks to determine if the C compiler is Sun or GNU, then adds
>> appropriate flags to show all warnings, depending on the compiler.
>> (-Wall for GNU. I'm not sure if my choice of flags with Sun's compiler
>> is best, but I'll find that out later).
>>
>> 7) Same as (5) above, but for the C++ compiler.
>>
>> 8) Checks if the C and C++ compilers are not a mix of Sun and GNU. In
>> other words, if you try to use a Sun C compiler with a GNU C++ compiler,
>> it will exit with an error.
>>
>> Ideally this needs extending to Fortran too.
>>
>> It would also be sensible to check that the version of gcc is the same
>> as the version of g++ and gfortran. It's quite possible the first C++
>> compiler in the path is version X, someone sets CC to a later version Y,
>> but does not do it with CXX. So the compilers are mixed. I don't
>> specifically know if that will break, but it can't be sensible.
>>
>> The latter part of spkg-install I've not touched. I'm interested in
>> general thoughts on this. I'd like to see a bit less of hiding of warnings.
>
> +1 overall, but hopefully most of this can be eventually factored out and
> made generic.
Agreed, it should be made generic. The big problem I have is time. If I
start looking at the generic code, it will just delay things more for
lcalc and Solaris. I want to try to get Sage working on Solaris asap.
Perhaps me adding some changes into lcalc's spkg-install, seeing how
they go, would be a sensible short-term solution. If they cause no
problems, then at a later date we look to make wider reaching changes. I
believe tests for things like different compilers should be done at an
earlier stage and not needed in every single spkg-install.
It's documented in one of sage-env that SAGE64 should be set to 'yes'.
IMHO, if 'yes' is going to be used for SAGE64, it should be used for
SAGE_DEBUG and everything else similar. That might break some code.
Accepting various options such as '1', 'yes', 'true' ,' Trus' , 'TRUE'
is a possibility, but that does get ugly.
I propose I integrate these changes into lcalc, with a view at a later
date to making them more generic.
dave
William Stein
Very nice. I like that.
>
>>> 3) It adds -m64 if the variable SAGE64 is set to 'yes'. (If you don't
>>> set SAGE64, the system will do whatever its default happens to be).
>>
>> +1. This only worked on OS X 64-bit because lcalc is not C linked
>> into Sage, i.e., it's only run via pexpect.
>
> But does it need an OS X specific test in there? If it will make no
> difference on any other platform, it seems pointless putting a test. I
> assumed it would be useful on Solaris. It if will cause problems on
> other platforms, then of course I can test for OSX, as we done before.
It doesn't need to be OS X specific. Go for it as you wish.
>>> 4) It builds with debug flags (-g) by default (as agreed the other day)
>>> if SAGE_DEBUG is not set.
>>>
>>> 5) If SAGE_DEBUG is set to 1, 'yes' or 'TRUE', then again it will build
>>> with -g.
>>>
>>> But any other setting of SAGE_DEBUG will result in -g not being in
>>> CFLAGS. Same for CXXFLAGS
>>
>> Just a thought: sage-env could check that SAGE_DEBUG is one of a
>> specific list of allowed values or not set, and if not, it immediately
>
> True. And perhaps that should be the place to check if the C, C++ and
> Fortran compilers are all the same versions, and not a mix of the lot.
> Certainly if one uses gcc,
>
> BTW, I don't see any way to specify a Fortran compiler in Sage - that
> sis something else we should do. Then it will allow one to test if the
> C, C++ and Fortran compilers are all the same.
See Sage's README.txt which explains how to specify the fortran compiler:
NOTE: If you're using Fortran on a platform without g95 binaries included
with Sage, e.g., Itanium, you must use a system-wide gfortran. You
have to explicitly tell the build process about the fortran
compiler and library location. Do this by typing
export SAGE_FORTRAN=/exact/path/to/gfortran
export SAGE_FORTRAN_LIB=/path/to/fortran/libs/libgfortran.so
>
>>> 6) It checks to determine if the C compiler is Sun or GNU, then adds
>>> appropriate flags to show all warnings, depending on the compiler.
>>> (-Wall for GNU. I'm not sure if my choice of flags with Sun's compiler
>>> is best, but I'll find that out later).
>>>
>>> 7) Same as (5) above, but for the C++ compiler.
>>>
>>> 8) Checks if the C and C++ compilers are not a mix of Sun and GNU. In
>>> other words, if you try to use a Sun C compiler with a GNU C++ compiler,
>>> it will exit with an error.
>>>
>>> Ideally this needs extending to Fortran too.
>>>
>>> It would also be sensible to check that the version of gcc is the same
>>> as the version of g++ and gfortran. It's quite possible the first C++
>>> compiler in the path is version X, someone sets CC to a later version Y,
>>> but does not do it with CXX. So the compilers are mixed. I don't
>>> specifically know if that will break, but it can't be sensible.
>>>
>>> The latter part of spkg-install I've not touched. I'm interested in
>>> general thoughts on this. I'd like to see a bit less of hiding of warnings.
>>
>> +1 overall, but hopefully most of this can be eventually factored out and
>> made generic.
>
> Agreed, it should be made generic. The big problem I have is time. If I
> start looking at the generic code, it will just delay things more for
> lcalc and Solaris. I want to try to get Sage working on Solaris asap.
Yep. With this sort of thing, it's best to write things that work in
a specific context, test them well, and only then factor them out.
> Perhaps me adding some changes into lcalc's spkg-install, seeing how
> they go, would be a sensible short-term solution. If they cause no
> problems, then at a later date we look to make wider reaching changes. I
> believe tests for things like different compilers should be done at an
> earlier stage and not needed in every single spkg-install.
+1
>
> It's documented in one of sage-env that SAGE64 should be set to 'yes'.
> IMHO, if 'yes' is going to be used for SAGE64, it should be used for
> SAGE_DEBUG and everything else similar. That might break some code.
> Accepting various options such as '1', 'yes', 'true' ,' Trus' , 'TRUE'
> is a possibility, but that does get ugly.
>
> I propose I integrate these changes into lcalc, with a view at a later
> date to making them more generic.
I support your proposal.
William
Ondrej Certik
Kirkby<david....@onetel.net> wrote:
> I noticed one package (lcalc) was passing GNU specific flags to the
> assembler to suppress warnings. I've seen other packages send warning to
> /dev/null.
>
> I decided to change the spkg-install for 'lcalc' somewhat, in a way that
> I think can be better for most packages.
Could you please post your spkg-install template somewhere to the
wiki? Here is mine, that I use all the time so far:
http://wiki.sagemath.org/OndrejCertik
(the very bottom of the page).
Ondrej
David Kirkby
Here is is. The idea is that this will be tested in 'lcalc' then some
of it integrated at a lower level, so some of the tests are done
elsewhere and don't all need to be repeated. But here is is.
Peter Jeremy
>> 1) It uses 'set -e' to exit the spkg-install if any error occurs.
>
>+1 we should do that for all spkg-install's
Agreed (and there are other shell-scripts that probably should also
have 'set -e'). But this is a very intrusive change since it touches
at least one file in virtually every spkg.
It's also non-trivial: The spkg-install files are full of constructs
like:
sage_fortran -c -fPIC *.f
if [ $? -ne 0 ]; then
echo "Error compiling blas."
exit 1
fi
In order to add 'set -e', this needs to be changed to directly test the
exit code rather than using $? - otherwise the script will immediately
exit when the command fails. The revised code should look like:
if ! sage_fortran -c -fPIC *.f; then
echo "Error compiling blas."
exit 1
fi
In addition to the above, there is a non-trivial amount of shell code
embedded in python scripts that also needs improved error checking.
Assuming that the change should be implemented, what approach should
be used:
1) Big bang: Someone goes through all the skpg files and fixes them
in one go with no other changes as part of a major release.
2) If you touch any part of an spkg, you must add 'set -e' and fixup
any subsequent fallout in all scripts in that spkg.
3) If you touch a script, you must fixup that script.
4) Fix it if you feel like it.
--
Peter Jeremy
Dr. David Kirkby
I think (2). As you note, it does need some care when applied. If it is
applied where it should not, it will not give a useful error message,
but it will at least exit. One could reasonably say that is better than
carrying on, when something is broken.
Dr. David Kirkby
> On 2009-Aug-02 12:24:26 -0700, William Stein <wst...@gmail.com> wrote:
>>> 1) It uses 'set -e' to exit the spkg-install if any error occurs.
>> +1 we should do that for all spkg-install's
>
> Agreed (and there are other shell-scripts that probably should also
> have 'set -e'). But this is a very intrusive change since it touches
> at least one file in virtually every spkg.
>
> It's also non-trivial: The spkg-install files are full of constructs
> like:
The new ECL package I created
http://sage.math.washington.edu/home/kirkby/Solaris-fixes/ecl-9.8.1/ecl-9.8.1.spkg
has an improved spkg-install.
http://sage.math.washington.edu/home/kirkby/Solaris-fixes/ecl-9.8.1/spkg-install
It does things like
1) Adds set -e, but uses set +e to remove where appropriate.
2) Checks the compilers are not a mix of Sun and GNU
3) Adds -g by default, but does allow it to be removed.
4) It forces a 64-bit build if SAGE64 is set to 'yes'. It sets all
compiler flags (CFLAGS, CXXFLAGS and FFLAGS) to include -m64.
Now lets see the things wrong with the original version of the
spkg-install in Sage:
http://sage.math.washington.edu/home/kirkby/Solaris-fixes/ecl-9.8.1/spkg-install-old
1) It set CFLAGS to -m64 on a 64-bit build, but not CXXFLAGS. Given
there are tons of C++ files, that was not a great idea. Also there is
one Fortran file, though I think that is only used on Cray. So setting
FFLAGS was sensible too.
2) It set LDFLAGS to -m64, but such an option is not portable. Neither
is it needed in 99% of the cases, as the linker will know whether to
create a 64 or 32 bit binary based on the format of the object files. (A
rare exception is if there is only archive libraries).
3) It added -fPIC on a 64-bit build, but not a 32-bit one. Given others
are building ECL on 64-bit, it makes me think that flag is not needed at
all. (In any case, it is non-portable).
There is the issue of where this sort of testing takes places though.
One could do a lot of the testing (identical compiler versions, mix of
compilers etc), specific compiles for specific version of the operating
system etc etc in the configure script when Sage starts to build. That
would simplify things a lot, as it only needs to be done once.
The downside to that approach is that if someone starts building, the
build gets stopped, and they mis-set CC or something, then they could
end up with a mix of things compilers. Hence at least some sanity
checking in spkg-install has some merit too, even if other (more
comprehensive) checks are performed early on.
Dave