Update python execution factory using conda.ts

src/client/common/process/pythonEnvironment.ts

@@ -2,16 +2,16 @@
 // Licensed under the MIT License.
 
 import { traceError, traceInfo } from '../../logging';
-import { CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
+import { Conda, CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
 import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
 import { InterpreterInformation } from '../../pythonEnvironments/info';
 import { getExecutablePath } from '../../pythonEnvironments/info/executable';
 import { getInterpreterInfo } from '../../pythonEnvironments/info/interpreter';
 import { IFileSystem } from '../platform/types';
 import * as internalPython from './internal/python';
-import { ExecutionResult, IProcessService, ShellOptions, SpawnOptions } from './types';
+import { ExecutionResult, IProcessService, IPythonEnvironment, ShellOptions, SpawnOptions } from './types';
 
-class PythonEnvironment {
+class PythonEnvironment implements IPythonEnvironment {
     private cachedExecutablePath: Map<string, Promise<string>> = new Map<string, Promise<string>>();
     private cachedInterpreterInformation: InterpreterInformation | undefined | null = null;
 
@@ -28,13 +28,13 @@ class PythonEnvironment {
         },
     ) {}
 
-    public getExecutionInfo(pythonArgs: string[] = []): PythonExecInfo {
+    public getExecutionInfo(pythonArgs: string[] = [], pythonExecutable?: string): PythonExecInfo {
         const python = this.deps.getPythonArgv(this.pythonPath);
-        return buildPythonExecInfo(python, pythonArgs);
+        return buildPythonExecInfo(python, pythonArgs, pythonExecutable);
     }
-    public getExecutionObservableInfo(pythonArgs: string[] = []): PythonExecInfo {
+    public getExecutionObservableInfo(pythonArgs: string[] = [], pythonExecutable?: string): PythonExecInfo {
         const python = this.deps.getObservablePythonArgv(this.pythonPath);
-        return buildPythonExecInfo(python, pythonArgs);
+        return buildPythonExecInfo(python, pythonArgs, pythonExecutable);
     }
 
     public async getInterpreterInformation(): Promise<InterpreterInformation | undefined> {
@@ -131,21 +131,18 @@ export function createPythonEnv(
     return new PythonEnvironment(pythonPath, deps);
 }
 
-export function createCondaEnv(
-    condaFile: string,
+export async function createCondaEnv(
     condaInfo: CondaEnvironmentInfo,
     pythonPath: string,
     // These are used to generate the deps.
     procs: IProcessService,
     fs: IFileSystem,
-): PythonEnvironment {
-    const runArgs = ['run'];
-    if (condaInfo.name === '') {
-        runArgs.push('-p', condaInfo.path);
-    } else {
-        runArgs.push('-n', condaInfo.name);
+): Promise<PythonEnvironment | undefined> {
+    const conda = await Conda.getConda();
+    const pythonArgv = await conda?.getRunPythonArgs({ name: condaInfo.name, prefix: condaInfo.path });
+    if (!pythonArgv) {
+        return undefined;
     }
-    const pythonArgv = [condaFile, ...runArgs, '--no-capture-output', 'python'];
     const deps = createDeps(
         async (filename) => fs.pathExists(filename),
         pythonArgv,

src/client/common/process/pythonExecutionFactory.ts

@@ -1,13 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { inject, injectable } from 'inversify';
-import { gte } from 'semver';
 
-import { Uri } from 'vscode';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
-import { IComponentAdapter, ICondaService } from '../../interpreter/contracts';
+import { IComponentAdapter } from '../../interpreter/contracts';
 import { IServiceContainer } from '../../ioc/types';
-import { CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
 import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { IFileSystem } from '../platform/types';
@@ -22,16 +19,14 @@ import {
     IProcessLogger,
     IProcessService,
     IProcessServiceFactory,
+    IPythonEnvironment,
     IPythonExecutionFactory,
     IPythonExecutionService,
 } from './types';
 import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelection/types';
 import { sleep } from '../utils/async';
 import { traceError } from '../../logging';
 
-// Minimum version number of conda required to be able to use 'conda run' and '--no-capture--output' option
-export const CONDA_RUN_VERSION = '4.9.0';
-
 @injectable()
 export class PythonExecutionFactory implements IPythonExecutionFactory {
     private readonly disposables: IDisposableRegistry;
@@ -45,7 +40,6 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         @inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
         @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
         @inject(IConfigurationService) private readonly configService: IConfigurationService,
-        @inject(ICondaService) private readonly condaService: ICondaService,
         @inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
         @inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
         @inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
@@ -90,13 +84,11 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
 
         const windowsStoreInterpreterCheck = this.pyenvs.isWindowsStoreInterpreter.bind(this.pyenvs);
 
-        return createPythonService(
-            pythonPath,
-            processService,
-            this.fileSystem,
-            undefined,
-            await windowsStoreInterpreterCheck(pythonPath),
-        );
+        const env = (await windowsStoreInterpreterCheck(pythonPath))
+            ? createWindowsStoreEnv(pythonPath, processService)
+            : createPythonEnv(pythonPath, processService, this.fileSystem);
+
+        return createPythonService(processService, env);
     }
 
     public async createActivatedEnvironment(
@@ -126,60 +118,28 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         if (condaExecutionService) {
             return condaExecutionService;
         }
-
-        return createPythonService(pythonPath, processService, this.fileSystem);
+        const env = createPythonEnv(pythonPath, processService, this.fileSystem);
+        return createPythonService(processService, env);
     }
 
     public async createCondaExecutionService(
         pythonPath: string,
-        processService?: IProcessService,
-        resource?: Uri,
+        processService: IProcessService,
     ): Promise<IPythonExecutionService | undefined> {
-        const processServicePromise = processService
-            ? Promise.resolve(processService)
-            : this.processServiceFactory.create(resource);
         const condaLocatorService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter);
-        const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([
-            this.condaService.getCondaVersion(),
-            condaLocatorService.getCondaEnvironment(pythonPath),
-            this.condaService.getCondaFile(),
-            processServicePromise,
-        ]);
-
-        if (condaVersion && gte(condaVersion, CONDA_RUN_VERSION) && condaEnvironment && condaFile && procService) {
-            // Add logging to the newly created process service
-            if (!processService) {
-                procService.on('exec', this.logger.logProcess.bind(this.logger));
-                this.disposables.push(procService);
-            }
-
-            return createPythonService(
-                pythonPath,
-                procService,
-                this.fileSystem,
-                // This is what causes a CondaEnvironment to be returned:
-                [condaFile, condaEnvironment],
-            );
+        const [condaEnvironment] = await Promise.all([condaLocatorService.getCondaEnvironment(pythonPath)]);
+        if (!condaEnvironment) {
+            return undefined;
         }
-
-        return Promise.resolve(undefined);
+        const env = await createCondaEnv(condaEnvironment, pythonPath, processService, this.fileSystem);
+        if (!env) {
+            return undefined;
+        }
+        return createPythonService(processService, env);
     }
 }
 
-function createPythonService(
-    pythonPath: string,
-    procService: IProcessService,
-    fs: IFileSystem,
-    conda?: [string, CondaEnvironmentInfo],
-    isWindowsStore?: boolean,
-): IPythonExecutionService {
-    let env = createPythonEnv(pythonPath, procService, fs);
-    if (conda) {
-        const [condaPath, condaInfo] = conda;
-        env = createCondaEnv(condaPath, condaInfo, pythonPath, procService, fs);
-    } else if (isWindowsStore) {
-        env = createWindowsStoreEnv(pythonPath, procService);
-    }
+function createPythonService(procService: IProcessService, env: IPythonEnvironment): IPythonExecutionService {
     const procs = createPythonProcessService(procService, env);
     return {
         getInterpreterInformation: () => env.getInterpreterInformation(),

src/client/common/process/pythonProcess.ts

@@ -5,7 +5,7 @@ import { PythonExecInfo } from '../../pythonEnvironments/exec';
 import { ErrorUtils } from '../errors/errorUtils';
 import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
 import * as internalPython from './internal/python';
-import { ExecutionResult, IProcessService, ObservableExecutionResult, SpawnOptions } from './types';
+import { ExecutionResult, IProcessService, IPythonEnvironment, ObservableExecutionResult, SpawnOptions } from './types';
 
 class PythonProcessService {
     constructor(
@@ -69,11 +69,7 @@ class PythonProcessService {
 export function createPythonProcessService(
     procs: IProcessService,
     // from PythonEnvironment:
-    env: {
-        getExecutionInfo(pythonArgs?: string[]): PythonExecInfo;
-        getExecutionObservableInfo(pythonArgs?: string[]): PythonExecInfo;
-        isModuleInstalled(moduleName: string): Promise<boolean>;
-    },
+    env: IPythonEnvironment,
 ) {
     const deps = {
         // from PythonService:

src/client/common/process/types.ts

@@ -82,8 +82,7 @@ export interface IPythonExecutionFactory {
     createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise<IPythonExecutionService>;
     createCondaExecutionService(
         pythonPath: string,
-        processService?: IProcessService,
-        resource?: Uri,
+        processService: IProcessService,
     ): Promise<IPythonExecutionService | undefined>;
 }
 export const IPythonExecutionService = Symbol('IPythonExecutionService');
@@ -102,6 +101,15 @@ export interface IPythonExecutionService {
     execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
 }
 
+export interface IPythonEnvironment {
+    getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
+    getExecutionObservableInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
+    getExecutablePath(): Promise<string>;
+    isModuleInstalled(moduleName: string): Promise<boolean>;
+    getModuleVersion(moduleName: string): Promise<string | undefined>;
+    getExecutionInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
+}
+
 export class StdErrError extends Error {
     constructor(message: string) {
         super(message);

src/test/common/process/pythonEnvironment.unit.test.ts

@@ -3,6 +3,7 @@
 
 import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
+import * as sinon from 'sinon';
 import { SemVer } from 'semver';
 import * as TypeMoq from 'typemoq';
 import { IFileSystem } from '../../../client/common/platform/types';
@@ -13,6 +14,8 @@ import {
 } from '../../../client/common/process/pythonEnvironment';
 import { IProcessService, StdErrError } from '../../../client/common/process/types';
 import { Architecture } from '../../../client/common/utils/platform';
+import { Conda } from '../../../client/pythonEnvironments/common/environmentManagers/conda';
+import { OUTPUT_MARKER_SCRIPT } from '../../../client/common/process/internal/scripts';
 
 use(chaiAsPromised);
 
@@ -275,64 +278,67 @@ suite('CondaEnvironment', () => {
     const condaFile = 'path/to/conda';
 
     setup(() => {
+        sinon.stub(Conda, 'getConda').resolves(new Conda(condaFile));
         processService = TypeMoq.Mock.ofType<IProcessService>(undefined, TypeMoq.MockBehavior.Strict);
         fileSystem = TypeMoq.Mock.ofType<IFileSystem>(undefined, TypeMoq.MockBehavior.Strict);
     });
 
-    test('getExecutionInfo with a named environment should return execution info using the environment name', () => {
+    teardown(() => sinon.restore());
+
+    test('getExecutionInfo with a named environment should return execution info using the environment name', async () => {
         const condaInfo = { name: 'foo', path: 'bar' };
-        const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
+        const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
 
-        const result = env.getExecutionInfo(args);
+        const result = env?.getExecutionInfo(args, pythonPath);
 
         expect(result).to.deep.equal({
             command: condaFile,
-            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
-            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
-            pythonExecutable: 'python',
+            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
+            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
+            pythonExecutable: pythonPath,
         });
     });
 
-    test('getExecutionInfo with a non-named environment should return execution info using the environment path', () => {
+    test('getExecutionInfo with a non-named environment should return execution info using the environment path', async () => {
         const condaInfo = { name: '', path: 'bar' };
-        const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
+        const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
 
-        const result = env.getExecutionInfo(args);
+        const result = env?.getExecutionInfo(args, pythonPath);
 
         expect(result).to.deep.equal({
             command: condaFile,
-            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
-            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
-            pythonExecutable: 'python',
+            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
+            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
+            pythonExecutable: pythonPath,
         });
     });
 
-    test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', () => {
+    test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', async () => {
         const condaInfo = { name: 'foo', path: 'bar' };
         const expected = {
             command: condaFile,
-            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
-            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
-            pythonExecutable: 'python',
+            args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
+            python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
+            pythonExecutable: pythonPath,
         };
-        const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
+        const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
 
-        const result = env.getExecutionObservableInfo(args);
+        const result = env?.getExecutionObservableInfo(args, pythonPath);
 
         expect(result).to.deep.equal(expected);
     });
 
-    test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', () => {
+    test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', async () => {
         const condaInfo = { name: '', path: 'bar' };
         const expected = {
             command: condaFile,
-            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
-            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
-            pythonExecutable: 'python',
+            args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
+            python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
+            pythonExecutable: pythonPath,
         };
-        const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
+        const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
 
-        const result = env.getExecutionObservableInfo(args);
+        const result = env?.getExecutionObservableInfo(args, pythonPath);
 
         expect(result).to.deep.equal(expected);
     });