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

DonJayamanne · web-flow · commit 4a1a7efe0828 · 2018-10-04T16:56:32.000-07:00
For #1882
diff --git a/src/client/common/logger.ts b/src/client/common/logger.ts
@@ -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}`);
 }
 
diff --git a/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts
@@ -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';
@@ -70,7 +69,6 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
                 default:
                     return this.getUnixCommands(
                         envInfo.name,
-                        await condaService.getCondaVersion() || '',
                         await condaService.getCondaFile()
                     );
             }
@@ -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()}`
+        ];
     }
 }
diff --git a/src/client/extension.ts b/src/client/extension.ts
@@ -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[]>(() => [])
         ]);
diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts
@@ -1,3 +1,4 @@
+import { SemVer } from 'semver';
 import { CodeLensProvider, ConfigurationTarget, Disposable, Event, TextDocument, Uri } from 'vscode';
 import { InterpreterInfomation } from '../common/process/types';
 
@@ -36,6 +37,7 @@ export type CondaInfo = {
     'sys.prefix'?: string;
     'python_version'?: string;
     default_prefix?: string;
+    conda_version?: string;
 };
 
 export const ICondaService = Symbol('ICondaService');
@@ -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;
diff --git a/src/client/interpreter/locators/services/condaService.ts b/src/client/interpreter/locators/services/condaService.ts
@@ -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';
@@ -74,19 +76,37 @@ 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 | undefined;
+        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() : stdOut;
+        }
+        if (!versionString) {
+            return;
+        }
+        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');
     }
 
     /**
diff --git a/src/test/common/terminals/activation.conda.unit.test.ts b/src/test/common/terminals/activation.conda.unit.test.ts
@@ -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';
@@ -105,15 +106,19 @@ 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'));
-        const expected = ['source activate EnvA'];
+            .returns(() => Promise.resolve(parse('4.3.1', true)!));
+        const expected = [`source ${path.join(path.dirname(condaPath), 'activate').fileToCommandArgument()} EnvA`];
 
         const provider = new CondaActivationCommandProvider(serviceContainer.object);
         const activationCommands = await provider.getActivationCommands(undefined, TerminalShellType.bash);
@@ -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').fileToCommandArgument()} EnvA`];
 
         const provider = new CondaActivationCommandProvider(serviceContainer.object);
         const activationCommands = await provider.getActivationCommands(undefined, TerminalShellType.bash);
diff --git a/src/test/interpreters/condaService.unit.test.ts b/src/test/interpreters/condaService.unit.test.ts
@@ -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';
@@ -557,27 +558,30 @@ suite('Interpreters Conda Service', () => {
     });
 
     test('isAvailable will return true if conda is available', async () => {
-        processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.resolve({ stdout: 'xyz' }));
+        processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.resolve({ stdout: '4.4.4' }));
         const isAvailable = await condaService.isCondaAvailable();
         assert.equal(isAvailable, true);
     });
 
     test('isAvailable will return false if conda is not available', async () => {
+        condaService.getCondaFile = () => Promise.resolve('conda');
         processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.reject(new Error('not found')));
         fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false));
         fileSystem.setup(fs => fs.search(TypeMoq.It.isAny())).returns(() => Promise.resolve([]));
         platformService.setup(p => p.isWindows).returns(() => false);
-
+        condaService.getCondaInfo = () => Promise.reject('Not Found');
         const isAvailable = await condaService.isCondaAvailable();
         assert.equal(isAvailable, false);
     });
 
     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 }));
+        condaService.getCondaInfo = () => Promise.reject('Not Found');
+        condaService.getCondaFile = () => Promise.resolve('conda');
+        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 () => {

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ export function error(title: string = '', message: any) {`
`38`	`38`	new Logger().logError(`${title}, ${message}`);
`39`	`39`	`}`
`40`	`40`	`// tslint:disable-next-line:no-any`
`41`		`-export function warn(title: string = '', message: any) {`
	`41`	`+export function warn(title: string = '', message: any = '') {`
`42`	`42`	new Logger().logWarning(`${title}, ${message}`);
`43`	`43`	`}`
`44`	`44`