Point release with fixes for debugger and activation of environments (#5939)

* Fixes to activation of Conda environments (#5934)

* Disable quoting paths sent to the debugger as args (#5936)

* Disable quoting paths sent to the debugger as args

* Remove unnecessary class

* Fix typo

* Fixes to tests

* Update version for new point release

Author: DonJayamanne

CHANGELOG.md

@@ -1,6 +1,15 @@
 # Changelog
 
-## 2019.5.3 (5 June 2019)
+## 2019.5.4 (6 June 2019)
+
+### Fixes
+
+1. Disable quoting of paths sent to the debugger as arguments.
+   ([#5861](https://github.com/microsoft/vscode-python/issues/5861))
+1. Fixes to activation of Conda environments.
+   ([#5929](https://github.com/microsoft/vscode-python/issues/5929))
+
+## 2019.5.18678 (5 June 2019)
 
 ### Fixes
 

package-lock.json

@@ -2,7 +2,7 @@
     "name": "python",
     "displayName": "Python",
     "description": "Linting, Debugging (multi-threaded, remote), Intellisense, code formatting, refactoring, unit tests, snippets, and more.",
-    "version": "2019.5.3",
+    "version": "2019.5.4",
     "languageServerVersion": "0.2.82",
     "publisher": "ms-python",
     "author": {

package.json

@@ -4,7 +4,7 @@
 'use strict';
 
 import { traceError } from './common/logger';
-import { RemoteDebuggerLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider';
+import { RemoteDebuggerExternalLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider';
 
 /*
  * Do not introduce any breaking changes to this API.
@@ -42,7 +42,7 @@ export function buildApi(ready: Promise<any>) {
         }),
         debug: {
             async getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean = true): Promise<string[]> {
-                return new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host, port, waitUntilDebuggerAttaches });
+                return new RemoteDebuggerExternalLauncherScriptProvider().getLauncherArgs({ host, port, waitUntilDebuggerAttaches });
             }
         }
     };

src/client/api.ts

@@ -52,12 +52,8 @@ export class TerminalHelper implements ITerminalHelper {
         this.sendTelemetry(resource, terminalShellType, EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL, promise).ignoreErrors();
         return promise;
     }
-    public async getEnvironmentActivationShellCommands(resource: Resource, interpreter?: PythonInterpreter): Promise<string[] | undefined> {
-        if (this.platform.osType === OSType.Unknown){
-            return;
-        }
-        const shell = this.shellDetector.identifyTerminalShell();
-        if (!shell) {
+    public async getEnvironmentActivationShellCommands(resource: Resource, shell: TerminalShellType, interpreter?: PythonInterpreter): Promise<string[] | undefined> {
+        if (this.platform.osType === OSType.Unknown) {
             return;
         }
         const providers = [this.bashCShellFish, this.commandPromptAndPowerShell];

src/client/common/terminal/helper.ts

@@ -57,7 +57,7 @@ export interface ITerminalHelper {
     identifyTerminalShell(terminal?: Terminal): TerminalShellType;
     buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]): string;
     getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise<string[] | undefined>;
-    getEnvironmentActivationShellCommands(resource: Resource, interpreter?: PythonInterpreter): Promise<string[] | undefined>;
+    getEnvironmentActivationShellCommands(resource: Resource, shell: TerminalShellType, interpreter?: PythonInterpreter): Promise<string[] | undefined>;
 }
 
 export const ITerminalActivator = Symbol('ITerminalActivator');

src/client/common/terminal/types.ts

@@ -15,19 +15,26 @@ export class NoDebugLauncherScriptProvider implements IDebugLauncherScriptProvid
     constructor(@optional() private script: string = pathToScript) { }
     public getLauncherArgs(options: LocalDebugOptions): string[] {
         const customDebugger = options.customDebugger ? '--custom' : '--default';
-        return [this.script.fileToCommandArgument(), customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()];
+        return [this.script, customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()];
     }
 }
 
 export class DebuggerLauncherScriptProvider implements IDebugLauncherScriptProvider<LocalDebugOptions>  {
     constructor(@optional() private script: string = pathToScript) { }
     public getLauncherArgs(options: LocalDebugOptions): string[] {
         const customDebugger = options.customDebugger ? '--custom' : '--default';
-        return [this.script.fileToCommandArgument(), customDebugger, '--client', '--host', options.host, '--port', options.port.toString()];
+        return [this.script, customDebugger, '--client', '--host', options.host, '--port', options.port.toString()];
     }
 }
 
-export class RemoteDebuggerLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider {
+/**
+ * This class is used to provide the launch scripts so external code can launch the debugger.
+ * As we're passing command arguments, we need to ensure the file paths are quoted.
+ * @export
+ * @class RemoteDebuggerExternalLauncherScriptProvider
+ * @implements {IRemoteDebugLauncherScriptProvider}
+ */
+export class RemoteDebuggerExternalLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider {
     constructor(@optional() private script: string = pathToScript) { }
     public getLauncherArgs(options: RemoteDebugOptions): string[] {
         const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : [];

src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts

@@ -9,7 +9,7 @@ import * as path from 'path';
 import { LogOptions, traceDecorators, traceError, traceVerbose } from '../../common/logger';
 import { IPlatformService } from '../../common/platform/types';
 import { IProcessServiceFactory } from '../../common/process/types';
-import { ITerminalHelper } from '../../common/terminal/types';
+import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types';
 import { ICurrentProcess, IDisposable, Resource } from '../../common/types';
 import {
     cacheResourceSpecificInterpreterData,
@@ -29,9 +29,9 @@ const getEnvironmentTimeout = 30000;
 
 // The shell under which we'll execute activation scripts.
 const defaultShells = {
-    [OSType.Windows]: 'cmd',
-    [OSType.OSX]: 'bash',
-    [OSType.Linux]: 'bash',
+    [OSType.Windows]: { shell: 'cmd', shellType: TerminalShellType.commandPrompt },
+    [OSType.OSX]: { shell: 'bash', shellType: TerminalShellType.bash },
+    [OSType.Linux]: { shell: 'bash', shellType: TerminalShellType.bash },
     [OSType.Unknown]: undefined
 };
 
@@ -54,14 +54,14 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
     @captureTelemetry(EventName.PYTHON_INTERPRETER_ACTIVATION_ENVIRONMENT_VARIABLES, { failed: false }, true)
     @cacheResourceSpecificInterpreterData('ActivatedEnvironmentVariables', cacheDuration)
     public async getActivatedEnvironmentVariables(resource: Resource, interpreter?: PythonInterpreter, allowExceptions?: boolean): Promise<NodeJS.ProcessEnv | undefined> {
-        const shell = defaultShells[this.platform.osType];
-        if (!shell) {
+        const shellInfo = defaultShells[this.platform.osType];
+        if (!shellInfo) {
             return;
         }
 
         try {
-            const activationCommands = await this.helper.getEnvironmentActivationShellCommands(resource, interpreter);
-            traceVerbose(`Activation Commands received ${activationCommands}`);
+            const activationCommands = await this.helper.getEnvironmentActivationShellCommands(resource, shellInfo.shellType, interpreter);
+            traceVerbose(`Activation Commands received ${activationCommands} for shell ${shellInfo.shell}`);
             if (!activationCommands || !Array.isArray(activationCommands) || activationCommands.length === 0) {
                 return;
             }
@@ -84,7 +84,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
             // See the discussion from hidesoon in this issue: https://github.com/Microsoft/vscode-python/issues/4424
             // His issue is conda never finishing during activate. This is a conda issue, but we
             // should at least tell the user.
-            const result = await processService.shellExec(command, { env, shell, timeout: getEnvironmentTimeout, maxBuffer: 1000 * 1000 });
+            const result = await processService.shellExec(command, { env, shell: shellInfo.shell, timeout: getEnvironmentTimeout, maxBuffer: 1000 * 1000 });
             if (result.stderr && result.stderr.length > 0) {
                 throw new Error(`StdErr from ShellExec, ${result.stderr}`);
             }

src/client/interpreter/activation/service.ts

@@ -315,9 +315,10 @@ suite('Terminal Service helpers', () => {
                     test('Activation command for Shell must be empty for unknown os', async () => {
                         when(platformService.osType).thenReturn(OSType.Unknown);
 
-                        const cmd = await helper.getEnvironmentActivationShellCommands(resource, interpreter);
-
-                        expect(cmd).to.equal(undefined, 'Command must be undefined');
+                        for (const item of getNamesAndValues<TerminalShellType>(TerminalShellType)) {
+                            const cmd = await helper.getEnvironmentActivationShellCommands(resource, item.value, interpreter);
+                            expect(cmd).to.equal(undefined, 'Command must be undefined');
+                        }
                     });
                 });
                 [undefined, pythonInterpreter].forEach(interpreter => {
@@ -332,7 +333,7 @@ suite('Terminal Service helpers', () => {
                             when(bashActivationProvider.isShellSupported(shellToExpect)).thenReturn(false);
                             when(cmdActivationProvider.isShellSupported(shellToExpect)).thenReturn(false);
 
-                            const cmd = await helper.getEnvironmentActivationShellCommands(resource, interpreter);
+                            const cmd = await helper.getEnvironmentActivationShellCommands(resource, shellToExpect, interpreter);
 
                             expect(cmd).to.equal(undefined, 'Command must be undefined');
                             verify(pythonSettings.terminal).once();

src/test/common/terminals/helper.unit.test.ts

@@ -7,7 +7,7 @@ import { expect } from 'chai';
 import * as fs from 'fs-extra';
 import * as path from 'path';
 import { EXTENSION_ROOT_DIR } from '../../../../client/common/constants';
-import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider';
+import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDebuggerExternalLauncherScriptProvider } from '../../../../client/debugger/debugAdapter/DebugClients/launcherProvider';
 
 const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
 
@@ -21,12 +21,12 @@ suite('Debugger - Launcher Script Provider', () => {
             {
                 testName: 'When path to ptvsd launcher does not contains spaces',
                 path: path.join('path', 'to', 'ptvsd_launcher'),
-                expectedPath: 'path/to/ptvsd_launcher'
+                expectedPath: path.join('path', 'to', 'ptvsd_launcher')
             },
             {
                 testName: 'When path to ptvsd launcher contains spaces',
                 path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'),
-                expectedPath: '"path/to/ptvsd_launcher/with spaces"'
+                expectedPath: path.join('path', 'to', 'ptvsd_launcher', 'with spaces')
             }
         ];
 
@@ -52,15 +52,33 @@ suite('Debugger - Launcher Script Provider', () => {
                 const expectedArgs = [testParams.expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
                 expect(args).to.be.deep.equal(expectedArgs);
             });
-            test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
-                const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
-                const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234'];
-                expect(args).to.be.deep.equal(expectedArgs);
-            });
-            test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
-                const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
-                const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
-                expect(args).to.be.deep.equal(expectedArgs);
+        });
+    });
+
+    suite('External Debug Launcher', () => {
+        [
+            {
+                testName: 'When path to ptvsd launcher does not contains spaces',
+                path: path.join('path', 'to', 'ptvsd_launcher'),
+                expectedPath: 'path/to/ptvsd_launcher'
+            },
+            {
+                testName: 'When path to ptvsd launcher contains spaces',
+                path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'),
+                expectedPath: '"path/to/ptvsd_launcher/with spaces"'
+            }
+        ].forEach(testParams => {
+            suite(testParams.testName, async () => {
+                test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
+                    const args = new RemoteDebuggerExternalLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
+                    const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234'];
+                    expect(args).to.be.deep.equal(expectedArgs);
+                });
+                test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
+                    const args = new RemoteDebuggerExternalLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
+                    const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
+                    expect(args).to.be.deep.equal(expectedArgs);
+                });
             });
         });
     });