search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 480 forks source link

Add scripts which check C and C++ compilers, and report what they are #7505

Closed bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 closed 14 years ago

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 15 years ago

In some parts of Sage, it is desirable to know what compiler is being used - in particular if its gcc or Sun, but there are other cases too.

The two scripts check the variable CC and CXX, and try to determine what the compiler is. I've no idea where the scripts should go though - ultimately they should be copied to SAGE_LOCAL/bin, but if great if someone can put them in a sensible place to start with. I would be appreciate that.

The scripts can report one of several answers.

These two scripts work by testing what pre-processor directives the compilers test. For example, gcc will define !GNUC!.

But so will the Intel and Sun compilers which try to be GNU like. But the Intel one will define !__INTEL_COMPILER and the Sun compilers define !__SUNPRO_C so it's possible to differentiate the GNU, Intel and Sun compilers, even though they all aim to be GCC-like.

Compiler documentation was obtained from various sites, and is documented in the scripts. == Testing == Obviously a reviewer can test these scripts on any platforms they have access to, but here are some tests I performed. I've not managed to check these scripts on every platform and compiler for which they are aimed to work on, due to lack of access to the hardware. 

drkirkby@swan:[~] $ uname -a
SunOS swan 5.10 Generic_139555-08 sun4u sparc SUNW,Sun-Blade-1000
drkirkby@swan:[~] $ echo $CC
/opt/xxxsunstudio12.1/bin/cc
drkirkby@swan:[~] $ echo $CXX
/opt/xxxsunstudio12.1/bin/CC
drkirkby@swan:[~] $ ./testcc
Sun_on_Solaris
drkirkby@swan:[~] $ ./testcxx
Sun_on_Solaris
drkirkby@swan:[~] $ export CC=gcc
drkirkby@swan:[~] $ export CXX=g++
drkirkby@swan:[~] $ ./testcc 
GNU
drkirkby@swan:[~] $ ./testcxx
GNU
SunOS hawk 5.11 snv_111b i86pc i386 i86pc
bash-3.2$ gcc hello.c -o hello
bash-3.2$ export CC=./hello
bash-3.2$ ./testcc
Unknown
$ uname -a
AIX client9 1 6 00C6B7C04C00
$ export CC=gcc
$ export CXX=g++
$ ./testcc
GNU
$ ./testcxx
GNU
$ uname -a
AIX client9 1 6 00C6B7C04C00
$ export CC=/usr/vacpp/bin/xlc
$ export CXX=/usr/vacpp/bin/xlc++
$ ./testcc
IBM_on_AIX
$ ./testcxx
IBM_on_AIX
$ uname -a
HP-UX hpbox B.11.11 U 9000/785 2016698240 unlimited-user license
$ export CC=gcc
$ export CXX=g++
$ ./testcc
GNU
$ ./testcxx
GNU
$ export CC=/opt/ansic/bin/cc
$ export CXX=/opt/aCC/bin/aCC
$ ./testcc
HP_on_HPUX
$ ./testcxx
HP_on_HPUX
kirkby@sage:~$ unset CC
kirkby@sage:~$ unset CXX
kirkby@sage:~$ ./testcc
Sorry, you should define the environment variable CC
kirkby@sage:~$ ./testcxx
Sorry, you should define the environment variable CXX
kirkby@sage:~$ uname -a
Linux sage.math.washington.edu 2.6.24-23-server #1 SMP Wed Apr 1 22:14:30 UTC 2009 x86_64 GNU/Linux
kirkby@sage:~$ export CC=/usr/bin/gcc
kirkby@sage:~$ export CXX=g++
kirkby@sage:~$ ./testcc
GNU
kirkby@sage:~$ ./testcxx
GNU
[kirkby@bsd ~]$ uname -a
Darwin bsd.local 10.2.0 Darwin Kernel Version 10.2.0: Tue Nov  3 10:37:10 PST 2009; root:xnu-1486.2.11~1/RELEASE_I386 i386 i386 MacPro1,1 Darwin
[kirkby@bsd ~]$ export CC=gcc
[kirkby@bsd ~]$ export CXX=g++
[kirkby@bsd ~]$ ./testcc
GNU
[kirkby@bsd ~]$ ./testcxx
GNU

The code was tested by someone else on Tru64, but I can not show the results of this.

I have not verified it with the Sun or Intel compilers which are GCC compatible, nor on Alpha linux. In each case, I've used compiler documentation, and hope it is right.

Dave

CC: @malb @sagetrac-drkirkby @williamstein @peterjeremy

Component: build

Author: David Kirkby, Peter Jeremy

Reviewer: Martin Albrecht, William Stein, Minh Van Nguyen

Merged: sage-4.3.1.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7505

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 15 years ago

Small shell script to check the type of C compiler

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 15 years ago

Attachment: testcc.gz

Small shell script to check the type of C++ compiler

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 15 years ago

Description changed:

--- 
+++ 
@@ -17,8 +17,12 @@
 These two scripts work by testing what pre-processor directives the compilers test. For example, gcc will define __GNUC__. 

 But so will the Intel and Sun compilers which try to be GNU like. But the Intel one will define __INTEL_COMPILER 
-Rhe Sun compilers defined __SUNPRO_C  so it's possible to differentiate the GNU, Intel and Sun compilers, even though they all aim to be GCC-like. 
- Compiler documentation was obtained from various sites, and is documented in the scripts. 
+
+The Sun compilers defined __SUNPRO_C  
+
+so it's possible to differentiate the GNU, Intel and Sun compilers, even though they all aim to be GCC-like. 
+
+Compiler documentation was obtained from various sites, and is documented in the scripts. 
  == Testing ==
 Obviously a reviewer can test these scripts on any platforms they have access to, but here are some tests I performed. I've not managed to check these scripts on every platform and compiler for which they are aimed to work on, due to lack of access to the hardware.
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 15 years ago
comment:1

Attachment: testcxx.gz

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Description changed:

--- 
+++ 
@@ -14,13 +14,9 @@
 * HP_on_Tru64 (HP's commerical compiler on Tru64)
 * Unknown  (The scripts can't work out what the compiler is). 

-These two scripts work by testing what pre-processor directives the compilers test. For example, gcc will define __GNUC__. 
+These two scripts work by testing what pre-processor directives the compilers test. For example, gcc will define !__GNUC!__. 

-But so will the Intel and Sun compilers which try to be GNU like. But the Intel one will define __INTEL_COMPILER 
-
-The Sun compilers defined __SUNPRO_C  
-
-so it's possible to differentiate the GNU, Intel and Sun compilers, even though they all aim to be GCC-like. 
+But so will the Intel and Sun compilers which try to be GNU like. But the Intel one will define !__INTEL_COMPILER and the Sun compilers define !__SUNPRO_C so it's possible to differentiate the GNU, Intel and Sun compilers, even though they all aim to be GCC-like. 

 Compiler documentation was obtained from various sites, and is documented in the scripts. 
  == Testing ==
malb commented 14 years ago
comment:3

I was worried that the sage_scripts repository would be installed too late for these scripts but I was wrong. I agree with Mike that this is where these scripts belong.

The only issue I can see is that # of C++ code, and checking what are defined. in testcc should be C code. There are a few of these copy'n'paste 'errors' in the docs, which are beautiful in general!

Also, I'd call the scripts testcc.sh and testcxx.sh to emphasise that they are shell scripts.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:4

Thank you for your comments. I am now exactly sure what you mean by the "The only issue I can see is that # of C++ code, and checking what are defined. in testcc should be C code."

I did intend making a change to these scripts. I thought it would be better if they returned 'GNU' for any compiler which was like gcc (i.e. whether produced by Sun, Intel or HP), and only gave very specific information about whether it was Sun/Intel/HP if an option was passed to the scripts. This is because, in 99% of cases, the user will only want to know if the compiler acts like gcc, and will probably not care if it the GNU distribution, or an improved one from HP, Intel or Sun.

Once I know exactly what you mean by your comment, I'll make the changes, rename the files with a .sh extension, and submit again for review. Dave

malb commented 14 years ago
comment:5

Oh, its just that you talk about C++ in the C script in the comments. That's all.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:6

I made a few changes.

I'm a bit concerned these need to be available very early on, before bzip2 is built, as that has options which depend on the compiler type, as it creates position independent code, the option for which is not standard. Wherever testcc.sh and testcxx.sh are added, then need execute permission.

Dave

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:8

I'm uploading a slightly changed version, which changes the output from 'Sun_on_Solaris' to 'Sun_Studio' as I'd overlooked the fact that Sun Studio can be installed on Linux too.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:10

I'm a bit worried about where these two files should go. Putting them in the 

sage_scripts-$version.spkg

will not be suitable, as this a bzip2 compressed file. Hence bzip2 needs to be built before that can extracted. But these two scripts which test the compiler needs to be available before bzip2 is compiled. They need to be available before bzip2 is compiled - particularly since bzip2 does have some compiler-specific flags, as it creates a shared library.

Dave

malb commented 14 years ago
comment:11

Hi, sorry for letting it bitrot so long. The scripts are fine, so you get a positive review as soon as we figured out where to put them. How about adding them to the prereq SPKG?

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:12

Hi, the prereq spkg seems a good idea, as that is a tar file, so needed nothing to extract it. I'll add that later today - I am in the middle of decorating now, and being the winter, there are not many daylight hours here in the UK

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:13

Having looked at prereq, it is not a collection of scripts, so I do not believe that is the appropriate place for them.

I believe if the two scripts are put in the directory 

$SAGE_ROOT/spkg/base/

and the file 

$SAGE_ROOT/spkg/install

is updated to copy these two scripts to $SAGE_LOCAL/bin (as it does some other files), then that will solve the problem. That 'SAGE_ROOT/spkg/install' script is run very early (before even prereq), so an added bonus is that the script could be called from within prereq if needed.

I'm not sure how to check these in properly though vi hg. I've never got to grips with that properly. I'll drop you an email about that.

I'm attaching the revised install script, and the patch file.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Updated 'install' scipt which copies testcc.sh and testcxx.sh to $SAGE_LOCAL/bin

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Attachment: install.gz

Patch, showing changes between the old an new versions of the install script

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:14

Attachment: install.patch.gz

The review of this has got a bit complex, mainly since agreeing where the scripts should go has not been without some problem, but I also slightly changed the output shown, in view of difficulty of the fact Sun Studio can be run on Solaris, and difficulties finding some information I wanted.

Hence I thought I'd clarify some things.

The scripts output one of:

I believe the scripts (see attached) should be:

$SAGE_ROOT/spkg/base/testcc.sh
$SAGE_ROOT/spkg/base/testcxx.sh

The file

$SAGE_ROOT/spkg/install

needs to be modified (patch attached), so it copies the two scripts to 

$SAGE_LOCAL/bin

in addition to the files it already copies there. This allows the scripts to be called from anywhere, including the prereq script, which might be necessary.

The scripts can't actually break anything in Sage, as they are not called at all. But calling them will simplify a lot of portability issues later one.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:15

Oops, I mean the fact Sun Studio could be run on Linux caused me to change the output - everyone known they can run on Solaris!

malb commented 14 years ago

Attachment: trac_7505.patch.gz

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:16

Peter Jeremy has found a cleaner way of testing the compiler, which I intend using. It makes use of the pre-processor in exactly the same way, but it is simpler to maintain, they way Peter wrote it.

This needs a bit of further testing, so I've stuck it back to 'needs work' for now. Then hopefully it can be reviewed when I've uploaded a couple of revised versions of testcc.sh and testcxx.sh

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Changed author from David Kirkby to David Kirkby, Peter Jeremy

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Simpler version of file to check the c-compiler

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Attachment: testcc.sh.gz

Attachment: testcxx.sh.gz

Simpler version of file to check the C++ compiler

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:18

Here are revised versions of the scripts, which report the compiler type. They use the same method as before (checking was macros are predefined), but avoid the use of some grep statments and generally look a lot cleaner. Thank you Peter for the revisions.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Attachment: trac_7505-test-scripts.patch.gz

A patch to be used in place of Martin's earlier version.

williamstein commented 14 years ago
comment:19

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
...
  1. 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
  1. 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.

  1. 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.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:20

Thank you for the helpful comments.

Dave

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

based on Sage 4.3

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Attachment: trac_7505-revision-control.patch.gz

version of kirkby's patch suitable for applying to spkg/base repository

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:21

Attachment: trac_7505-scripts.patch.gz

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Reviewer: Martin Albrecht, Minh Van Nguyen

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:22

The patch trac_7505-test-scripts.patch contains two shell scripts: one for testing the type of C compiler, and the other for testing the type of C++ compiler. Here are the results of my testing them on different combinations of platform and hardware:

In each case, the test scripts work as claimed. We can say that the scripts have been tested and work as claimed on various versions of the following platforms with GCC and the relevant commercial compilers: AIX, Cygwin on Windows XP, Fedora, HP-UX, Mac OS X, openSUSE, Red Hat Enterprise Linux, Solaris, SUSE Linux Enterprise Server, Ubuntu. So far, no one has been able to test the scripts on Tru64 or Alpha Linux because we don't have access to the relevant platform/hardware combinations. At this sage-devel thread, Peter Jeremy mentions that he has access to a Tru64 machine, but he won't be able to test the scripts on that machine within the next few weeks from now. But that should not prevent the scripts from being merged in Sage.

The patch trac_7505-test-scripts.patch needs to be applied to SAGE_ROOT/spkg/base. But note that with Sage 4.3 some files under SAGE_ROOT/spkg/base are not yet managed by revision control:

[mvngu@boxen base]$ pwd
/dev/shm/mvngu/sage-4.3/spkg/base
[mvngu@boxen base]$ hg st
M sage-env
M sage-spkg
! prereq-0.3-install
? prereq-0.5-install
? prereq-0.5.tar

The file prereq-0.3-install is now superseded by prereq-0.5-install so prereq-0.3-install can be removed as follows:

[mvngu@boxen base]$ hg remove prereq-0.3-install
[mvngu@boxen base]$ hg st
M sage-env
M sage-spkg
R prereq-0.3-install
? prereq-0.5-install
? prereq-0.5.tar

Also, the file prereq-0.5-install needs to be checked in with:

[mvngu@boxen base]$ hg add prereq-0.5-install
[mvngu@boxen base]$ hg st
M sage-env
M sage-spkg
A prereq-0.5-install
R prereq-0.3-install
? prereq-0.5.tar

But the file prereq-0.5.tar need not be under revision control. In that case, the release manager could edit the file .hgignore to ignore that particular tarball. Anyway, I have attached the patch trac_7505-revision-control.patch which should take care of editing .hgignore, adding prereq-0.5-install and removing prereq-0.3-install. I have attached a version of David Kirkby's patch that contains his name so that when his scripts are checked in, they are committed in his name. Finally, note that the files sage-env and sage-spkg must be first checked in before applying any of the above patches. That is, patches/changes should be applied/made to the directory SAGE_ROOT/spkg/base in the following order:

  1. Check in the changes to sage-env and sage-spkg.
  2. Apply trac_7505-revision-control.patch to properly put three existing files under revision control.
  3. Finally, apply trac_7505-scripts.patch which has the same changes as trac_7505-test-scripts.patch, but any check ins would be made in David's name.
7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago

Changed reviewer from Martin Albrecht, Minh Van Nguyen to Martin Albrecht, William Stein, Minh Van Nguyen

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:25

I forgot to say that someone needs to make sure that my patch trac_7505-revision-control.patch works as claimed. Once that is done, I think the whole ticket would receive positive review.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Tests C compiler reading from the command line, not environment variable CC

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Attachment: testcc.2.sh.gz

Attachment: testcxx.2.sh.gz

Tests C++ compiler reading from the command line, not environment variable CXX

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:26

I've attached the revised versions. I did not overwrite the file names, and note they now have a 2 appended to the names, which was done by the trac server, not me.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:27

Oops my comments seems to have been missed.

Minh

did you not see William's comments? The revised scripts attach address them.

I do not know if the trac server is in some way out of sync, but I thought I put this comment a few mins ago. The fact Minh seems to have missed something William wrote, makes me wonder if there is a problem with the trac server

dave

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:28

Replying to @sagetrac-drkirkby:

did you not see William's comments?

I missed those comments.

The revised scripts attach address them.

I'll have a look at them.

I do not know if the trac server is in some way out of sync, but I thought I put this comment a few mins ago. The fact Minh seems to have missed something William wrote, makes me wonder if there is a problem with the trac server

I wasn't CC'd on the ticket. So when I was reviewing your previous attachments, I missed William's comments. There's no need to worry. I can review your newer attachments.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:29

PS, regarding your comment about checking in prereq 0.5, see #7781

Dave

7c09a680-e216-4024-bb8e-9bfd4aa7f313 commented 14 years ago
comment:30

There is a mismatch in documentation in testcc.2.sh:

54  # Exit if the user supplies any command-line arguments.
55  if [ $# -ne 1 ] ; then
56    usage
57    exit 1
58  fi
59  CC_LOCAL=$1

Do you also want to change the above section to something like the following?

# Exit if the user does not supply one command line argument.
if [ $# -ne 1 ] ; then
  usage
  exit 1
fi

CC_LOCAL=$1 # Compiler name or path as argument to script.

This would also be in line with the corresponding documentation in testcxx.2.sh.

PS: If you want to replace an earlier version of an attachment with the same name, you could do so. On the page for uploading files, there should be a check box to indicate that you want to replace files of the same name.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago

Attachment: testcc.3.sh.gz

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:31

Hi Minh, yes, I did want the comment updated, as now the scripts take one argument, not zero as before. I'd purposely chosen to leave the older version on the server, but perhaps with hindsight that was not wise.

Dave

williamstein commented 14 years ago
comment:32

Would it be acceptable to you if the file name was much more complex, say [...]

Yes. "/tmp/test.$$.c" seems just a little too generic. I like the change you made in your latest script.

Minh -- thanks for all your testing!

Definite positive review.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 14 years ago
comment:33

Note to Mike (release manager)

This ticket might have got a bit confusing. Just to recap, these things need going in.

mwhansen commented 14 years ago

Merged: sage-4.3.1.alpha0