diff --git a/news/1 Enhancements/7696.md b/news/1 Enhancements/7696.md new file mode 100644 index 000000000000..ce5bf882046c --- /dev/null +++ b/news/1 Enhancements/7696.md @@ -0,0 +1 @@ +Add support for conda run without output, using `--no-capture-output` flag. diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts index 9364f1d90d8c..786adb5bc0c5 100644 --- a/src/client/common/installer/productInstaller.ts +++ b/src/client/common/installer/productInstaller.ts @@ -94,6 +94,7 @@ abstract class BaseInstaller { resource?: InterpreterUri, cancel?: CancellationToken, flags?: ModuleInstallFlags, + bypassCondaExecution?: boolean, ): Promise { if (product === Product.unittest) { return InstallerResponse.Installed; @@ -113,7 +114,7 @@ abstract class BaseInstaller { .installModule(product, resource, cancel, flags) .catch((ex) => traceError(`Error in installing the product '${ProductNames.get(product)}', ${ex}`)); - return this.isInstalled(product, resource).then((isInstalled) => { + return this.isInstalled(product, resource, bypassCondaExecution).then((isInstalled) => { sendTelemetryEvent(EventName.PYTHON_INSTALL_PACKAGE, undefined, { installer: installer.displayName, productName: ProductNames.get(product), @@ -178,7 +179,11 @@ abstract class BaseInstaller { } } - public async isInstalled(product: Product, resource?: InterpreterUri): Promise { + public async isInstalled( + product: Product, + resource?: InterpreterUri, + bypassCondaExecution?: boolean, + ): Promise { if (product === Product.unittest) { return true; } @@ -191,7 +196,12 @@ abstract class BaseInstaller { if (isModule) { const pythonProcess = await this.serviceContainer .get(IPythonExecutionFactory) - .createActivatedEnvironment({ resource: uri, interpreter, allowEnvironmentFetchExceptions: true }); + .createActivatedEnvironment({ + resource: uri, + interpreter, + allowEnvironmentFetchExceptions: true, + bypassCondaExecution, + }); return pythonProcess.isModuleInstalled(executableName); } const process = await this.serviceContainer.get(IProcessServiceFactory).create(uri); @@ -595,12 +605,17 @@ export class ProductInstaller implements IInstaller { resource?: InterpreterUri, cancel?: CancellationToken, flags?: ModuleInstallFlags, + bypassCondaExecution?: boolean, ): Promise { - return this.createInstaller(product).install(product, resource, cancel, flags); + return this.createInstaller(product).install(product, resource, cancel, flags, bypassCondaExecution); } - public async isInstalled(product: Product, resource?: InterpreterUri): Promise { - return this.createInstaller(product).isInstalled(product, resource); + public async isInstalled( + product: Product, + resource?: InterpreterUri, + bypassCondaExecution?: boolean, + ): Promise { + return this.createInstaller(product).isInstalled(product, resource, bypassCondaExecution); } // eslint-disable-next-line class-methods-use-this diff --git a/src/client/common/process/pythonEnvironment.ts b/src/client/common/process/pythonEnvironment.ts index 0c4a54cc5fd5..14828b9fd120 100644 --- a/src/client/common/process/pythonEnvironment.ts +++ b/src/client/common/process/pythonEnvironment.ts @@ -145,15 +145,11 @@ export function createCondaEnv( } else { runArgs.push('-n', condaInfo.name); } - const pythonArgv = [condaFile, ...runArgs, 'python']; + const pythonArgv = [condaFile, ...runArgs, '--no-capture-output', 'python']; const deps = createDeps( async (filename) => fs.pathExists(filename), pythonArgv, - - // TODO: Use pythonArgv here once 'conda run' can be - // run without buffering output. - // See https://github.com/microsoft/vscode-python/issues/8473. - undefined, + pythonArgv, (file, args, opts) => procs.exec(file, args, opts), (command, opts) => procs.shellExec(command, opts), ); diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index f48a526c23bb..d89ef9f14b26 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -29,8 +29,8 @@ import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelectio import { sleep } from '../utils/async'; import { traceError } from '../../logging'; -// Minimum version number of conda required to be able to use 'conda run' -export const CONDA_RUN_VERSION = '4.6.0'; +// 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 { @@ -83,6 +83,14 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { } const processService: IProcessService = await this.processServiceFactory.create(options.resource); + // Allow parts of the code to ignore conda run. + if (!options.bypassCondaExecution) { + const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService); + if (condaExecutionService) { + return condaExecutionService; + } + } + const windowsStoreInterpreterCheck = this.pyenvs.isWindowsStoreInterpreter.bind(this.pyenvs); return createPythonService( @@ -108,6 +116,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { return this.create({ resource: options.resource, pythonPath: options.interpreter ? options.interpreter.path : undefined, + bypassCondaExecution: options.bypassCondaExecution, }); } const pythonPath = options.interpreter @@ -117,11 +126,17 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { processService.on('exec', this.logger.logProcess.bind(this.logger)); this.disposables.push(processService); + // Allow parts of the code to ignore conda run. + if (!options.bypassCondaExecution) { + const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService); + if (condaExecutionService) { + return condaExecutionService; + } + } + return createPythonService(pythonPath, processService, this.fileSystem); } - // Not using this function for now because there are breaking issues with conda run (conda 4.8, PVSC 2020.1). - // See https://github.com/microsoft/vscode-python/issues/9490 public async createCondaExecutionService( pythonPath: string, processService?: IProcessService, @@ -144,6 +159,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { procService.on('exec', this.logger.logProcess.bind(this.logger)); this.disposables.push(procService); } + return createPythonService( pythonPath, procService, diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index 2f22ea013261..13e7911e3ac3 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -65,6 +65,7 @@ export const IPythonExecutionFactory = Symbol('IPythonExecutionFactory'); export type ExecutionFactoryCreationOptions = { resource?: Uri; pythonPath?: string; + bypassCondaExecution?: boolean; }; export type ExecutionFactoryCreateWithEnvironmentOptions = { resource?: Uri; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index f71ebf890645..e0826c433f41 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -121,8 +121,9 @@ export interface IInstaller { resource?: InterpreterUri, cancel?: CancellationToken, flags?: ModuleInstallFlags, + bypassCondaExecution?: boolean, ): Promise; - isInstalled(product: Product, resource?: InterpreterUri): Promise; + isInstalled(product: Product, resource?: InterpreterUri, bypassCondaExecution?: boolean): Promise; isProductVersionCompatible( product: Product, semVerRequirement: string, diff --git a/src/client/linters/flake8.ts b/src/client/linters/flake8.ts index 3ad39fd1faf4..c034cfb60639 100644 --- a/src/client/linters/flake8.ts +++ b/src/client/linters/flake8.ts @@ -14,7 +14,7 @@ export class Flake8 extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { const messages = await this.run( - ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], + ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation, ); diff --git a/src/client/linters/pycodestyle.ts b/src/client/linters/pycodestyle.ts index 6ef61b0ccdbe..c7f83a8ab974 100644 --- a/src/client/linters/pycodestyle.ts +++ b/src/client/linters/pycodestyle.ts @@ -14,7 +14,7 @@ export class Pycodestyle extends BaseLinter { protected async runLinter(document: TextDocument, cancellation: CancellationToken): Promise { const messages = await this.run( - ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], + ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', document.uri.fsPath], document, cancellation, ); diff --git a/src/test/common/installer.test.ts b/src/test/common/installer.test.ts index 0309b0df7bf7..880e7d486b5d 100644 --- a/src/test/common/installer.test.ts +++ b/src/test/common/installer.test.ts @@ -296,7 +296,7 @@ suite('Installer', () => { } callback({ stdout: '' }); }); - await installer.isInstalled(product, resource); + await installer.isInstalled(product, resource, true); await checkInstalledDef.promise; } getNamesAndValues(Product).forEach((prod) => { @@ -333,7 +333,7 @@ suite('Installer', () => { checkInstalledDef.resolve(); } }); - await installer.install(product); + await installer.install(product, undefined, undefined, undefined, true); await checkInstalledDef.promise; } getNamesAndValues(Product).forEach((prod) => { diff --git a/src/test/common/process/pythonEnvironment.unit.test.ts b/src/test/common/process/pythonEnvironment.unit.test.ts index c446e44e368e..a0ed2c2cde2d 100644 --- a/src/test/common/process/pythonEnvironment.unit.test.ts +++ b/src/test/common/process/pythonEnvironment.unit.test.ts @@ -295,8 +295,8 @@ suite('CondaEnvironment', () => { expect(result).to.deep.equal({ command: condaFile, - args: ['run', '-n', condaInfo.name, 'python', ...args], - python: [condaFile, 'run', '-n', condaInfo.name, 'python'], + args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args], + python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'], pythonExecutable: 'python', }); }); @@ -309,15 +309,20 @@ suite('CondaEnvironment', () => { expect(result).to.deep.equal({ command: condaFile, - args: ['run', '-p', condaInfo.path, 'python', ...args], - python: [condaFile, 'run', '-p', condaInfo.path, 'python'], + args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args], + python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'], pythonExecutable: 'python', }); }); - test('getExecutionObservableInfo with a named environment should return execution info using pythonPath only', () => { - const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath }; + test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', () => { 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', + }; const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object); const result = env.getExecutionObservableInfo(args); @@ -325,9 +330,14 @@ suite('CondaEnvironment', () => { expect(result).to.deep.equal(expected); }); - test('getExecutionObservableInfo with a non-named environment should return execution info using pythonPath only', () => { - const expected = { command: pythonPath, args, python: [pythonPath], pythonExecutable: pythonPath }; + test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', () => { 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', + }; const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object); const result = env.getExecutionObservableInfo(args); diff --git a/src/test/common/process/pythonExecutionFactory.unit.test.ts b/src/test/common/process/pythonExecutionFactory.unit.test.ts index 4177bbc5837c..d9db9b03abbb 100644 --- a/src/test/common/process/pythonExecutionFactory.unit.test.ts +++ b/src/test/common/process/pythonExecutionFactory.unit.test.ts @@ -257,9 +257,7 @@ suite('Process - PythonExecutionFactory', () => { assert.strictEqual(createInvoked, false); }); - test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () { - return this.skip(); - + test('Ensure `create` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => { const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); @@ -284,9 +282,7 @@ suite('Process - PythonExecutionFactory', () => { verify(condaService.getCondaFile()).once(); }); - test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () { - return this.skip(); - + test('Ensure `create` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => { const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); when(processFactory.create(resource)).thenResolve(processService.object); @@ -305,9 +301,7 @@ suite('Process - PythonExecutionFactory', () => { verify(condaService.getCondaFile()).once(); }); - test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async function () { - return this.skip(); - + test('Ensure `createActivatedEnvironment` returns a CondaExecutionService instance if createCondaExecutionService() returns a valid object', async () => { const pythonPath = 'path/to/python'; const pythonSettings = mock(PythonSettings); @@ -336,9 +330,7 @@ suite('Process - PythonExecutionFactory', () => { } }); - test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async function () { - return this.skip(); - + test('Ensure `createActivatedEnvironment` returns a PythonExecutionService instance if createCondaExecutionService() returns undefined', async () => { let createInvoked = false; const pythonPath = 'path/to/python'; const mockExecService = 'mockService'; diff --git a/src/test/format/extension.sort.test.ts b/src/test/format/extension.sort.test.ts index 14cfb501a8f8..e8d5601c92d0 100644 --- a/src/test/format/extension.sort.test.ts +++ b/src/test/format/extension.sort.test.ts @@ -107,7 +107,7 @@ suite('Sorting', () => { await window.showTextDocument(textDocument); await commands.executeCommand(Commands.Sort_Imports); assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed'); - }); + }).timeout(TEST_TIMEOUT * 3); test('With Config', async () => { const textDocument = await workspace.openTextDocument(fileToFormatWithConfig); @@ -130,7 +130,7 @@ suite('Sorting', () => { await window.showTextDocument(textDocument); await commands.executeCommand(Commands.Sort_Imports); assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed'); - }); + }).timeout(TEST_TIMEOUT * 3); test('With Changes and Config in Args', async () => { await updateSetting( @@ -164,7 +164,7 @@ suite('Sorting', () => { const originalContent = textDocument.getText(); await commands.executeCommand(Commands.Sort_Imports); assert.notStrictEqual(originalContent, textDocument.getText(), 'Contents have not changed'); - }).timeout(TEST_TIMEOUT * 2); + }).timeout(TEST_TIMEOUT * 3); test('With Changes and Config implicit from cwd', async () => { const textDocument = await workspace.openTextDocument(fileToFormatWithConfig); diff --git a/src/test/linters/lint.args.test.ts b/src/test/linters/lint.args.test.ts index b9908900d449..cf2a000720d0 100644 --- a/src/test/linters/lint.args.test.ts +++ b/src/test/linters/lint.args.test.ts @@ -142,12 +142,12 @@ suite('Linting - Arguments', () => { } test('Flake8', async () => { const linter = new Flake8(serviceContainer); - const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; + const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; await testLinter(linter, expectedArgs); }); test('Pycodestyle', async () => { const linter = new Pycodestyle(serviceContainer); - const expectedArgs = ['--format=%(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; + const expectedArgs = ['--format= %(row)d,%(col)d,%(code).1s,%(code)s:%(text)s', fileUri.fsPath]; await testLinter(linter, expectedArgs); }); test('Prospector', async () => { diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index e0fc71aafa89..b029773c0d40 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -625,6 +625,11 @@ class TestFixture extends BaseTestFixture { const serviceContainer = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const configService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const processLogger = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + const componentAdapter = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + componentAdapter + .setup((c) => c.getCondaEnvironment(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(undefined)); + const filesystem = new FileSystem(); processLogger .setup((p) => p.logProcess(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) @@ -637,6 +642,9 @@ class TestFixture extends BaseTestFixture { serviceContainer .setup((s) => s.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny())) .returns(() => filesystem); + serviceContainer + .setup((s) => s.get(TypeMoq.It.isValue(IComponentAdapter), TypeMoq.It.isAny())) + .returns(() => componentAdapter.object); const platformService = new PlatformService();