Skip to content

Commit aec4fb3

Browse files
DonJayamanneDonJayamanne
authored
Refactor extension to remove old way of spawning python processes (#439)
Fixes #354 Code refactoring to remove execPythonFile function and use the new execution layer
1 parent 9c3d9f7 commit aec4fb3

21 files changed

Lines changed: 418 additions & 975 deletions

File tree

‎src/client/common/configSettings.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { EventEmitter } from 'events';
55
import * as path from 'path';
66
import * as vscode from 'vscode';
77
import { Uri } from 'vscode';
8-
import { InterpreterInfoCache } from './interpreterInfoCache';
98
import { SystemVariables } from './variables/systemVariables';
109

1110
// tslint:disable-next-line:no-require-imports no-var-requires
@@ -188,12 +187,10 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
188187
// tslint:disable-next-line:no-unsafe-any
189188
this.disposables.forEach(disposable => disposable.dispose());
190189
this.disposables = [];
191-
InterpreterInfoCache.clear();
192190
}
193191

194192
// tslint:disable-next-line:cyclomatic-complexity max-func-body-length
195193
private initializeSettings() {
196-
InterpreterInfoCache.clear();
197194
const workspaceRoot = this.workspaceRoot.fsPath;
198195
const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
199196
const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot);

‎src/client/common/interpreterInfoCache.ts‎

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎src/client/common/utils.ts‎

Lines changed: 6 additions & 254 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@ import * as fs from 'fs';
88
import * as fsExtra from 'fs-extra';
99
import * as os from 'os';
1010
import * as path from 'path';
11-
import { CancellationToken, Range, TextDocument, Uri } from 'vscode';
11+
import { Position, Range, TextDocument, Uri } from 'vscode';
1212
import * as settings from './configSettings';
13-
import { mergeEnvVariables, parseEnvFile } from './envFileParser';
14-
import { isNotInstalledError } from './helpers';
15-
import { InterpreterInfoCache } from './interpreterInfoCache';
13+
import { parseEnvFile } from './envFileParser';
1614

1715
export const IS_WINDOWS = /^win/.test(process.platform);
1816
export const Is_64Bit = os.arch() === 'x64';
@@ -53,51 +51,6 @@ export function fsReaddirAsync(root: string): Promise<string[]> {
5351
});
5452
}
5553

56-
async function getPythonInterpreterDirectory(resource?: Uri): Promise<string> {
57-
const cache = InterpreterInfoCache.get(resource);
58-
const pythonFileName = settings.PythonSettings.getInstance(resource).pythonPath;
59-
60-
// If we already have it and the python path hasn't changed, yay
61-
if (cache.pythonInterpreterDirectory && cache.pythonInterpreterDirectory.length > 0
62-
&& cache.pythonSettingsPath === pythonFileName) {
63-
return cache.pythonInterpreterDirectory;
64-
}
65-
66-
// Check if we have the path
67-
if (path.basename(pythonFileName) === pythonFileName) {
68-
try {
69-
const pythonInterpreterPath = await getPathFromPythonCommand(pythonFileName);
70-
const pythonInterpreterDirectory = path.dirname(pythonInterpreterPath);
71-
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonInterpreterPath, pythonInterpreterDirectory);
72-
return pythonInterpreterDirectory;
73-
// tslint:disable-next-line:variable-name
74-
} catch (_ex) {
75-
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, '');
76-
return '';
77-
}
78-
}
79-
80-
return new Promise<string>(resolve => {
81-
// If we can execute the python, then get the path from the fully qualified name
82-
child_process.execFile(pythonFileName, ['-c', 'print(1234)'], (error, stdout, stderr) => {
83-
// Yes this is a valid python path
84-
if (stdout.startsWith('1234')) {
85-
const pythonInterpreterDirectory = path.dirname(pythonFileName);
86-
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, pythonInterpreterDirectory);
87-
resolve(pythonInterpreterDirectory);
88-
} else {
89-
// No idea, didn't work, hence don't reject, but return empty path
90-
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, '');
91-
resolve('');
92-
}
93-
});
94-
});
95-
}
96-
export async function getFullyQualifiedPythonInterpreterPath(resource?: Uri): Promise<string> {
97-
const pyDir = await getPythonInterpreterDirectory(resource);
98-
const cache = InterpreterInfoCache.get(resource);
99-
return cache.pythonInterpreterPath;
100-
}
10154
export async function getPathFromPythonCommand(pythonPath: string): Promise<string> {
10255
return await new Promise<string>((resolve, reject) => {
10356
child_process.execFile(pythonPath, ['-c', 'import sys;print(sys.executable)'], (_, stdout) => {
@@ -110,197 +63,6 @@ export async function getPathFromPythonCommand(pythonPath: string): Promise<stri
11063
});
11164
});
11265
}
113-
async function getEnvVariables(resource?: Uri): Promise<{}> {
114-
const cache = InterpreterInfoCache.get(resource);
115-
if (cache.customEnvVariables) {
116-
return cache.customEnvVariables;
117-
}
118-
119-
const pyPath = await getPythonInterpreterDirectory(resource);
120-
let customEnvVariables = await getCustomEnvVars(resource) || {};
121-
122-
if (pyPath.length > 0) {
123-
// Ensure to include the path of the current python.
124-
let newPath = '';
125-
const currentPath = typeof customEnvVariables[PATH_VARIABLE_NAME] === 'string' ? customEnvVariables[PATH_VARIABLE_NAME] : process.env[PATH_VARIABLE_NAME];
126-
if (IS_WINDOWS) {
127-
newPath = `${pyPath}\\${path.delimiter}${path.join(pyPath, 'Scripts\\')}${path.delimiter}${currentPath}`;
128-
// This needs to be done for windows.
129-
process.env[PATH_VARIABLE_NAME] = newPath;
130-
} else {
131-
newPath = `${pyPath}${path.delimiter}${currentPath}`;
132-
}
133-
customEnvVariables = mergeEnvVariables(customEnvVariables, process.env);
134-
customEnvVariables[PATH_VARIABLE_NAME] = newPath;
135-
}
136-
137-
InterpreterInfoCache.setCustomEnvVariables(resource, customEnvVariables);
138-
return customEnvVariables;
139-
}
140-
export async function execPythonFile(resource: string | Uri | undefined, file: string, args: string[], cwd: string, includeErrorAsResponse: boolean = false, stdOut: (line: string) => void = null, token?: CancellationToken): Promise<string> {
141-
const resourceUri = typeof resource === 'string' ? Uri.file(resource) : resource;
142-
const env = await getEnvVariables(resourceUri);
143-
const options = { cwd, env };
144-
145-
if (stdOut) {
146-
return spawnFileInternal(file, args, options, includeErrorAsResponse, stdOut, token);
147-
}
148-
149-
const fileIsPythonInterpreter = (file.toUpperCase() === 'PYTHON' || file === settings.PythonSettings.getInstance(resourceUri).pythonPath);
150-
const execAsModule = fileIsPythonInterpreter && args.length > 0 && args[0] === '-m';
151-
152-
if (execAsModule) {
153-
return getFullyQualifiedPythonInterpreterPath(resourceUri)
154-
.then(p => execPythonModule(p, args, options, includeErrorAsResponse, token));
155-
}
156-
return execFileInternal(file, args, options, includeErrorAsResponse, token);
157-
}
158-
159-
function handleResponse(file: string, includeErrorAsResponse: boolean, error: Error, stdout: string, stderr: string, token?: CancellationToken): Promise<string> {
160-
if (token && token.isCancellationRequested) {
161-
return Promise.resolve(undefined);
162-
}
163-
if (isNotInstalledError(error)) {
164-
return Promise.reject(error);
165-
}
166-
167-
// pylint:
168-
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
169-
// These error messages are useless when using pylint
170-
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
171-
return Promise.resolve(stdout + '\n' + stderr);
172-
}
173-
174-
let hasErrors = (error && error.message.length > 0) || (stderr && stderr.length > 0);
175-
if (hasErrors && (typeof stdout !== 'string' || stdout.length === 0)) {
176-
let errorMsg = (error && error.message) ? error.message : (stderr && stderr.length > 0 ? stderr + '' : '');
177-
return Promise.reject(errorMsg);
178-
}
179-
else {
180-
return Promise.resolve(stdout + '');
181-
}
182-
}
183-
function handlePythonModuleResponse(includeErrorAsResponse: boolean, error: Error, stdout: string, stderr: string, token?: CancellationToken): Promise<string> {
184-
if (token && token.isCancellationRequested) {
185-
return Promise.resolve(undefined);
186-
}
187-
if (isNotInstalledError(error)) {
188-
return Promise.reject(error);
189-
}
190-
191-
// pylint:
192-
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
193-
// These error messages are useless when using pylint
194-
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
195-
return Promise.resolve(stdout + '\n' + stderr);
196-
}
197-
if (!includeErrorAsResponse && stderr.length > 0) {
198-
return Promise.reject(stderr);
199-
}
200-
201-
return Promise.resolve(stdout + '');
202-
}
203-
function execPythonModule(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, token?: CancellationToken): Promise<string> {
204-
options.maxBuffer = options.maxBuffer ? options.maxBuffer : 1024 * 102400;
205-
return new Promise<string>((resolve, reject) => {
206-
let proc = child_process.execFile(file, args, options, (error, stdout, stderr) => {
207-
handlePythonModuleResponse(includeErrorAsResponse, error, stdout, stderr, token)
208-
.then(resolve)
209-
.catch(reject);
210-
});
211-
if (token && token.onCancellationRequested) {
212-
token.onCancellationRequested(() => {
213-
if (proc) {
214-
proc.kill();
215-
proc = null;
216-
}
217-
});
218-
}
219-
});
220-
}
221-
222-
function execFileInternal(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, token?: CancellationToken): Promise<string> {
223-
options.maxBuffer = options.maxBuffer ? options.maxBuffer : 1024 * 102400;
224-
return new Promise<string>((resolve, reject) => {
225-
let proc = child_process.execFile(file, args, options, (error, stdout, stderr) => {
226-
handleResponse(file, includeErrorAsResponse, error, stdout, stderr, token)
227-
.then(data => resolve(data))
228-
.catch(err => reject(err));
229-
});
230-
if (token && token.onCancellationRequested) {
231-
token.onCancellationRequested(() => {
232-
if (proc) {
233-
proc.kill();
234-
proc = null;
235-
}
236-
});
237-
}
238-
});
239-
}
240-
function spawnFileInternal(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, stdOut: (line: string) => void, token?: CancellationToken): Promise<string> {
241-
return new Promise<string>((resolve, reject) => {
242-
options.env = options.env || {};
243-
options.env['PYTHONIOENCODING'] = 'UTF-8';
244-
let proc = child_process.spawn(file, args, options);
245-
let error = '';
246-
let exited = false;
247-
if (token && token.onCancellationRequested) {
248-
token.onCancellationRequested(() => {
249-
if (!exited && proc) {
250-
proc.kill();
251-
proc = null;
252-
}
253-
});
254-
}
255-
proc.on('error', error => {
256-
reject(error);
257-
});
258-
proc.stdout.setEncoding('utf8');
259-
proc.stderr.setEncoding('utf8');
260-
proc.stdout.on('data', function (data: string) {
261-
if (token && token.isCancellationRequested) {
262-
return;
263-
}
264-
stdOut(data);
265-
});
266-
267-
proc.stderr.on('data', function (data: string) {
268-
if (token && token.isCancellationRequested) {
269-
return;
270-
}
271-
if (includeErrorAsResponse) {
272-
stdOut(data);
273-
}
274-
else {
275-
error += data;
276-
}
277-
});
278-
279-
proc.on('exit', function (code) {
280-
exited = true;
281-
282-
if (token && token.isCancellationRequested) {
283-
return reject();
284-
}
285-
if (error.length > 0) {
286-
return reject(error);
287-
}
288-
289-
resolve();
290-
});
291-
292-
});
293-
}
294-
function execInternal(command: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean): Promise<string> {
295-
return new Promise<string>((resolve, reject) => {
296-
child_process.exec([command].concat(args).join(' '), options, (error, stdout, stderr) => {
297-
handleResponse(command, includeErrorAsResponse, error, stdout, stderr)
298-
.then(data => resolve(data))
299-
.catch(err => reject(err));
300-
});
301-
});
302-
}
303-
30466
export function formatErrorForLogging(error: Error | string): string {
30567
let message: string = '';
30668
if (typeof error === 'string') {
@@ -332,7 +94,7 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
33294
if (error) {
33395
return resolve([]);
33496
}
335-
const subDirs = [];
97+
const subDirs: string[] = [];
33698
files.forEach(name => {
33799
const fullPath = path.join(rootDir, name);
338100
try {
@@ -341,7 +103,7 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
341103
}
342104
}
343105
// tslint:disable-next-line:no-empty
344-
catch (ex) {}
106+
catch (ex) { }
345107
});
346108
resolve(subDirs);
347109
});
@@ -396,7 +158,7 @@ export function getWindowsLineEndingCount(document: TextDocument, offset: Number
396158
// In order to prevent the one-time loading of large files from taking up too much memory
397159
for (let pos = 0; pos < offset; pos += readBlock) {
398160
let startAt = document.positionAt(pos);
399-
let endAt = null;
161+
let endAt: Position;
400162

401163
if (offsetDiff >= readBlock) {
402164
endAt = document.positionAt(pos + readBlock);
@@ -405,7 +167,7 @@ export function getWindowsLineEndingCount(document: TextDocument, offset: Number
405167
endAt = document.positionAt(pos + offsetDiff);
406168
}
407169

408-
let text = document.getText(new Range(startAt, endAt));
170+
let text = document.getText(new Range(startAt, endAt!));
409171
let cr = text.match(eolPattern);
410172

411173
count += cr ? cr.length : 0;
@@ -422,13 +184,3 @@ export function arePathsSame(path1: string, path2: string) {
422184
return path1 === path2;
423185
}
424186
}
425-
426-
export async function getInterpreterVersion(pythonPath: string) {
427-
return await new Promise<string>((resolve, reject) => {
428-
child_process.execFile(pythonPath, ['--version'], (error, stdout, stdErr) => {
429-
const out = (typeof stdErr === 'string' ? stdErr : '') + os.EOL + (typeof stdout === 'string' ? stdout : '');
430-
const lines = out.split(/\r?\n/g).map(line => line.trim()).filter(line => line.length > 0);
431-
resolve(lines.length > 0 ? lines[0] : '');
432-
});
433-
});
434-
}

0 commit comments

Comments
 (0)