Use new environment variable parser

src/client/common/envFileParser.ts

@@ -1,5 +1,6 @@
 import * as fs from 'fs-extra';
 import 'reflect-metadata';
+import { PathUtils } from './platform/pathUtils';
 import { EnvironmentVariablesService } from './variables/environment';
 import { EnvironmentVariables } from './variables/types';
 export const IS_WINDOWS = /^win/.test(process.platform);
@@ -38,7 +39,7 @@ export function parseEnvFile(envFile: string, mergeWithProcessEnvVars: boolean =
  * @returns {EnvironmentVariables}
  */
 export function mergeEnvVariables(targetEnvVars: EnvironmentVariables, sourceEnvVars: EnvironmentVariables = process.env): EnvironmentVariables {
-    const service = new EnvironmentVariablesService(IS_WINDOWS);
+    const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
     service.mergeVariables(sourceEnvVars, targetEnvVars);
     service.appendPythonPath(targetEnvVars, sourceEnvVars.PYTHONPATH);
     return targetEnvVars;
@@ -56,7 +57,7 @@ export function mergePythonPath(env: EnvironmentVariables, currentPythonPath: st
     if (typeof currentPythonPath !== 'string' || currentPythonPath.length === 0) {
         return env;
     }
-    const service = new EnvironmentVariablesService(IS_WINDOWS);
+    const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
     service.appendPythonPath(env, currentPythonPath!);
     return env;
 }

src/client/common/platform/pathUtils.ts

@@ -0,0 +1,12 @@
+import { inject, injectable } from 'inversify';
+import 'reflect-metadata';
+import { IPathUtils, IsWindows } from '../types';
+import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants';
+
+@injectable()
+export class PathUtils implements IPathUtils {
+    constructor( @inject(IsWindows) private isWindows: boolean) { }
+    public getPathVariableName() {
+        return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
+    }
+}

src/client/common/process/proc.ts

@@ -19,6 +19,10 @@ export class ProcessService implements IProcessService {
         if (!spawnOptions.env || Object.keys(spawnOptions).length === 0) {
             spawnOptions.env = process.env;
         }
+
+        // Always ensure we have unbuffered output.
+        spawnOptions.env.PYTHONUNBUFFERED = '1';
+
         const proc = spawn(file, args, spawnOptions);
         let procExited = false;
 
@@ -72,6 +76,10 @@ export class ProcessService implements IProcessService {
         if (!spawnOptions.env || Object.keys(spawnOptions).length === 0) {
             spawnOptions.env = process.env;
         }
+
+        // Always ensure we have unbuffered output.
+        spawnOptions.env.PYTHONUNBUFFERED = '1';
+
         const proc = spawn(file, args, spawnOptions);
         const deferred = createDeferred<ExecutionResult<string>>();
         const disposables: Disposable[] = [];

src/client/common/serviceRegistry.ts

@@ -8,13 +8,15 @@ import { Installer } from './installer';
 import { Logger } from './logger';
 import { PersistentStateFactory } from './persistentState';
 import { IS_WINDOWS as isWindows } from './platform/constants';
-import { IDiposableRegistry, IInstaller, ILogger, IPersistentStateFactory, IsWindows } from './types';
+import { PathUtils } from './platform/pathUtils';
+import { IDiposableRegistry, IInstaller, ILogger, IPathUtils, IPersistentStateFactory, IsWindows } from './types';
 
 export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingletonInstance<boolean>(IsWindows, isWindows);
     serviceManager.addSingleton<IPersistentStateFactory>(IPersistentStateFactory, PersistentStateFactory);
     serviceManager.addSingleton<IInstaller>(IInstaller, Installer);
     serviceManager.addSingleton<ILogger>(ILogger, Logger);
+    serviceManager.addSingleton<IPathUtils>(IPathUtils, PathUtils);
 
     const disposableRegistry = serviceManager.get<Disposable[]>(IDiposableRegistry);
     disposableRegistry.push(serviceManager.get<IInstaller>(IInstaller));

src/client/common/types.ts

@@ -67,3 +67,9 @@ export interface IInstaller extends Disposable {
     isInstalled(product: Product, resource?: Uri): Promise<boolean | undefined>;
     disableLinter(product: Product, resource?: Uri): Promise<void>;
 }
+
+export const IPathUtils = Symbol('IPathUtils');
+
+export interface IPathUtils {
+    getPathVariableName(): 'Path' | 'PATH';
+}

src/client/common/variables/environment.ts

@@ -5,13 +5,15 @@ import * as fs from 'fs-extra';
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import 'reflect-metadata';
-import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../platform/constants';
-import { IsWindows } from '../types';
+import { IPathUtils } from '../types';
 import { EnvironmentVariables, IEnvironmentVariablesService } from './types';
 
 @injectable()
 export class EnvironmentVariablesService implements IEnvironmentVariablesService {
-    constructor( @inject(IsWindows) private isWidows: boolean) { }
+    private readonly pathVariable: 'PATH' | 'Path';
+    constructor( @inject(IPathUtils) pathUtils: IPathUtils) {
+        this.pathVariable = pathUtils.getPathVariableName();
+    }
     public async parseFile(filePath: string): Promise<EnvironmentVariables | undefined> {
         const exists = await fs.pathExists(filePath);
         if (!exists) {
@@ -27,8 +29,7 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
                     return resolve(undefined);
                 }
                 this.appendPythonPath(vars, process.env.PYTHONPATH);
-                const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
-                this.appendPath(vars, process.env[pathVariable]);
+                this.appendPath(vars, process.env[this.pathVariable]);
                 resolve(vars);
             });
         });
@@ -37,8 +38,7 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         if (!target) {
             return;
         }
-        const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
-        const settingsNotToMerge = ['PYTHONPATH', pathVariable];
+        const settingsNotToMerge = ['PYTHONPATH', this.pathVariable];
         Object.keys(source).forEach(setting => {
             if (settingsNotToMerge.indexOf(setting) >= 0) {
                 return;
@@ -55,12 +55,10 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         return this.appendOrPrependPaths(vars, 'PYTHONPATH', true, ...pythonPaths);
     }
     public prependPath(vars: EnvironmentVariables, ...paths: string[]) {
-        const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
-        return this.appendOrPrependPaths(vars, pathVariable, false, ...paths);
+        return this.appendOrPrependPaths(vars, this.pathVariable, false, ...paths);
     }
     public appendPath(vars: EnvironmentVariables, ...paths: string[]) {
-        const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
-        return this.appendOrPrependPaths(vars, pathVariable, true, ...paths);
+        return this.appendOrPrependPaths(vars, this.pathVariable, true, ...paths);
     }
     private appendOrPrependPaths(vars: EnvironmentVariables, variableName: 'PATH' | 'Path' | 'PYTHONPATH', append: boolean, ...pythonPaths: string[]) {
         const pathToInsert = pythonPaths.filter(item => typeof item === 'string' && item.length > 0).join(path.delimiter);

src/client/debugger/Common/Utils.ts

@@ -4,8 +4,7 @@ import * as child_process from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as untildify from 'untildify';
-import { mergeEnvVariables, mergePythonPath, parseEnvFile } from '../../common/envFileParser';
-import { IPythonEvaluationResult, IPythonModule, IPythonProcess, IPythonThread } from './Contracts';
+import { IPythonModule, IPythonProcess, IPythonThread } from './Contracts';
 
 export const IS_WINDOWS = /^win/.test(process.platform);
 export const PATH_VARIABLE_NAME = IS_WINDOWS ? 'Path' : 'PATH';
@@ -31,7 +30,7 @@ export function validatePathSync(filePath: string): boolean {
         return false;
     }
     if (PathValidity.has(filePath)) {
-        return PathValidity.get(filePath);
+        return PathValidity.get(filePath)!;
     }
     const exists = fs.existsSync(filePath);
     PathValidity.set(filePath, exists);
@@ -116,30 +115,3 @@ function isValidPythonPath(pythonPath): boolean {
         return false;
     }
 }
-
-type EnvVars = Object & { [key: string]: string };
-
-export function getCustomEnvVars(envVars: Object, envFile: string, mergeWithProcessEnvVars: boolean = true): EnvVars {
-    let envFileVars: EnvVars = null;
-    if (typeof envFile === 'string' && envFile.length > 0 && fs.existsSync(envFile)) {
-        try {
-            envFileVars = parseEnvFile(envFile, mergeWithProcessEnvVars);
-        } catch (ex) {
-            console.error('Failed to load env file');
-            console.error(ex);
-        }
-    }
-    if (envFileVars && Object.keys(envFileVars).length > 0) {
-        if (!envVars || Object.keys(envVars).length === 0) {
-            return envFileVars;
-        } else {
-            envVars = envVars || {};
-            return mergeEnvVariables(envVars as EnvVars, envFileVars);
-        }
-    }
-    if (!envVars || Object.keys(envVars).length === 0) {
-        return null;
-    }
-
-    return mergePythonPath(envVars as EnvVars, process.env.PYTHONPATH);
-}

src/client/debugger/DebugClients/LocalDebugClient.ts

@@ -4,9 +4,12 @@ import * as path from 'path';
 import { DebugSession, OutputEvent } from 'vscode-debugadapter';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { open } from '../../common/open';
+import { PathUtils } from '../../common/platform/pathUtils';
+import { EnvironmentVariablesService } from '../../common/variables/environment';
+import { EnvironmentVariables } from '../../common/variables/types';
 import { IDebugServer, IPythonProcess } from '../Common/Contracts';
 import { LaunchRequestArguments } from '../Common/Contracts';
-import { getCustomEnvVars } from '../Common/Utils';
+import { IS_WINDOWS } from '../Common/Utils';
 import { BaseDebugServer } from '../DebugServers/BaseDebugServer';
 import { LocalDebugServer } from '../DebugServers/LocalDebugServer';
 import { DebugClient, DebugType } from './DebugClient';
@@ -17,10 +20,25 @@ const VALID_DEBUG_OPTIONS = [
     'BreakOnSystemExitZero',
     'DjangoDebugging'];
 
+enum DebugServerStatus {
+    Unknown = 1,
+    Running = 2,
+    NotRunning = 3
+}
+
 export class LocalDebugClient extends DebugClient {
-    protected pyProc: child_process.ChildProcess;
+    protected pyProc: child_process.ChildProcess | undefined;
     protected pythonProcess: IPythonProcess;
-    protected debugServer: BaseDebugServer;
+    protected debugServer: BaseDebugServer | undefined;
+    private get debugServerStatus(): DebugServerStatus {
+        if (this.debugServer && this.debugServer!.IsRunning) {
+            return DebugServerStatus.Running;
+        }
+        if (this.debugServer && !this.debugServer!.IsRunning) {
+            return DebugServerStatus.NotRunning;
+        }
+        return DebugServerStatus.Unknown;
+    }
     // tslint:disable-next-line:no-any
     constructor(args: any, debugSession: DebugSession, private canLaunchTerminal: boolean) {
         super(args, debugSession);
@@ -38,24 +56,24 @@ export class LocalDebugClient extends DebugClient {
 
     public Stop() {
         if (this.debugServer) {
-            this.debugServer.Stop();
-            this.debugServer = null;
+            this.debugServer!.Stop();
+            this.debugServer = undefined;
         }
 
         if (this.pyProc) {
             try {
-                this.pyProc.send('EXIT');
+                this.pyProc!.send('EXIT');
                 // tslint:disable-next-line:no-empty
             } catch { }
             try {
-                this.pyProc.stdin.write('EXIT');
+                this.pyProc!.stdin.write('EXIT');
                 // tslint:disable-next-line:no-empty
             } catch { }
             try {
-                this.pyProc.disconnect();
+                this.pyProc!.disconnect();
                 // tslint:disable-next-line:no-empty
             } catch { }
-            this.pyProc = null;
+            this.pyProc = undefined;
         }
     }
     protected getLauncherFilePath(): string {
@@ -71,7 +89,8 @@ export class LocalDebugClient extends DebugClient {
         }
     }
     // tslint:disable-next-line:max-func-body-length member-ordering no-any
-    public LaunchApplicationToDebug(dbgServer: IDebugServer, processErrored: (error: any) => void): Promise<any> {
+    public async LaunchApplicationToDebug(dbgServer: IDebugServer, processErrored: (error: any) => void): Promise<any> {
+        const environmentVariables = await this.getEnvironmentVariables();
         // tslint:disable-next-line:max-func-body-length cyclomatic-complexity no-any
         return new Promise<any>((resolve, reject) => {
             const fileDir = this.args && this.args.program ? path.dirname(this.args.program) : '';
@@ -83,8 +102,6 @@ export class LocalDebugClient extends DebugClient {
             if (typeof this.args.pythonPath === 'string' && this.args.pythonPath.trim().length > 0) {
                 pythonPath = this.args.pythonPath;
             }
-            let environmentVariables = getCustomEnvVars(this.args.env, this.args.envFile, false);
-            environmentVariables = environmentVariables ? environmentVariables : {};
             if (!environmentVariables.hasOwnProperty('PYTHONIOENCODING')) {
                 environmentVariables.PYTHONIOENCODING = 'UTF-8';
             }
@@ -103,12 +120,16 @@ export class LocalDebugClient extends DebugClient {
                     break;
                 }
                 default: {
+                    // As we're spawning the process, we need to ensure all env variables are passed.
+                    // Including those from the current process (i.e. everything, not just custom vars).
+                    const envParser = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
+                    envParser.mergeVariables(process.env as EnvironmentVariables, environmentVariables);
                     this.pyProc = child_process.spawn(pythonPath, args, { cwd: processCwd, env: environmentVariables });
-                    this.handleProcessOutput(this.pyProc, reject);
+                    this.handleProcessOutput(this.pyProc!, reject);
 
                     // Here we wait for the application to connect to the socket server.
                     // Only once connected do we know that the application has successfully launched.
-                    this.debugServer.DebugClientConnected
+                    this.debugServer!.DebugClientConnected
                         .then(resolve)
                         .catch(ex => console.error('Python Extension: debugServer.DebugClientConnected', ex));
                 }
@@ -120,10 +141,11 @@ export class LocalDebugClient extends DebugClient {
         proc.on('error', error => {
             // If debug server has started, then don't display errors.
             // The debug adapter will get this info from the debugger (e.g. ptvsd lib).
-            if (!this.debugServer && this.debugServer.IsRunning) {
+            const status = this.debugServerStatus;
+            if (status === DebugServerStatus.Running) {
                 return;
             }
-            if (!this.debugServer.IsRunning && typeof (error) === 'object' && error !== null) {
+            if (status === DebugServerStatus.NotRunning && typeof (error) === 'object' && error !== null) {
                 return failedToLaunch(error);
             }
             // This could happen when the debugger didn't launch at all, e.g. python doesn't exist.
@@ -136,13 +158,14 @@ export class LocalDebugClient extends DebugClient {
 
             // Either way, we need some code in here so we read the stdout of the python process,
             // Else it just keep building up (related to issue #203 and #52).
-            if (this.debugServer && !this.debugServer.IsRunning) {
+            if (this.debugServerStatus === DebugServerStatus.NotRunning) {
                 return failedToLaunch(error);
             }
         });
         proc.stdout.on('data', d => {
             // This is necessary so we read the stdout of the python process,
             // Else it just keep building up (related to issue #203 and #52).
+            // tslint:disable-next-line:prefer-const no-unused-variable
             let x = 0;
         });
     }
@@ -193,12 +216,32 @@ export class LocalDebugClient extends DebugClient {
                     this.pyProc = proc;
                     resolve();
                 }, error => {
-                    if (!this.debugServer && this.debugServer.IsRunning) {
+                    if (this.debugServerStatus === DebugServerStatus.Running) {
                         return;
                     }
                     reject(error);
                 });
             }
         });
     }
+    private async getEnvironmentVariables(): Promise<EnvironmentVariables> {
+        const args = this.args as LaunchRequestArguments;
+        const envParser = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS));
+        const envFileVars = await envParser.parseFile(args.envFile);
+
+        const hasEnvVars = args.env && Object.keys(args.env).length > 0;
+        if (!envFileVars && !hasEnvVars) {
+            return {};
+        }
+        if (envFileVars && !hasEnvVars) {
+            return envFileVars!;
+        }
+        if (!envFileVars && hasEnvVars) {
+            return args.env as EnvironmentVariables;
+        }
+        // Merge the two sets of environment variables.
+        const env = { ...args.env } as EnvironmentVariables;
+        envParser.mergeVariables(envFileVars!, env);
+        return env;
+    }
 }

src/client/debugger/DebugClients/NonDebugClient.ts

@@ -1,10 +1,7 @@
 import { ChildProcess } from 'child_process';
 import * as path from 'path';
-import { DebugSession, OutputEvent } from 'vscode-debugadapter';
-import { IPythonProcess } from '../Common/Contracts';
+import { DebugSession } from 'vscode-debugadapter';
 import { LaunchRequestArguments } from '../Common/Contracts';
-import { BaseDebugServer } from '../DebugServers/BaseDebugServer';
-import { LocalDebugServer } from '../DebugServers/LocalDebugServer';
 import { DebugType } from './DebugClient';
 import { LocalDebugClient } from './LocalDebugClient';
 
@@ -22,10 +19,10 @@ export class NonDebugClient extends LocalDebugClient {
         super.Stop();
         if (this.pyProc) {
             try {
-                this.pyProc.kill();
+                this.pyProc!.kill();
                 // tslint:disable-next-line:no-empty
             } catch { }
-            this.pyProc = null;
+            this.pyProc = undefined;
         }
     }
     protected handleProcessOutput(proc: ChildProcess, _failedToLaunch: (error: Error | string | Buffer) => void) {