#7505: Add scripts which check C and C++ compilers, and report what they are.
---------------------------+------------------------------------------------
Reporter: drkirkby | Owner: tbd
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.3.1
Component: build | Keywords:
Work_issues: | Author: David Kirkby, Peter Jeremy
Upstream: N/A | Reviewer:
Merged: |
---------------------------+------------------------------------------------
Changes (by was):
* status: needs_review => needs_work
Comment:
REFEREE REPORT:
Please read everything below before making any changes.
1. In the usage it says:
"Will return one of the following, to indicate the C compiler".
However, shell scripts don't return anything. It is more precise to say
"Will print one of the following to stdout, to indicate the C compiler."
2. I would expect to also get the usage message when CC isn't defined.
{{{
flat:Downloads wstein$ ./testcc.sh
Sorry, you must define the environment variable CC
}}}
Currently, the *only* way to get the usage is to call the command with an
extra argument (in which case one doesn't get an error message, by the
way):
{{{
flat:Downloads wstein$ ./testcc.sh foo
Usage: ./testcc.sh
Will return one of the following, to indicate the C compiler
...
}}}
3. Perhaps change the message to say "You must export the CC environment
variable", because of course defining the CC variable as instructed above
doesn't work:
{{{
flat:Downloads wstein$ CC=gcc
flat:Downloads wstein$ ./testcc.sh
Sorry, you must define the environment variable CC
flat:Downloads wstein$ CC=gcc; export CC
flat:Downloads wstein$ ./testcc.sh
GCC
}}}
4. I wonder about the design. If I were writing this script I would make
it take a single argument:
{{{
$ ./testcc.sh <path to compiler>
}}}
instead of using an environment variable. Then the usage info would
naturally be printed no matter what when one does
{{{
$ ./testcc.sh
}}}
and one doesn't have to mess with (= muck up) the environment at all in
order to use this command. You could use it like you already plan to
by just doing
{{{
testcc.sh "$CC"
}}}
To me, that would fit much, much better with standard conventions for
command line tools.
5. I think the script should immediately bail in case
{{{
TESTFILE=/tmp/test.$$.c
}}}
already exists. Its entirely possible somebody else makes a shell script
that uses a similar "strategy" for making temp files and your script just
clobbers them. (The right way to make temp files is to use the tmpfile C
library calls. It would be reasonably easy to rewrite this script, say in
Python, to do that, and then temp files would be properly handled and
never clash. This should be done later, since your script is nicely
laying down the interface. Yes, this would depend on some Python being
installed system-wide, but that's OK.)
Maybe you could completely avoid using temp files by using - to get input
from stdin, e.g. use {{{$CC -}}} and pipe the input in.
{{{
$ echo "main(){}" | gcc - -E
# 1 "<stdin>"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "<stdin>"
main(){}
}}}
SUMMARY: I personally think that testcc and testcxx should take one
command line argument instead of using the environment to pass arguments.
Their documentation could be slightly clarified. The use of the temp file
could also be very slightly made more robust. In fact -- much better --
perhaps just rewrite the script to not use temp files at all.
Sorry for being so picky. It's just because I just spent a bunch of my
limited time reading through this all very carefully and just want to
contribute something of value.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7505#comment:19>
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.
