Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workaround for bug in pythonExtension.getExecutionDetails() method #1514

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# v 1.5.0 (?)
Changes in this release:
- Drop support for iso4/iso5
- Add workaround for bug in the python extension where `getExecutionDetails().execCommand[0]` returns the path to the root of the venv instead of the path to the python binary in that venv.

# v 1.4.0 (2023-06-06)
Changes in this release:
Expand Down
41 changes: 38 additions & 3 deletions src/python_extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { StatusBarAlignment, ThemeColor, window, workspace, TextDocument, Worksp
import { IExtensionApi, Resource } from './types';
import { fileOrDirectoryExists, log, getOuterMostWorkspaceFolder, logMap} from './utils';
import { getLanguageMap, getLastActiveFolder} from './extension';
import * as fs from "fs";


export const PYTHONEXTENSIONID = "ms-python.python";
Expand Down Expand Up @@ -31,13 +32,45 @@ export class PythonExtension {
* @returns {string} A string representing the path to the Python interpreter.
*/
get pythonPath(): string {
return this.executionDetails.execCommand[0];
return PythonExtension.getPathPythonBinary(this.executionDetails.execCommand[0]);
}

get virtualEnvName(): string | null {
return this.pythonPathToEnvName(this.pythonPath);
}


/**
* Due to bug https://github.com/microsoft/vscode-python/issues/22617, the `this.pythonApi.settings.getExecutionDetails()` method
* might return the path to the root of the venv instead of the path to the python binary in that venv. This method exists to work
* around that issue.
*/
private static getPathPythonBinary(execCommand: string): string {
if(PythonExtension.isFile(execCommand)){
return execCommand;
}
for(const pythonSuffix of ["python3", "python"]){
const execCommandWithSuffix = execCommand + "/bin/" + pythonSuffix;
if(PythonExtension.isFile(execCommandWithSuffix)){
return execCommandWithSuffix
}
}
throw new Error(`Failed to find python binary ${execCommand}`);
}


/**
* @returns {boolean} True iff the given path references a file (or symbolic link to file).
*/
private static isFile(path: string): boolean {
try{
const stat = fs.statSync(path);
return stat.isFile();
} catch(err){
return false;
}
}

pythonPathToEnvName(path: string) : string {
/**
* Match the virtual environment name using a regular expression to transform
Expand Down Expand Up @@ -141,7 +174,8 @@ export class PythonExtension {

getPathForResource(resource) {
try{
return this.pythonApi.settings.getExecutionDetails(resource).execCommand[0];
const execDetails = this.pythonApi.settings.getExecutionDetails(resource);
return PythonExtension.getPathPythonBinary(execDetails.execCommand[0]);
} catch (error){
console.error(`Failed to getPathForResource :` + error.name + error.message);
}
Expand All @@ -160,8 +194,9 @@ export class PythonExtension {

if(this.executionDetails.execCommand[0] !== newExecutionDetails.execCommand[0]){
this.executionDetails = newExecutionDetails;
const newPathPythonBinary = PythonExtension.getPathPythonBinary(newExecutionDetails.execCommand[0]);
for (const callback of this.callBacksOnchange) {
callback(newExecutionDetails.execCommand[0], outermost);
callback(newPathPythonBinary, outermost);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ async function main() {

const vscodeExecutablePath = await downloadAndUnzipVSCode('stable');
const cliPath = resolveCliPathFromVSCodeExecutablePath(vscodeExecutablePath, "linux-x64");
cp.spawnSync(cliPath, ['--install-extension', 'ms-python.python'], {
cp.spawnSync(cliPath, ['--install-extension', 'ms-python.python', '--force'], {
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't work without this anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This flag makes sure that we always check whether there is a newer version of the extensions available and upgrade to it if there is one. It should just make sure that we get faster feedback when there is a breaking change. So to answer your question: without this flag the tests should also work.

encoding: 'utf-8',
stdio: 'inherit'
});
Expand Down