sage.env: _add_variable_or_fallback depends on the order of a dict

[jhpalmieri]

In sage/env.py, the function _add_variable_or_fallback does some variable substitution: if a string contains $VAR, it substitutes the value of VAR in the dictionary SAGE_ENV. It does this by iterating through the items of SAGE_ENV, which means that if both VAR and VAR_OTHER are in SAGE_ENV, it's sort of a coin toss as to what happens when it comes across $VAR_OTHER.

(This arose when I was working on #21469: the string $SAGE_SRC_ROOT was being replaced by the value of SAGE_SRC, followed by the string _ROOT.)

CC: @mkoeppe

Component: build

Author: John Palmieri

Branch: 343d685

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/23758

[jhpalmieri]

Branch: u/jhpalmieri/variable_replacement

[jhpalmieri]

New commits:

60927e5

trac 23758: in env.py, _add_variable_or_fallback should depend less

[jhpalmieri]

Commit: 60927e5

[sagetrac-git]

Changed commit from 60927e5 to c23cc1c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c23cc1c

trac 23758: restore "import os"

[mkoeppe]

comment:4

I don't know, maybe we can just get rid of this whole variable substitution business, by using the Python variable SAGE_ROOT instead of "$SAGE_ROOT"?

[mkoeppe]

comment:5

(Could be a follow-up ticket.)

[mkoeppe]

comment:6

Perhaps add a doctest?

[sagetrac-git]

Changed commit from c23cc1c to 343d685

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

343d685

trac 23758: add a doctest

[jhpalmieri]

comment:8

Here is a doctest. The same test fails for me in the develop branch, but I suppose that failure may not be repeatable on all platforms, since dictionaries are not ordered.

[jhpalmieri]

comment:9

Replying to @mkoeppe:

I don't know, maybe we can just get rid of this whole variable substitution business, by using the Python variable SAGE_ROOT instead of "$SAGE_ROOT"?

I agree, a followup ticket if we want to address this. I'm not sure what the point of the substitution is, compared to using shell variables or some shortcut for accessing SAGE_ENV['foo'].

[mkoeppe]

Author: John H. Palmieri

[mkoeppe]

Reviewer: Matthias Koeppe

[mkoeppe]

comment:11

Follow up ticket is #23762.

[sagetrac-git]

Changed commit from 343d685 to c7cff80

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

eb4137b	trac 23762: in sage/env.py, get rid of the variable substitution
c7cff80	Merge branch 't/23758/variable_replacement' into env

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

[sagetrac-git]

Changed commit from c7cff80 to 343d685

[jhpalmieri]

comment:14

Sorry, I accidentally pushed a branch here instead of to the appropriate ticket. The positively reviewed branch has been restored.

[vbraun]

Changed branch from u/jhpalmieri/variable_replacement to 343d685

[jdemeyer]

Changed commit from 343d685 to none

[jdemeyer]

Changed author from John H. Palmieri to John Palmieri