fix: use run.executable for interpreter identification instead of activatedRun.executable#949
Open
renan-r-santos wants to merge 1 commit intomicrosoft:mainfrom
Conversation
…reter identification getInterpreterDetails, getSettingsPythonPath, and getExecutableCommand need the actual Python binary path, not the activated run command. Using activatedRun.executable breaks when environment managers set it to a wrapper command (e.g. pixi run ... python). This unblocks managers like conda and pixi from using wrapper commands in activatedRun without breaking the debugger, while remaining backwards-compatible since the two values are currently identical for all existing managers.
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 microsoft/vscode-python-environments#987
Hi @eleanorjboyd. I saw you merged test discovery 2.0 and was very excited to test it. I still have environment activation issues after that PR was merged, so I decided to investigate it a bit further. I'd appreciate it if you could review this PR.
The debugger reads
activatedRun.executablein three places to figure out which Python binary to use. This works today because every environment manager happens to setactivatedRun.executableto the actual Python path, but it breaks when a manager sets it to a wrapper command (likepixi run ... pythonor how conda used to doconda run ... pythonbefore microsoft/vscode-python-environments#860).I believe the three call sites (
getInterpreterDetails,getSettingsPythonPath,getExecutableCommand) aren't running Python, they're identifying the interpreter for things likeresolveEnvironment()and spawning the debug adapter. And I think that's whatrun.executableis for.activatedRunis for execution contexts likerunInBackgroundin vscode-python-environments.The change should be backwards compatible since
run.executableandactivatedRun.executableare currently the same value for all existing managers (venv, system, conda). It only matters when they differ, which is the case for pixi-code (see microsoft/vscode-python-environments#987 and #872).Debuggee activation is not affected, that still goes through
getActivatedEnvironmentVariables()and terminalenvVarCollection.This unblocks environment managers from setting
activatedRunto a wrapper command likepixi run ... python, which is needed to fix pytest discovery not activating the environment (microsoft/vscode-python-environments#987). Right nowrunInBackground(used by test discovery) relies onactivatedRunfor execution, but managers can't set it to anything other than the bare Python path without breaking the debugger.