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

* Fixes to activation of Conda environments (microsoft#5934) * Disable quoting paths sent to the debugger as args (microsoft#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);
+ });
  });
  });
  });