Skip to content

Commit 5419388

Browse files
authored
Merging isolate branch changes to main (#14966)
* Add setting to control isolation (#14845) * Add placeholder setting field to control isolation * Set field value from settings. * Update tests * Change description text for useIsolation * Check useIsolation setting when using internal scripts (#14952) * Check useIsolation setting when using internal scripts * fix lint * handle case when python settings are not available * Use isolation setting with custom python code * Address comments
1 parent 73062ee commit 5419388

File tree

6 files changed

+55
-33
lines changed

6 files changed

+55
-33
lines changed

package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,6 +1772,12 @@
17721772
"description": "Enable auto run test discovery when saving a test file.",
17731773
"scope": "resource"
17741774
},
1775+
"python.useIsolation": {
1776+
"type": "boolean",
1777+
"default": true,
1778+
"description": "Execute tools in isolation from the workspace.",
1779+
"scope": "machine"
1780+
},
17751781
"python.venvFolders": {
17761782
"type": "array",
17771783
"default": [],

src/client/common/configSettings.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ export class PythonSettings implements IPythonSettings {
113113
public experiments!: IExperiments;
114114
public languageServer: LanguageServerType = LanguageServerType.Microsoft;
115115
public logging: ILoggingSettings = { level: LogLevel.Error };
116+
public useIsolation: boolean = true;
116117

117118
protected readonly changed = new EventEmitter<void>();
118119
private workspaceRoot: Resource;
@@ -244,6 +245,8 @@ export class PythonSettings implements IPythonSettings {
244245
pythonSettings.get<boolean>('autoUpdateLanguageServer', true)
245246
)!;
246247

248+
this.useIsolation = systemVariables.resolveAny(pythonSettings.get<boolean>('useIsolation', true))!;
249+
247250
let ls = pythonSettings.get<LanguageServerType>('languageServer') ?? LanguageServerType.Jedi;
248251
ls = systemVariables.resolveAny(ls);
249252
if (!Object.values(LanguageServerType).includes(ls)) {

src/client/common/process/internal/python.ts

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { _ISOLATED as ISOLATED } from './scripts';
4+
import { _ISOLATED as ISOLATED, getUseIsolationSetting, maybeIsolated } from './scripts';
55

66
// "python" contains functions corresponding to the various ways that
77
// the extension invokes a Python executable internally. Each function
@@ -16,9 +16,9 @@ import { _ISOLATED as ISOLATED } from './scripts';
1616
// string as the stdout text and returns the relevant data.
1717

1818
export function execCode(code: string, isolated = true): string[] {
19-
const args = ['-c', code];
19+
let args = ['-c', code];
2020
if (isolated) {
21-
args.splice(0, 0, ISOLATED);
21+
args = maybeIsolated(args);
2222
}
2323
// "code" isn't specific enough to know how to parse it,
2424
// so we only return the args.
@@ -27,7 +27,7 @@ export function execCode(code: string, isolated = true): string[] {
2727

2828
export function execModule(name: string, moduleArgs: string[], isolated = true): string[] {
2929
const args = ['-m', name, ...moduleArgs];
30-
if (isolated) {
30+
if (isolated && getUseIsolationSetting()) {
3131
args[0] = ISOLATED; // replace
3232
}
3333
// "code" isn't specific enough to know how to parse it,
@@ -47,7 +47,7 @@ export function getVersion(): [string[], (out: string) => string] {
4747
}
4848

4949
export function getSysPrefix(): [string[], (out: string) => string] {
50-
const args = [ISOLATED, '-c', 'import sys;print(sys.prefix)'];
50+
const args = maybeIsolated(['-c', 'import sys;print(sys.prefix)']);
5151

5252
function parse(out: string): string {
5353
return out.trim();
@@ -57,7 +57,7 @@ export function getSysPrefix(): [string[], (out: string) => string] {
5757
}
5858

5959
export function getExecutable(): [string[], (out: string) => string] {
60-
const args = [ISOLATED, '-c', 'import sys;print(sys.executable)'];
60+
const args = maybeIsolated(['-c', 'import sys;print(sys.executable)']);
6161

6262
function parse(out: string): string {
6363
return out.trim();
@@ -67,14 +67,10 @@ export function getExecutable(): [string[], (out: string) => string] {
6767
}
6868

6969
export function getSitePackages(): [string[], (out: string) => string] {
70-
const args = [
71-
ISOLATED,
72-
'-c',
73-
// On windows we also need the libs path (second item will
74-
// return c:\xxx\lib\site-packages). This is returned by
75-
// the following:
76-
'from distutils.sysconfig import get_python_lib; print(get_python_lib())'
77-
];
70+
// On windows we also need the libs path (second item will
71+
// return c:\xxx\lib\site-packages). This is returned by
72+
// the following: get_python_lib
73+
const args = maybeIsolated(['-c', 'from distutils.sysconfig import get_python_lib; print(get_python_lib())']);
7874

7975
function parse(out: string): string {
8076
return out.trim();
@@ -84,7 +80,7 @@ export function getSitePackages(): [string[], (out: string) => string] {
8480
}
8581

8682
export function getUserSitePackages(): [string[], (out: string) => string] {
87-
const args = [ISOLATED, 'site', '--user-site'];
83+
const args = maybeIsolated(['site', '--user-site']);
8884

8985
function parse(out: string): string {
9086
return out.trim();
@@ -105,7 +101,7 @@ export function isValid(): [string[], (out: string) => boolean] {
105101
}
106102

107103
export function isModuleInstalled(name: string): [string[], (out: string) => boolean] {
108-
const args = [ISOLATED, '-c', `import ${name}`];
104+
const args = maybeIsolated(['-c', `import ${name}`]);
109105

110106
function parse(_out: string): boolean {
111107
// If the command did not fail then the module is installed.
@@ -116,7 +112,7 @@ export function isModuleInstalled(name: string): [string[], (out: string) => boo
116112
}
117113

118114
export function getModuleVersion(name: string): [string[], (out: string) => string] {
119-
const args = [ISOLATED, name, '--version'];
115+
const args = maybeIsolated([name, '--version']);
120116

121117
function parse(out: string): string {
122118
return out.trim();

src/client/common/process/internal/scripts/index.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
import * as path from 'path';
5+
import { workspace } from 'vscode';
56
import { EXTENSION_ROOT_DIR } from '../../../constants';
67

78
// It is simpler to hard-code it instead of using vscode.ExtensionContext.extensionPath.
@@ -10,6 +11,22 @@ const SCRIPTS_DIR = _SCRIPTS_DIR;
1011
export const _ISOLATED = path.join(_SCRIPTS_DIR, 'pyvsc-run-isolated.py');
1112
const ISOLATED = _ISOLATED;
1213

14+
export function getUseIsolationSetting(): boolean {
15+
try {
16+
return workspace.getConfiguration('python').get<boolean>('useIsolation', true);
17+
} catch (ex) {
18+
// If we can't get the setting for any reason we assume default
19+
return true;
20+
}
21+
}
22+
23+
export function maybeIsolated(args: string[]): string[] {
24+
if (getUseIsolationSetting()) {
25+
args.splice(0, 0, ISOLATED);
26+
}
27+
return args;
28+
}
29+
1330
// "scripts" contains everything relevant to the scripts found under
1431
// the top-level "pythonFiles" directory. Each of those scripts has
1532
// a function in this module which matches the script's filename.
@@ -49,7 +66,7 @@ export type InterpreterInfoJson = {
4966

5067
export function interpreterInfo(): [string[], (out: string) => InterpreterInfoJson] {
5168
const script = path.join(SCRIPTS_DIR, 'interpreterInfo.py');
52-
const args = [ISOLATED, script];
69+
const args = maybeIsolated([script]);
5370

5471
function parse(out: string): InterpreterInfoJson {
5572
let json: InterpreterInfoJson;
@@ -159,7 +176,7 @@ namespace _completion {
159176

160177
export function completion(jediPath?: string): [string[], (out: string) => _completion.Response[]] {
161178
const script = path.join(SCRIPTS_DIR, 'completion.py');
162-
const args = [ISOLATED, script];
179+
const args = maybeIsolated([script]);
163180
if (jediPath) {
164181
args.push('custom');
165182
args.push(jediPath);
@@ -177,7 +194,7 @@ export function completion(jediPath?: string): [string[], (out: string) => _comp
177194

178195
export function sortImports(filename: string, sortArgs?: string[]): [string[], (out: string) => string] {
179196
const script = path.join(SCRIPTS_DIR, 'sortImports.py');
180-
const args = [ISOLATED, script, filename, '--diff'];
197+
const args = maybeIsolated([script, filename, '--diff']);
181198
if (sortArgs) {
182199
args.push(...sortArgs);
183200
}
@@ -195,7 +212,7 @@ export function sortImports(filename: string, sortArgs?: string[]): [string[], (
195212

196213
export function refactor(root: string): [string[], (out: string) => object[]] {
197214
const script = path.join(SCRIPTS_DIR, 'refactor.py');
198-
const args = [ISOLATED, script, root];
215+
const args = maybeIsolated([script, root]);
199216

200217
// tslint:disable-next-line:no-suspicious-comment
201218
// TODO: Make the return type more specific, like we did
@@ -217,7 +234,7 @@ export function refactor(root: string): [string[], (out: string) => object[]] {
217234

218235
export function normalizeForInterpreter(): [string[], (out: string) => string] {
219236
const script = path.join(SCRIPTS_DIR, 'normalizeForInterpreter.py');
220-
const args = [ISOLATED, script];
237+
const args = maybeIsolated([script]);
221238

222239
function parse(out: string) {
223240
// The text will be used as-is.
@@ -232,7 +249,7 @@ export function normalizeForInterpreter(): [string[], (out: string) => string] {
232249

233250
export function normalizeSelection(): [string[], (out: string) => string] {
234251
const script = path.join(SCRIPTS_DIR, 'normalizeSelection.py');
235-
const args = [ISOLATED, script];
252+
const args = maybeIsolated([script]);
236253

237254
function parse(out: string) {
238255
// The text will be used as-is.
@@ -272,7 +289,7 @@ export function symbolProvider(
272289
text?: string
273290
): [string[], (out: string) => _symbolProvider.Symbols] {
274291
const script = path.join(SCRIPTS_DIR, 'symbolProvider.py');
275-
const args = [ISOLATED, script, filename];
292+
const args = maybeIsolated([script, filename]);
276293
if (text) {
277294
args.push(text);
278295
}
@@ -289,7 +306,7 @@ export function symbolProvider(
289306

290307
export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessEnv] {
291308
const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgument();
292-
const args = [ISOLATED, script];
309+
const args = maybeIsolated([script]);
293310

294311
function parse(out: string): NodeJS.ProcessEnv {
295312
return JSON.parse(out);
@@ -303,7 +320,7 @@ export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessE
303320

304321
export function printEnvVariablesToFile(filename: string): [string[], (out: string) => NodeJS.ProcessEnv] {
305322
const script = path.join(SCRIPTS_DIR, 'printEnvVariablesToFile.py');
306-
const args = [ISOLATED, script, filename.fileToCommandArgument()];
323+
const args = maybeIsolated([script, filename.fileToCommandArgument()]);
307324

308325
function parse(out: string): NodeJS.ProcessEnv {
309326
return JSON.parse(out);
@@ -319,15 +336,14 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[
319336
const script = path.join(SCRIPTS_DIR, 'shell_exec.py');
320337
// We don't bother with a "parse" function since the output
321338
// could be anything.
322-
return [
323-
ISOLATED,
339+
return maybeIsolated([
324340
script,
325341
command.fileToCommandArgument(),
326342
// The shell args must come after the command
327343
// but before the lockfile.
328344
...shellArgs,
329345
lockfile.fileToCommandArgument()
330-
];
346+
]);
331347
}
332348

333349
//============================
@@ -336,7 +352,7 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[
336352
export function testlauncher(testArgs: string[]): string[] {
337353
const script = path.join(SCRIPTS_DIR, 'testlauncher.py');
338354
// There is no output to parse, so we do not return a function.
339-
return [ISOLATED, script, ...testArgs];
355+
return maybeIsolated([script, ...testArgs]);
340356
}
341357

342358
//============================
@@ -353,5 +369,5 @@ export function visualstudio_py_testlauncher(testArgs: string[]): string[] {
353369

354370
export function tensorboardLauncher(args: string[]) {
355371
const script = path.join(SCRIPTS_DIR, 'tensorboard_launcher.py');
356-
return [ISOLATED, script, ...args];
372+
return maybeIsolated([script, ...args]);
357373
}

src/client/common/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ export interface IPythonSettings {
190190
readonly languageServer: LanguageServerType;
191191
readonly defaultInterpreterPath: string;
192192
readonly logging: ILoggingSettings;
193+
readonly useIsolation: boolean;
193194
}
194195
export interface ISortImportSettings {
195196
readonly path: string;

src/test/common/configSettings/configSettings.unit.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ suite('Python Settings', async () => {
8282
}
8383

8484
// boolean settings
85-
for (const name of ['downloadLanguageServer', 'autoUpdateLanguageServer']) {
85+
for (const name of ['downloadLanguageServer', 'autoUpdateLanguageServer', 'useIsolation']) {
8686
config
8787
.setup((c) => c.get<boolean>(name, true))
8888
// tslint:disable-next-line:no-any
@@ -148,7 +148,7 @@ suite('Python Settings', async () => {
148148
});
149149

150150
suite('Boolean settings', async () => {
151-
['downloadLanguageServer', 'autoUpdateLanguageServer', 'globalModuleInstallation'].forEach(
151+
['downloadLanguageServer', 'autoUpdateLanguageServer', 'globalModuleInstallation', 'useIsolation'].forEach(
152152
async (settingName) => {
153153
testIfValueIsUpdated(settingName, true);
154154
}

0 commit comments

Comments
 (0)