Repository navigation
[pathfinder] Fix site-packages search order for venv created with --system-site-packages - #1717
Merged
Conversation
…ixes NVIDIA#1716) In a venv created with --system-site-packages, user-site-packages were being searched before the virtual environment's site-packages, which violates PEP 405. The fix ensures that: - In a venv: venv site-packages come first, then user-site-packages, then system site-packages (PEP 405) - Outside a venv: user-site-packages come before system site-packages (PEP 370) Added tests to verify the correct ordering in both scenarios. Made-with: Cursor
Contributor
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
Author
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
cpcloud
approved these changes
Mar 4, 2026
cpcloud
left a comment
cpcloud
left a comment
Contributor
There was a problem hiding this comment.
Looks good: fixes the venv (PEP 405) vs user-site (PEP 370) site-packages ordering for #1716 and adds targeted tests. Local Pixi run for cuda_pathfinder is clean. The aarch64 failures appear infra-related (apt mirror sync / GitHub API 502) rather than caused by this change.
Contributor
|
This was referenced Nov 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1716
Fix
Updated
find_sub_dirs_all_sitepackages()to detect when running in a virtual environment (by checking ifsys.prefix != sys.base_prefix) and insert user-site-packages at the correct position:Testing
Added three test cases to
test_utils_find_sub_dirs.py:test_find_sub_dirs_all_sitepackages_venv_order: Verifies correct ordering (venv → user → system) in a venv with--system-site-packagestest_find_sub_dirs_all_sitepackages_non_venv_order: Verifies correct ordering (user → system) outside a venvtest_find_sub_dirs_all_sitepackages_venv_first_match: Ensures venv site-packages are searched before user site-packages