Skip to content

fix: treat cross-drive paths as outside the virtualenv - #4912

Open
r3wretrhy wants to merge 2 commits into
SCons:masterfrom
r3wretrhy:fix/virtualenv-cross-drive-3614
Open

r3wretrhy wants to merge 2 commits into
SCons:masterfrom
r3wretrhy:fix/virtualenv-cross-drive-3614

Conversation

@r3wretrhy

Copy link
Copy Markdown
Contributor

Summary

SCons.Platform.virtualenv._is_path_in() called os.path.relpath(path, base) without handling ValueError. On Windows that raises when path and base are on different drives, so --enable-virtualenv crashed when the venv lived on another drive than PATH entries (e.g. venv on W: while system Python / PATH entries on C:).

Catch ValueError and treat the path as outside the virtualenv (False), matching the intended "is this under the venv prefix?" check.

Fixes #3614.

Test plan

  • python runtest.py SCons/Platform/virtualenvTests.py — 9 passed (includes new cross-drive / ValueError coverage)

_is_path_in() used os.path.relpath() without catching ValueError. On
Windows that raises when path and base are on different drives, so
--enable-virtualenv crashed when the venv lived on another drive than
PATH entries (issue SCons#3614).

Catch ValueError and return False. Unit test covers the Windows cases
and mocks the ValueError path on all platforms.

Assisted-by: Claude
Signed-off-by: Zhaoqi Xu <lzy00419@outlook.com>
@mwichmann mwichmann moved this to In review in Next Release Oct 2, 2026
@bdbaddog

bdbaddog commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Can you add a note in CHANGES/RELEASE with the issue this fixes?
Otherwise looks good and ready to merge.

Assisted-by: Claude
Signed-off-by: Zhaoqi Xu <lzy00419@outlook.com>
@r3wretrhy

Copy link
Copy Markdown
Contributor Author

Done in fbfa853: the CHANGES.txt entry now ends with "Fixes #3614." and the RELEASE.txt FIXES entry says "(issue #3614)". I also dropped the extra blank line I'd added in CHANGES.txt. No code changes; runtest.py SCons/Platform/virtualenvTests.py still passes (9 tests).

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

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Crash on Win32 when using --enable-virtualenv (and two drive letters involved)

3 participants