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

Author: DonJayamanne

src/client/common/configSettings.ts

@@ -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
@@ -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);

src/client/common/interpreterInfoCache.ts

@@ -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';
@@ -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) => {
@@ -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') {
@@ -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 {
@@ -341,7 +103,7 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
                     }
                 }
                 // tslint:disable-next-line:no-empty
-                catch (ex) {}
+                catch (ex) { }
             });
             resolve(subDirs);
         });
@@ -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);
@@ -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;
@@ -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] : '');
-        });
-    });
-}