diff --git a/src/rez/rex.py b/src/rez/rex.py index da26ef5185..1946753842 100644 --- a/src/rez/rex.py +++ b/src/rez/rex.py @@ -12,8 +12,8 @@ from enum import Enum from contextlib import contextmanager from string import Formatter -from collections.abc import MutableMapping -from typing import Any, Iterable, Mapping +from collections.abc import Mapping, MutableMapping +from typing import Any, Iterable from rez.system import system from rez.config import config @@ -187,7 +187,10 @@ def __init__(self, interpreter: ActionInterpreter, interpreter: string or `ActionInterpreter` the interpreter to use when executing rex actions parent_environ: environment to execute the actions within. If None, - defaults to the current environment. + defaults to the current environment. On Windows, a caller-supplied + mapping is wrapped in a read-only, case-insensitive snapshot so that + lookups against it are case-insensitive, matching how :data:`os.environ` + behaves natively (see #2089); :data:`os.environ` and None are used as-is. parent_variables: List of variables to append/prepend to, rather than overwriting on first reference. If this is set to True instead of a list, all variables are treated as parent variables. @@ -200,9 +203,24 @@ def __init__(self, interpreter: ActionInterpreter, ''' self.interpreter = interpreter self.verbose = verbose - self.parent_environ = os.environ if parent_environ is None else parent_environ + # On Windows env-var keys are case-insensitive; os.environ already + # behaves that way but a plain dict supplied by the caller does not, + # so wrap it on the Windows path only. See #2089. + self.parent_environ: Mapping[str, str] + if parent_environ is None or parent_environ is os.environ: + # os.environ is already case-insensitive on Windows, so use it + # as-is (preserving liveness and identity); None means "current". + self.parent_environ = os.environ + elif platform_.name == "windows" and not isinstance( + parent_environ, _CaseInsensitiveEnvironProxy): + self.parent_environ = _CaseInsensitiveEnvironProxy(parent_environ) + else: + self.parent_environ = parent_environ self.parent_variables = True if parent_variables is True \ else set(parent_variables or []) + # Variables set during rex execution. Keys are stored verbatim and are + # not case-folded on Windows (unlike the parent_environ snapshot above); + # that broader normalization is a separate concern, tracked in #2164. self.environ = {} self.formatter = formatter or str self.actions = [] @@ -447,6 +465,47 @@ def _keytoken(self, key): return self.interpreter.get_key_token(key) +class _CaseInsensitiveEnvironProxy(Mapping): + """Read-only, case-insensitive snapshot of an env-var mapping. + + Used by :class:`ActionManager` to wrap a caller-supplied ``parent_environ`` + on Windows so lookups against it are case-insensitive regardless of + the casing used to populate the input dict, matching how :data:`os.environ` + behaves natively on Windows. + + Keys are normalized (upper-cased) once at construction, so this is a + point-in-time snapshot: later mutations to the source mapping are not + reflected. That matches how ``parent_environ`` is used in practice (fixed + for the lifetime of an executor). + + Kept private to this module on purpose; promote to a shared util + only when a second caller needs it. + """ + def __init__(self, data): + # keys are upper-cased once on the way in; same shape as + # os.environ on Windows. Last write wins on case collisions, + # which mirrors how os.environ resolves them too. + self._data = {k.upper(): v for k, v in data.items()} + + def __getitem__(self, key): + return self._data[key.upper()] + + def __contains__(self, key): + return key.upper() in self._data + + def __iter__(self): + return iter(self._data) + + def __len__(self): + return len(self._data) + + def copy(self): + # Return a plain dict with normalized (upper-cased) keys, mirroring + # os.environ.copy() on Windows. Defensive: lets callers that expect a + # dict-like parent_environ (e.g. `self.parent_environ.copy()`) work. + return dict(self._data) + + #=============================================================================== # Interpreters #=============================================================================== diff --git a/src/rez/tests/test_rex.py b/src/rez/tests/test_rex.py index ac3a8966df..fe27bb9278 100644 --- a/src/rez/tests/test_rex.py +++ b/src/rez/tests/test_rex.py @@ -13,10 +13,12 @@ from rez.exceptions import RexError, RexUndefinedVariableError from rez.config import config import unittest +from unittest import mock from rez.version import Version from rez.version import Requirement from rez.tests.util import TestBase from rez.utils.backcompat import convert_old_commands +from rez.utils.platform_ import platform_ from rez.package_repository import package_repository_manager from rez.packages import iter_package_families import inspect @@ -547,6 +549,110 @@ def test_intersects_ephemerals(self) -> None: self.assertRaises(RuntimeError, # no default intersects, ephemerals.get_range("foo.bar"), "0") + def test_case_insensitive_environ_proxy(self) -> None: + """Unit-test the case-normalizing proxy directly (platform independent). + + Runs on every host so the core logic behind the #2089 fix has real + CI coverage even though the ActionManager wiring is Windows-only. + """ + from rez.rex import _CaseInsensitiveEnvironProxy + + proxy = _CaseInsensitiveEnvironProxy({"SomeVariable": "covfefe"}) + for key in ("SomeVariable", "SOMEVARIABLE", "somevariable"): + self.assertEqual(proxy[key], "covfefe") + self.assertIn(key, proxy) + self.assertNotIn("OtherVariable", proxy) + self.assertRaises(KeyError, lambda: proxy["OtherVariable"]) + + # copy() yields a plain dict with normalized (upper-cased) keys, + # mirroring os.environ.copy() on Windows. + copied = proxy.copy() + self.assertIsInstance(copied, dict) + self.assertEqual(copied, {"SOMEVARIABLE": "covfefe"}) + + # normalized iteration / length + self.assertEqual(len(proxy), 1) + self.assertEqual(set(proxy), {"SOMEVARIABLE"}) + + # last-write-wins on case collision, mirroring os.environ + collided = _CaseInsensitiveEnvironProxy({"Foo": "a", "FOO": "b"}) + self.assertEqual(collided["foo"], "b") + + @unittest.skipIf(platform_.name == "windows", + "parent_environ passthrough is non-Windows behaviour") + def test_parent_environ_identity_passthrough_on_non_windows(self) -> None: + """On non-Windows, a caller dict is used as-is (no wrapping). #2089.""" + parent_env = {"SomeVariable": "covfefe"} + ex = self._create_executor(parent_env) + self.assertIs(ex.manager.parent_environ, parent_env) + + @unittest.skipIf(platform_.name != "windows", "Windows-only behaviour") + def test_parent_environ_case_insensitive_on_windows(self) -> None: + """Regression for #2089. + + On Windows env vars are case-insensitive; os.environ already + behaves that way, but a plain dict copy of it doesn't. Passing + such a dict as parent_environ used to break env. + lookups inside commands(). We now wrap on the Windows side. + """ + parent_env = {"SomeVariable": "covfefe"} + ex = self._create_executor(parent_env) + + # any casing of the key resolves + self.assertEqual(ex.manager.parent_environ["SomeVariable"], "covfefe") + self.assertEqual(ex.manager.parent_environ["SOMEVARIABLE"], "covfefe") + self.assertEqual(ex.manager.parent_environ["somevariable"], "covfefe") + self.assertIn("SOMEVARIABLE", ex.manager.parent_environ) + self.assertNotIn("OtherVariable", ex.manager.parent_environ) + + # end-to-end via the real package-commands entrypoint: rex references + # a different casing than the caller's dict, exercised through both + # execute_function and execute_code (what commands() actually run). + # Pre-fix this raised RexUndefinedVariableError. Note: the caller dict + # is named `parent_env`, not `env`, so the nested rex function does not + # close over it and shadow the rex `env` builtin. + def _rex() -> None: + env.RESULT = env.SOMEVARIABLE # noqa: F821 - rex builtin + + self._test( + func=_rex, + env=parent_env, + expected_actions=[Setenv("RESULT", "covfefe")], + expected_output={"RESULT": "covfefe"}, + ) + + def test_parent_environ_os_environ_passthrough(self) -> None: + """Explicit os.environ is used as-is (identity), never wrapped or + copied -- it is already case-insensitive on Windows. #2089. + """ + ex = self._create_executor(os.environ) + self.assertIs(ex.manager.parent_environ, os.environ) + + @mock.patch.object(platform_, "name", "windows") + def test_parent_environ_case_insensitive_windows_mocked(self) -> None: + """Companion to test_parent_environ_case_insensitive_on_windows that + runs on every host by mocking the platform, so the Windows-only + wrapping branch keeps CI coverage (and local validation) off Windows + too. #2089. + """ + from rez.rex import _CaseInsensitiveEnvironProxy + + parent_env = {"SomeVariable": "covfefe"} + ex = self._create_executor(parent_env) + self.assertIsInstance( + ex.manager.parent_environ, _CaseInsensitiveEnvironProxy) + self.assertEqual(ex.manager.parent_environ["SOMEVARIABLE"], "covfefe") + + def _rex() -> None: + env.RESULT = env.SOMEVARIABLE # noqa: F821 - rex builtin + + self._test( + func=_rex, + env=parent_env, + expected_actions=[Setenv("RESULT", "covfefe")], + expected_output={"RESULT": "covfefe"}, + ) + if __name__ == '__main__': unittest.main()