palp spkg ignores global CC and CFLAGS variables

[sagetrac-drkirkby]

Using

• Solaris 10 update 7 on SPARC
• sage-4.1.2.alpha4
• Sun Studio 12.1
• An updated configure script to allow the Sun compiler to be used Update prereq from 0.3 to 0.4 #7021

This is one of the many packages that ignore the setting of the variable CC.

palp-1.1.p1/src/GNUmakefile
palp-1.1.p1/src/mori.c
Finished extraction
****************************************************
Host system
uname -a:
SunOS swan 5.10 Generic_139555-08 sun4u sparc SUNW,Sun-Blade-1000
****************************************************
****************************************************
CC Version
/opt/xxxsunstudio12.1/bin/cc -v
usage: cc [ options] files.  Use 'cc -flags' for details
****************************************************
make[2]: Entering directory `/export/home/drkirkby/sage/sage-4.1.2.alpha4/spkg/build/palp-1.1.p1/src'
gcc -O3 -g -W -Wall -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE   -c -o poly.o poly.c
gcc -O3 -g -W -Wall -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE   -c -o Coord.o Coord.c
gcc -O3 -g -W -Wall -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE   -c -o Rat.o Rat.c
gcc -O3 -g -W -Wall -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE   -c -o Vertex.o Vertex.c

To update:

• Replace the palp spkg with http://wstein.org/home/ohanar/spkgs/palp-2.0.p1.spkg

Depends on #12055

CC: @orlitzky

Component: build

Author: R. Andrew Ohana

Reviewer: Volker Braun

Merged: sage-5.0.beta7

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

[ohanar]

Author: R. Andrew Ohana

[ohanar]

Attachment: palp-1.1.p4.patch.gz

for review purposes

[orlitzky]

Work Issues: respect CFLAGS too

[orlitzky]

comment:2

Can we pass CFLAGS, too? That will allow us to get rid of that horrible sed. Might as well fix the "xyes" test also

[vbraun]

comment:3

Palp 2 is out and I have a preliminary spkg at #12055. Please base your work on the new version.

[ohanar]

comment:4

ok, I've based my changes off of the new version at #12055 and have made CFLAGS respected as well.

[ohanar]

Dependencies: #12055

[ohanar]

Attachment: palp-2.0.p1.patch.gz

for review purposes

[vbraun]

Changed work issues from respect CFLAGS too to none

[vbraun]

comment:5

Looks good!

[vbraun]

Reviewer: Volker Braun

[vbraun]

Patch from #12055 needs to be applied

[vbraun]

comment:6

Attachment: trac-12055-SAGELOCAL_BIN.patch.gz

I've switched the old ticket #12055 to invalid to not make Jeroen replace the spkg twice. But we still need the patch to the Sage library from there.

[sagetrac-drkirkby]

comment:7

Replying to @orlitzky:

Can we pass CFLAGS, too? That will allow us to get rid of that horrible sed. Might as well fix the "xyes" test also

The "xyes" test, as it is called above, is the safest, most portable way to test for a string, as other methods, like the proposed change, can fail under obscure conditions. One might argue they don't fail with modern versions of bash, but IMHO is it worthwhile to write scripts which will always work under all conditions. The original code will always work - the proposed change is less portable. I suggest you take a look at the scripts created by autoconf. You will find they use a similar method to what was in Sage, as it is known to always work.

As such, I believe the change is a retrograde step.

Dave

[vbraun]

comment:9

I disagree. The xblah test makes it more difficult to read the test, which both increases the chance for errors as well as the long-term maintenance effort.

A quick survey shows that at least half of the Sage spkgs use the simplified test, so clearly nobody has yet encountered a shell ancient enough to not work. I'm pretty sure many upstream sources use simplified tests, too, so there is basically no hope of ever compiling Sage on such a system without installing a shell that isn't from the middle ages. Autotools output really is a totally different issue, since their scripts are autogenerated readability is not an issue (and usually is pretty bad, in fact).

If you disagree we can discuss this on the sage-devel, but its such a pervasive issue throughout Sage that it doesn't really matter if we use the simplified test in one spkg more or less.

[jdemeyer]

Merged: sage-5.0.beta7