Skip to content

Commit

Permalink
sagemathgh-36364: Do not run sage-env more than once, for real.
Browse files Browse the repository at this point in the history
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->

Do not run sage-env more than once.

<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->

In the previous version of sage-env, in the lines
```
SAGE_ENV_VERSION="6:$SAGE_LOCAL:$SAGE_VENV:$SAGE_SRC"
if [ "$SAGE_ENV_SOURCED" = "$SAGE_ENV_VERSION" ]; then
    # Already sourced, nothing to do.
    return 0
fi
export SAGE_ENV_SOURCED="$SAGE_ENV_VERSION"
```
the test would fail the first time, as it should, but when the rest of
the file was executed, SAGE_SRC would get changed. That meant that the
test would fail the next time and so the file would get executed again.
We move the last line to the end of the file, and also change it to
explicitly use `SAGE_LOCAL`, `SAGE_VENV`, and `SAGE_SRC` so that it uses
the up-to-date versions.

This should fix part of the problem in sagemath#36337: it should prevent
duplicate `-rpath` entries which were arising because sage-env was
getting executed twice.

<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [X] The title is concise, informative, and self-explanatory.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36364
Reported by: John H. Palmieri
Reviewer(s): Dima Pasechnik, John H. Palmieri, Matthias Köppe
  • Loading branch information
Release Manager committed Oct 1, 2023
2 parents 53abf0a + b081169 commit 6819a3c
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/bin/sage-env
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ if [ "$SAGE_ENV_SOURCED" = "$SAGE_ENV_VERSION" ]; then
# Already sourced, nothing to do.
return 0
fi
export SAGE_ENV_SOURCED="$SAGE_ENV_VERSION"
# Set SAGE_ENV_SOURCED to the appropriate value at the end of this file, once
# $SAGE_LOCAL, $SAGE_VENV, $SAGE_SRC have been set.

# The compilers are set in order of priority by
# 1) environment variables
Expand Down Expand Up @@ -643,3 +644,7 @@ fi
# our doctests predictable (which was the status quo with earlier
# versions of debugpy).
export PYDEVD_DISABLE_FILE_VALIDATION=1

# Finally, set SAGE_ENV_SOURCED as evidence that this script has been
# run successfully.
export SAGE_ENV_SOURCED="6:$SAGE_LOCAL:$SAGE_VENV:$SAGE_SRC"

0 comments on commit 6819a3c

Please sign in to comment.