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

Activate conda envs with fully qualified paths to activate script #2801

Merged
merged 5 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/client/common/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function error(title: string = '', message: any) {
new Logger().logError(`${title}, ${message}`);
}
// tslint:disable-next-line:no-any
export function warn(title: string = '', message: any) {
export function warn(title: string = '', message: any = '') {
new Logger().logWarning(`${title}, ${message}`);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import { injectable } from 'inversify';
import * as path from 'path';
import { Uri } from 'vscode';
import { compareVersion } from '../../../../utils/version';
import { ICondaService } from '../../../interpreter/contracts';
import { IServiceContainer } from '../../../ioc/types';
import '../../extensions';
Expand Down Expand Up @@ -70,7 +69,6 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
default:
return this.getUnixCommands(
envInfo.name,
await condaService.getCondaVersion() || '',
await condaService.getCondaFile()
);
}
Expand Down Expand Up @@ -134,28 +132,12 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman

public async getUnixCommands(
envName: string,
version: string,
conda: string
): Promise<string[] | undefined> {
// Conda changed how activation works in the 4.4.0 release, so
// we accommodate the two ways distinctly.
if (version === '4.4.0' || compareVersion(version, '4.4.0') > 0) {
// Note that this requires the user to have already followed
// the conda instructions such that "conda" is on their
// $PATH. While we *could* use "source <abs-path-to-activate>"
// (after resolving the absolute path to the "activate"
// script), we're going to avoid operating contrary to
// conda's recommendations.
return [
`${conda.fileToCommandArgument()} activate ${envName.toCommandArgument()}`
];
} else {
// tslint:disable-next-line:no-suspicious-comment
// TODO: Handle pre-4.4 case where "activate" script not on $PATH.
// (Locate script next to "conda" binary and use absolute path.
return [
`source activate ${envName.toCommandArgument()}`
];
}
const condaDir = path.dirname(conda);
const activateFile = path.join(condaDir, 'activate');
return [
`source ${activateFile.fileToCommandArgument()} ${envName.toCommandArgument()}`
];
}
}
2 changes: 1 addition & 1 deletion src/client/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ async function sendStartupTelemetry(activatedPromise: Promise<void>, serviceCont
const condaLocator = serviceContainer.get<ICondaService>(ICondaService);
const interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
const [condaVersion, interpreter, interpreters] = await Promise.all([
condaLocator.getCondaVersion().catch(() => undefined),
condaLocator.getCondaVersion().then(ver => ver ? ver.raw : '').catch<string>(() => ''),
interpreterService.getActiveInterpreter().catch<PythonInterpreter | undefined>(() => undefined),
interpreterService.getInterpreters().catch<PythonInterpreter[]>(() => [])
]);
Expand Down
4 changes: 3 additions & 1 deletion src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SemVer } from 'semver';
import { CodeLensProvider, ConfigurationTarget, Disposable, Event, TextDocument, Uri } from 'vscode';
import { InterpreterInfomation } from '../common/process/types';

Expand Down Expand Up @@ -36,6 +37,7 @@ export type CondaInfo = {
'sys.prefix'?: string;
'python_version'?: string;
default_prefix?: string;
conda_version?: string;
};

export const ICondaService = Symbol('ICondaService');
Expand All @@ -44,7 +46,7 @@ export interface ICondaService {
readonly condaEnvironmentsFile: string | undefined;
getCondaFile(): Promise<string>;
isCondaAvailable(): Promise<boolean>;
getCondaVersion(): Promise<string | undefined>;
getCondaVersion(): Promise<SemVer | undefined>;
getCondaInfo(): Promise<CondaInfo | undefined>;
getCondaEnvironments(ignoreCache: boolean): Promise<({ name: string; path: string }[]) | undefined>;
getInterpreterPath(condaEnvironmentPath: string): string;
Expand Down
29 changes: 23 additions & 6 deletions src/client/interpreter/locators/services/condaService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { inject, injectable, named, optional } from 'inversify';
import * as path from 'path';
import { parse, SemVer } from 'semver';
import { compareVersion } from '../../../../utils/version';
import { warn } from '../../../common/logger';
import { IFileSystem, IPlatformService } from '../../../common/platform/types';
import { IProcessServiceFactory } from '../../../common/process/types';
import { IConfigurationService, ILogger, IPersistentStateFactory } from '../../../common/types';
Expand Down Expand Up @@ -71,19 +73,34 @@ export class CondaService implements ICondaService {
return this.isAvailable;
}
return this.getCondaVersion()
.then(version => this.isAvailable = typeof version === 'string')
.then(version => this.isAvailable = version !== undefined)
.catch(() => this.isAvailable = false);
}

/**
* Return the conda version.
*/
public async getCondaVersion(): Promise<string | undefined> {
public async getCondaVersion(): Promise<SemVer | undefined> {
const processService = await this.processServiceFactory.create();
return this.getCondaFile()
.then(condaFile => processService.exec(condaFile, ['--version'], {}))
.then(result => result.stdout.trim())
.catch(() => undefined);
const info = await this.getCondaInfo().catch<CondaInfo | undefined>(() => undefined);
let versionString: string;
if (info && info.conda_version) {
versionString = info.conda_version;
} else {
const stdOut = await this.getCondaFile()
.then(condaFile => processService.exec(condaFile, ['--version'], {}))
.then(result => result.stdout.trim())
.catch<string | undefined>(() => undefined);

versionString = (stdOut && stdOut.startsWith('conda ')) ? stdOut.substring('conda '.length).trim() : '';
}
const version = parse(versionString, true);
if (version) {
return version;
}
// Use a bogus version, at least to indicate the fact that a version was returned.
warn(`Unable to parse Version of Conda, ${versionString}`);
return new SemVer('0.0.1');
}

/**
Expand Down
15 changes: 12 additions & 3 deletions src/test/common/terminals/activation.conda.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { expect } from 'chai';
import * as path from 'path';
import { parse } from 'semver';
import * as TypeMoq from 'typemoq';
import { Disposable } from 'vscode';
import '../../../client/common/extensions';
Expand Down Expand Up @@ -105,14 +106,18 @@ suite('Terminal Environment Activation conda', () => {
test('Conda activation on bash uses "source" before 4.4.0', async () => {
const envName = 'EnvA';
const pythonPath = 'python3';
const condaPath = path.join('a', 'b', 'c', 'conda');
platformService.setup(p => p.isWindows).returns(() => false);
condaService.reset();
condaService.setup(c => c.getCondaEnvironment(TypeMoq.It.isAny()))
.returns(() => Promise.resolve({
name: envName,
path: path.dirname(pythonPath)
}));
condaService.setup(c => c.getCondaFile())
.returns(() => Promise.resolve(condaPath));
condaService.setup(c => c.getCondaVersion())
.returns(() => Promise.resolve('4.3.1'));
.returns(() => Promise.resolve(parse('4.3.1', true)!));
const expected = ['source activate EnvA'];

const provider = new CondaActivationCommandProvider(serviceContainer.object);
Expand All @@ -124,15 +129,19 @@ suite('Terminal Environment Activation conda', () => {
test('Conda activation on bash uses "conda" after 4.4.0', async () => {
const envName = 'EnvA';
const pythonPath = 'python3';
const condaPath = path.join('a', 'b', 'c', 'conda');
platformService.setup(p => p.isWindows).returns(() => false);
condaService.reset();
condaService.setup(c => c.getCondaEnvironment(TypeMoq.It.isAny()))
.returns(() => Promise.resolve({
name: envName,
path: path.dirname(pythonPath)
}));
condaService.setup(c => c.getCondaFile())
.returns(() => Promise.resolve(condaPath));
condaService.setup(c => c.getCondaVersion())
.returns(() => Promise.resolve('4.4.0'));
const expected = [`${conda} activate EnvA`];
.returns(() => Promise.resolve(parse('4.4.0', true)!));
const expected = [`source ${path.join(path.dirname(condaPath), 'activate')} EnvA`];

const provider = new CondaActivationCommandProvider(serviceContainer.object);
const activationCommands = await provider.getActivationCommands(undefined, TerminalShellType.bash);
Expand Down
7 changes: 4 additions & 3 deletions src/test/interpreters/condaService.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as assert from 'assert';
import { expect } from 'chai';
import { EOL } from 'os';
import * as path from 'path';
import { parse } from 'semver';
import * as TypeMoq from 'typemoq';
import { FileSystem } from '../../client/common/platform/fileSystem';
import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
Expand Down Expand Up @@ -555,11 +556,11 @@ suite('Interpreters Conda Service', () => {
});

test('Version info from conda process will be returned in getCondaVersion', async () => {
const expectedVersion = new Date().toString();
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.resolve({ stdout: expectedVersion }));
const expectedVersion = parse('4.4.4')!.raw;
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.resolve({ stdout: '4.4.4' }));

const version = await condaService.getCondaVersion();
assert.equal(version, expectedVersion);
assert.equal(version!.raw, expectedVersion);
});

test('isCondaInCurrentPath will return true if conda is available', async () => {
Expand Down