Skip to content

Fix rex parent_environ case sensitivity on Windows#2098

Open
thc1006 wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
thc1006:fix/rex-windows-parent-environ-case-2089
Open

Fix rex parent_environ case sensitivity on Windows#2098
thc1006 wants to merge 1 commit into
AcademySoftwareFoundation:mainfrom
thc1006:fix/rex-windows-parent-environ-case-2089

Conversation

@thc1006
Copy link
Copy Markdown

@thc1006 thc1006 commented May 7, 2026

On Windows, environment variable keys are case-insensitive natively. os.environ preserves that contract through Python's os._Environ wrapper, but a plain dict copy of it (or any user-built dict) does not. Until now, ActionManager consumed whatever Mapping the caller passed as-is, so a package whose commands() referenced env.SomeVariable against a parent_environ that held the same variable under a different casing would raise:

Error in commands in package '...':
Referenced undefined environment variable: SomeVariable

Issue #2089 has a clean reproducer that follows exactly this shape: dict(os.environ) is passed to ResolvedContext.get_environ, and a package looks up env.SomeVariable while the dict only has SOMEVARIABLE.

The fix wraps a caller-supplied parent_environ in a small read-only case-insensitive proxy, gated on platform_.name == "windows". On Linux and macOS the assignment reduces to the existing identity passthrough, so non-Windows is byte-for-byte unchanged. The proxy is a 5-method Mapping private to rex.py; keys are upper-cased once at construction time so iteration matches what os.environ already yields on Windows, and last-write-wins on case collisions matches how os.environ itself collapses duplicates. The not isinstance(...) guard in the gate keeps wrapping idempotent.

The new test_parent_environ_case_insensitive_on_windows mirrors the issue's reproducer through _create_executor, asserting both direct parent_environ[...] lookup across casings and an end-to-end getenv("SOMEVARIABLE") / setenv("RESULT", ...) flow against a {"SomeVariable": ...} dict. It skips on non-Windows since the wrapping is a no-op there.

Verified locally: pytest src/rez/tests/test_rex.py is green (15 pass + 1 skip on Linux), full rez-selftest is green in a hermetic Python 3.11 container (Ran 259 tests in 24.997s, OK), flake8 is clean, and mypy | mypy-baseline filter reports no new violations above the pinned baseline.

Closes #2089

Reported by @nrusch. Independent diagnosis previously posted in #2093 (closed because the author could not satisfy the EasyCLA/DCO requirement, not for technical reasons).

@thc1006 thc1006 requested a review from a team as a code owner May 7, 2026 05:12
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 7, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: thc1006 / name: thc1006 (f3db9fb)

@thc1006
Copy link
Copy Markdown
Author

thc1006 commented May 7, 2026

Closing+reopening to retrigger Read the Docs (initial build failed during clone, 2s duration, generic clone error — fork branch is public and accessible).

@thc1006 thc1006 closed this May 7, 2026
@thc1006 thc1006 reopened this May 7, 2026
On Windows env-var keys are case-insensitive natively. os.environ
preserves that contract via os._Environ, but a plain dict copy of it
(or any user-built dict) does not. Until now, ActionManager consumed
whatever Mapping the caller passed as-is, so a package commands()
that referenced env.SomeVariable against a parent_environ holding the
same key under a different case raised RexUndefinedVariableError.

Wrap the caller-supplied parent_environ in a small read-only
case-insensitive proxy on the Windows path only. The proxy is module
private; the wrapping is gated on platform_.name == "windows" and is
idempotent so re-entering the gate is a no-op. Linux and macOS take
the existing identity assignment unchanged.

Also adds a Windows-only regression test that mirrors the issue
reproducer and asserts both direct lookup and end-to-end rex code
paths against a mixed-case parent_environ.

Closes AcademySoftwareFoundation#2089

Reported by @nrusch.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 force-pushed the fix/rex-windows-parent-environ-case-2089 branch from f3db9fb to fa436d7 Compare May 7, 2026 05:21
@thc1006
Copy link
Copy Markdown
Author

thc1006 commented May 7, 2026

Status note for reviewers:

  • The six core GitHub Actions workflows (tests, flake8, mypy, copyright, installation, benchmark) are queued in action_required state, awaiting maintainer approval per the first-time-contributor policy. Nothing to do on my side.
  • The Read the Docs failure is git fetch origin pull/2098/head returning fatal: couldn't find remote ref pull/2098/head. The GitHub git API for this PR currently exposes only refs/pull/2098/merge; refs/pull/2098/head is missing from matching-refs/pull/2098, while other open PRs (e.g. Allow building with cmake on Windows #2090, Fix shell auto-completion #2091) have both refs as expected. An amend force-push was attempted to nudge GitHub into recreating the head ref but it has not reappeared. This appears to be a GitHub-side propagation glitch independent of the diff. Approving the workflows may incidentally fix it; if not, an RTD admin rebuild should clear it.
  • DCO and EasyCLA are both green.

Local pre-push validation: pytest src/rez/tests/test_rex.py is green (15 pass + 1 Windows-only skip); full rez-selftest on a hermetic Python 3.11.14 container reports Ran 259 tests in 24.997s, OK (skipped=10); flake8 src/rez/rex.py src/rez/tests/test_rex.py is clean; mypy | mypy-baseline filter --ignore-categories=note --allow-unsynced reports no new violations above the pinned baseline.

Copilot AI review requested due to automatic review settings May 7, 2026 06:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rex environment lookups do not respect platform case-sensitivity rules

2 participants