Skip to content

Fix Python executable detection in virtual environments#7485

Merged
andife merged 3 commits intoonnx:mainfrom
aoguntayo:fix-python-executable-detection
Dec 2, 2025
Merged

Fix Python executable detection in virtual environments#7485
andife merged 3 commits intoonnx:mainfrom
aoguntayo:fix-python-executable-detection

Conversation

@aoguntayo
Copy link
Copy Markdown
Contributor

@aoguntayo aoguntayo commented Nov 25, 2025 •
edited
Loading

topic: #7226

The current get_python_execute() function prioritizes a directory-based search over sys.executable, which can incorrectly select the system Python instead of the active runtime Python in virtual environments and containerized builds.

This causes build failures when the system Python version differs from the runtime Python version. For example, when building with Python 3.12 in a virtual environment, the function may incorrectly return /usr/bin/python3 (Python 3.9), leading to compatibility issues.

Changes:

  • Check if sys.executable is valid and executable before attempting directory traversal
  • Only use the include-path-based search as a fallback when sys.executable is invalid
  • Add docstring explaining the function's behavior
  • Maintain backward compatibility for the cpython issue #84399 edge case

This ensures the build uses the correct Python executable in:

  • Virtual environments (venv, virtualenv)
  • Containerized builds
  • Custom Python installations
  • Standard system installations

Motivation and Context

Fixes #

@aoguntayo aoguntayo requested a review from a team as a code owner November 25, 2025 12:04
@github-project-automation github-project-automation Bot moved this to In progress in PR Tracker Nov 25, 2025
EmilienM
EmilienM approved these changes Nov 25, 2025
View reviewed changes
Copy link
Copy Markdown

@EmilienM EmilienM left a comment

Choose a reason for hiding this comment

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

thanks, lgtm

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 25, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 93.20388% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.25%. Comparing base (46c78b5) to head (1fbe740).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
onnx/test/test_env_python_executable.py 93.20% 2 Missing and 5 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7485      +/-   ##
==========================================
+ Coverage   55.12%   55.25%   +0.12%     
==========================================
  Files         512      513       +1     
  Lines       32108    32211     +103     
  Branches     2885     2895      +10     
==========================================
+ Hits        17701    17797      +96     
- Misses      13618    13620       +2     
- Partials      789      794       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EmilienM
Copy link
Copy Markdown

EmilienM commented Nov 25, 2025

@justinchuby @andife please take a look

@titaiwangms titaiwangms requested a review from andife November 25, 2025 17:17
@andife andife added the python Pull requests that update Python code label Nov 26, 2025
@andife
Copy link
Copy Markdown
Member

andife commented Nov 26, 2025

I think the patch is fine as it is, but I wonder if we should add some tests to check the behavior in general?

@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in PR Tracker Nov 26, 2025
@EmilienM
Copy link
Copy Markdown

EmilienM commented Dec 1, 2025

I think the patch is fine as it is, but I wonder if we should add some tests to check the behavior in general?

@aoguntayo please look at that comments and if no tests can be provided, please copy/paste a proof of it working for you when building the wheel. Thank you

@aoguntayo
Copy link
Copy Markdown
Contributor Author

aoguntayo commented Dec 2, 2025

@EmilienM & @andife
I've added a test, so hopefully this is okay to merge.
Results:

$ pytest test_env_python_executable.py -v
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.13.5, pytest-9.0.1, pluggy-1.5.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/aoguntay/onnx
plugins: requests-mock-1.12.1
collected 9 items                                                                                                                                                                                                 

test_env_python_executable.py::TestGetPythonExecutable::test_cpython_issue_84399_fallback PASSED                                                                                                            [ 11%]
test_env_python_executable.py::TestGetPythonExecutable::test_executable_permission_check PASSED                                                                                                             [ 22%]
test_env_python_executable.py::TestGetPythonExecutable::test_fallback_finds_python3_in_bin PASSED                                                                                                           [ 33%]
test_env_python_executable.py::TestGetPythonExecutable::test_fallback_finds_python_in_bin PASSED                                                                                                            [ 44%]
test_env_python_executable.py::TestGetPythonExecutable::test_invalid_sys_executable_falls_back PASSED                                                                                                       [ 55%]
test_env_python_executable.py::TestGetPythonExecutable::test_real_environment PASSED                                                                                                                        [ 66%]
test_env_python_executable.py::TestGetPythonExecutable::test_valid_sys_executable_is_preferred PASSED                                                                                                       [ 77%]
test_env_python_executable.py::TestGetPythonExecutable::test_virtual_environment_detection PASSED                                                                                                           [ 88%]
test_env_python_executable.py::TestGetPythonExecutable::test_windows_returns_sys_executable PASSED                                                                                                          [100%]

================================================================================================ 9 passed in 0.04s ================================================================================================

github-advanced-security[bot]
github-advanced-security AI found potential problems Dec 2, 2025
View reviewed changes
Comment thread onnx/test/test_env_python_executable.py Fixed
Comment thread onnx/test/test_env_python_executable.py Fixed
Comment thread onnx/test/test_env_python_executable.py Fixed
github-advanced-security[bot]
github-advanced-security AI found potential problems Dec 2, 2025
View reviewed changes
Comment thread onnx/test/test_env_python_executable.py Fixed
@andife
Copy link
Copy Markdown
Member

andife commented Dec 2, 2025 •
edited
Loading

@aoguntayo Could you check the windows test failures and the linting errors?

@aoguntayo
Fix Python executable detection in virtual environments
916ca78 
The current get_python_execute() function prioritizes a directory-based
search over sys.executable, which can incorrectly select the system
Python instead of the active runtime Python in virtual environments
and containerized builds.

This causes build failures when the system Python version differs from
the runtime Python version. For example, when building with Python 3.12
in a virtual environment, the function may incorrectly return
/usr/bin/python3 (Python 3.9), leading to compatibility issues.

Changes:
- Check if sys.executable is valid and executable before attempting
  directory traversal
- Only use the include-path-based search as a fallback when
  sys.executable is invalid
- Add docstring explaining the function's behavior
- Maintain backward compatibility for the cpython issue #84399 edge case

This ensures the build uses the correct Python executable in:
- Virtual environments (venv, virtualenv)
- Containerized builds
- Custom Python installations
- Standard system installations

Signed-off-by: Anu Oguntayo <aoguntay@redhat.com>
@aoguntayo
Fix Python executable detection in virtual environments #7485
49c7f35 
Added a test file to validate the get_python_execute() function

Signed-off-by: Anu Oguntayo <aoguntay@redhat.com>
@aoguntayo aoguntayo force-pushed the fix-python-executable-detection branch from 6ef3356 to 49c7f35 Compare December 2, 2025 12:57
@aoguntayo
Copy link
Copy Markdown
Contributor Author

aoguntayo commented Dec 2, 2025

@andife Should be okay

@andife
Copy link
Copy Markdown
Member

andife commented Dec 2, 2025

@andife Should be okay

@aoguntayo Have a look at the tests.

@aoguntayo aoguntayo force-pushed the fix-python-executable-detection branch 2 times, most recently from 09fe620 to 471b105 Compare December 2, 2025 17:36
@aoguntayo
Fix Python executable detection in virtual environments #7485
1fbe740 
Enable pytest `onnx/test/test_env_python_executable.py` to support windows

Signed-off-by: Anu Oguntayo <aoguntay@redhat.com>
@aoguntayo aoguntayo force-pushed the fix-python-executable-detection branch from 471b105 to 1fbe740 Compare December 2, 2025 19:03
@aoguntayo
Copy link
Copy Markdown
Contributor Author

aoguntayo commented Dec 2, 2025

@andife Should be okay

@aoguntayo Have a look at the tests.

haha sorry for ping pong back and forth XD
The test should be fix

@andife andife added the topic: build Issues related to ONNX builds and packages label Dec 2, 2025
@andife andife enabled auto-merge (squash) December 2, 2025 19:46
@andife andife merged commit 5b5566e into onnx:main Dec 2, 2025
54 of 56 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in PR Tracker Dec 2, 2025
@aoguntayo aoguntayo deleted the fix-python-executable-detection branch December 3, 2025 09:24
@andife
Copy link
Copy Markdown
Member

andife commented Dec 3, 2025

@aoguntayo

did we miss

image

?

@andife
Copy link
Copy Markdown
Member

andife commented Dec 7, 2025

#7511 @aoguntayo

@BrewTestBot BrewTestBot mentioned this pull request Mar 27, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andife andife andife approved these changes

+1 more reviewer

@EmilienM EmilienM EmilienM approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

python Pull requests that update Python code topic: build Issues related to ONNX builds and packages

Projects

PR Tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@aoguntayo @EmilienM @andife @github-advanced-security