From f6e13383b24c4d1b2a72ad1066a4a1762fbc1055 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Sun, 19 Nov 2023 13:39:37 -0800 Subject: [PATCH] Use worker threads for fetching conda environments and interpreter related info (#22481) --- .github/workflows/build.yml | 2 +- .github/workflows/pr-check.yml | 4 +- .gitignore | 1 + .vscode/settings.json | 5 +- build/webpack/webpack.extension.config.js | 8 + package-lock.json | 31 +++ package.json | 1 + src/client/common/constants.ts | 3 +- src/client/common/process/proc.ts | 9 +- src/client/common/process/rawProcessApis.ts | 1 - src/client/common/process/types.ts | 3 +- src/client/common/process/worker/main.ts | 42 ++++ .../common/process/worker/plainExec.worker.ts | 16 ++ .../process/worker/rawProcessApiWrapper.ts | 26 +++ .../common/process/worker/shellExec.worker.ts | 16 ++ src/client/common/process/worker/types.ts | 38 ++++ .../process/worker/workerRawProcessApis.ts | 213 ++++++++++++++++++ src/client/common/terminal/shellDetector.ts | 5 +- .../shellDetectors/baseShellDetector.ts | 3 - .../shellDetectors/settingsShellDetector.ts | 2 - .../userEnvironmentShellDetector.ts | 2 - src/client/common/utils/decorators.ts | 3 +- src/client/interpreter/activation/service.ts | 1 - .../virtualEnvs/activatedEnvLaunch.ts | 5 +- src/client/pythonEnvironments/base/locator.ts | 2 +- .../composite/envsCollectionService.ts | 3 +- .../base/locators/lowLevel/condaLocator.ts | 9 +- .../lowLevel/windowsKnownPathsLocator.ts | 15 +- .../lowLevel/windowsRegistryLocator.ts | 7 +- .../base/locators/wrappers.ts | 11 +- .../common/environmentManagers/conda.ts | 26 ++- .../common/environmentManagers/pipenv.ts | 4 +- .../common/externalDependencies.ts | 34 +-- ...ryKeysWorker.ts => registryKeys.worker.ts} | 0 ...luesWorker.ts => registryValues.worker.ts} | 0 .../common/windowsRegistry.ts | 6 +- .../envCollectionActivation/service.ts | 2 + src/test/common/process/proc.exec.test.ts | 22 ++ .../lowLevel/condaLocator.testvirtualenvs.ts | 34 ++- .../lowLevel/condaLocator.unit.test.ts | 4 +- .../windowsRegistryLocator.testvirtualenvs.ts | 2 +- .../windowsRegistryLocator.unit.test.ts | 2 + .../environmentManagers/conda.unit.test.ts | 5 + 43 files changed, 537 insertions(+), 91 deletions(-) create mode 100644 src/client/common/process/worker/main.ts create mode 100644 src/client/common/process/worker/plainExec.worker.ts create mode 100644 src/client/common/process/worker/rawProcessApiWrapper.ts create mode 100644 src/client/common/process/worker/shellExec.worker.ts create mode 100644 src/client/common/process/worker/types.ts create mode 100644 src/client/common/process/worker/workerRawProcessApis.ts rename src/client/pythonEnvironments/common/{registryKeysWorker.ts => registryKeys.worker.ts} (100%) rename src/client/pythonEnvironments/common/{registryValuesWorker.ts => registryValues.worker.ts} (100%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d17d5fff4b92..2a155b5ebf98 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -282,7 +282,7 @@ jobs: shell: pwsh if: matrix.test-suite == 'venv' run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` + # 1. For `*.testvirtualenvs.test.ts` if ('${{ matrix.os }}' -match 'windows-latest') { $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 3283d3281e5b..eb8cce0bce39 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -255,7 +255,7 @@ jobs: shell: pwsh if: matrix.test-suite == 'venv' run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` + # 1. For `*.testvirtualenvs.test.ts` if ('${{ matrix.os }}' -match 'windows-latest') { $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda @@ -451,7 +451,7 @@ jobs: PYTHON_VIRTUAL_ENVS_LOCATION: './src/tmp/envPaths.json' shell: pwsh run: | - # 1. For `terminalActivation.testvirtualenvs.test.ts` + # 1. For `*.testvirtualenvs.test.ts` if ('${{ matrix.os }}' -match 'windows-latest') { $condaPythonPath = Join-Path -Path $Env:CONDA -ChildPath python.exe $condaExecPath = Join-Path -Path $Env:CONDA -ChildPath Scripts | Join-Path -ChildPath conda diff --git a/.gitignore b/.gitignore index 046d01588573..69166ccac1a4 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ log.log **/node_modules *.pyc *.vsix +envVars.txt **/.vscode/.ropeproject/** **/testFiles/**/.cache/** *.noseids diff --git a/.vscode/settings.json b/.vscode/settings.json index 3251f7efcad5..50864f0b5cd8 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -25,7 +25,8 @@ "[python]": { "editor.formatOnSave": true, "editor.codeActionsOnSave": { - "source.organizeImports.isort": true + "source.fixAll.eslint": "explicit", + "source.organizeImports.isort": "explicit" }, "editor.defaultFormatter": "ms-python.black-formatter", }, @@ -51,7 +52,7 @@ "prettier.printWidth": 120, "prettier.singleQuote": true, "editor.codeActionsOnSave": { - "source.fixAll.eslint": true + "source.fixAll.eslint": "explicit" }, "python.languageServer": "Default", "typescript.preferences.importModuleSpecifier": "relative", diff --git a/build/webpack/webpack.extension.config.js b/build/webpack/webpack.extension.config.js index a33508e5d96a..082ce52a4d32 100644 --- a/build/webpack/webpack.extension.config.js +++ b/build/webpack/webpack.extension.config.js @@ -19,6 +19,10 @@ const config = { target: 'node', entry: { extension: './src/client/extension.ts', + 'shellExec.worker': './src/client/common/process/worker/shellExec.worker.ts', + 'plainExec.worker': './src/client/common/process/worker/plainExec.worker.ts', + 'registryKeys.worker': 'src/client/pythonEnvironments/common/registryKeys.worker.ts', + 'registryValues.worker': 'src/client/pythonEnvironments/common/registryValues.worker.ts', }, devtool: 'source-map', node: { @@ -51,6 +55,10 @@ const config = { }, ], }, + { + test: /\.worker\.js$/, + use: { loader: 'worker-loader' }, + }, ], }, externals: [ diff --git a/package-lock.json b/package-lock.json index d2ab2fbb1953..adae0dddc8b6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -113,6 +113,7 @@ "webpack-merge": "^5.8.0", "webpack-node-externals": "^3.0.0", "webpack-require-from": "^1.8.6", + "worker-loader": "^3.0.8", "yargs": "^15.3.1" }, "engines": { @@ -14966,6 +14967,26 @@ "wipe-node-cache": "^2.1.0" } }, + "node_modules/worker-loader": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-3.0.8.tgz", + "integrity": "sha512-XQyQkIFeRVC7f7uRhFdNMe/iJOdO6zxAaR3EWbDp45v3mDhrTi+++oswKNxShUNjPC/1xUp5DB29YKLhFo129g==", + "dev": true, + "dependencies": { + "loader-utils": "^2.0.0", + "schema-utils": "^3.0.0" + }, + "engines": { + "node": ">= 10.13.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/webpack" + }, + "peerDependencies": { + "webpack": "^4.0.0 || ^5.0.0" + } + }, "node_modules/workerpool": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.0.tgz", @@ -26974,6 +26995,16 @@ "wipe-node-cache": "^2.1.0" } }, + "worker-loader": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/worker-loader/-/worker-loader-3.0.8.tgz", + "integrity": "sha512-XQyQkIFeRVC7f7uRhFdNMe/iJOdO6zxAaR3EWbDp45v3mDhrTi+++oswKNxShUNjPC/1xUp5DB29YKLhFo129g==", + "dev": true, + "requires": { + "loader-utils": "^2.0.0", + "schema-utils": "^3.0.0" + } + }, "workerpool": { "version": "6.2.0", "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.0.tgz", diff --git a/package.json b/package.json index 6bb410fa3fba..26e3ab028c01 100644 --- a/package.json +++ b/package.json @@ -1665,6 +1665,7 @@ "typescript": "4.5.5", "uuid": "^8.3.2", "webpack": "^5.76.0", + "worker-loader": "^3.0.8", "webpack-bundle-analyzer": "^4.5.0", "webpack-cli": "^4.9.2", "webpack-fix-default-import-plugin": "^1.0.3", diff --git a/src/client/common/constants.ts b/src/client/common/constants.ts index 6fc743fb8a0a..ac9387784c2c 100644 --- a/src/client/common/constants.ts +++ b/src/client/common/constants.ts @@ -94,7 +94,8 @@ export namespace ThemeIcons { export const DEFAULT_INTERPRETER_SETTING = 'python'; -export const isCI = process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined; +export const isCI = + process.env.TRAVIS === 'true' || process.env.TF_BUILD !== undefined || process.env.GITHUB_ACTIONS === 'true'; export function isTestExecution(): boolean { return process.env.VSC_PYTHON_CI_TEST === '1' || isUnitTestExecution(); diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 18add7daf6fa..4a5aa984fa44 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -7,6 +7,7 @@ import { IDisposable } from '../types'; import { EnvironmentVariables } from '../variables/types'; import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis'; import { ExecutionResult, IProcessService, ObservableExecutionResult, ShellOptions, SpawnOptions } from './types'; +import { workerPlainExec, workerShellExec } from './worker/rawProcessApiWrapper'; export class ProcessService extends EventEmitter implements IProcessService { private processesToKill = new Set(); @@ -47,14 +48,20 @@ export class ProcessService extends EventEmitter implements IProcessService { } public exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { + this.emit('exec', file, args, options); + if (options.useWorker) { + return workerPlainExec(file, args, options); + } const execOptions = { ...options, doNotLog: true }; const promise = plainExec(file, args, execOptions, this.env, this.processesToKill); - this.emit('exec', file, args, options); return promise; } public shellExec(command: string, options: ShellOptions = {}): Promise> { this.emit('exec', command, undefined, options); + if (options.useWorker) { + return workerShellExec(command, options); + } const disposables = new Set(); const shellOptions = { ...options, doNotLog: true }; return shellExec(command, shellOptions, this.env, disposables).finally(() => { diff --git a/src/client/common/process/rawProcessApis.ts b/src/client/common/process/rawProcessApis.ts index 27cd88d42851..5e3641328b69 100644 --- a/src/client/common/process/rawProcessApis.ts +++ b/src/client/common/process/rawProcessApis.ts @@ -56,7 +56,6 @@ export function shellExec( disposables?: Set, ): Promise> { const shellOptions = getDefaultOptions(options, defaultEnv); - traceVerbose(`Shell Exec: ${command} with options: ${JSON.stringify(shellOptions, null, 4)}`); if (!options.doNotLog) { const processLogger = new ProcessLogger(new WorkspaceService()); processLogger.logProcess(command, undefined, shellOptions); diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index d4b742718e36..9263e69cbe21 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -26,9 +26,10 @@ export type SpawnOptions = ChildProcessSpawnOptions & { extraVariables?: NodeJS.ProcessEnv; outputChannel?: OutputChannel; stdinStr?: string; + useWorker?: boolean; }; -export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean }; +export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean; useWorker?: boolean }; export type ExecutionResult = { stdout: T; diff --git a/src/client/common/process/worker/main.ts b/src/client/common/process/worker/main.ts new file mode 100644 index 000000000000..324673618942 --- /dev/null +++ b/src/client/common/process/worker/main.ts @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Worker } from 'worker_threads'; +import * as path from 'path'; +import { traceVerbose, traceError } from '../../../logging/index'; + +/** + * Executes a worker file. Make sure to declare the worker file as a entry in the webpack config. + * @param workerFileName Filename of the worker file to execute, it has to end with ".worker.js" for webpack to bundle it. + * @param workerData Arguments to the worker file. + * @returns + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { + if (!workerFileName.endsWith('.worker.js')) { + throw new Error('Worker file must end with ".worker.js" for webpack to bundle webworkers'); + } + return new Promise((resolve, reject) => { + const worker = new Worker(workerFileName, { workerData }); + const id = worker.threadId; + traceVerbose( + `Worker id ${id} for file ${path.basename(workerFileName)} with data ${JSON.stringify(workerData)}`, + ); + worker.on('message', (msg: { err: Error; res: unknown }) => { + if (msg.err) { + reject(msg.err); + } + resolve(msg.res); + }); + worker.on('error', (ex: Error) => { + traceError(`Error in worker ${workerFileName}`, ex); + reject(ex); + }); + worker.on('exit', (code) => { + traceVerbose(`Worker id ${id} exited with code ${code}`); + if (code !== 0) { + reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); + } + }); + }); +} diff --git a/src/client/common/process/worker/plainExec.worker.ts b/src/client/common/process/worker/plainExec.worker.ts new file mode 100644 index 000000000000..f44ea15f9653 --- /dev/null +++ b/src/client/common/process/worker/plainExec.worker.ts @@ -0,0 +1,16 @@ +import { parentPort, workerData } from 'worker_threads'; +import { _workerPlainExecImpl } from './workerRawProcessApis'; + +_workerPlainExecImpl(workerData.file, workerData.args, workerData.options) + .then((res) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ res }); + }) + .catch((err) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ err }); + }); diff --git a/src/client/common/process/worker/rawProcessApiWrapper.ts b/src/client/common/process/worker/rawProcessApiWrapper.ts new file mode 100644 index 000000000000..e6476df5d8fa --- /dev/null +++ b/src/client/common/process/worker/rawProcessApiWrapper.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { SpawnOptions } from 'child_process'; +import * as path from 'path'; +import { executeWorkerFile } from './main'; +import { ExecutionResult, ShellOptions } from './types'; + +export function workerShellExec(command: string, options: ShellOptions): Promise> { + return executeWorkerFile(path.join(__dirname, 'shellExec.worker.js'), { + command, + options, + }); +} + +export function workerPlainExec( + file: string, + args: string[], + options: SpawnOptions = {}, +): Promise> { + return executeWorkerFile(path.join(__dirname, 'plainExec.worker.js'), { + file, + args, + options, + }); +} diff --git a/src/client/common/process/worker/shellExec.worker.ts b/src/client/common/process/worker/shellExec.worker.ts new file mode 100644 index 000000000000..f4e9809a29a5 --- /dev/null +++ b/src/client/common/process/worker/shellExec.worker.ts @@ -0,0 +1,16 @@ +import { parentPort, workerData } from 'worker_threads'; +import { _workerShellExecImpl } from './workerRawProcessApis'; + +_workerShellExecImpl(workerData.command, workerData.options, workerData.defaultEnv) + .then((res) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ res }); + }) + .catch((ex) => { + if (!parentPort) { + throw new Error('Not in a worker thread'); + } + parentPort.postMessage({ ex }); + }); diff --git a/src/client/common/process/worker/types.ts b/src/client/common/process/worker/types.ts new file mode 100644 index 000000000000..5c58aec10214 --- /dev/null +++ b/src/client/common/process/worker/types.ts @@ -0,0 +1,38 @@ +/* eslint-disable @typescript-eslint/no-empty-function */ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ +import { ExecOptions, SpawnOptions as ChildProcessSpawnOptions } from 'child_process'; + +export function noop() {} +export interface IDisposable { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dispose(): void | undefined | Promise; +} +export type EnvironmentVariables = Record; +export class StdErrError extends Error { + // eslint-disable-next-line @typescript-eslint/no-useless-constructor + constructor(message: string) { + super(message); + } +} + +export type SpawnOptions = ChildProcessSpawnOptions & { + encoding?: string; + // /** + // * Can't use `CancellationToken` here as it comes from vscode which is not available in worker threads. + // */ + // token?: CancellationToken; + mergeStdOutErr?: boolean; + throwOnStdErr?: boolean; + extraVariables?: NodeJS.ProcessEnv; + // /** + // * Can't use `OutputChannel` here as it comes from vscode which is not available in worker threads. + // */ + // outputChannel?: OutputChannel; + stdinStr?: string; +}; +export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean }; + +export type ExecutionResult = { + stdout: T; + stderr?: T; +}; diff --git a/src/client/common/process/worker/workerRawProcessApis.ts b/src/client/common/process/worker/workerRawProcessApis.ts new file mode 100644 index 000000000000..5b04aaa40b0a --- /dev/null +++ b/src/client/common/process/worker/workerRawProcessApis.ts @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// !!!! IMPORTANT: DO NOT IMPORT FROM VSCODE MODULE AS IT IS NOT AVAILABLE INSIDE WORKER THREADS !!!! + +import { exec, execSync, spawn } from 'child_process'; +import { Readable } from 'stream'; +import { createDeferred } from '../../utils/async'; +import { DEFAULT_ENCODING } from '../constants'; +import { decodeBuffer } from '../decoder'; +import { + ShellOptions, + SpawnOptions, + EnvironmentVariables, + IDisposable, + noop, + StdErrError, + ExecutionResult, +} from './types'; + +const PS_ERROR_SCREEN_BOGUS = /your [0-9]+x[0-9]+ screen size is bogus\. expect trouble/; + +function getDefaultOptions(options: T, defaultEnv?: EnvironmentVariables): T { + const defaultOptions = { ...options }; + const execOptions = defaultOptions as SpawnOptions; + if (execOptions) { + execOptions.encoding = + typeof execOptions.encoding === 'string' && execOptions.encoding.length > 0 + ? execOptions.encoding + : DEFAULT_ENCODING; + const { encoding } = execOptions; + delete execOptions.encoding; + execOptions.encoding = encoding; + } + if (!defaultOptions.env || Object.keys(defaultOptions.env).length === 0) { + const env = defaultEnv || process.env; + defaultOptions.env = { ...env }; + } else { + defaultOptions.env = { ...defaultOptions.env }; + } + + if (execOptions && execOptions.extraVariables) { + defaultOptions.env = { ...defaultOptions.env, ...execOptions.extraVariables }; + } + + // Always ensure we have unbuffered output. + defaultOptions.env.PYTHONUNBUFFERED = '1'; + if (!defaultOptions.env.PYTHONIOENCODING) { + defaultOptions.env.PYTHONIOENCODING = 'utf-8'; + } + + return defaultOptions; +} + +export function _workerShellExecImpl( + command: string, + options: ShellOptions, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): Promise> { + const shellOptions = getDefaultOptions(options, defaultEnv); + return new Promise((resolve, reject) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const callback = (e: any, stdout: any, stderr: any) => { + if (e && e !== null) { + reject(e); + } else if (shellOptions.throwOnStdErr && stderr && stderr.length) { + reject(new Error(stderr)); + } else { + stdout = filterOutputUsingCondaRunMarkers(stdout); + // Make sure stderr is undefined if we actually had none. This is checked + // elsewhere because that's how exec behaves. + resolve({ stderr: stderr && stderr.length > 0 ? stderr : undefined, stdout }); + } + }; + let procExited = false; + const proc = exec(command, shellOptions, callback); // NOSONAR + proc.once('close', () => { + procExited = true; + }); + proc.once('exit', () => { + procExited = true; + }); + proc.once('error', () => { + procExited = true; + }); + const disposable: IDisposable = { + dispose: () => { + // If process has not exited nor killed, force kill it. + if (!procExited && !proc.killed) { + if (proc.pid) { + killPid(proc.pid); + } else { + proc.kill(); + } + } + }, + }; + if (disposables) { + disposables.add(disposable); + } + }); +} + +export function _workerPlainExecImpl( + file: string, + args: string[], + options: SpawnOptions & { doNotLog?: boolean } = {}, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): Promise> { + const spawnOptions = getDefaultOptions(options, defaultEnv); + const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; + const proc = spawn(file, args, spawnOptions); + // Listen to these errors (unhandled errors in streams tears down the process). + // Errors will be bubbled up to the `error` event in `proc`, hence no need to log. + proc.stdout?.on('error', noop); + proc.stderr?.on('error', noop); + const deferred = createDeferred>(); + const disposable: IDisposable = { + dispose: () => { + // If process has not exited nor killed, force kill it. + if (!proc.killed && !deferred.completed) { + if (proc.pid) { + killPid(proc.pid); + } else { + proc.kill(); + } + } + }, + }; + disposables?.add(disposable); + const internalDisposables: IDisposable[] = []; + + // eslint-disable-next-line @typescript-eslint/ban-types + const on = (ee: Readable | null, name: string, fn: Function) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ee?.on(name, fn as any); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + internalDisposables.push({ dispose: () => ee?.removeListener(name, fn as any) as any }); + }; + + // Tokens not supported yet as they come from vscode module which is not available. + // if (options.token) { + // internalDisposables.push(options.token.onCancellationRequested(disposable.dispose)); + // } + + const stdoutBuffers: Buffer[] = []; + on(proc.stdout, 'data', (data: Buffer) => { + stdoutBuffers.push(data); + }); + const stderrBuffers: Buffer[] = []; + on(proc.stderr, 'data', (data: Buffer) => { + if (options.mergeStdOutErr) { + stdoutBuffers.push(data); + stderrBuffers.push(data); + } else { + stderrBuffers.push(data); + } + }); + + proc.once('close', () => { + if (deferred.completed) { + return; + } + const stderr: string | undefined = + stderrBuffers.length === 0 ? undefined : decodeBuffer(stderrBuffers, encoding); + if ( + stderr && + stderr.length > 0 && + options.throwOnStdErr && + // ignore this specific error silently; see this issue for context: https://github.com/microsoft/vscode/issues/75932 + !(PS_ERROR_SCREEN_BOGUS.test(stderr) && stderr.replace(PS_ERROR_SCREEN_BOGUS, '').trim().length === 0) + ) { + deferred.reject(new StdErrError(stderr)); + } else { + let stdout = decodeBuffer(stdoutBuffers, encoding); + stdout = filterOutputUsingCondaRunMarkers(stdout); + deferred.resolve({ stdout, stderr }); + } + internalDisposables.forEach((d) => d.dispose()); + disposable.dispose(); + }); + proc.once('error', (ex) => { + deferred.reject(ex); + internalDisposables.forEach((d) => d.dispose()); + disposable.dispose(); + }); + + return deferred.promise; +} + +function filterOutputUsingCondaRunMarkers(stdout: string) { + // These markers are added if conda run is used or `interpreterInfo.py` is + // run, see `get_output_via_markers.py`. + const regex = />>>PYTHON-EXEC-OUTPUT([\s\S]*)<<= 2 ? match[1].trim() : undefined; + return filteredOut !== undefined ? filteredOut : stdout; +} + +function killPid(pid: number): void { + try { + if (process.platform === 'win32') { + // Windows doesn't support SIGTERM, so execute taskkill to kill the process + execSync(`taskkill /pid ${pid} /T /F`); // NOSONAR + } else { + process.kill(pid); + } + } catch { + console.warn('Unable to kill process with pid', pid); + } +} diff --git a/src/client/common/terminal/shellDetector.ts b/src/client/common/terminal/shellDetector.ts index 98cda5953fe8..ad515d42c734 100644 --- a/src/client/common/terminal/shellDetector.ts +++ b/src/client/common/terminal/shellDetector.ts @@ -53,9 +53,6 @@ export class ShellDetector { for (const detector of shellDetectors) { shell = detector.identify(telemetryProperties, terminal); - traceVerbose( - `${detector}. Shell identified as ${shell} ${terminal ? `(Terminal name is ${terminal.name})` : ''}`, - ); if (shell && shell !== TerminalShellType.other) { telemetryProperties.failed = false; break; @@ -66,7 +63,7 @@ export class ShellDetector { // This impacts executing code in terminals and activation of environments in terminal. // So, the better this works, the better it is for the user. sendTelemetryEvent(EventName.TERMINAL_SHELL_IDENTIFICATION, undefined, telemetryProperties); - traceVerbose(`Shell identified as '${shell}'`); + traceVerbose(`Shell identified as ${shell} ${terminal ? `(Terminal name is ${terminal.name})` : ''}`); // If we could not identify the shell, use the defaults. if (shell === undefined || shell === TerminalShellType.other) { diff --git a/src/client/common/terminal/shellDetectors/baseShellDetector.ts b/src/client/common/terminal/shellDetectors/baseShellDetector.ts index 29a5e548891e..4262bdf80364 100644 --- a/src/client/common/terminal/shellDetectors/baseShellDetector.ts +++ b/src/client/common/terminal/shellDetectors/baseShellDetector.ts @@ -5,7 +5,6 @@ import { injectable, unmanaged } from 'inversify'; import { Terminal } from 'vscode'; -import { traceVerbose } from '../../../logging'; import { IShellDetector, ShellIdentificationTelemetry, TerminalShellType } from '../types'; /* @@ -75,7 +74,5 @@ export function identifyShellFromShellPath(shellPath: string): TerminalShellType return matchedShell; }, TerminalShellType.other); - traceVerbose(`Shell path '${shellPath}', base path '${basePath}'`); - traceVerbose(`Shell path identified as shell '${shell}'`); return shell; } diff --git a/src/client/common/terminal/shellDetectors/settingsShellDetector.ts b/src/client/common/terminal/shellDetectors/settingsShellDetector.ts index 7ffc168db28b..3eeb9d2e85da 100644 --- a/src/client/common/terminal/shellDetectors/settingsShellDetector.ts +++ b/src/client/common/terminal/shellDetectors/settingsShellDetector.ts @@ -5,7 +5,6 @@ import { inject, injectable } from 'inversify'; import { Terminal } from 'vscode'; -import { traceVerbose } from '../../../logging'; import { IWorkspaceService } from '../../application/types'; import { IPlatformService } from '../../platform/types'; import { OSType } from '../../utils/platform'; @@ -62,7 +61,6 @@ export class SettingsShellDetector extends BaseShellDetector { } else { telemetryProperties.shellIdentificationSource = 'settings'; } - traceVerbose(`Shell path from user settings '${shellPath}'`); return shell; } } diff --git a/src/client/common/terminal/shellDetectors/userEnvironmentShellDetector.ts b/src/client/common/terminal/shellDetectors/userEnvironmentShellDetector.ts index 7d8ed34ebf62..bed2848ece92 100644 --- a/src/client/common/terminal/shellDetectors/userEnvironmentShellDetector.ts +++ b/src/client/common/terminal/shellDetectors/userEnvironmentShellDetector.ts @@ -5,7 +5,6 @@ import { inject, injectable } from 'inversify'; import { Terminal } from 'vscode'; -import { traceVerbose } from '../../../logging'; import { IPlatformService } from '../../platform/types'; import { ICurrentProcess } from '../../types'; import { OSType } from '../../utils/platform'; @@ -41,7 +40,6 @@ export class UserEnvironmentShellDetector extends BaseShellDetector { if (shell !== TerminalShellType.other) { telemetryProperties.shellIdentificationSource = 'environment'; } - traceVerbose(`Shell path from user env '${shellPath}'`); return shell; } } diff --git a/src/client/common/utils/decorators.ts b/src/client/common/utils/decorators.ts index 689eb9acad44..44a82ee13760 100644 --- a/src/client/common/utils/decorators.ts +++ b/src/client/common/utils/decorators.ts @@ -1,5 +1,5 @@ import '../../common/extensions'; -import { traceError, traceVerbose } from '../../logging'; +import { traceError } from '../../logging'; import { isTestExecution } from '../constants'; import { createDeferred, Deferred } from './async'; import { getCacheKeyFromFunctionArgs, getGlobalCacheStore } from './cacheUtils'; @@ -161,7 +161,6 @@ export function cache(expiryDurationMs: number, cachePromise = false, expiryDura } const cachedItem = cacheStoreForMethods.get(key); if (cachedItem && (cachedItem.expiry > Date.now() || expiryDurationMs === -1)) { - traceVerbose(`Cached data exists ${key}`); return Promise.resolve(cachedItem.data); } const expiryMs = diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index ff03e95836fd..586bad0d765c 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -294,7 +294,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const oldWarnings = env[PYTHON_WARNINGS]; env[PYTHON_WARNINGS] = 'ignore'; - traceVerbose(`${hasCustomEnvVars ? 'Has' : 'No'} Custom Env Vars`); traceVerbose(`Activating Environment to capture Environment variables, ${command}`); // Do some wrapping of the call. For two reasons: diff --git a/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts b/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts index b4dcfe36e095..d5470e528ab9 100644 --- a/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts +++ b/src/client/interpreter/virtualEnvs/activatedEnvLaunch.ts @@ -91,12 +91,9 @@ export class ActivatedEnvironmentLaunch implements IActivatedEnvironmentLaunch { } if (process.env.VSCODE_CLI !== '1') { // We only want to select the interpreter if VS Code was launched from the command line. - traceVerbose( - 'VS Code was not launched from the command line, not selecting activated interpreter', - JSON.stringify(process.env, undefined, 4), - ); return undefined; } + traceVerbose('VS Code was not launched from the command line'); const prefix = await this.getPrefixOfSelectedActivatedEnv(); if (!prefix) { this._promptIfApplicable().ignoreErrors(); diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index 8fefe8e2d0dd..ab3b17629bc5 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -178,7 +178,7 @@ export interface ILocator extends * @param query - if provided, the locator will limit results to match * @returns - the fast async iterator of Python envs, which may have incomplete info */ - iterEnvs(query?: QueryForEvent, useWorkerThreads?: boolean): IPythonEnvsIterator; + iterEnvs(query?: QueryForEvent): IPythonEnvsIterator; } export type ICompositeLocator = Omit, 'providerId'>; diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index fb1a791d07ed..cc37b1f82cfd 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -88,14 +88,13 @@ export class EnvsCollectionService extends PythonEnvsWatcher { traceError(`Failed to resolve ${path}`, ex); return undefined; }); - traceVerbose(`Resolved ${path} to ${JSON.stringify(resolved)}`); + traceVerbose(`Resolved ${path} using downstream locator`); if (resolved) { this.cache.addEnv(resolved, true); } diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts index 7cac0cb7df90..651a43ff8868 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/condaLocator.ts @@ -6,6 +6,8 @@ import { BasicEnvInfo, IPythonEnvsIterator } from '../../locator'; import { Conda, getCondaEnvironmentsTxt } from '../../../common/environmentManagers/conda'; import { traceError, traceVerbose } from '../../../../logging'; import { FSWatchingLocator } from './fsWatchingLocator'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; +import { inExperiment } from '../../../common/externalDependencies'; export class CondaEnvironmentLocator extends FSWatchingLocator { public readonly providerId: string = 'conda-envs'; @@ -19,8 +21,11 @@ export class CondaEnvironmentLocator extends FSWatchingLocator { } // eslint-disable-next-line class-methods-use-this - public async *doIterEnvs(): IPythonEnvsIterator { - const conda = await Conda.getConda(); + public async *doIterEnvs( + _: unknown, + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + ): IPythonEnvsIterator { + const conda = await Conda.getConda(undefined, useWorkerThreads); if (conda === undefined) { traceVerbose(`Couldn't locate the conda binary.`); return; diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index 5bfc62d99d48..b2f5123069b0 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -63,7 +63,14 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos // Note that we do no filtering here, including to check if files // are valid executables. That is left to callers (e.g. composite // locators). - return this.locators.iterEnvs(query); + async function* iterator(it: IPythonEnvsIterator) { + traceVerbose(`Searching windows known paths locator`); + for await (const env of it) { + yield env; + } + traceVerbose(`Finished searching windows known paths locator`); + } + return iterator(this.locators.iterEnvs(query)); } } @@ -93,11 +100,7 @@ function getDirFilesLocator( // rather than in each low-level locator. In the meantime we // take a naive approach. async function* iterEnvs(query: PythonLocatorQuery): IPythonEnvsIterator { - traceVerbose('Searching for windows path interpreters'); - yield* await getEnvs(locator.iterEnvs(query)).then((res) => { - traceVerbose('Finished searching for windows path interpreters'); - return res; - }); + yield* await getEnvs(locator.iterEnvs(query)).then((res) => res); } return { providerId: locator.providerId, diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index 947d90ee9cb5..16b2167021db 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -7,12 +7,17 @@ import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; import { getRegistryInterpreters } from '../../../common/windowsUtils'; import { traceError, traceVerbose } from '../../../../logging'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; +import { inExperiment } from '../../../common/externalDependencies'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; export class WindowsRegistryLocator extends Locator { public readonly providerId: string = 'windows-registry'; // eslint-disable-next-line class-methods-use-this - public iterEnvs(_?: unknown, useWorkerThreads = true): IPythonEnvsIterator { + public iterEnvs( + _?: unknown, + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + ): IPythonEnvsIterator { const iterator = async function* () { traceVerbose('Searching for windows registry interpreters'); const interpreters = await getRegistryInterpreters(useWorkerThreads); diff --git a/src/client/pythonEnvironments/base/locators/wrappers.ts b/src/client/pythonEnvironments/base/locators/wrappers.ts index 1334c60fa2e0..bfaede584f6f 100644 --- a/src/client/pythonEnvironments/base/locators/wrappers.ts +++ b/src/client/pythonEnvironments/base/locators/wrappers.ts @@ -3,13 +3,10 @@ // eslint-disable-next-line max-classes-per-file import { Uri } from 'vscode'; -import { DiscoveryUsingWorkers } from '../../../common/experiments/groups'; import { IDisposable } from '../../../common/types'; import { iterEmpty } from '../../../common/utils/async'; import { getURIFilter } from '../../../common/utils/misc'; import { Disposables } from '../../../common/utils/resourceLifecycle'; -import { traceLog } from '../../../logging'; -import { inExperiment } from '../../common/externalDependencies'; import { PythonEnvInfo } from '../info'; import { BasicEnvInfo, ILocator, IPythonEnvsIterator, PythonLocatorQuery } from '../locator'; import { combineIterators, Locators } from '../locators'; @@ -20,8 +17,6 @@ import { LazyResourceBasedLocator } from './common/resourceBasedLocator'; */ export class ExtensionLocators extends Locators { - private readonly useWorkerThreads: boolean; - constructor( // These are expected to be low-level locators (e.g. system). private readonly nonWorkspace: ILocator[], @@ -30,10 +25,6 @@ export class ExtensionLocators extends Locators { private readonly workspace: ILocator, ) { super([...nonWorkspace, workspace]); - this.useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); - if (this.useWorkerThreads) { - traceLog('Using worker threads for discovery...'); - } } public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { @@ -42,7 +33,7 @@ export class ExtensionLocators extends Locators { const nonWorkspace = query?.providerId ? this.nonWorkspace.filter((locator) => query.providerId === locator.providerId) : this.nonWorkspace; - iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query, this.useWorkerThreads))); + iterators.push(...nonWorkspace.map((loc) => loc.iterEnvs(query))); } return combineIterators(iterators); } diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 842f81b99a4e..bc98e57829e9 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -265,16 +265,21 @@ export class Conda { * @param command - Command used to spawn conda. This has the same meaning as the * first argument of spawn() - i.e. it can be a full path, or just a binary name. */ - constructor(readonly command: string, shellCommand?: string, private readonly shellPath?: string) { + constructor( + readonly command: string, + shellCommand?: string, + private readonly shellPath?: string, + private readonly useWorkerThreads = true, + ) { this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { Conda.condaPromise = new Map>(); }); } - public static async getConda(shellPath?: string): Promise { + public static async getConda(shellPath?: string, useWorkerThreads?: boolean): Promise { if (Conda.condaPromise.get(shellPath) === undefined || isTestExecution()) { - Conda.condaPromise.set(shellPath, Conda.locate(shellPath)); + Conda.condaPromise.set(shellPath, Conda.locate(shellPath, useWorkerThreads)); } return Conda.condaPromise.get(shellPath); } @@ -285,10 +290,15 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(shellPath?: string): Promise { + private static async locate(shellPath?: string, useWorkerThreads = true): Promise { traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); - const customCondaPath = getPythonSetting(CONDAPATH_SETTING_KEY); + let customCondaPath: string | undefined = 'conda'; + try { + customCondaPath = getPythonSetting(CONDAPATH_SETTING_KEY); + } catch (ex) { + traceError(`Failed to get conda path setting, ${ex}`); + } const suffix = getOSType() === OSType.Windows ? 'Scripts\\conda.exe' : 'bin/conda'; // Produce a list of candidate binaries to be probed by exec'ing them. @@ -307,7 +317,7 @@ export class Conda { } async function* getCandidatesFromRegistry() { - const interps = await getRegistryInterpreters(true); + const interps = await getRegistryInterpreters(useWorkerThreads); const candidates = interps .filter((interp) => interp.interpreterPath && interp.distroOrgName === 'ContinuumAnalytics') .map((interp) => path.join(path.win32.dirname(interp.interpreterPath), suffix)); @@ -385,7 +395,7 @@ export class Conda { // Probe the candidates, and pick the first one that exists and does what we need. for await (const condaPath of getCandidates()) { traceVerbose(`Probing conda binary: ${condaPath}`); - let conda = new Conda(condaPath, undefined, shellPath); + let conda = new Conda(condaPath, undefined, shellPath, useWorkerThreads); try { await conda.getInfo(); if (getOSType() === OSType.Windows && (isTestExecution() || condaPath !== customCondaPath)) { @@ -440,7 +450,7 @@ export class Conda { if (shellPath) { options.shell = shellPath; } - const resultPromise = exec(command, ['info', '--json'], options); + const resultPromise = exec(command, ['info', '--json'], options, this.useWorkerThreads); // It has been observed that specifying a timeout is still not reliable to terminate the Conda process, see #27915. // Hence explicitly continue execution after timeout has been reached. const success = await Promise.race([ diff --git a/src/client/pythonEnvironments/common/environmentManagers/pipenv.ts b/src/client/pythonEnvironments/common/environmentManagers/pipenv.ts index 80a185dc2991..d8b1b2ff649e 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/pipenv.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/pipenv.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { getEnvironmentVariable } from '../../../common/utils/platform'; -import { traceError } from '../../../logging'; +import { traceError, traceVerbose } from '../../../logging'; import { arePathsSame, normCasePath, pathExists, readFile } from '../externalDependencies'; function getSearchHeight() { @@ -85,7 +85,7 @@ async function getProjectDir(envFolder: string): Promise { } const projectDir = (await readFile(dotProjectFile)).trim(); if (!(await pathExists(projectDir))) { - traceError( + traceVerbose( `The .project file inside environment folder: ${envFolder} doesn't contain a valid path to the project`, ); return undefined; diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index 357967236c9e..2ecbbdf0cb11 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -3,7 +3,6 @@ import * as fsapi from 'fs-extra'; import * as path from 'path'; -import { Worker } from 'worker_threads'; import * as vscode from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; import { ExecutionResult, IProcessServiceFactory, ShellOptions, SpawnOptions } from '../../common/process/types'; @@ -12,6 +11,7 @@ import { chain, iterable } from '../../common/utils/async'; import { getOSType, OSType } from '../../common/utils/platform'; import { IServiceContainer } from '../../ioc/types'; import { traceError, traceVerbose } from '../../logging'; +import { DiscoveryUsingWorkers } from '../../common/experiments/groups'; let internalServiceContainer: IServiceContainer; export function initializeExternalDependencies(serviceContainer: IServiceContainer): void { @@ -21,12 +21,20 @@ export function initializeExternalDependencies(serviceContainer: IServiceContain // processes export async function shellExecute(command: string, options: ShellOptions = {}): Promise> { + const useWorker = inExperiment(DiscoveryUsingWorkers.experiment); const service = await internalServiceContainer.get(IProcessServiceFactory).create(); + options = { ...options, useWorker }; return service.shellExec(command, options); } -export async function exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { +export async function exec( + file: string, + args: string[], + options: SpawnOptions = {}, + useWorker = inExperiment(DiscoveryUsingWorkers.experiment), +): Promise> { const service = await internalServiceContainer.get(IProcessServiceFactory).create(); + options = { ...options, useWorker }; return service.exec(file, args, options); } @@ -201,25 +209,3 @@ export function onDidChangePythonSetting(name: string, callback: () => void, roo } }); } - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -export async function executeWorkerFile(workerFileName: string, workerData: any): Promise { - return new Promise((resolve, reject) => { - const worker = new Worker(workerFileName, { workerData }); - worker.on('message', (res: { err: Error; res: unknown }) => { - if (res.err) { - reject(res.err); - } - resolve(res.res); - }); - worker.on('error', (ex: Error) => { - traceError(`Error in worker ${workerFileName}`, ex); - reject(ex); - }); - worker.on('exit', (code) => { - if (code !== 0) { - reject(new Error(`Worker ${workerFileName} stopped with exit code ${code}`)); - } - }); - }); -} diff --git a/src/client/pythonEnvironments/common/registryKeysWorker.ts b/src/client/pythonEnvironments/common/registryKeys.worker.ts similarity index 100% rename from src/client/pythonEnvironments/common/registryKeysWorker.ts rename to src/client/pythonEnvironments/common/registryKeys.worker.ts diff --git a/src/client/pythonEnvironments/common/registryValuesWorker.ts b/src/client/pythonEnvironments/common/registryValues.worker.ts similarity index 100% rename from src/client/pythonEnvironments/common/registryValuesWorker.ts rename to src/client/pythonEnvironments/common/registryValues.worker.ts diff --git a/src/client/pythonEnvironments/common/windowsRegistry.ts b/src/client/pythonEnvironments/common/windowsRegistry.ts index efac5bb3209f..801ef0c907b1 100644 --- a/src/client/pythonEnvironments/common/windowsRegistry.ts +++ b/src/client/pythonEnvironments/common/windowsRegistry.ts @@ -5,7 +5,7 @@ import { HKCU, HKLM, Options, REG_SZ, Registry, RegistryItem } from 'winreg'; import * as path from 'path'; import { createDeferred } from '../../common/utils/async'; -import { executeWorkerFile } from './externalDependencies'; +import { executeWorkerFile } from '../../common/process/worker/main'; export { HKCU, HKLM, REG_SZ, Options }; @@ -39,7 +39,7 @@ export async function readRegistryValues(options: Options, useWorkerThreads: boo }); return deferred.promise; } - return executeWorkerFile(path.join(__dirname, 'registryValuesWorker.js'), options); + return executeWorkerFile(path.join(__dirname, 'registryValues.worker.js'), options); } export async function readRegistryKeys(options: Options, useWorkerThreads: boolean): Promise { @@ -56,5 +56,5 @@ export async function readRegistryKeys(options: Options, useWorkerThreads: boole }); return deferred.promise; } - return executeWorkerFile(path.join(__dirname, 'registryKeysWorker.js'), options); + return executeWorkerFile(path.join(__dirname, 'registryKeys.worker.js'), options); } diff --git a/src/client/terminals/envCollectionActivation/service.ts b/src/client/terminals/envCollectionActivation/service.ts index c9fa125324a0..68c098c4c928 100644 --- a/src/client/terminals/envCollectionActivation/service.ts +++ b/src/client/terminals/envCollectionActivation/service.ts @@ -402,6 +402,8 @@ function shouldSkip(env: string) { 'SHLVL', // Even though this maybe returned, setting it can result in output encoding errors in terminal. 'PYTHONUTF8', + // We have deactivate service which takes care of setting it. + '_OLD_VIRTUAL_PATH', ].includes(env); } diff --git a/src/test/common/process/proc.exec.test.ts b/src/test/common/process/proc.exec.test.ts index c193df95d080..7e771e884b82 100644 --- a/src/test/common/process/proc.exec.test.ts +++ b/src/test/common/process/proc.exec.test.ts @@ -34,6 +34,16 @@ suite('ProcessService Observable', () => { expect(result.stderr).to.equal(undefined, 'stderr not undefined'); }); + test('When using worker threads, exec should output print statements', async () => { + const procService = new ProcessService(); + const printOutput = '1234'; + const result = await procService.exec(pythonPath, ['-c', `print("${printOutput}")`], { useWorker: true }); + + expect(result).not.to.be.an('undefined', 'result is undefined'); + expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output'); + expect(result.stderr).to.equal(undefined, 'stderr not undefined'); + }); + test('exec should output print unicode characters', async function () { // This test has not been working for many months in Python 2.7 under // Windows. Tracked by #2546. (unicode under Py2.7 is tough!) @@ -241,6 +251,18 @@ suite('ProcessService Observable', () => { expect(result.stderr).to.equal(undefined, 'stderr not empty'); expect(result.stdout.trim()).to.be.equal(printOutput, 'Invalid output'); }); + test('When using worker threads, shellExec should be able to run python and filter output using conda related markers', async () => { + const procService = new ProcessService(); + const printOutput = '1234'; + const result = await procService.shellExec( + `"${pythonPath}" -c "print('>>>PYTHON-EXEC-OUTPUT');print('${printOutput}');print('<< { const procService = new ProcessService(); const result = procService.shellExec('invalid command'); diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.testvirtualenvs.ts b/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.testvirtualenvs.ts index 1b2ea8715d35..b3e1084a56be 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.testvirtualenvs.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.testvirtualenvs.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. @@ -10,9 +11,14 @@ import { CondaEnvironmentLocator } from '../../../../../client/pythonEnvironment import { sleep } from '../../../../core'; import { createDeferred, Deferred } from '../../../../../client/common/utils/async'; import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher'; -import { TEST_TIMEOUT } from '../../../../constants'; +import { EXTENSION_ROOT_DIR_FOR_TESTS, TEST_TIMEOUT } from '../../../../constants'; import { traceWarn } from '../../../../../client/logging'; import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants'; +import { getEnvs } from '../../common'; +import { assertBasicEnvsEqual } from '../envTestUtils'; +import { PYTHON_VIRTUAL_ENVS_LOCATION } from '../../../../ciConstants'; +import { isCI } from '../../../../../client/common/constants'; +import * as externalDependencies from '../../../../../client/pythonEnvironments/common/externalDependencies'; class CondaEnvs { private readonly condaEnvironmentsTxt; @@ -50,9 +56,13 @@ class CondaEnvs { } } -suite('Conda Env Watcher', async () => { +suite('Conda Env Locator', async () => { let locator: CondaEnvironmentLocator; let condaEnvsTxt: CondaEnvs; + const envsLocation = + PYTHON_VIRTUAL_ENVS_LOCATION !== undefined + ? path.join(EXTENSION_ROOT_DIR_FOR_TESTS, PYTHON_VIRTUAL_ENVS_LOCATION) + : path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'tmp', 'envPaths.json'); async function waitForChangeToBeDetected(deferred: Deferred) { const timeout = setTimeout(() => { @@ -61,11 +71,21 @@ suite('Conda Env Watcher', async () => { }, TEST_TIMEOUT); await deferred.promise; } + let envPaths: any; + + suiteSetup(async () => { + if (isCI) { + envPaths = await fs.readJson(envsLocation); + } + }); setup(async () => { sinon.stub(platformUtils, 'getUserHomeDir').returns(TEST_LAYOUT_ROOT); condaEnvsTxt = new CondaEnvs(); await condaEnvsTxt.cleanUp(); + if (isCI) { + sinon.stub(externalDependencies, 'getPythonSetting').returns(envPaths.condaExecPath); + } }); async function setupLocator(onChanged: (e: PythonEnvsChangedEvent) => Promise) { @@ -111,4 +131,14 @@ suite('Conda Env Watcher', async () => { assert.deepEqual(actualEvent!, expectedEvent, 'Unexpected event emitted'); }); + + test('Worker thread to fetch conda environments is working', async () => { + locator = new CondaEnvironmentLocator(); + const items = await getEnvs(locator.doIterEnvs(undefined, false)); + const workerItems = await getEnvs(locator.doIterEnvs(undefined, true)); + console.log('Number of items Conda locator returned:', items.length); + // Make sure items returned when using worker threads v/s not are the same. + assertBasicEnvsEqual(items, workerItems); + assert(workerItems.length > 0, 'No environments found'); + }).timeout(TEST_TIMEOUT * 2); }); diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.unit.test.ts index 276f28cd665b..1bf3ef19398d 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/condaLocator.unit.test.ts @@ -17,14 +17,14 @@ suite('Conda Python Version Parser Tests', () => { setup(() => { readFileStub = sinon.stub(externalDeps, 'readFile'); + sinon.stub(externalDeps, 'inExperiment').returns(false); pathExistsStub = sinon.stub(externalDeps, 'pathExists'); pathExistsStub.resolves(true); }); teardown(() => { - readFileStub.restore(); - pathExistsStub.restore(); + sinon.restore(); }); interface ICondaPythonVersionTestData { diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts index 6f0ff0e1bb5e..693d7c1b7fee 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.testvirtualenvs.ts @@ -18,7 +18,7 @@ suite('Windows Registry Locator', async () => { return undefined; }); - test('Make sure worker thread to fetch environments is working', async () => { + test('Worker thread to fetch registry interpreters is working', async () => { const items = await getEnvs(locator.iterEnvs(undefined, false)); const workerItems = await getEnvs(locator.iterEnvs(undefined, true)); console.log('Number of items Windows registry locator returned:', items.length); diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts index c4621b267ad6..a69a643f52d4 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts @@ -11,6 +11,7 @@ import { WindowsRegistryLocator } from '../../../../../client/pythonEnvironments import { createBasicEnv } from '../../common'; import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants'; import { assertBasicEnvsEqual } from '../envTestUtils'; +import * as externalDependencies from '../../../../../client/pythonEnvironments/common/externalDependencies'; suite('Windows Registry', () => { let stubReadRegistryValues: sinon.SinonStub; @@ -200,6 +201,7 @@ suite('Windows Registry', () => { } setup(async () => { + sinon.stub(externalDependencies, 'inExperiment').returns(false); stubReadRegistryValues = sinon.stub(winreg, 'readRegistryValues'); stubReadRegistryKeys = sinon.stub(winreg, 'readRegistryKeys'); stubReadRegistryValues.callsFake(fakeRegistryValues); diff --git a/src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts b/src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts index 1e9de68ad77a..268c5333a42f 100644 --- a/src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts +++ b/src/test/pythonEnvironments/common/environmentManagers/conda.unit.test.ts @@ -593,6 +593,11 @@ suite('Conda and its environments are located correctly', () => { }, }, }; + sinon.stub(externalDependencies, 'inExperiment').returns(false); + }); + + teardown(() => { + sinon.restore(); }); test('Must compute conda environment name from prefix', async () => {