#10469: Don't (effectively) source sage-env more than once
---------------------------+------------------------------------------------
Reporter: leif | Owner: GeorgSWeber
Type: defect | Status: needs_review
Priority: major | Milestone: sage-4.7
Component: build | Keywords: environment variables sage-sage
scripts
Author: Ivan Andrus | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
---------------------------+------------------------------------------------
Old description:
> This can currently happen because
>
> * some scripts source it themselves (like e.g. at least `sage-spkg`,
> which is necessary since `sage-spkg` isn't always called through `sage-
> sage`, which also sources it),
>
> * some receipts in the top-level `Makefile` source it before some other
> script is called through `sage-sage`,
>
> * `sage-sage` itself or other (e.g. Python) code may run `sage ...`
> commands such that `sage-sage` is then called recursively, again sourcing
> `sage-env`.
>
> To achieve this, we could simply add
> {{{
> #!sh
> # Don't execute the commands below more than once:
> if [ -z "$SAGE_ENV_SOURCED" ]; then
> export SAGE_ENV_SOURCED=1 # or "yes", "true" or alike, but see below
> (versioning)
> else
> # Already sourced, nothing to do.
> return 0
> fi
> }}}
> near its top.
>
> We ''may'' even put a version number into that variable and execute
> (perhaps only some of) the commands in `sage-env` "again" in case it was
> modified during running scripts, which could be helpful when upgrading
> Sage.
>
> ----
>
> Such a variable also allows still generic, but safer tests than the usual
> `[ -z "$SAGE_LOCAL" ]`, since `SAGE_LOCAL` being defined doesn't really
> imply `sage-env` was sourced, but we usually intend to ensure that some
> environment variables (like e.g. `CC` or `UNAME`) are properly set up,
> without testing each of these individually.
>
> Effectively sourcing (at least parts of) `sage-env` only once also
>
> * avoids redundant entries in `PATH` etc.,
> * avoids special tests if some variable was already defined / modified
> (like e.g. `SAGE_ORIG_LD_LIBRARY_PATH`), also simplifying other scripts
> (cf. #10286),
> * IMHO reduces the risk of unintentional side-effects, and
> * speeds up execution.
>
> ----
>
> Also, we should replace the useless
> {{{
> #!sh
> #!/usr/bin/env bash
> }}}
> at its top by (e.g.)
> {{{
> #!sh
> #!/this/script/must/be/sourced
> }}}
> which causes an error if `sage-env` is called rather than sourced (which
> makes no sense and therefore ''is'' indeed an error), with an
> "appropriate" error message since such an interpreter certainly does not
> exist.
New description:
This can currently happen because
* some scripts source it themselves (like e.g. at least `sage-spkg`,
which is necessary since `sage-spkg` isn't always called through `sage-
sage`, which also sources it),
* some receipts in the top-level `Makefile` source it before some other
script is called through `sage-sage`,
* `sage-sage` itself or other (e.g. Python) code may run `sage ...`
commands such that `sage-sage` is then called recursively, again sourcing
`sage-env`.
To achieve this, we could simply add
{{{
#!sh
# Don't execute the commands below more than once:
if [ -z "$SAGE_ENV_SOURCED" ]; then
export SAGE_ENV_SOURCED=1 # or "yes", "true" or alike, but see below
(versioning)
else
# Already sourced, nothing to do.
return 0
fi
}}}
near its top.
We ''may'' even put a version number into that variable and execute
(perhaps only some of) the commands in `sage-env` "again" in case it was
modified during running scripts, which could be helpful when upgrading
Sage.
----
Such a variable also allows still generic, but safer tests than the usual
`[ -z "$SAGE_LOCAL" ]`, since `SAGE_LOCAL` being defined doesn't really
imply `sage-env` was sourced, but we usually intend to ensure that some
environment variables (like e.g. `CC` or `UNAME`) are properly set up,
without testing each of these individually.
Effectively sourcing (at least parts of) `sage-env` only once also
* avoids redundant entries in `PATH` etc.,
* avoids special tests if some variable was already defined / modified
(like e.g. `SAGE_ORIG_LD_LIBRARY_PATH`), also simplifying other scripts
(cf. #10286),
* IMHO reduces the risk of unintentional side-effects, and
* speeds up execution.
----
Also, we should replace the useless
{{{
#!sh
#!/usr/bin/env bash
}}}
at its top by (e.g.)
{{{
#!sh
#!/this/script/must/be/sourced
}}}
which causes an error if `sage-env` is called rather than sourced (which
makes no sense and therefore ''is'' indeed an error), with an
"appropriate" error message since such an interpreter certainly does not
exist.
Apply trac_10469.v3.patch to local/bin
--
Comment(by kini):
While we're at it, why don't we just make sage-env non-executable? Patch
attached.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10469#comment:4>
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.
