#11687: Sanitize `sage-env`
-------------------------------------------------+-------------------------
Reporter: leif | Owner: leif
Type: defect | Status: new
Priority: minor | Milestone: sage-6.1
Component: scripts | Resolution:
Keywords: BUILD sage-spkg pkg-config | Merged in:
.pc pkgconfig PKG_CONFIG_PATH SAGE_PATH | Reviewers:
Cygwin environment variables | Work issues:
Authors: | Commit:
Report Upstream: N/A | Stopgaps:
Branch: |
Dependencies: |
-------------------------------------------------+-------------------------
Description changed by jdemeyer:
Old description:
> There are currently a couple of issues with `sage-env`; approximately
> ordered by importance:
>
> * The definition of `UNAME` (currently on line 284) has to be moved up,
> preferably right before `PATH` etc. get set / modified (currently around
> line 135), and `$UNAME` should be used consistently.
> * The following (starting on line 348) has also to be moved up, since we
> return from `sage-env` earlier above (line 238) in case `"$1" =
> "-short"`:
> {{{
> #!sh
> if [ "$UNAME" = "CYGWIN" ]; then
> PATH="$PATH:$SAGE_LOCAL/lib" && export PATH
> fi
> }}}
> Moreover, I think the directory should be '''pre'''pendend to the
> path, but perhaps in a way that it is still preceded by
> `$SAGE_ROOT/local/bin`.
> * `SAGE_PATH` (followed by a colon) is prepended to `PYTHONPATH` even if
> it is empty / undefined. Some programs interpret this as if the current
> working directory ("`.`") was prepended. [[BR]]
> It also shouldn't be set to (or contain) "`.`", since this causes
> trouble with `easy_install` / distutils / `.pth` files. (Cf. #10192,
> #10176; the former has a patch to remove it [at least] in `spkg-install`,
> which isn't necessarily sufficient.).
> * We should save the original, unmodified `PATH` (to `SAGE_ORIG_PATH`),
> analoguous to `SAGE_ORIG_LD_LIBRARY_PATH` etc., and perhaps record this
> in `SAGE_ORIG_PATH_SET`. This should also be done close to the top, of
> course before modifying `PATH`. (#10202 also suggests / introduces this.)
> * There's partially useless code like
> {{{
> #!sh
> if [ "$LDFLAGS" = "" ]; then
> LDFLAGS="" && export LDFLAGS
> fi
> }}}
> * If `CFLAGS` (as specified by the user) are non-empty, we could append
> `-fno-strict-aliasing`, since Cython code will break if `CFLAGS` are set
> but don't contain this, but this should better be done where Cython code
> is compiled, i.e. in `devel/sage/setup.py` and `sage.misc.cython`. [[BR]]
> Note that even `export CFLAGS=""` causes distutils to drop the
> defaults, which do contain `-fno-strict-aliasing` (which could be solved
> by `unset`ting `CFLAGS` if they are empty). [[BR]]
> The result of such broken compiled Cython code shows up as weird
> import errors when starting Sage, which a user will hardly relate to a
> probably empty environment variable.
> * ''All'' variables that are expected to be either "`yes`", "`no`" or
> empty / unset should be checked for [in]valid settings, and perhaps
> normalized (i.e., for example set to "`no`" in case they're empty or not
> set). [[BR]]
> We currently only do this for `SAGE64`:
> {{{
> #!sh
> if [ "$SAGE64" = "" ]; then
> SAGE64="no"
> fi
>
> if [ "$SAGE64" != "yes" -a "$SAGE64" != "no" ]; then
> echo "The environment variable SAGE64 (=$SAGE64) must be either
> unset, yes or no."
> return 1
> fi
> }}}
> * The ''default'' for `CFLAG64` (`-m64`) and similar should be set in
> `sage-env`, not each and every `spkg-install`.
>
> (Line numbers as of Sage 4.7.1.rc2.)
>
> ----
>
> We should IMHO also save the original settings of `CC`, `CFLAGS`, `LD`,
> `LDFLAGS`, `MAKE` etc., since it is otherwise impossible for scripts
> (like the `spkg-install`s) to determine whether or which parts of the
> current settings were made by Sage, and which originate from the user
> (intentionally or not, e.g. from system-wide or default `.*rc` files).
>
> A better approach would be to have and use `SAGE_CC` etc. instead, but
> this would require changes to all spkgs and other build-related files.
>
> ----
>
> Some of the rather minor issues can be fixed on their own / follow-up
> tickets, but it would be odd to have a dozen "concurring" tickets all
> modifying `sage-env` at the same time.
New description:
There are currently a couple of issues with `sage-env`; approximately
ordered by importance:
* The definition of `UNAME` (currently on line 458) has to be moved up,
preferably right before `PATH` etc. get set / modified (currently around
line 135), and `$UNAME` should be used consistently.
* `SAGE_PATH` (followed by a colon) shouldn't be set to (or contain)
"`.`", since this causes trouble with `easy_install` / distutils / `.pth`
files. (Cf. #10192, #10176; the former has a patch to remove it [at least]
in `spkg-install`, which isn't necessarily sufficient.).
* We should save the original, unmodified `PATH` (to `SAGE_ORIG_PATH`),
analoguous to `SAGE_ORIG_LD_LIBRARY_PATH` etc., and perhaps record this in
`SAGE_ORIG_PATH_SET`. This should also be done close to the top, of course
before modifying `PATH`. (#10202 also suggests / introduces this.)
* There's partially useless code like
{{{
#!sh
if [ "$LDFLAGS" = "" ]; then
LDFLAGS="" && export LDFLAGS
fi
}}}
* If `CFLAGS` (as specified by the user) are non-empty, we could append
`-fno-strict-aliasing`, since Cython code will break if `CFLAGS` are set
but don't contain this, but this should better be done where Cython code
is compiled, i.e. in `devel/sage/setup.py` and `sage.misc.cython`. [[BR]]
Note that even `export CFLAGS=""` causes distutils to drop the
defaults, which do contain `-fno-strict-aliasing` (which could be solved
by `unset`ting `CFLAGS` if they are empty). [[BR]]
The result of such broken compiled Cython code shows up as weird import
errors when starting Sage, which a user will hardly relate to a probably
empty environment variable.
* ''All'' variables that are expected to be either "`yes`", "`no`" or
empty / unset should be checked for [in]valid settings, and perhaps
normalized (i.e., for example set to "`no`" in case they're empty or not
set). [[BR]]
We currently only do this for `SAGE64`
* The ''default'' for `CFLAG64` (`-m64`) and similar should be set in
`sage-env`, not each and every `spkg-install`.
----
We should IMHO also save the original settings of `CC`, `CFLAGS`, `LD`,
`LDFLAGS`, `MAKE` etc., since it is otherwise impossible for scripts (like
the `spkg-install`s) to determine whether or which parts of the current
settings were made by Sage, and which originate from the user
(intentionally or not, e.g. from system-wide or default `.*rc` files).
A better approach would be to have and use `SAGE_CC` etc. instead, but
this would require changes to all spkgs and other build-related files.
----
Some of the rather minor issues can be fixed on their own / follow-up
tickets, but it would be odd to have a dozen "concurring" tickets all
modifying `sage-env` at the same time.
--
--
Ticket URL: <http://trac.sagemath.org/ticket/11687#comment:13>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
