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

Refactor extension to remove old way of spawning python processes #439

Merged
merged 58 commits into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
ecc1ca9
Fix Microsoft/vscode#37627 (#1368)
octref Nov 3, 2017
7c5778c
Version 0.7.0 of extension (#1381)
DonJayamanne Nov 9, 2017
9d1bf82
Update README.md
DonJayamanne Nov 9, 2017
ffba179
Update README.md
DonJayamanne Nov 9, 2017
905c713
sync fork with upstream
DonJayamanne Nov 10, 2017
acc2109
fix readme
DonJayamanne Nov 10, 2017
d470523
Merge branch 'master' of https://github.com/Microsoft/vscode-python
DonJayamanne Nov 16, 2017
d392e8b
merged upstream
DonJayamanne Nov 16, 2017
92f775f
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 20, 2017
32a6e53
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 21, 2017
4b30f2c
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 22, 2017
e396752
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 22, 2017
eff4792
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
4553c28
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
3c6520a
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
966e516
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
63d2d65
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
f6d469e
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 28, 2017
029e055
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 30, 2017
e8c71c0
Merge remote-tracking branch 'upstream/master'
DonJayamanne Nov 30, 2017
51cf9d2
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 1, 2017
7aadc43
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 1, 2017
f0f5c59
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 4, 2017
b2b9da9
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 4, 2017
30a4091
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 5, 2017
b16d2f9
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 6, 2017
c8db345
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 7, 2017
0df7f16
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 8, 2017
3ccc881
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 9, 2017
bb0709e
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
2c19004
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
8f224ab
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 11, 2017
41b7080
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
dab38dc
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
ae22dd4
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 12, 2017
d2340d2
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 14, 2017
bcb8879
use new exec engine instead of spawning manually
DonJayamanne Dec 14, 2017
c6d6f50
refactor to use new execution framework
DonJayamanne Dec 15, 2017
65a949b
refactor to use new exec framework
DonJayamanne Dec 15, 2017
c8559ea
fix linter errors
DonJayamanne Dec 15, 2017
51a2802
refactor
DonJayamanne Dec 15, 2017
c778493
disable messages and copy config files
DonJayamanne Dec 15, 2017
a34a62c
remove old execution layer
DonJayamanne Dec 15, 2017
52bb7ae
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 15, 2017
5760886
merged master
DonJayamanne Dec 15, 2017
1bdc95d
fix tests
DonJayamanne Dec 15, 2017
0313000
fix tests
DonJayamanne Dec 15, 2017
e7eb19e
disable message D102
DonJayamanne Dec 15, 2017
d09b3ef
fix bug introduced into test
DonJayamanne Dec 15, 2017
0467d4c
fix tests
DonJayamanne Dec 15, 2017
ffdfd5d
fixed code review comments
DonJayamanne Dec 15, 2017
b6b2531
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 19, 2017
2b21cec
merged master
DonJayamanne Dec 19, 2017
21601f2
Fixed code review comments
DonJayamanne Dec 19, 2017
8d8d2fc
Merge remote-tracking branch 'upstream/master'
DonJayamanne Dec 19, 2017
6641ec8
merged master
DonJayamanne Dec 19, 2017
25d3758
fixed code review comments
DonJayamanne Jan 2, 2018
7a45b2d
removed unused var
DonJayamanne Jan 2, 2018
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
3 changes: 0 additions & 3 deletions src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { EventEmitter } from 'events';
import * as path from 'path';
import * as vscode from 'vscode';
import { Uri } from 'vscode';
import { InterpreterInfoCache } from './interpreterInfoCache';
import { SystemVariables } from './variables/systemVariables';

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

// tslint:disable-next-line:cyclomatic-complexity max-func-body-length
private initializeSettings() {
InterpreterInfoCache.clear();
const workspaceRoot = this.workspaceRoot.fsPath;
const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
const pythonSettings = vscode.workspace.getConfiguration('python', this.workspaceRoot);
Expand Down
54 changes: 0 additions & 54 deletions src/client/common/interpreterInfoCache.ts

This file was deleted.

260 changes: 6 additions & 254 deletions src/client/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ import * as fs from 'fs';
import * as fsExtra from 'fs-extra';
import * as os from 'os';
import * as path from 'path';
import { CancellationToken, Range, TextDocument, Uri } from 'vscode';
import { Position, Range, TextDocument, Uri } from 'vscode';
import * as settings from './configSettings';
import { mergeEnvVariables, parseEnvFile } from './envFileParser';
import { isNotInstalledError } from './helpers';
import { InterpreterInfoCache } from './interpreterInfoCache';
import { parseEnvFile } from './envFileParser';

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

async function getPythonInterpreterDirectory(resource?: Uri): Promise<string> {
const cache = InterpreterInfoCache.get(resource);
const pythonFileName = settings.PythonSettings.getInstance(resource).pythonPath;

// If we already have it and the python path hasn't changed, yay
if (cache.pythonInterpreterDirectory && cache.pythonInterpreterDirectory.length > 0
&& cache.pythonSettingsPath === pythonFileName) {
return cache.pythonInterpreterDirectory;
}

// Check if we have the path
if (path.basename(pythonFileName) === pythonFileName) {
try {
const pythonInterpreterPath = await getPathFromPythonCommand(pythonFileName);
const pythonInterpreterDirectory = path.dirname(pythonInterpreterPath);
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonInterpreterPath, pythonInterpreterDirectory);
return pythonInterpreterDirectory;
// tslint:disable-next-line:variable-name
} catch (_ex) {
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, '');
return '';
}
}

return new Promise<string>(resolve => {
// If we can execute the python, then get the path from the fully qualified name
child_process.execFile(pythonFileName, ['-c', 'print(1234)'], (error, stdout, stderr) => {
// Yes this is a valid python path
if (stdout.startsWith('1234')) {
const pythonInterpreterDirectory = path.dirname(pythonFileName);
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, pythonInterpreterDirectory);
resolve(pythonInterpreterDirectory);
} else {
// No idea, didn't work, hence don't reject, but return empty path
InterpreterInfoCache.setPaths(resource, pythonFileName, pythonFileName, '');
resolve('');
}
});
});
}
export async function getFullyQualifiedPythonInterpreterPath(resource?: Uri): Promise<string> {
const pyDir = await getPythonInterpreterDirectory(resource);
const cache = InterpreterInfoCache.get(resource);
return cache.pythonInterpreterPath;
}
export async function getPathFromPythonCommand(pythonPath: string): Promise<string> {
return await new Promise<string>((resolve, reject) => {
child_process.execFile(pythonPath, ['-c', 'import sys;print(sys.executable)'], (_, stdout) => {
Expand All @@ -110,197 +63,6 @@ export async function getPathFromPythonCommand(pythonPath: string): Promise<stri
});
});
}
async function getEnvVariables(resource?: Uri): Promise<{}> {
const cache = InterpreterInfoCache.get(resource);
if (cache.customEnvVariables) {
return cache.customEnvVariables;
}

const pyPath = await getPythonInterpreterDirectory(resource);
let customEnvVariables = await getCustomEnvVars(resource) || {};

if (pyPath.length > 0) {
// Ensure to include the path of the current python.
let newPath = '';
const currentPath = typeof customEnvVariables[PATH_VARIABLE_NAME] === 'string' ? customEnvVariables[PATH_VARIABLE_NAME] : process.env[PATH_VARIABLE_NAME];
if (IS_WINDOWS) {
newPath = `${pyPath}\\${path.delimiter}${path.join(pyPath, 'Scripts\\')}${path.delimiter}${currentPath}`;
// This needs to be done for windows.
process.env[PATH_VARIABLE_NAME] = newPath;
} else {
newPath = `${pyPath}${path.delimiter}${currentPath}`;
}
customEnvVariables = mergeEnvVariables(customEnvVariables, process.env);
customEnvVariables[PATH_VARIABLE_NAME] = newPath;
}

InterpreterInfoCache.setCustomEnvVariables(resource, customEnvVariables);
return customEnvVariables;
}
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> {
const resourceUri = typeof resource === 'string' ? Uri.file(resource) : resource;
const env = await getEnvVariables(resourceUri);
const options = { cwd, env };

if (stdOut) {
return spawnFileInternal(file, args, options, includeErrorAsResponse, stdOut, token);
}

const fileIsPythonInterpreter = (file.toUpperCase() === 'PYTHON' || file === settings.PythonSettings.getInstance(resourceUri).pythonPath);
const execAsModule = fileIsPythonInterpreter && args.length > 0 && args[0] === '-m';

if (execAsModule) {
return getFullyQualifiedPythonInterpreterPath(resourceUri)
.then(p => execPythonModule(p, args, options, includeErrorAsResponse, token));
}
return execFileInternal(file, args, options, includeErrorAsResponse, token);
}

function handleResponse(file: string, includeErrorAsResponse: boolean, error: Error, stdout: string, stderr: string, token?: CancellationToken): Promise<string> {
if (token && token.isCancellationRequested) {
return Promise.resolve(undefined);
}
if (isNotInstalledError(error)) {
return Promise.reject(error);
}

// pylint:
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
// These error messages are useless when using pylint
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
return Promise.resolve(stdout + '\n' + stderr);
}

let hasErrors = (error && error.message.length > 0) || (stderr && stderr.length > 0);
if (hasErrors && (typeof stdout !== 'string' || stdout.length === 0)) {
let errorMsg = (error && error.message) ? error.message : (stderr && stderr.length > 0 ? stderr + '' : '');
return Promise.reject(errorMsg);
}
else {
return Promise.resolve(stdout + '');
}
}
function handlePythonModuleResponse(includeErrorAsResponse: boolean, error: Error, stdout: string, stderr: string, token?: CancellationToken): Promise<string> {
if (token && token.isCancellationRequested) {
return Promise.resolve(undefined);
}
if (isNotInstalledError(error)) {
return Promise.reject(error);
}

// pylint:
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
// These error messages are useless when using pylint
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
return Promise.resolve(stdout + '\n' + stderr);
}
if (!includeErrorAsResponse && stderr.length > 0) {
return Promise.reject(stderr);
}

return Promise.resolve(stdout + '');
}
function execPythonModule(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, token?: CancellationToken): Promise<string> {
options.maxBuffer = options.maxBuffer ? options.maxBuffer : 1024 * 102400;
return new Promise<string>((resolve, reject) => {
let proc = child_process.execFile(file, args, options, (error, stdout, stderr) => {
handlePythonModuleResponse(includeErrorAsResponse, error, stdout, stderr, token)
.then(resolve)
.catch(reject);
});
if (token && token.onCancellationRequested) {
token.onCancellationRequested(() => {
if (proc) {
proc.kill();
proc = null;
}
});
}
});
}

function execFileInternal(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, token?: CancellationToken): Promise<string> {
options.maxBuffer = options.maxBuffer ? options.maxBuffer : 1024 * 102400;
return new Promise<string>((resolve, reject) => {
let proc = child_process.execFile(file, args, options, (error, stdout, stderr) => {
handleResponse(file, includeErrorAsResponse, error, stdout, stderr, token)
.then(data => resolve(data))
.catch(err => reject(err));
});
if (token && token.onCancellationRequested) {
token.onCancellationRequested(() => {
if (proc) {
proc.kill();
proc = null;
}
});
}
});
}
function spawnFileInternal(file: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean, stdOut: (line: string) => void, token?: CancellationToken): Promise<string> {
return new Promise<string>((resolve, reject) => {
options.env = options.env || {};
options.env['PYTHONIOENCODING'] = 'UTF-8';
let proc = child_process.spawn(file, args, options);
let error = '';
let exited = false;
if (token && token.onCancellationRequested) {
token.onCancellationRequested(() => {
if (!exited && proc) {
proc.kill();
proc = null;
}
});
}
proc.on('error', error => {
reject(error);
});
proc.stdout.setEncoding('utf8');
proc.stderr.setEncoding('utf8');
proc.stdout.on('data', function (data: string) {
if (token && token.isCancellationRequested) {
return;
}
stdOut(data);
});

proc.stderr.on('data', function (data: string) {
if (token && token.isCancellationRequested) {
return;
}
if (includeErrorAsResponse) {
stdOut(data);
}
else {
error += data;
}
});

proc.on('exit', function (code) {
exited = true;

if (token && token.isCancellationRequested) {
return reject();
}
if (error.length > 0) {
return reject(error);
}

resolve();
});

});
}
function execInternal(command: string, args: string[], options: child_process.ExecFileOptions, includeErrorAsResponse: boolean): Promise<string> {
return new Promise<string>((resolve, reject) => {
child_process.exec([command].concat(args).join(' '), options, (error, stdout, stderr) => {
handleResponse(command, includeErrorAsResponse, error, stdout, stderr)
.then(data => resolve(data))
.catch(err => reject(err));
});
});
}

export function formatErrorForLogging(error: Error | string): string {
let message: string = '';
if (typeof error === 'string') {
Expand Down Expand Up @@ -332,7 +94,7 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
if (error) {
return resolve([]);
}
const subDirs = [];
const subDirs: string[] = [];
files.forEach(name => {
const fullPath = path.join(rootDir, name);
try {
Expand All @@ -341,7 +103,7 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
}
}
// tslint:disable-next-line:no-empty
catch (ex) {}
catch (ex) { }
});
resolve(subDirs);
});
Expand Down Expand Up @@ -396,7 +158,7 @@ export function getWindowsLineEndingCount(document: TextDocument, offset: Number
// In order to prevent the one-time loading of large files from taking up too much memory
for (let pos = 0; pos < offset; pos += readBlock) {
let startAt = document.positionAt(pos);
let endAt = null;
let endAt: Position;

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

let text = document.getText(new Range(startAt, endAt));
let text = document.getText(new Range(startAt, endAt!));
let cr = text.match(eolPattern);

count += cr ? cr.length : 0;
Expand All @@ -422,13 +184,3 @@ export function arePathsSame(path1: string, path2: string) {
return path1 === path2;
}
}

export async function getInterpreterVersion(pythonPath: string) {
return await new Promise<string>((resolve, reject) => {
child_process.execFile(pythonPath, ['--version'], (error, stdout, stdErr) => {
const out = (typeof stdErr === 'string' ? stdErr : '') + os.EOL + (typeof stdout === 'string' ? stdout : '');
const lines = out.split(/\r?\n/g).map(line => line.trim()).filter(line => line.length > 0);
resolve(lines.length > 0 ? lines[0] : '');
});
});
}
Loading