Skip to content

Cherry pick fixes for release #18874

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

Merged
merged 7 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions news/2 Fixes/18200.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure `conda info` command isn't run multiple times during startup when large number of conda interpreters are present.
1 change: 1 addition & 0 deletions news/2 Fixes/18530.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If a conda environment is not returned via the `conda env list` command, consider it as unknown env type.
1 change: 1 addition & 0 deletions news/2 Fixes/18722.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Wrap file paths containg an ampersand in double quotation marks for running commands in a shell.
1 change: 1 addition & 0 deletions news/2 Fixes/18835.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes regression with support for python binaries not following the standard names.
1 change: 1 addition & 0 deletions news/2 Fixes/18847.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix launch of Python Debugger when using conda environments.
14 changes: 8 additions & 6 deletions src/client/common/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ declare interface String {
* Appropriately formats a string so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
toCommandArgument(): string;
toCommandArgumentForPythonExt(): string;
/**
* Appropriately formats a a file path so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
fileToCommandArgument(): string;
fileToCommandArgumentForPythonExt(): string;
/**
* String.format() implementation.
* Tokens such as {0}, {1} will be replaced with corresponding positional arguments.
Expand Down Expand Up @@ -69,22 +69,24 @@ String.prototype.splitLines = function (
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
* @param {String} value.
*/
String.prototype.toCommandArgument = function (this: string): string {
String.prototype.toCommandArgumentForPythonExt = function (this: string): string {
if (!this) {
return this;
}
return this.indexOf(' ') >= 0 && !this.startsWith('"') && !this.endsWith('"') ? `"${this}"` : this.toString();
return (this.indexOf(' ') >= 0 || this.indexOf('&') >= 0) && !this.startsWith('"') && !this.endsWith('"')
? `"${this}"`
: this.toString();
};

/**
* Appropriately formats a a file path so it can be used as an argument for a command in a shell.
* E.g. if an argument contains a space, then it will be enclosed within double quotes.
*/
String.prototype.fileToCommandArgument = function (this: string): string {
String.prototype.fileToCommandArgumentForPythonExt = function (this: string): string {
if (!this) {
return this;
}
return this.toCommandArgument().replace(/\\/g, '/');
return this.toCommandArgumentForPythonExt().replace(/\\/g, '/');
};

/**
Expand Down
13 changes: 8 additions & 5 deletions src/client/common/installer/condaInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ export class CondaInstaller extends ModuleInstaller {
flags: ModuleInstallFlags = 0,
): Promise<ExecutionInfo> {
const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
const condaFile = await condaService.getCondaFile();
// Installation using `conda.exe` sometimes fails with a HTTP error on Windows:
// https://github.com/conda/conda/issues/11399
// Execute in a shell which uses a `conda.bat` file instead, using which installation works.
const useShell = true;
const condaFile = await condaService.getCondaFile(useShell);

const pythonPath = isResource(resource)
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath
Expand Down Expand Up @@ -100,11 +104,11 @@ export class CondaInstaller extends ModuleInstaller {
if (info && info.name) {
// If we have the name of the conda environment, then use that.
args.push('--name');
args.push(info.name.toCommandArgument());
args.push(info.name.toCommandArgumentForPythonExt());
} else if (info && info.path) {
// Else provide the full path to the environment path.
args.push('--prefix');
args.push(info.path.fileToCommandArgument());
args.push(info.path.fileToCommandArgumentForPythonExt());
}
if (flags & ModuleInstallFlags.updateDependencies) {
args.push('--update-deps');
Expand All @@ -117,8 +121,7 @@ export class CondaInstaller extends ModuleInstaller {
return {
args,
execPath: condaFile,
// Execute in a shell as `conda` on windows refers to `conda.bat`, which requires a shell to work.
useShell: true,
useShell,
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/client/common/installer/moduleInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ export abstract class ModuleInstaller implements IModuleInstaller {
const argv = [command, ...args];
// Concat these together to make a set of quoted strings
const quoted = argv.reduce(
(p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`),
(p, c) =>
p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`,
'',
);
await processService.shellExec(quoted);
Expand Down
6 changes: 3 additions & 3 deletions src/client/common/process/internal/scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function normalizeSelection(): [string[], (out: string) => string] {
// printEnvVariables.py

export function printEnvVariables(): [string[], (out: string) => NodeJS.ProcessEnv] {
const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgument();
const script = path.join(SCRIPTS_DIR, 'printEnvVariables.py').fileToCommandArgumentForPythonExt();
const args = [script];

function parse(out: string): NodeJS.ProcessEnv {
Expand All @@ -113,11 +113,11 @@ export function shell_exec(command: string, lockfile: string, shellArgs: string[
// could be anything.
return [
script,
command.fileToCommandArgument(),
command.fileToCommandArgumentForPythonExt(),
// The shell args must come after the command
// but before the lockfile.
...shellArgs,
lockfile.fileToCommandArgument(),
lockfile.fileToCommandArgumentForPythonExt(),
];
}

Expand Down
2 changes: 1 addition & 1 deletion src/client/common/process/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class ProcessLogger implements IProcessLogger {
return;
}
let command = args
? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgument()).join(' ')
? [fileOrCommand, ...args].map((e) => e.trimQuotes().toCommandArgumentForPythonExt()).join(' ')
: fileOrCommand;
const info = [`> ${this.getDisplayCommands(command)}`];
if (options && options.cwd) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ export class Bash extends VenvBaseActivationCommandProvider {
if (!scriptFile) {
return;
}
return [`source ${scriptFile.fileToCommandArgument()}`];
return [`source ${scriptFile.fileToCommandArgumentForPythonExt()}`];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ export class CommandPromptAndPowerShell extends VenvBaseActivationCommandProvide
}

if (targetShell === TerminalShellType.commandPrompt && scriptFile.endsWith('activate.bat')) {
return [scriptFile.fileToCommandArgument()];
return [scriptFile.fileToCommandArgumentForPythonExt()];
} else if (
(targetShell === TerminalShellType.powershell || targetShell === TerminalShellType.powershellCore) &&
scriptFile.endsWith('Activate.ps1')
) {
return [`& ${scriptFile.fileToCommandArgument()}`];
return [`& ${scriptFile.fileToCommandArgumentForPythonExt()}`];
} else if (targetShell === TerminalShellType.commandPrompt && scriptFile.endsWith('Activate.ps1')) {
// lets not try to run the powershell file from command prompt (user may not have powershell)
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,11 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
const interpreterPath = await this.condaService.getInterpreterPathForEnvironment(envInfo);
const condaPath = await this.condaService.getCondaFileFromInterpreter(interpreterPath, envInfo.name);
if (condaPath) {
const activatePath = path.join(path.dirname(condaPath), 'activate').fileToCommandArgument();
const activatePath = path
.join(path.dirname(condaPath), 'activate')
.fileToCommandArgumentForPythonExt();
const firstActivate = this.platform.isWindows ? activatePath : `source ${activatePath}`;
return [firstActivate, `conda activate ${condaEnv.toCommandArgument()}`];
return [firstActivate, `conda activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}
}
}
Expand Down Expand Up @@ -116,15 +118,15 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
const condaScriptsPath: string = path.dirname(condaExePath);
// prefix the cmd with the found path, and ensure it's quoted properly
activateCmd = path.join(condaScriptsPath, activateCmd);
activateCmd = activateCmd.toCommandArgument();
activateCmd = activateCmd.toCommandArgumentForPythonExt();
}

return activateCmd;
}

public async getWindowsCommands(condaEnv: string): Promise<string[] | undefined> {
const activate = await this.getWindowsActivateCommand();
return [`${activate} ${condaEnv.toCommandArgument()}`];
return [`${activate} ${condaEnv.toCommandArgumentForPythonExt()}`];
}
}

Expand All @@ -135,16 +137,16 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
* Extension will not attempt to work around issues by trying to setup shell for user.
*/
export async function _getPowershellCommands(condaEnv: string): Promise<string[] | undefined> {
return [`conda activate ${condaEnv.toCommandArgument()}`];
return [`conda activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}

async function getFishCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
// https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28
return [`${condaFile.fileToCommandArgument()} activate ${condaEnv.toCommandArgument()}`];
return [`${condaFile.fileToCommandArgumentForPythonExt()} activate ${condaEnv.toCommandArgumentForPythonExt()}`];
}

async function getUnixCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
const condaDir = path.dirname(condaFile);
const activateFile = path.join(condaDir, 'activate');
return [`source ${activateFile.fileToCommandArgument()} ${condaEnv.toCommandArgument()}`];
return [`source ${activateFile.fileToCommandArgumentForPythonExt()} ${condaEnv.toCommandArgumentForPythonExt()}`];
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma
}
}
const execName = this.pipEnvExecution.executable;
return [`${execName.fileToCommandArgument()} shell`];
return [`${execName.fileToCommandArgumentForPythonExt()} shell`];
}

public async getActivationCommandsForInterpreter(pythonPath: string): Promise<string[] | undefined> {
Expand All @@ -51,6 +51,6 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma
}

const execName = this.pipEnvExecution.executable;
return [`${execName.fileToCommandArgument()} shell`];
return [`${execName.fileToCommandArgumentForPythonExt()} shell`];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class PyEnvActivationCommandProvider implements ITerminalActivationComman
return;
}

return [`pyenv shell ${interpreter.envName.toCommandArgument()}`];
return [`pyenv shell ${interpreter.envName.toCommandArgumentForPythonExt()}`];
}

public async getActivationCommandsForInterpreter(
Expand All @@ -40,6 +40,6 @@ export class PyEnvActivationCommandProvider implements ITerminalActivationComman
return;
}

return [`pyenv shell ${interpreter.envName.toCommandArgument()}`];
return [`pyenv shell ${interpreter.envName.toCommandArgumentForPythonExt()}`];
}
}
4 changes: 2 additions & 2 deletions src/client/common/terminal/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ export class TerminalHelper implements ITerminalHelper {
terminalShellType === TerminalShellType.powershell ||
terminalShellType === TerminalShellType.powershellCore;
const commandPrefix = isPowershell ? '& ' : '';
const formattedArgs = args.map((a) => a.toCommandArgument());
const formattedArgs = args.map((a) => a.toCommandArgumentForPythonExt());

return `${commandPrefix}${command.fileToCommandArgument()} ${formattedArgs.join(' ')}`.trim();
return `${commandPrefix}${command.fileToCommandArgumentForPythonExt()} ${formattedArgs.join(' ')}`.trim();
}
public async getEnvironmentActivationCommands(
terminalShellType: TerminalShellType,
Expand Down
35 changes: 1 addition & 34 deletions src/client/debugger/extension/adapter/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import { IApplicationShell } from '../../../common/application/types';
import { EXTENSION_ROOT_DIR } from '../../../constants';
import { IInterpreterService } from '../../../interpreter/contracts';
import { traceLog, traceVerbose } from '../../../logging';
import { Conda } from '../../../pythonEnvironments/common/environmentManagers/conda';
import { EnvironmentType, PythonEnvironment } from '../../../pythonEnvironments/info';
import { PythonEnvironment } from '../../../pythonEnvironments/info';
import { sendTelemetryEvent } from '../../../telemetry';
import { EventName } from '../../../telemetry/constants';
import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
Expand Down Expand Up @@ -143,40 +142,8 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
return this.getExecutableCommand(interpreters[0]);
}

private async getCondaCommand(): Promise<Conda | undefined> {
const condaCommand = await Conda.getConda();
const isCondaRunSupported = await condaCommand?.isCondaRunSupported();
return isCondaRunSupported ? condaCommand : undefined;
}

private async getExecutableCommand(interpreter: PythonEnvironment | undefined): Promise<string[]> {
if (interpreter) {
if (interpreter.envType === EnvironmentType.Conda) {
const condaCommand = await this.getCondaCommand();
if (condaCommand) {
if (interpreter.envName) {
return [
condaCommand.command,
'run',
'-n',
interpreter.envName,
'--no-capture-output',
'--live-stream',
'python',
];
} else if (interpreter.envPath) {
return [
condaCommand.command,
'run',
'-p',
interpreter.envPath,
'--no-capture-output',
'--live-stream',
'python',
];
}
}
}
return interpreter.path.length > 0 ? [interpreter.path] : [];
}
return [];
Expand Down
7 changes: 6 additions & 1 deletion src/client/debugger/extension/adapter/remoteLaunchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ type RemoteDebugOptions = {

export function getDebugpyLauncherArgs(options: RemoteDebugOptions, debuggerPath: string = pathToDebugger) {
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait-for-client'] : [];
return [debuggerPath.fileToCommandArgument(), '--listen', `${options.host}:${options.port}`, ...waitArgs];
return [
debuggerPath.fileToCommandArgumentForPythonExt(),
'--listen',
`${options.host}:${options.port}`,
...waitArgs,
];
}

export function getDebugpyPackagePath(): string {
Expand Down
4 changes: 2 additions & 2 deletions src/client/interpreter/activation/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
let command: string | undefined;
let [args, parse] = internalScripts.printEnvVariables();
args.forEach((arg, i) => {
args[i] = arg.toCommandArgument();
args[i] = arg.toCommandArgumentForPythonExt();
});
interpreter = interpreter ?? (await this.interpreterService.getActiveInterpreter(resource));
if (interpreter?.envType === EnvironmentType.Conda) {
Expand All @@ -185,7 +185,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi
});
if (pythonArgv) {
// Using environment prefix isn't needed as the marker script already takes care of it.
command = [...pythonArgv, ...args].map((arg) => arg.toCommandArgument()).join(' ');
command = [...pythonArgv, ...args].map((arg) => arg.toCommandArgumentForPythonExt()).join(' ');
}
}
if (!command) {
Expand Down
2 changes: 1 addition & 1 deletion src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const ICondaService = Symbol('ICondaService');
* Interface carries the properties which are not available via the discovery component interface.
*/
export interface ICondaService {
getCondaFile(): Promise<string>;
getCondaFile(forShellExecution?: boolean): Promise<string>;
isCondaAvailable(): Promise<boolean>;
getCondaVersion(): Promise<SemVer | undefined>;
getInterpreterPathForEnvironment(condaEnv: CondaEnvironmentInfo): Promise<string | undefined>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async function buildEnvironmentInfoUsingCondaRun(env: PythonEnvInfo): Promise<In
if (!condaEnv) {
return undefined;
}
const python = await conda?.getRunPythonArgs(condaEnv);
const python = await conda?.getRunPythonArgs(condaEnv, true);
if (!python) {
return undefined;
}
Expand Down
5 changes: 4 additions & 1 deletion src/client/pythonEnvironments/base/info/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ export async function getInterpreterInfo(
const argv = [info.command, ...info.args];

// Concat these together to make a set of quoted strings
const quoted = argv.reduce((p, c) => (p ? `${p} ${c.toCommandArgument()}` : `${c.toCommandArgument()}`), '');
const quoted = argv.reduce(
(p, c) => (p ? `${p} ${c.toCommandArgumentForPythonExt()}` : `${c.toCommandArgumentForPythonExt()}`),
'',
);

// Try shell execing the command, followed by the arguments. This will make node kill the process if it
// takes too long.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class PythonEnvsResolver implements IResolvingLocator {
);
} else if (seen[event.index] !== undefined) {
const old = seen[event.index];
seen[event.index] = await resolveBasicEnv(event.update);
seen[event.index] = await resolveBasicEnv(event.update, true);
didUpdate.fire({ old, index: event.index, update: seen[event.index] });
this.resolveInBackground(event.index, state, didUpdate, seen).ignoreErrors();
} else {
Expand All @@ -92,7 +92,8 @@ export class PythonEnvsResolver implements IResolvingLocator {

let result = await iterator.next();
while (!result.done) {
const currEnv = await resolveBasicEnv(result.value);
// Use cache from the current refresh where possible.
const currEnv = await resolveBasicEnv(result.value, true);
seen.push(currEnv);
yield currEnv;
this.resolveInBackground(seen.indexOf(currEnv), state, didUpdate, seen).ignoreErrors();
Expand Down
Loading