Don't (effectively) source sage-env more than once

[leif]

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

# 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 change the permissions of sage-env to non-executable (it must be sourced, not executed).

• Apply trac_10469.v5[1.patch] to the scripts repo.
• Apply trac_10469-sage-repo.patch​ to the Sage library repo.

[leif]

Thoughts?

[iandrus]

It seems reasonable to me, so I've included a patch which does everything except versioning. This will obsolete #4029.

[jhpalmieri]

scripts repo; replaces other patch

[kini]

While we're at it, why don't we just make sage-env non-executable? Patch attached.

[iandrus]

What is the purpose of checking SAGE_ROOT but not doing anything with it? Is it just so that it prints a warning? It seems that if sage-env has been sourced SAGE_ROOT should be set correctly.

[jhpalmieri]

Replying to iandrus:

What is the purpose of checking SAGE_ROOT but not doing anything with it? Is it just so that it prints a warning? It seems that if sage-env has been sourced SAGE_ROOT should be set correctly.

You're probably right, and if this is always called via a call to sage, this could probably be deleted. As it stands, the changes here look pretty safe to me, but removing that check seems a bit riskier, and something which would require more serious testing.

[jhpalmieri]

I'm happy with all of the changes here, but since I helped to write them, I don't think I should give this a positive review. Anyone else? Ivan or Keshav?

[iandrus]

I'm happy to give it a positive review.

[jdemeyer]

I don't like the #!/this/script/must/be/sourced In my v4 patch, I removed that line. Instead, I think a much better solution is simply to make sage-env not executable.

[kini]

John, Mercurial tracks file modes. The diffs it exports (and thus the patches mq exports) do not mention them unless you put "[diff]\ngit = true" in your .hgrc. My patch does this - you can just remove the hashbang line from it.

Generally putting "[diff]\ngit = true" in your .hgrc is a good idea, since it frees Mercurial from having to remain compatible with ancient pre-dvcs diff formats.

[kini]

Oh, I'm sorry Jeroen. Somehow I thought you were John Palmieri...

[kini]

git format of Jeroen's v4 patch, which tracks the filemode change

[kini]

Added a patch as described and simplified the application instructions so the patchbot can read them more easily (?)

[jdemeyer]

Doctest error:

sage -t  -force_lib devel/sage/sage/misc/dist.py
**********************************************************************
File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.alpha2-10469/devel/sage-main/sage/misc/dist.py", line 46:
    sage: install_scripts(SAGE_TMP)
Expected:
    Checking that Sage has the command 'gap' installed
    Created script ...
Got:
    Checking that Sage has the command 'gap' installed
    The command 'gap' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'gp' installed
    The command 'gp' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'singular' installed
    The command 'singular' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'maxima' installed
    The command 'maxima' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'M2' installed
    The command 'M2' is not available; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'kash' installed
    The command 'kash' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'mwrank' installed
    The command 'mwrank' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'ipython' installed
    The command 'ipython' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'hg' installed
    The command 'hg' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Checking that Sage has the command 'R' installed
    The command 'R' is installed outside of Sage; not adding shortcut
    <BLANKLINE>
    Finished creating scripts.
    You need not do this again even if you upgrade or move Sage.
    The only requirement is that the command 'sage' is in the PATH.
**********************************************************************

[jhpalmieri]

Okay, I know what's going on. This is a bug in a different aspect of Sage, but perhaps it can be fixed on this ticket. On sage.math.washington.edu, the directory /scratch is actually a link, pointing to /mnt/usb1/scratch/. If you have a copy of Sage in a subdirectory of /scratch, then for reasons I don't understand, sometimes Sage thinks SAGE_ROOT is /scratch/... and sometimes it thinks it is /mnt/usb1/scratch/.... This doctest is failing because when the doctest runs, it thinks SAGE_ROOT is /mnt/usb1/scratch/..., while the output from "which gap" starts with /scratch/..., so the first entry in $PATH is the right directory, but with the wrong name. Maybe when sage-env gets run twice, it manages to use /mnt/... both times.

Here's a patch (to the Sage library) which applies os.path.realpath to both the value of SAGE_ROOT and the output from "which gap" (etc.). This fixes the problem for me.

[jhpalmieri]

main Sage repo