From b22bc8170f68438c73339883148e78bfdb3d0a4b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 27 Apr 2018 11:40:35 -0700 Subject: [PATCH] Fix activation of environments --- news/2 Fixes/1520.md | 1 + news/2 Fixes/1534.md | 1 + .../commandPrompt.ts | 4 +-- .../condaActivationProvider.ts | 12 +++++-- .../activation.commandPrompt.test.ts | 8 ++--- .../common/terminals/activation.conda.test.ts | 34 +++++++++++++++---- 6 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 news/2 Fixes/1520.md create mode 100644 news/2 Fixes/1534.md diff --git a/news/2 Fixes/1520.md b/news/2 Fixes/1520.md new file mode 100644 index 000000000000..5f5d52ec9e44 --- /dev/null +++ b/news/2 Fixes/1520.md @@ -0,0 +1 @@ +Fixes the issue where Conda environments created using the latest version of Anaconda are not activated in Powershell. diff --git a/news/2 Fixes/1534.md b/news/2 Fixes/1534.md new file mode 100644 index 000000000000..c0e5b965116c --- /dev/null +++ b/news/2 Fixes/1534.md @@ -0,0 +1 @@ +Fix activation of environments with spaces in the python path when using Powershell. diff --git a/src/client/common/terminal/environmentActivationProviders/commandPrompt.ts b/src/client/common/terminal/environmentActivationProviders/commandPrompt.ts index b7d8f24d86b9..f9c8ca77311b 100644 --- a/src/client/common/terminal/environmentActivationProviders/commandPrompt.ts +++ b/src/client/common/terminal/environmentActivationProviders/commandPrompt.ts @@ -12,7 +12,7 @@ import { BaseActivationCommandProvider } from './baseActivationProvider'; @injectable() export class CommandPromptAndPowerShell extends BaseActivationCommandProvider { - constructor( @inject(IServiceContainer) serviceContainer: IServiceContainer) { + constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { super(serviceContainer); } public isShellSupported(targetShell: TerminalShellType): boolean { @@ -41,7 +41,7 @@ export class CommandPromptAndPowerShell extends BaseActivationCommandProvider { const powershellExe = targetShell === TerminalShellType.powershell ? 'powershell' : 'pwsh'; const activationCmd = scriptFile.fileToCommandArgument(); return [ - `& cmd /k "${activationCmd} & ${powershellExe}"` + `& cmd /k "${activationCmd.replace(/"/g, '""')} & ${powershellExe}"` ]; } else { // Powershell on non-windows os, we cannot execute the batch file. diff --git a/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts index 51cb12e88e7e..e3f73deecf90 100644 --- a/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts @@ -8,8 +8,7 @@ import { IServiceContainer } from '../../../ioc/types'; import '../../extensions'; import { IPlatformService } from '../../platform/types'; import { IConfigurationService } from '../../types'; -import { TerminalShellType } from '../types'; -import { ITerminalActivationCommandProvider } from '../types'; +import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; @injectable() export class CondaActivationCommandProvider implements ITerminalActivationCommandProvider { @@ -29,8 +28,15 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman const isWindows = this.serviceContainer.get(IPlatformService).isWindows; if (targetShell === TerminalShellType.powershell || targetShell === TerminalShellType.powershellCore) { + if (!isWindows) { + return; + } // https://github.com/conda/conda/issues/626 - return; + // On windows, the solution is to go into cmd, then run the batch (.bat) file and go back into powershell. + const powershellExe = targetShell === TerminalShellType.powershell ? 'powershell' : 'pwsh'; + return [ + `& cmd /k "activate ${envInfo.name.toCommandArgument().replace(/"/g, '""')} & ${powershellExe}"` + ]; } else if (targetShell === TerminalShellType.fish) { // https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28 return [`conda activate ${envInfo.name.toCommandArgument()}`]; diff --git a/src/test/common/terminals/activation.commandPrompt.test.ts b/src/test/common/terminals/activation.commandPrompt.test.ts index f6c7509ce00d..ce1942bc3b88 100644 --- a/src/test/common/terminals/activation.commandPrompt.test.ts +++ b/src/test/common/terminals/activation.commandPrompt.test.ts @@ -97,17 +97,17 @@ suite('Terminal Environment Activation (cmd/powershell)', () => { }); test('Ensure batch files are supported by powershell (on windows)', async () => { - const bash = new CommandPromptAndPowerShell(serviceContainer.object); + const batch = new CommandPromptAndPowerShell(serviceContainer.object); platform.setup(p => p.isWindows).returns(() => true); const pathToScriptFile = path.join(path.dirname(pythonPath), 'activate.bat'); fileSystem.setup(fs => fs.fileExistsAsync(TypeMoq.It.isValue(pathToScriptFile))).returns(() => Promise.resolve(true)); - const command = await bash.getActivationCommands(resource, TerminalShellType.powershell); + const command = await batch.getActivationCommands(resource, TerminalShellType.powershell); // Executing batch files from powershell requires going back to cmd, then into powershell const activationCommand = pathToScriptFile.fileToCommandArgument(); - const commands = [`& cmd /k "${activationCommand} & powershell"`]; + const commands = [`& cmd /k "${activationCommand.replace(/"/g, '""')} & powershell"`]; expect(command).to.be.deep.equal(commands, 'Invalid command'); }); @@ -122,7 +122,7 @@ suite('Terminal Environment Activation (cmd/powershell)', () => { // Executing batch files from powershell requires going back to cmd, then into powershell const activationCommand = pathToScriptFile.fileToCommandArgument(); - const commands = [`& cmd /k "${activationCommand} & pwsh"`]; + const commands = [`& cmd /k "${activationCommand.replace(/"/g, '""')} & pwsh"`]; expect(command).to.be.deep.equal(commands, 'Invalid command'); }); diff --git a/src/test/common/terminals/activation.conda.test.ts b/src/test/common/terminals/activation.conda.test.ts index c9570ef54d94..d1d868dcc44f 100644 --- a/src/test/common/terminals/activation.conda.test.ts +++ b/src/test/common/terminals/activation.conda.test.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { Disposable } from 'vscode'; import { EnumEx } from '../../../client/common/enumUtils'; +import '../../../client/common/extensions'; import { IFileSystem, IPlatformService } from '../../../client/common/platform/types'; import { IProcessService } from '../../../client/common/process/types'; import { CondaActivationCommandProvider } from '../../../client/common/terminal/environmentActivationProviders/condaActivationProvider'; @@ -67,29 +68,32 @@ suite('Terminal Environment Activation conda', () => { expect(activationCommands).to.equal(undefined, 'Activation commands should be undefined'); }); - async function expectNoCondaActivationCommandForPowershell(isWindows: boolean, isOsx: boolean, isLinux: boolean, pythonPath: string, shellType: TerminalShellType) { + async function expectNoCondaActivationCommandForPowershell(isWindows: boolean, isOsx: boolean, isLinux: boolean, pythonPath: string, shellType: TerminalShellType, hasSpaceInEnvironmentName = false) { terminalSettings.setup(t => t.activateEnvironment).returns(() => true); platformService.setup(p => p.isLinux).returns(() => isLinux); platformService.setup(p => p.isWindows).returns(() => isWindows); platformService.setup(p => p.isMac).returns(() => isOsx); condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); pythonSettings.setup(s => s.pythonPath).returns(() => pythonPath); - condaService.setup(c => c.getCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve({ name: 'EnvA', path: path.dirname(pythonPath) })); + const envName = hasSpaceInEnvironmentName ? 'EnvA' : 'Env A'; + condaService.setup(c => c.getCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve({ name: envName, path: path.dirname(pythonPath) })); const activationCommands = await new CondaActivationCommandProvider(serviceContainer.object).getActivationCommands(undefined, shellType); let expectedActivationCommamnd: string[] | undefined; switch (shellType) { case TerminalShellType.powershell: case TerminalShellType.powershellCore: { - expectedActivationCommamnd = undefined; + const powershellExe = shellType === TerminalShellType.powershell ? 'powershell' : 'pwsh'; + const envNameForCmd = envName.toCommandArgument().replace(/"/g, '""'); + expectedActivationCommamnd = isWindows ? [`& cmd /k \"activate ${envNameForCmd} & ${powershellExe}\"`] : undefined; break; } case TerminalShellType.fish: { - expectedActivationCommamnd = ['conda activate EnvA']; + expectedActivationCommamnd = [`conda activate ${envName.toCommandArgument()}`]; break; } default: { - expectedActivationCommamnd = isWindows ? ['activate EnvA'] : ['source activate EnvA']; + expectedActivationCommamnd = isWindows ? [`activate ${envName.toCommandArgument()}`] : [`source activate ${envName.toCommandArgument()}`]; break; } } @@ -101,16 +105,32 @@ suite('Terminal Environment Activation conda', () => { await expectNoCondaActivationCommandForPowershell(true, false, false, pythonPath, shellType.value); }); - test(`Conda activation command for shell ${shellType.name} on (windows)`, async () => { + test(`Conda activation command for shell ${shellType.name} on (linux)`, async () => { const pythonPath = path.join('users', 'xyz', '.conda', 'envs', 'enva', 'bin', 'python'); await expectNoCondaActivationCommandForPowershell(false, false, true, pythonPath, shellType.value); }); - test(`Conda activation command for shell ${shellType.name} on (linux)`, async () => { + test(`Conda activation command for shell ${shellType.name} on (mac)`, async () => { const pythonPath = path.join('users', 'xyz', '.conda', 'envs', 'enva', 'bin', 'python'); await expectNoCondaActivationCommandForPowershell(false, true, false, pythonPath, shellType.value); }); }); + EnumEx.getNamesAndValues(TerminalShellType).forEach(shellType => { + test(`Conda activation command for shell ${shellType.name} on (windows), containing spaces in environment name`, async () => { + const pythonPath = path.join('c', 'users', 'xyz', '.conda', 'envs', 'enva', 'python.exe'); + await expectNoCondaActivationCommandForPowershell(true, false, false, pythonPath, shellType.value, true); + }); + + test(`Conda activation command for shell ${shellType.name} on (linux), containing spaces in environment name`, async () => { + const pythonPath = path.join('users', 'xyz', '.conda', 'envs', 'enva', 'bin', 'python'); + await expectNoCondaActivationCommandForPowershell(false, false, true, pythonPath, shellType.value, true); + }); + + test(`Conda activation command for shell ${shellType.name} on (mac), containing spaces in environment name`, async () => { + const pythonPath = path.join('users', 'xyz', '.conda', 'envs', 'enva', 'bin', 'python'); + await expectNoCondaActivationCommandForPowershell(false, true, false, pythonPath, shellType.value, true); + }); + }); async function expectCondaActivationCommand(isWindows: boolean, isOsx: boolean, isLinux: boolean, pythonPath: string) { terminalSettings.setup(t => t.activateEnvironment).returns(() => true); platformService.setup(p => p.isLinux).returns(() => isLinux);