Skip to content

Commit 5140ddd

Browse files
isabelizimmseeMsharon-wang
authored
python: respect defaultInterpreterPath when specified (#6414)
Adds in a selection process for implicitly or immediately starting a runtime before discovery in `recommendWorkspaceRuntime()`. The hope here is that, if there is a local env or a user-defined environment we know about _before_ discovery, we can automatically select it. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - #6281 - #6204 #### Bug Fixes - N/A ### QA Notes <!-- Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues. --> I have a few scenarios highlighted and what is expected. Since this part of the codebase cares a lot about past selections, I'd highly recommend clearing state and then restoring settings by running: ``` rm -rf ~/.vscode-oss-dev/; rm -rf ~/.positron-dev; mkdir -p ~/.vscode-oss-dev/User; touch ~/.vscode-oss-dev/User/settings.json; echo '{"python.defaultInterpreterPath": "PATH TO SOME PYTHON"}' > ~/.vscode-oss-dev/User/settings.json ``` | scenario | expected | | ------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | | in workspace:<br>has .venv<br>has user level defaultInterpreterPath set | .venv<br>immediate startup | | in workspace:<br>had active runtime when Positron shut down<br>has user level defaultInterpreterPath | previous active runtime<br>immediate startup<br> | | in workspace:<br>brand new, no affiliated runtime<br>has user level defaultInterpreterPath set | user level defaultInterpreterPath<br>implicit startup | | in workspace:<br>no affiliated runtime<br>has workspace level defaultInterpreterPath set<br>has user level defaultInterpreterPath set | workspace level defaultInterpreterPath<br>implicit startup | | no workspace:<br>has user level defaultInterpreterPath set | user level defaultInterpreterPath<br>implicit startup | --------- Signed-off-by: Isabel Zimmerman <[email protected]> Co-authored-by: Wasim Lorgat <[email protected]> Co-authored-by: sharon <[email protected]>
1 parent e16a655 commit 5140ddd

File tree

9 files changed

+412
-20
lines changed

9 files changed

+412
-20
lines changed

extensions/positron-python/package.nls.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"python.condaPath.description": "Path to the conda executable to use for activation (version 4.4+).",
5151
"python.environmentProviders.enable.markdownDescription": "Configures which Python environment providers are enabled in Positron. Check the box to enable or uncheck the box to disable an environment provider.\n\nRequires a restart to take effect.",
5252
"python.debugger.deprecatedMessage": "This configuration will be deprecated soon. Please replace `python` with `debugpy` to use the new Python Debugger extension.",
53-
"python.defaultInterpreterPath.description": "Path to default Python to use when extension loads up for the first time, no longer used once an interpreter is selected for the workspace. See [here](https://aka.ms/AAfekmf) to understand when this is used",
53+
"python.defaultInterpreterPath.description": "Path to default Python to use when extension loads up for the first time, no longer used once an interpreter is selected for the workspace. Must be an absolute path.",
5454
"python.interpreters.include.markdownDescription": "List of folders (absolute paths) to search for Python installations. These folders are searched in addition to the default folders for your operating system.\n\nExample: if you have Python located at `/custom/pythons/3.10.4/bin/python`, add path `/custom/pythons` or `/custom/pythons/3.10.4` to this setting.\n\nNote: If an interpreter is both included via `python.interpreters.include` and excluded via `python.interpreters.exclude`, it will not be displayed in the Positron UI.\n\nRequires a restart to take effect.",
5555
"python.interpreters.exclude.markdownDescription": "List of interpreter paths or folders (absolute paths) to exclude from the available Python installations. These interpreters will not be displayed in the Positron UI.\n\nExample: Add `/usr/bin/python3` to exclude the specific interpreter, or `/opt/homebrew` to exclude all brew-installed Pythons on macOS.\n\nRequires a restart to take effect.",
5656
"python.envFile.description": "Absolute path to a file containing environment variable definitions.",

extensions/positron-python/src/client/common/interpreterPathService.ts

+8-5
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,14 @@ export class InterpreterPathService implements IInterpreterPathService {
111111
): Promise<void> {
112112
resource = PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri;
113113
if (configTarget === ConfigurationTarget.Global) {
114-
const pythonConfig = this.workspaceService.getConfiguration('python');
115-
const globalValue = pythonConfig.inspect<string>('defaultInterpreterPath')!.globalValue;
116-
if (globalValue !== pythonPath) {
117-
await pythonConfig.update('defaultInterpreterPath', pythonPath, true);
118-
}
114+
// --- Start Positron ---
115+
// do not update global interpreter path setting via the Select Interpreter dropdown
116+
// const pythonConfig = this.workspaceService.getConfiguration('python');
117+
// const globalValue = pythonConfig.inspect<string>('defaultInterpreterPath')!.globalValue;
118+
// if (globalValue !== pythonPath) {
119+
// await pythonConfig.update('defaultInterpreterPath', pythonPath, true);
120+
// }
121+
// --- End Positron ---
119122
return;
120123
}
121124
if (!resource) {

extensions/positron-python/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts

+9
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import { sendTelemetryEvent } from '../../telemetry';
1515
import { EventName } from '../../telemetry/constants';
1616
import { IPythonPathUpdaterServiceManager } from '../configuration/types';
1717
import { IActivatedEnvironmentLaunch, IInterpreterService } from '../contracts';
18+
// --- Start Positron ---
19+
import { getUserDefaultInterpreter } from '../../positron/interpreterSettings';
20+
// --- End Positron ---
1821

1922
@injectable()
2023
export class ActivatedEnvironmentLaunch implements IActivatedEnvironmentLaunch {
@@ -101,6 +104,12 @@ export class ActivatedEnvironmentLaunch implements IActivatedEnvironmentLaunch {
101104
}
102105
this.wasSelected = true;
103106
this.inMemorySelection = prefix;
107+
// --- Start Positron ---
108+
// prefer user setting defaultInterpreterPath (if set) if no workspace is opened
109+
if (getUserDefaultInterpreter().globalValue && !this.workspaceService.workspaceFolders) {
110+
return undefined;
111+
}
112+
// --- End Positron ---
104113
traceLog(
105114
`VS Code was launched from an activated environment: '${path.basename(
106115
prefix,

extensions/positron-python/src/client/positron/discoverer.ts

+1-8
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { createPythonRuntimeMetadata } from './runtime';
1616
import { comparePythonVersionDescending } from '../interpreter/configuration/environmentTypeComparer';
1717
import { MINIMUM_PYTHON_VERSION } from '../common/constants';
1818
import { isVersionSupported, shouldIncludeInterpreter } from './interpreterSettings';
19+
import { hasFiles } from './util';
1920

2021
/**
2122
* Provides Python language runtime metadata to Positron; called during the
@@ -151,11 +152,3 @@ function sortInterpreters(
151152
});
152153
return copy;
153154
}
154-
155-
// Check if the current workspace contains files matching any of the passed glob ptaterns
156-
async function hasFiles(includes: string[]): Promise<boolean> {
157-
// Create a single glob pattern e.g. ['a', 'b'] => '{a,b}'
158-
const include = `{${includes.join(',')}}`;
159-
// Exclude node_modules for performance reasons
160-
return (await vscode.workspace.findFiles(include, '**/node_modules/**', 1)).length > 0;
161-
}

extensions/positron-python/src/client/positron/interpreterSettings.ts

+33-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import path from 'path';
7-
87
import { traceInfo, traceVerbose } from '../logging';
98
import { getConfiguration } from '../common/vscodeApis/workspaceApis';
109
import { arePathsSame, isParentPath } from '../pythonEnvironments/common/externalDependencies';
@@ -16,6 +15,7 @@ import {
1615
import { untildify } from '../common/helpers';
1716
import { PythonEnvironment } from '../pythonEnvironments/info';
1817
import { PythonVersion } from '../pythonEnvironments/info/pythonVersion';
18+
import { Resource, InspectInterpreterSettingType } from '../common/types';
1919
import { comparePythonVersionDescending } from '../interpreter/configuration/environmentTypeComparer';
2020

2121
/**
@@ -209,3 +209,35 @@ export function printInterpreterDebugInfo(interpreters: PythonEnvironment[]): vo
209209
traceInfo('================ [END] PYTHON INTERPRETER DEBUG INFO ================');
210210
traceInfo('=====================================================================');
211211
}
212+
213+
/**
214+
* Retrieves the user's default Python interpreter path from VS Code settings
215+
*
216+
* @returns The configured Python interpreter path if it exists and is not 'python',
217+
* otherwise returns an empty string
218+
*/
219+
export function getUserDefaultInterpreter(scope?: Resource): InspectInterpreterSettingType {
220+
const configuration = getConfiguration('python', scope);
221+
const defaultInterpreterPath: InspectInterpreterSettingType =
222+
configuration?.inspect<string>('defaultInterpreterPath') ?? {};
223+
224+
const processPath = (value: string | undefined): string => {
225+
// 'python' is the default for this setting. we only want to know if it has changed
226+
if (value === 'python') {
227+
return '';
228+
}
229+
if (value) {
230+
if (!path.isAbsolute(value)) {
231+
traceInfo(`[getUserDefaultInterpreter]: interpreter path ${value} is not absolute...ignoring`);
232+
return '';
233+
}
234+
return value;
235+
}
236+
return value ?? '';
237+
};
238+
239+
defaultInterpreterPath.globalValue = processPath(defaultInterpreterPath.globalValue);
240+
defaultInterpreterPath.workspaceValue = processPath(defaultInterpreterPath.workspaceValue);
241+
defaultInterpreterPath.workspaceFolderValue = processPath(defaultInterpreterPath.workspaceFolderValue);
242+
return defaultInterpreterPath;
243+
}

extensions/positron-python/src/client/positron/manager.ts

+55-3
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ import { EXTENSION_ROOT_DIR, MINIMUM_PYTHON_VERSION } from '../common/constants'
2525
import { JupyterKernelSpec } from '../positron-supervisor.d';
2626
import { IEnvironmentVariablesProvider } from '../common/variables/types';
2727
import { checkAndInstallPython } from './extension';
28-
import { shouldIncludeInterpreter, isVersionSupported } from './interpreterSettings';
28+
import { shouldIncludeInterpreter, isVersionSupported, getUserDefaultInterpreter } from './interpreterSettings';
2929
import { parseVersion, toSemverLikeVersion } from '../pythonEnvironments/base/info/pythonVersion';
3030
import { PythonVersion } from '../pythonEnvironments/info/pythonVersion';
31+
import { hasFiles } from './util';
3132

3233
export const IPythonRuntimeManager = Symbol('IPythonRuntimeManager');
3334

@@ -97,12 +98,63 @@ export class PythonRuntimeManager implements IPythonRuntimeManager {
9798
return this.discoverPythonRuntimes();
9899
}
99100

101+
/**
102+
* Get the recommended Python interpreter path for the workspace.
103+
* Returns an object with the path and whether it should be immediately selected.
104+
*/
105+
private async recommendedWorkspaceInterpreterPath(
106+
workspaceUri: vscode.Uri | undefined,
107+
): Promise<{ path: string | undefined; isImmediate: boolean }> {
108+
const userInterpreterSettings = getUserDefaultInterpreter(workspaceUri);
109+
let interpreterPath: string | undefined;
110+
let isImmediate = false;
111+
112+
if (!workspaceUri) {
113+
if (userInterpreterSettings.globalValue) {
114+
interpreterPath = userInterpreterSettings.globalValue;
115+
} else {
116+
return { path: undefined, isImmediate };
117+
}
118+
} else if (await hasFiles(['.venv/**/*'])) {
119+
interpreterPath = path.join(workspaceUri.fsPath, '.venv', 'bin', 'python');
120+
isImmediate = true;
121+
} else if (await hasFiles(['.conda/**/*'])) {
122+
interpreterPath = path.join(workspaceUri.fsPath, '.conda', 'bin', 'python');
123+
isImmediate = true;
124+
} else if (await hasFiles(['*/bin/python'])) {
125+
// if we found */bin/python but not .venv or .conda, use the first one we find
126+
const files = await vscode.workspace.findFiles('*/bin/python', '**/node_modules/**');
127+
if (files.length > 0) {
128+
interpreterPath = files[0].fsPath;
129+
isImmediate = true;
130+
}
131+
} else {
132+
interpreterPath =
133+
userInterpreterSettings.workspaceValue ||
134+
userInterpreterSettings.workspaceFolderValue ||
135+
userInterpreterSettings.globalValue;
136+
}
137+
138+
return { path: interpreterPath, isImmediate };
139+
}
140+
100141
/**
101142
* Recommend a Python language runtime based on the workspace.
102143
*/
103144
async recommendedWorkspaceRuntime(): Promise<positron.LanguageRuntimeMetadata | undefined> {
104-
// TODO: This is where we could recommend a runtime based on the
105-
// workspace, e.g. if it contains a virtualenv
145+
// TODO: may need other handling for multiroot workspaces
146+
const workspaceUri = vscode.workspace.workspaceFolders?.[0]?.uri;
147+
const { path: interpreterPath, isImmediate } = await this.recommendedWorkspaceInterpreterPath(workspaceUri);
148+
149+
if (interpreterPath) {
150+
const interpreter = await this.interpreterService.getInterpreterDetails(interpreterPath, workspaceUri);
151+
if (interpreter) {
152+
const metadata = await createPythonRuntimeMetadata(interpreter, this.serviceContainer, isImmediate);
153+
traceInfo(`Recommended runtime for workspace: ${interpreter.path}`);
154+
return metadata;
155+
}
156+
}
157+
traceInfo('No recommended workspace runtime found.');
106158
return undefined;
107159
}
108160

extensions/positron-python/src/client/positron/util.ts

+16
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import * as vscode from 'vscode';
7+
import { traceVerbose } from '../logging';
8+
69
export class PromiseHandles<T> {
710
resolve!: (value: T | Promise<T>) => void;
811

@@ -26,3 +29,16 @@ export async function whenTimeout<T>(ms: number, fn: () => T): Promise<T> {
2629
await delay(ms);
2730
return fn();
2831
}
32+
33+
// Check if the current workspace contains files matching any of the passed glob ptaterns
34+
export async function hasFiles(includes: string[]): Promise<boolean> {
35+
// Create a single glob pattern e.g. ['a', 'b'] => '{a,b}'
36+
const include = `{${includes.join(',')}}`;
37+
traceVerbose(`Searching for _files_ with pattern: ${include}`);
38+
39+
// Exclude node_modules for performance reasons
40+
const files = await vscode.workspace.findFiles(include, '**/node_modules/**', 1);
41+
traceVerbose(`Found _files_: ${files.map((file) => file.fsPath)}`);
42+
43+
return files.length > 0;
44+
}

extensions/positron-python/src/test/common/interpreterPathService.unit.test.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,10 @@ suite('Interpreter Path Service', async () => {
9595
workspaceConfig
9696
.setup((w) => w.update('defaultInterpreterPath', interpreterPath, true))
9797
.returns(() => Promise.resolve())
98-
.verifiable(TypeMoq.Times.once());
98+
/// --- Start Positron ---
99+
// Do not reset defaultInterpreterPath setting unless the user does
100+
.verifiable(TypeMoq.Times.never());
101+
// --- End Positron ---
99102

100103
await interpreterPathService.update(resource, ConfigurationTarget.Global, interpreterPath);
101104

0 commit comments

Comments
 (0)