Skip to content

Commit bd942a0

Browse files
author
Eric Snow
authored
Run all extension-internal uses of Python "isolated". (#10943)
For #10681. This change updates all other internal uses of Python in the extension to run "isolated".
1 parent 10f8a6b commit bd942a0

22 files changed

Lines changed: 970 additions & 789 deletions

news/3 Code Health/10681.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Run internal modules and scripts in isolated manner.
2+
3+
This helps avoid problems like shadowing stdlib modules.

pythonFiles/pyvsc-run-isolated.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313
del sys.path[0]
1414
del sys.argv[0]
1515
module = sys.argv[0]
16-
if module.startswith("-"):
16+
if module == "-c":
17+
ns = {}
18+
for code in sys.argv[1:]:
19+
exec(code, ns, ns)
20+
elif module.startswith("-"):
1721
raise NotImplementedError(sys.argv)
1822
elif module.endswith(".py"):
1923
runpy.run_path(module, run_name="__main__")

src/client/common/configSettings.ts

Lines changed: 706 additions & 704 deletions
Large diffs are not rendered by default.

src/client/common/installer/moduleInstaller.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { IApplicationShell } from '../application/types';
1212
import { wrapCancellationTokens } from '../cancellation';
1313
import { STANDARD_OUTPUT_CHANNEL } from '../constants';
1414
import { IFileSystem } from '../platform/types';
15+
import * as internalPython from '../process/internal/python';
1516
import { ITerminalServiceFactory } from '../terminal/types';
1617
import { ExecutionInfo, IConfigurationService, IOutputChannel } from '../types';
1718
import { Products } from '../utils/localize';
@@ -38,13 +39,13 @@ export abstract class ModuleInstaller implements IModuleInstaller {
3839
if (executionInfo.moduleName) {
3940
const configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
4041
const settings = configService.getSettings(uri);
41-
const args = ['-m', executionInfo.moduleName].concat(executionInfoArgs);
4242

4343
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
4444
const interpreter = isResource(resource)
4545
? await interpreterService.getActiveInterpreter(resource)
4646
: resource;
4747
const pythonPath = isResource(resource) ? settings.pythonPath : resource.path;
48+
const args = internalPython.execModule(executionInfo.moduleName, executionInfoArgs);
4849
if (!interpreter || interpreter.type !== InterpreterType.Unknown) {
4950
await terminalService.sendCommand(pythonPath, args, token);
5051
} else if (settings.globalModuleInstallation) {
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { _ISOLATED as ISOLATED } from './scripts';
5+
6+
// "python" contains functions corresponding to the various ways that
7+
// the extension invokes a Python executable internally. Each function
8+
// takes arguments relevant to the specific use case. However, each
9+
// always *returns* a list of strings for the commandline arguments that
10+
// should be used when invoking the Python executable for the specific
11+
// use case, whether through spawn/exec or a terminal.
12+
//
13+
// Where relevant (nearly always), the function also returns a "parse"
14+
// function that may be used to deserialize the stdout of the command
15+
// into the corresponding object or objects. "parse()" takes a single
16+
// string as the stdout text and returns the relevant data.
17+
18+
export function execCode(code: string, isolated = true): string[] {
19+
const args = ['-c', code];
20+
if (isolated) {
21+
args.splice(0, 0, ISOLATED);
22+
}
23+
// "code" isn't specific enough to know how to parse it,
24+
// so we only return the args.
25+
return args;
26+
}
27+
28+
export function execModule(name: string, moduleArgs: string[], isolated = true): string[] {
29+
const args = ['-m', name, ...moduleArgs];
30+
if (isolated) {
31+
args[0] = ISOLATED; // replace
32+
}
33+
// "code" isn't specific enough to know how to parse it,
34+
// so we only return the args.
35+
return args;
36+
}
37+
38+
export function getVersion(): [string[], (out: string) => string] {
39+
// There is no need to isolate this.
40+
const args = ['--version'];
41+
42+
function parse(out: string): string {
43+
return out.trim();
44+
}
45+
46+
return [args, parse];
47+
}
48+
49+
export function getSysPrefix(): [string[], (out: string) => string] {
50+
const args = [ISOLATED, '-c', 'import sys;print(sys.prefix)'];
51+
52+
function parse(out: string): string {
53+
return out.trim();
54+
}
55+
56+
return [args, parse];
57+
}
58+
59+
export function getExecutable(): [string[], (out: string) => string] {
60+
const args = [ISOLATED, '-c', 'import sys;print(sys.executable)'];
61+
62+
function parse(out: string): string {
63+
return out.trim();
64+
}
65+
66+
return [args, parse];
67+
}
68+
69+
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+
];
78+
79+
function parse(out: string): string {
80+
return out.trim();
81+
}
82+
83+
return [args, parse];
84+
}
85+
86+
export function getUserSitePackages(): [string[], (out: string) => string] {
87+
const args = [ISOLATED, 'site', '--user-site'];
88+
89+
function parse(out: string): string {
90+
return out.trim();
91+
}
92+
93+
return [args, parse];
94+
}
95+
96+
export function isValid(): [string[], (out: string) => boolean] {
97+
// There is no need to isolate this.
98+
const args = ['-c', 'print(1234)'];
99+
100+
function parse(out: string): boolean {
101+
return out.startsWith('1234');
102+
}
103+
104+
return [args, parse];
105+
}
106+
107+
export function isModuleInstalled(name: string): [string[], (out: string) => boolean] {
108+
const args = [ISOLATED, '-c', `import ${name}`];
109+
110+
function parse(_out: string): boolean {
111+
// If the command did not fail then the module is installed.
112+
return true;
113+
}
114+
115+
return [args, parse];
116+
}
117+
118+
export function getModuleVersion(name: string): [string[], (out: string) => string] {
119+
const args = [ISOLATED, name, '--version'];
120+
121+
function parse(out: string): string {
122+
return out.trim();
123+
}
124+
125+
return [args, parse];
126+
}

src/client/common/process/pythonDaemonPool.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { IDisposableRegistry } from '../types';
1717
import { createDeferred, sleep } from '../utils/async';
1818
import { noop } from '../utils/misc';
1919
import { StopWatch } from '../utils/stopWatch';
20+
import * as internalPython from './internal/python';
2021
import { ProcessService } from './proc';
2122
import { PythonDaemonExecutionService } from './pythonDaemon';
2223
import {
@@ -101,7 +102,8 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
101102
return this.pythonExecutionService.getExecutionInfo(pythonArgs);
102103
}
103104
public async isModuleInstalled(moduleName: string): Promise<boolean> {
104-
const msg = { args: ['-m', moduleName] };
105+
const args = internalPython.execModule(moduleName, []);
106+
const msg = { args };
105107
return this.wrapCall((daemon) => daemon.isModuleInstalled(moduleName), msg);
106108
}
107109
public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
@@ -110,10 +112,11 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
110112
}
111113
public async execModule(
112114
moduleName: string,
113-
args: string[],
115+
moduleArgs: string[],
114116
options: SpawnOptions
115117
): Promise<ExecutionResult<string>> {
116-
const msg = { args: ['-m', moduleName].concat(args), options };
118+
const args = internalPython.execModule(moduleName, moduleArgs);
119+
const msg = { args, options };
117120
return this.wrapCall((daemon) => daemon.execModule(moduleName, args, options), msg);
118121
}
119122
public execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
@@ -122,10 +125,11 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
122125
}
123126
public execModuleObservable(
124127
moduleName: string,
125-
args: string[],
128+
moduleArgs: string[],
126129
options: SpawnOptions
127130
): ObservableExecutionResult<string> {
128-
const msg = { args: ['-m', moduleName].concat(args), options };
131+
const args = internalPython.execModule(moduleName, moduleArgs);
132+
const msg = { args, options };
129133
return this.wrapObservableCall((daemon) => daemon.execModuleObservable(moduleName, args, options), msg);
130134
}
131135
/**

src/client/common/process/pythonEnvironment.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { traceError, traceInfo } from '../logger';
66
import { IFileSystem } from '../platform/types';
77
import { Architecture } from '../utils/platform';
88
import { parsePythonVersion } from '../utils/version';
9+
import * as internalPython from './internal/python';
910
import * as internalScripts from './internal/scripts';
1011
import {
1112
ExecutionResult,
@@ -61,15 +62,18 @@ class PythonEnvironment {
6162
return this.pythonPath;
6263
}
6364

64-
const { command, args } = this.getExecutionInfo(['-c', 'import sys;print(sys.executable)']);
65-
const proc = await this.deps.exec(command, args);
66-
return proc.stdout.trim();
65+
const [args, parse] = internalPython.getExecutable();
66+
const info = this.getExecutionInfo(args);
67+
const proc = await this.deps.exec(info.command, info.args);
68+
return parse(proc.stdout);
6769
}
6870

6971
public async isModuleInstalled(moduleName: string): Promise<boolean> {
70-
const { command, args } = this.getExecutionInfo(['-c', `import ${moduleName}`]);
72+
// prettier-ignore
73+
const [args,] = internalPython.isModuleInstalled(moduleName);
74+
const info = this.getExecutionInfo(args);
7175
try {
72-
await this.deps.exec(command, args);
76+
await this.deps.exec(info.command, info.args);
7377
} catch {
7478
return false;
7579
}

src/client/common/process/pythonProcess.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { ErrorUtils } from '../errors/errorUtils';
55
import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
6+
import * as internalPython from './internal/python';
67
import {
78
ExecutionResult,
89
IProcessService,
@@ -33,11 +34,12 @@ class PythonProcessService {
3334

3435
public execModuleObservable(
3536
moduleName: string,
36-
args: string[],
37+
moduleArgs: string[],
3738
options: SpawnOptions
3839
): ObservableExecutionResult<string> {
40+
const args = internalPython.execModule(moduleName, moduleArgs);
3941
const opts: SpawnOptions = { ...options };
40-
const executable = this.deps.getExecutionObservableInfo(['-m', moduleName, ...args]);
42+
const executable = this.deps.getExecutionObservableInfo(args);
4143
return this.deps.execObservable(executable.command, executable.args, opts);
4244
}
4345

@@ -49,11 +51,12 @@ class PythonProcessService {
4951

5052
public async execModule(
5153
moduleName: string,
52-
args: string[],
54+
moduleArgs: string[],
5355
options: SpawnOptions
5456
): Promise<ExecutionResult<string>> {
57+
const args = internalPython.execModule(moduleName, moduleArgs);
5558
const opts: SpawnOptions = { ...options };
56-
const executable = this.deps.getExecutionInfo(['-m', moduleName, ...args]);
59+
const executable = this.deps.getExecutionInfo(args);
5760
const result = await this.deps.exec(executable.command, executable.args, opts);
5861

5962
// If a module is not installed we'll have something in stderr.

src/client/interpreter/display/shebangCodeLensProvider.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { inject, injectable } from 'inversify';
22
import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri } from 'vscode';
33
import { IWorkspaceService } from '../../common/application/types';
44
import { IPlatformService } from '../../common/platform/types';
5+
import * as internalPython from '../../common/process/internal/python';
56
import { IProcessServiceFactory } from '../../common/process/types';
67
import { IConfigurationService } from '../../common/types';
78
import { IShebangCodeLensProvider } from '../contracts';
@@ -37,20 +38,20 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
3738
}
3839
private async getFullyQualifiedPathToInterpreter(pythonPath: string, resource: Uri) {
3940
let cmdFile = pythonPath;
40-
let args = ['-c', 'import sys;print(sys.executable)'];
41+
const [args, parse] = internalPython.getExecutable();
4142
if (pythonPath.indexOf('bin/env ') >= 0 && !this.platformService.isWindows) {
4243
// In case we have pythonPath as '/usr/bin/env python'.
4344
const parts = pythonPath
4445
.split(' ')
4546
.map((part) => part.trim())
4647
.filter((part) => part.length > 0);
4748
cmdFile = parts.shift()!;
48-
args = parts.concat(args);
49+
args.splice(0, 0, ...parts);
4950
}
5051
const processService = await this.processServiceFactory.create(resource);
5152
return processService
5253
.exec(cmdFile, args)
53-
.then((output) => output.stdout.trim())
54+
.then((output) => parse(output.stdout))
5455
.catch(() => '');
5556
}
5657
private async createShebangCodeLens(document: TextDocument) {

src/client/interpreter/interpreterVersion.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { inject, injectable } from 'inversify';
22
import '../common/extensions';
3+
import * as internalPython from '../common/process/internal/python';
34
import { IProcessServiceFactory } from '../common/process/types';
45
import { IInterpreterVersionService } from './contracts';
56

@@ -9,21 +10,24 @@ export const PIP_VERSION_REGEX = '\\d+\\.\\d+(\\.\\d+)?';
910
export class InterpreterVersionService implements IInterpreterVersionService {
1011
constructor(@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory) {}
1112
public async getVersion(pythonPath: string, defaultValue: string): Promise<string> {
13+
const [args, parse] = internalPython.getVersion();
1214
const processService = await this.processServiceFactory.create();
1315
return processService
14-
.exec(pythonPath, ['--version'], { mergeStdOutErr: true })
15-
.then((output) => output.stdout.splitLines()[0])
16+
.exec(pythonPath, args, { mergeStdOutErr: true })
17+
.then((output) => parse(output.stdout).splitLines()[0])
1618
.then((version) => (version.length === 0 ? defaultValue : version))
1719
.catch(() => defaultValue);
1820
}
1921
public async getPipVersion(pythonPath: string): Promise<string> {
22+
const [args, parse] = internalPython.getModuleVersion('pip');
2023
const processService = await this.processServiceFactory.create();
21-
const output = await processService.exec(pythonPath, ['-m', 'pip', '--version'], { mergeStdOutErr: true });
22-
if (output.stdout.length > 0) {
24+
const output = await processService.exec(pythonPath, args, { mergeStdOutErr: true });
25+
const version = parse(output.stdout);
26+
if (version.length > 0) {
2327
// Here's a sample output:
2428
// pip 9.0.1 from /Users/donjayamanne/anaconda3/lib/python3.6/site-packages (python 3.6).
2529
const re = new RegExp(PIP_VERSION_REGEX, 'g');
26-
const matches = re.exec(output.stdout);
30+
const matches = re.exec(version);
2731
if (matches && matches.length > 0) {
2832
return matches[0].trim();
2933
}

0 commit comments

Comments
 (0)