#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, John Palmieri, Keshav Kini  |    Upstream:  N/A      
                              
   Reviewer:  Ivan Andrus                              |      Merged:           
                              
Work_issues:                                           |  
-------------------------------------------------------+--------------------
Description changed by jdemeyer:

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.
>
> Apply trac_10469.v3.patch to local/bin

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 [attachment:trac_10469.v4.patch] to `local/bin` and change
 permissions of `local/bin/sage-env` to 0644.

--

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10469#comment:11>
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.

Reply via email to