Revert using conda run for conda environments for running python files (#18661)

Kartik Raj · web-flow · commit 054407df4bcd · 2022-03-17T22:19:10.000+05:30
* Revert "Use conda run for conda environments for running python files (#18520)" This reverts commit 8fc8648. * Revert "Fix unit tests related to terminal code execution (#18549)" This reverts commit c5e5979. * News entry
diff --git a/news/2 Fixes/18634.md b/news/2 Fixes/18634.md
@@ -0,0 +1 @@
+Fixes regression caused due to using `conda run` for executing files.
diff --git a/src/client/common/process/pythonEnvironment.ts b/src/client/common/process/pythonEnvironment.ts
@@ -149,13 +149,9 @@ export async function createCondaEnv(
     // These are used to generate the deps.
     procs: IProcessService,
     fs: IFileSystem,
-    executeAsAProcess?: boolean,
 ): Promise<PythonEnvironment | undefined> {
     const conda = await Conda.getConda();
-    const pythonArgv = await conda?.getRunPythonArgs(
-        { name: condaInfo.name, prefix: condaInfo.path },
-        executeAsAProcess,
-    );
+    const pythonArgv = await conda?.getRunPythonArgs({ name: condaInfo.name, prefix: condaInfo.path });
     if (!pythonArgv) {
         return undefined;
     }
diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts
@@ -77,11 +77,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         }
         const processService: IProcessService = await this.processServiceFactory.create(options.resource);
 
-        const condaExecutionService = await this.createCondaExecutionService(
-            pythonPath,
-            processService,
-            options.executeAsAProcess,
-        );
+        const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
         if (condaExecutionService) {
             return condaExecutionService;
         }
@@ -129,20 +125,13 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
     public async createCondaExecutionService(
         pythonPath: string,
         processService: IProcessService,
-        executeAsAProcess?: boolean,
     ): Promise<IPythonExecutionService | undefined> {
         const condaLocatorService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter);
         const [condaEnvironment] = await Promise.all([condaLocatorService.getCondaEnvironment(pythonPath)]);
         if (!condaEnvironment) {
             return undefined;
         }
-        const env = await createCondaEnv(
-            condaEnvironment,
-            pythonPath,
-            processService,
-            this.fileSystem,
-            executeAsAProcess,
-        );
+        const env = await createCondaEnv(condaEnvironment, pythonPath, processService, this.fileSystem);
         if (!env) {
             return undefined;
         }
diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts
@@ -65,10 +65,6 @@ export const IPythonExecutionFactory = Symbol('IPythonExecutionFactory');
 export type ExecutionFactoryCreationOptions = {
     resource?: Uri;
     pythonPath?: string;
-    /**
-     * Whether to execute using `NodeJS.child_process` API, considered `true` as default.
-     */
-    executeAsAProcess?: boolean;
 };
 export type ExecutionFactoryCreateWithEnvironmentOptions = {
     resource?: Uri;
diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts
@@ -405,7 +405,7 @@ export class Conda {
         return envList.find((e) => isParentPath(executable, e.prefix));
     }
 
-    public async getRunPythonArgs(env: CondaEnvInfo, executeAsAProcess = true): Promise<string[] | undefined> {
+    public async getRunPythonArgs(env: CondaEnvInfo): Promise<string[] | undefined> {
         const condaVersion = await this.getCondaVersion();
         if (condaVersion && lt(condaVersion, CONDA_RUN_VERSION)) {
             return undefined;
@@ -416,11 +416,7 @@ export class Conda {
         } else {
             args.push('-p', env.prefix);
         }
-        const pythonArgs = [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python'];
-        if (executeAsAProcess) {
-            pythonArgs.push(OUTPUT_MARKER_SCRIPT);
-        }
-        return pythonArgs;
+        return [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT];
     }
 
     /**
diff --git a/src/client/terminals/codeExecution/djangoShellCodeExecution.ts b/src/client/terminals/codeExecution/djangoShellCodeExecution.ts
@@ -9,7 +9,6 @@ import { Disposable, Uri } from 'vscode';
 import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IFileSystem, IPlatformService } from '../../common/platform/types';
-import { IPythonExecutionFactory } from '../../common/process/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import { copyPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
@@ -27,16 +26,8 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
         @inject(ICommandManager) commandManager: ICommandManager,
         @inject(IFileSystem) fileSystem: IFileSystem,
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
-        @inject(IPythonExecutionFactory) pythonExecutionFactory: IPythonExecutionFactory,
     ) {
-        super(
-            terminalServiceFactory,
-            configurationService,
-            workspace,
-            disposableRegistry,
-            platformService,
-            pythonExecutionFactory,
-        );
+        super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
         this.terminalTitle = 'Django Shell';
         disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));
     }
diff --git a/src/client/terminals/codeExecution/repl.ts b/src/client/terminals/codeExecution/repl.ts
@@ -7,7 +7,6 @@ import { inject, injectable } from 'inversify';
 import { Disposable } from 'vscode';
 import { IWorkspaceService } from '../../common/application/types';
 import { IPlatformService } from '../../common/platform/types';
-import { IPythonExecutionFactory } from '../../common/process/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import { TerminalCodeExecutionProvider } from './terminalCodeExecution';
@@ -20,16 +19,8 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
         @inject(IWorkspaceService) workspace: IWorkspaceService,
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
         @inject(IPlatformService) platformService: IPlatformService,
-        @inject(IPythonExecutionFactory) pythonExecutionFactory: IPythonExecutionFactory,
     ) {
-        super(
-            terminalServiceFactory,
-            configurationService,
-            workspace,
-            disposableRegistry,
-            platformService,
-            pythonExecutionFactory,
-        );
+        super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
         this.terminalTitle = 'REPL';
     }
 }
diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts
@@ -9,7 +9,6 @@ import { Disposable, Uri } from 'vscode';
 import { IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IPlatformService } from '../../common/platform/types';
-import { IPythonExecutionFactory } from '../../common/process/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
@@ -27,7 +26,6 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         @inject(IWorkspaceService) protected readonly workspace: IWorkspaceService,
         @inject(IDisposableRegistry) protected readonly disposables: Disposable[],
         @inject(IPlatformService) protected readonly platformService: IPlatformService,
-        @inject(IPythonExecutionFactory) private readonly pythonExecutionFactory: IPythonExecutionFactory,
     ) {}
 
     public async executeFile(file: Uri) {
@@ -63,12 +61,11 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
 
     public async getExecutableInfo(resource?: Uri, args: string[] = []): Promise<PythonExecInfo> {
         const pythonSettings = this.configurationService.getSettings(resource);
-        const executionFactory = await this.pythonExecutionFactory.create({ resource, executeAsAProcess: false });
-        const execInfo = executionFactory.getExecutionInfo();
-        const pythonArgs = execInfo.args;
-        const command = this.platformService.isWindows ? execInfo.command.replace(/\\/g, '/') : execInfo.command;
+        const command = this.platformService.isWindows
+            ? pythonSettings.pythonPath.replace(/\\/g, '/')
+            : pythonSettings.pythonPath;
         const launchArgs = pythonSettings.terminal.launchArgs;
-        return buildPythonExecInfo(command, [...pythonArgs, ...launchArgs, ...args]);
+        return buildPythonExecInfo(command, [...launchArgs, ...args]);
     }
 
     // Overridden in subclasses, see djangoShellCodeExecution.ts
diff --git a/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts b/src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts
@@ -10,11 +10,7 @@ import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../c
 import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
 import { createCondaEnv } from '../../../client/common/process/pythonEnvironment';
 import { createPythonProcessService } from '../../../client/common/process/pythonProcess';
-import {
-    IProcessService,
-    IPythonExecutionFactory,
-    IPythonExecutionService,
-} from '../../../client/common/process/types';
+import { IProcessService, IPythonExecutionFactory } from '../../../client/common/process/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../../client/common/terminal/types';
 import { IConfigurationService, IPythonSettings, ITerminalSettings } from '../../../client/common/types';
 import { DjangoShellCodeExecutionProvider } from '../../../client/terminals/codeExecution/djangoShellCodeExecution';
@@ -23,7 +19,6 @@ import { PYTHON_PATH } from '../../common';
 import { Conda, CONDA_RUN_VERSION } from '../../../client/pythonEnvironments/common/environmentManagers/conda';
 import { SemVer } from 'semver';
 import assert from 'assert';
-import { PythonExecInfo } from '../../../client/pythonEnvironments/exec';
 
 suite('Terminal - Django Shell Code Execution', () => {
     let executor: ICodeExecutionService;
@@ -62,7 +57,6 @@ suite('Terminal - Django Shell Code Execution', () => {
             commandManager.object,
             fileSystem.object,
             disposables,
-            pythonExecutionFactory.object,
         );
 
         terminalFactory.setup((f) => f.getTerminalService(TypeMoq.It.isAny())).returns(() => terminalService.object);
@@ -91,14 +85,7 @@ suite('Terminal - Django Shell Code Execution', () => {
         resource?: Uri,
     ) {
         platform.setup((p) => p.isWindows).returns(() => isWindows);
-        const executionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
-        pythonExecutionFactory
-            .setup((p) => p.create(TypeMoq.It.isAny()))
-            .returns(() => Promise.resolve(executionService.object));
-        executionService.setup((p) => (p as any).then).returns(() => undefined);
-        executionService
-            .setup((e) => e.getExecutionInfo())
-            .returns(() => (({ command: pythonPath, args: [] } as unknown) as PythonExecInfo));
+        settings.setup((s) => s.pythonPath).returns(() => pythonPath);
         terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs);
 
         const replCommandArgs = await (executor as DjangoShellCodeExecutionProvider).getExecutableInfo(resource);
@@ -217,14 +204,7 @@ suite('Terminal - Django Shell Code Execution', () => {
         condaEnv: { name: string; path: string },
         resource?: Uri,
     ) {
-        const executionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
-        pythonExecutionFactory
-            .setup((p) => p.create(TypeMoq.It.isAny()))
-            .returns(() => Promise.resolve(executionService.object));
-        executionService.setup((p) => (p as any).then).returns(() => undefined);
-        executionService
-            .setup((e) => e.getExecutionInfo())
-            .returns(() => (({ command: pythonPath, args: [] } as unknown) as PythonExecInfo));
+        settings.setup((s) => s.pythonPath).returns(() => pythonPath);
         terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs);
 
         const condaFile = 'conda';
diff --git a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fixes regression caused due to using `conda run` for executing files.
Original file line number	Diff line number	Diff line change
`@@ -405,7 +405,7 @@ export class Conda {`
`405`	`405`	`return envList.find((e) => isParentPath(executable, e.prefix));`
`406`	`406`	`}`
`407`	`407`
`408`		`- public async getRunPythonArgs(env: CondaEnvInfo, executeAsAProcess = true): Promise<string[] \| undefined> {`
	`408`	`+ public async getRunPythonArgs(env: CondaEnvInfo): Promise<string[] \| undefined> {`
`409`	`409`	`const condaVersion = await this.getCondaVersion();`
`410`	`410`	`if (condaVersion && lt(condaVersion, CONDA_RUN_VERSION)) {`
`411`	`411`	`return undefined;`
`@@ -416,11 +416,7 @@ export class Conda {`
`416`	`416`	`} else {`
`417`	`417`	`args.push('-p', env.prefix);`
`418`	`418`	`}`
`419`		`- const pythonArgs = [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python'];`
`420`		`- if (executeAsAProcess) {`
`421`		`- pythonArgs.push(OUTPUT_MARKER_SCRIPT);`
`422`		`- }`
`423`		`- return pythonArgs;`
	`419`	`+ return [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT];`
`424`	`420`	`}`
`425`	`421`
`426`	`422`	`/**`