Skip to content
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

#1944: Support a new "python.condaPath" setting, in case conda not on PATH. #2605

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/1944.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a new "python.condaPath" to use if conda not found on PATH.
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,12 @@
"description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.",
"scope": "resource"
},
"python.condaPath": {
"type": "string",
"default": "",
"description": "Path to the conda executable to use for activation (version 4.4+).",
"scope": "resource"
},
"python.sortImports.args": {
"type": "array",
"description": "Arguments passed in. Each argument is a separate item in the array.",
Expand Down
1 change: 1 addition & 0 deletions src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
public envFile = '';
public venvPath = '';
public venvFolders: string[] = [];
public condaPath = '';
public devOptions: string[] = [];
public linting!: ILintingSettings;
public formatting!: IFormattingSettings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
`& cmd /k "activate ${envInfo.name.toCommandArgument().replace(/"/g, '""')} & ${powershellExe}"`
];
} else if (targetShell === TerminalShellType.fish) {
const conda = await condaService.getCondaFile();
// https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28
return [`conda activate ${envInfo.name.toCommandArgument()}`];
return [`${conda.fileToCommandArgument()} activate ${envInfo.name.toCommandArgument()}`];
} else if (isWindows) {
return [`activate ${envInfo.name.toCommandArgument()}`];
} else {
Expand Down
1 change: 1 addition & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export interface IPythonSettings {
readonly pythonPath: string;
readonly venvPath: string;
readonly venvFolders: string[];
readonly condaPath: string;
readonly downloadLanguageServer: boolean;
readonly jediEnabled: boolean;
readonly jediPath: string;
Expand Down
8 changes: 7 additions & 1 deletion src/client/interpreter/locators/services/condaService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as path from 'path';
import { compareVersion } from '../../../../utils/version';
import { IFileSystem, IPlatformService } from '../../../common/platform/types';
import { IProcessServiceFactory } from '../../../common/process/types';
import { ILogger, IPersistentStateFactory } from '../../../common/types';
import { IConfigurationService, ILogger, IPersistentStateFactory } from '../../../common/types';
import { IServiceContainer } from '../../../ioc/types';
import { CondaInfo, ICondaService, IInterpreterLocatorService, InterpreterType, PythonInterpreter, WINDOWS_REGISTRY_SERVICE } from '../../contracts';
import { CondaHelper } from './condaHelper';
Expand Down Expand Up @@ -223,6 +223,12 @@ export class CondaService implements ICondaService {
* Return the path to the "conda file", if there is one (in known locations).
*/
private async getCondaFileImpl() {
const settings = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings();
const setting = settings.condaPath;
if (setting && setting !== '') {
return setting;
}

const isAvailable = await this.isCondaInCurrentPath();
if (isAvailable) {
return 'conda';
Expand Down
18 changes: 18 additions & 0 deletions src/test/common/terminals/activation.conda.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ suite('Terminal Environment Activation conda', () => {
let processService: TypeMoq.IMock<IProcessService>;
let procServiceFactory: TypeMoq.IMock<IProcessServiceFactory>;
let condaService: TypeMoq.IMock<ICondaService>;
let conda: string;

setup(() => {
conda = 'conda';
serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
disposables = [];
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDisposableRegistry), TypeMoq.It.isAny())).returns(() => disposables);
Expand All @@ -39,6 +41,8 @@ suite('Terminal Environment Activation conda', () => {
platformService = TypeMoq.Mock.ofType<IPlatformService>();
processService = TypeMoq.Mock.ofType<IProcessService>();
condaService = TypeMoq.Mock.ofType<ICondaService>();
condaService.setup(c => c.getCondaFile()).returns(() => Promise.resolve(conda));

processService.setup((x: any) => x.then).returns(() => undefined);
procServiceFactory = TypeMoq.Mock.ofType<IProcessServiceFactory>();
procServiceFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(processService.object));
Expand Down Expand Up @@ -73,6 +77,20 @@ suite('Terminal Environment Activation conda', () => {
expect(activationCommands).to.equal(undefined, 'Activation commands should be undefined');
});

test('Conda activation for fish escapes spaces in conda filename', async () => {
conda = 'path to conda';
const envName = 'EnvA';
const pythonPath = 'python3';
platformService.setup(p => p.isWindows).returns(() => false);
condaService.setup(c => c.getCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve({ name: envName, path: path.dirname(pythonPath) }));
const expected = ['"path to conda" activate EnvA'];

const provider = new CondaActivationCommandProvider(serviceContainer.object);
const activationCommands = await provider.getActivationCommands(undefined, TerminalShellType.fish);

expect(activationCommands).to.deep.equal(expected, 'Incorrect Activation command');
});

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);
Expand Down
27 changes: 26 additions & 1 deletion src/test/interpreters/condaService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as TypeMoq from 'typemoq';
import { FileSystem } from '../../client/common/platform/fileSystem';
import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
import { IProcessService, IProcessServiceFactory } from '../../client/common/process/types';
import { ILogger, IPersistentStateFactory } from '../../client/common/types';
import { IConfigurationService, ILogger, IPersistentStateFactory, IPythonSettings } from '../../client/common/types';
import { IInterpreterLocatorService, InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts';
import { CondaService } from '../../client/interpreter/locators/services/condaService';
import { IServiceContainer } from '../../client/ioc/types';
Expand Down Expand Up @@ -35,16 +35,22 @@ suite('Interpreters Conda Service', () => {
let platformService: TypeMoq.IMock<IPlatformService>;
let condaService: CondaService;
let fileSystem: TypeMoq.IMock<IFileSystem>;
let config: TypeMoq.IMock<IConfigurationService>;
let settings: TypeMoq.IMock<IPythonSettings>;
let registryInterpreterLocatorService: TypeMoq.IMock<IInterpreterLocatorService>;
let serviceContainer: TypeMoq.IMock<IServiceContainer>;
let procServiceFactory: TypeMoq.IMock<IProcessServiceFactory>;
let logger: TypeMoq.IMock<ILogger>;
let condaPathSetting: string;
setup(async () => {
condaPathSetting = '';
logger = TypeMoq.Mock.ofType<ILogger>();
processService = TypeMoq.Mock.ofType<IProcessService>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
registryInterpreterLocatorService = TypeMoq.Mock.ofType<IInterpreterLocatorService>();
fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
config = TypeMoq.Mock.ofType<IConfigurationService>();
settings = TypeMoq.Mock.ofType<IPythonSettings>();
procServiceFactory = TypeMoq.Mock.ofType<IProcessServiceFactory>();
processService.setup((x: any) => x.then).returns(() => undefined);
procServiceFactory.setup(p => p.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(processService.object));
Expand All @@ -54,6 +60,9 @@ suite('Interpreters Conda Service', () => {
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService), TypeMoq.It.isAny())).returns(() => platformService.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ILogger), TypeMoq.It.isAny())).returns(() => logger.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny())).returns(() => fileSystem.object);
serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IConfigurationService), TypeMoq.It.isAny())).returns(() => config.object);
config.setup(c => c.getSettings(TypeMoq.It.isValue(undefined))).returns(() => settings.object);
settings.setup(p => p.condaPath).returns(() => condaPathSetting);
condaService = new CondaService(serviceContainer.object, registryInterpreterLocatorService.object);

fileSystem.setup(fs => fs.arePathsSame(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p1, p2) => {
Expand Down Expand Up @@ -331,6 +340,22 @@ suite('Interpreters Conda Service', () => {
assert.equal(condaExe, 'conda', 'Failed to identify conda.exe');
});

test('Must use \'python.condaPath\' setting if set', async () => {
condaPathSetting = 'spam-spam-conda-spam-spam';
// We ensure that conda would otherwise be found.
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version'])))
.returns(() => Promise.resolve({ stdout: 'xyz' }))
.verifiable(TypeMoq.Times.never());

const condaExe = await condaService.getCondaFile();
assert.equal(condaExe, 'spam-spam-conda-spam-spam', 'Failed to identify conda.exe');
ericsnowcurrently marked this conversation as resolved.
Show resolved Hide resolved

// We should not try to call other unwanted methods.
processService.verifyAll();
platformService.verify(p => p.isWindows, TypeMoq.Times.never());
registryInterpreterLocatorService.verify(r => r.getInterpreters(TypeMoq.It.isAny()), TypeMoq.Times.never());
});

test('Must use \'conda\' if is available in the current path', async () => {
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']))).returns(() => Promise.resolve({ stdout: 'xyz' }));

Expand Down