Add support for a "sagerc" script

[jdemeyer]

This deals with adding support for a file

$DOT_SAGE/sagerc

which will be sourced after sage-env, so it can be used to override the environment variables used in Sage.

Apply:

1. attachment: 12647_sagerc.patch to SAGE_ROOT.
2. attachment: 12647_sagerc_doc.patch, attachment: trac_12647-review.patch to the Sage library.

Component: scripts

Author: Jeroen Demeyer

Reviewer: John Palmieri

Merged: sage-5.0.beta9

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

[jdemeyer]

comment:1

This still needs a documentation patch.

[jdemeyer]

Author: Jeroen Demeyer

[jhpalmieri]

comment:2

Where do you suggest adding documentation? The reference manual (here or here, perhaps)? The top-level README.txt?

[jdemeyer]

comment:3

Attachment: 12647_sagerc_doc.patch.gz

[jdemeyer]

comment:4

Replying to @jhpalmieri:

Where do you suggest adding documentation? The reference manual (here or here, perhaps)? The top-level README.txt?

I created a new file in the reference manual mentioning sagerc and init.sage.

[jhpalmieri]

Reviewer: John Palmieri

[jhpalmieri]

comment:5

In a line like

if [ "$SAGE_RC_FILE" = "" ]; then

is there any advantage to replacing the test with [ -z "$SAGE_RC_FILE" ]? (I guess you did it the way you did for consistency with other parts of the script?)

See the review patch for two minor changes to the documentation.

Otherwise, positive review.

[jhpalmieri]

Attachment: trac_12647-review.patch.gz

[jhpalmieri]

comment:6

(The "review patch" needs review.)

[jdemeyer]

comment:7

Replying to @jhpalmieri:

In a line like

if [ "$SAGE_RC_FILE" = "" ]; then

is there any advantage to replacing the test with [ -z "$SAGE_RC_FILE" ]?

As far as I know, these two tests are identical. But I changed it to use -z.

Review patch is obviously fine.

[nexttime]

comment:8

How about adding a commented template rc file, with examples for CC, CFLAGS, MAKE (e.g. conditionally, depending on the hostname), SAGE_BROWSER, SAGE_ATLAS_LIB etc.?

Maybe on a follow-up ticket?

(Unfortunately I meanwhile forgot most of what I had in mind... :-/ )

[nexttime]

comment:9

Minor flaw:

        echo >&2 "Error sourcing $DOT_SAGE/sagerc"

should read

        echo >&2 "Error sourcing $SAGE_RC_FILE"

AFAIK . is more portable than source; does Bash 2.04 support the latter?

[jdemeyer]

comment:10

Replying to @nexttime:

AFAIK . is more portable than source; does Bash 2.04 support the latter?

GNU bash, version 2.05b.0(1)-release (powerpc-apple-darwin8.0)
Copyright (C) 2002 Free Software Foundation, Inc.

does support . and source.

[jdemeyer]

comment:11

Setting MAKE in sagerc is a bad idea by the way: currently, sage-env is not sourced by spkg/install which runs the top-level $MAKE. So, setting MAKE=make -j4 in sagerc would only build individual packages in parallel, it would not build multiple packages concurrently.

Fixed the minor flaw, thanks for that.

[ppurka]

comment:12

Attachment: 12647_sagerc.patch.gz

In some other ticket there were some concerns raised about the non-portability of x\+ in sed. Can't remember which ticket it was. The portable solutions were either xx* or x\{1,\}.

[jdemeyer]

comment:13

Replying to @ppurka:

In some other ticket there were some concerns raised about the non-portability of x\+ in sed. Can't remember which ticket it was. The portable solutions were either xx* or x\{1,\}.

Presumably, the sed on Cygwin (which is all that matters for this) does support \+. I know the OS X 10.4 sed does not support \+.

[nexttime]

comment:14

Replying to @jdemeyer:

Replying to @ppurka:

In some other ticket there were some concerns raised about the non-portability of x\+ in sed. Can't remember which ticket it was. The portable solutions were either xx* or x\{1,\}.

Presumably, the sed on Cygwin (which is all that matters for this) does support \+. I know the OS X 10.4 sed does not support \+.

Sun's "default" sed (not the one in /usr/xpg*/bin/) doesn't either IIRC.

Probably even the XPG one doesn't; I don't recall. It at least used to be a GNU extension.

[jhpalmieri]

comment:15

Regarding the use of sed here, this script is used every time Sage is run, and it hasn't caused any problems on any platform, presumably because (as Jeroen points out) this part only matters on Cygwin. So I don't see any need to change anything.

[nexttime]

comment:16

Replying to @jhpalmieri:

Regarding the use of sed here, this script is used every time Sage is run, and it hasn't caused any problems on any platform, presumably because (as Jeroen points out) this part only matters on Cygwin. So I don't see any need to change anything.

Oh, I thought this discussion was unrelated to the ticket anyway... 8)

But now I see it is on topic...
In this case, s/WIN.*/WIN/ would work as well by the way, and there shouldn't be two invocations of uname. (And using the shell-builtin pattern matching, case ... in ..., would be faster and safer / more portable anyway.)

But never mind...

[nexttime]

comment:17

Replying to @jdemeyer:

Setting MAKE in sagerc is a bad idea by the way: currently, sage-env is not sourced by spkg/install which runs the top-level $MAKE. So, setting MAKE=make -j4 in sagerc would only build individual packages in parallel, it would not build multiple packages concurrently.

I see. I was pretty sure we'd source sage-env (at least) in the build rule (of course on the same line we call spkg/install) of the top-level Makefile ($SAGE_ROOT/Makefile).

We could of course implement ./sage --make ... :-)

(But perhaps the easiest way is to make make an alias to a script that behaves differently in case the current directory is the root of a Sage installation, and just calls make otherwise.)

[jdemeyer]

Merged: sage-5.0.beta9

[nexttime]

comment:19

The patch to sage-env causes a beta9 regression since CFLAGS are now no longer exported, which at least some spkgs relied on (i.e., their spkg-install sets or modifies them, but doesn't export them by itself since this used to be redundant).

[jhpalmieri]

comment:20

Replying to @nexttime:

The patch to sage-env causes a beta9 regression since CFLAGS are now no longer exported, which at least some spkgs relied on (i.e., their spkg-install sets or modifies them, but doesn't export them by itself since this used to be redundant).

Can you explain that? I thought that first sage-env was run, then spkg-install was run (via sage-spkg). So how can exporting (an empty) CFLAGS in sage-env help if that variable is later modified in spkg-install?

[nexttime]

comment:21

Replying to @nexttime:

The patch to sage-env causes a beta9 regression since CFLAGS are now no longer exported, which at least some spkgs relied on (i.e., their spkg-install sets or modifies them, but doesn't export them by itself since this used to be redundant).

... or probably not (it's late).

Actually any spkg-install modifying CFLAGS should export them (at least if they've been empty / unset) ...

[nexttime]

comment:22

Replying to @jhpalmieri:

Replying to @nexttime:

The patch to sage-env causes a beta9 regression since CFLAGS are now no longer exported, which at least some spkgs relied on (i.e., their spkg-install sets or modifies them, but doesn't export them by itself since this used to be redundant).

Can you explain that? I thought that first sage-env was run, then spkg-install was run (via sage-spkg). So how can exporting (an empty) CFLAGS in sage-env help if that variable is later modified in spkg-install?

It would have helped only if CFLAGS were (e.g.) set to "" as well (if they had been empty or unset before), which on the other hand would currently break building the Sage library, which needs -fno-strict-aliasing, which [also] comes from Python's CFLAGS, but only if CFLAGS are not set to "", i.e., not set / exported at all.

[jdemeyer]

comment:23

Exporting a variable which you don't set has no effect. Since CFLAGS wasn't (and still isn't) set in sage-env, the export did nothing.

Example (GNU bash version 4.0.35(1)-release if it matters):

jdemeyer@arcanis:~$ ( export FOO; env ) | grep FOO
jdemeyer@arcanis:~$ ( export FOO; FOO=; env ) | grep FOO
FOO=

[nexttime]

comment:24

Replying to @jdemeyer:

Exporting a variable which you don't set has no effect. Since CFLAGS wasn't (and still isn't) set in sage-env, the export did nothing.

Example (GNU bash version 4.0.35(1)-release if it matters):

jdemeyer@arcanis:~$ ( export FOO; env ) | grep FOO
jdemeyer@arcanis:~$ ( export FOO; FOO=; env ) | grep FOO
FOO=

Of course. But we [still] have things like

if [ "$LDFLAGS" = "" ]; then
    LDFLAGS=""          && export LDFLAGS
fi


if [ "$CXXFLAGS" = "" ]; then
    export CXXFLAGS="$CFLAGS"
fi

in sage-env, which will always cause LDFLAGS and CXXFLAGS to be exported, no matter whether they've previously been unset, or exported empty. So currently (re)exporting these in e.g. spkg-install scripts is indeed redundant.

I think we should either do that there for all such variables, or none of them. Especially setting CXXFLAGS to $CFLAGS even if a user did export CXXFLAGS="" may have somewhat surprising effects, or is (I think) at least unexpected behaviour.