Skip to content

Commit

Permalink
fix(PATH): push PATH updates into os.environ on change
Browse files Browse the repository at this point in the history
This issue was raised in
#5050.

If users (or `xontribs`) make use of `subprocess`, the `PATH` available
to `subprocess` is the value of `PATH` in `os.environ`, which is set at
the first import of `os.environ`.

This means (meant) that if a user updates their `PATH` in `xonshrc`,
those changes aren't visible to a bare `subprocess` call.
(This is what `UPDATE_OS_ENVIRON` is nominally for, but since changes to
`PATH` are mutating a mutable object, we don't hit any of the
`__setitem__` checks in the env object).

So, instead, I've added an explicit call to update the `os.environ`
value of `PATH` after each mutating call to our `EnvPath` class, so this
should also work for the various `_PATH` variables (e.g. `LDD_LIBRARY_PATH`).
  • Loading branch information
gforsyth committed Jul 21, 2023
1 parent 224d9e6 commit 64b907d
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 2 deletions.
25 changes: 25 additions & 0 deletions news/fix_os_environ_path.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Changes to ``$PATH`` are propagated to ``os.environ`` if
``UPDATE_OS_ENVIRON=True`` so user and ``xontrib`` usage of ``subprocess``
reflects the current ``xonsh`` value for ``$PATH``

**Security:**

* <news item>
2 changes: 1 addition & 1 deletion tests/test_environ.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def test_register_custom_var_env_path():


def test_deregister_custom_var():
env = Env()
env = Env({"UPDATE_OS_ENVIRON": True})

env.register("MY_SPECIAL_VAR", type="env_path")
env.deregister("MY_SPECIAL_VAR")
Expand Down
16 changes: 16 additions & 0 deletions tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -2048,3 +2048,19 @@ def test_is_regex_false():
)
def test_is_tok_color_dict(val, exp):
assert is_tok_color_dict(val) == exp


def test_path_updates_propagate_to_os_environ(xession):
with xession.env.swap(
{"PATH": EnvPath(xession.env["PATH"]), "UPDATE_OS_ENVIRON": True}
):
fakepath = "not/a/real/path/at/all"
xession.env["PATH"].insert(0, fakepath)
assert fakepath in os.environ["PATH"]

xession.env["PATH"].pop(0)
assert fakepath not in os.environ["PATH"]

xession.env["UPDATE_OS_ENVIRON"] = False
xession.env["PATH"].insert(0, fakepath)
assert fakepath not in os.environ["PATH"]
2 changes: 1 addition & 1 deletion xonsh/environ.py
Original file line number Diff line number Diff line change
Expand Up @@ -2345,7 +2345,7 @@ def register(
)

def deregister(self, name):
"""Deregister an enviornment variable and all its type handling,
"""Deregister an environment variable and all its type handling,
default value, doc.
Parameters
Expand Down
21 changes: 21 additions & 0 deletions xonsh/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ def findfirst(s, substrs):
return i, result


@lazyobject
def path_events():
from xonsh.events import events

return events


class EnvPath(cabc.MutableSequence):
"""A class that implements an environment path, which is a list of
strings. Provides a custom method that expands all paths if the
Expand Down Expand Up @@ -219,16 +226,30 @@ def __getitem__(self, item):
return _expandpath(self._l[item])

def __setitem__(self, index, item):
old_value = repr(self)
self._l.__setitem__(index, item)
self._update_env(old_value=old_value)

def __len__(self):
return len(self._l)

def __delitem__(self, key):
old_value = repr(self)
self._l.__delitem__(key)
self._update_env(old_value=old_value)

def insert(self, index, value):
old_value = repr(self)
self._l.insert(index, value)
self._update_env(old_value=old_value)

def _update_env(self, old_value=None):
new_value = repr(self)
if xsh.env.get("UPDATE_OS_ENVIRON"):
xsh.env["PATH"] = self
path_events.on_envvar_change.fire(
name="PATH", oldvalue=old_value, newvalue=new_value
)

@property
def paths(self):
Expand Down

0 comments on commit 64b907d

Please sign in to comment.