From 7352a0a5d6ce73138f541ae63ace966c7014fc83 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 9 Feb 2021 15:11:12 -0800 Subject: [PATCH 01/21] Cherry pick fix for native notebook support into release branch (#15369) * Fix problem with notebook apis not being used. (#15366) * Update changelog * Remove news file --- CHANGELOG.md | 2 ++ src/client/common/application/notebook.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 290fb2d196f3..93d944df5799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ ([#15232](https://github.com/Microsoft/vscode-python/issues/15232)) 1. Ensure target environment is activated in the terminal when running install scripts. ([#15285](https://github.com/Microsoft/vscode-python/issues/15285)) +1. Allow support for using notebook APIs in the VS code stable build. + ([#15364](https://github.com/Microsoft/vscode-python/issues/15364)) ### Code Health diff --git a/src/client/common/application/notebook.ts b/src/client/common/application/notebook.ts index 52557d820184..bd5a873d7361 100644 --- a/src/client/common/application/notebook.ts +++ b/src/client/common/application/notebook.ts @@ -85,7 +85,7 @@ export class VSCodeNotebook implements IVSCodeNotebook { @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IApplicationEnvironment) readonly env: IApplicationEnvironment, ) { - if (this.useProposedApi && this.env.channel === 'insiders') { + if (this.useProposedApi) { this.addEventHandlers(); this.canUseNotebookApi = true; } From 3f78ccb7a7dbc8a7b9d6cc56c6d855d5fd6179d7 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 9 Feb 2021 15:21:52 -0800 Subject: [PATCH 02/21] Add extraPaths support to JediLSP (#15365) * Add extraPaths support * Remove duplicate opt option * Eslint cleanup * Fix tests --- .eslintignore | 1 - package.json | 2 - src/client/activation/jedi/analysisOptions.ts | 42 ++++++++- src/client/providers/jediProxy.ts | 88 +++++++++++++----- .../jedi/jediAnalysisOptions.unit.test.ts | 93 +++++++++++++++++++ 5 files changed, 198 insertions(+), 28 deletions(-) create mode 100644 src/test/activation/jedi/jediAnalysisOptions.unit.test.ts diff --git a/.eslintignore b/.eslintignore index d7240625a999..55d41fe9132c 100644 --- a/.eslintignore +++ b/.eslintignore @@ -383,7 +383,6 @@ src/client/providers/docStringFoldingProvider.ts src/client/providers/linterProvider.ts src/client/providers/simpleRefactorProvider.ts src/client/providers/completionProvider.ts -src/client/providers/jediProxy.ts src/client/providers/definitionProvider.ts src/client/providers/referenceProvider.ts src/client/providers/terminalProvider.ts diff --git a/package.json b/package.json index cdd48b18e0fb..68e97e23b293 100644 --- a/package.json +++ b/package.json @@ -1044,7 +1044,6 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "tryPylance", - "jediLSP", "debuggerDataViewer", "pythonSendEntireLineToREPL", "pythonTensorboardExperiment", @@ -1074,7 +1073,6 @@ "DeprecatePythonPath - experiment", "RunByLine - experiment", "tryPylance", - "jediLSP", "debuggerDataViewer", "pythonSendEntireLineToREPL", "pythonTensorboardExperiment", diff --git a/src/client/activation/jedi/analysisOptions.ts b/src/client/activation/jedi/analysisOptions.ts index f2ba1555fcd8..6eb2e296c31c 100644 --- a/src/client/activation/jedi/analysisOptions.ts +++ b/src/client/activation/jedi/analysisOptions.ts @@ -1,8 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import { WorkspaceFolder } from 'vscode'; +import { IWorkspaceService } from '../../common/application/types'; +import { IConfigurationService, Resource } from '../../common/types'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; +import { PythonEnvironment } from '../../pythonEnvironments/info'; import { LanguageServerAnalysisOptionsWithEnv } from '../common/analysisOptions'; import { ILanguageServerOutputChannel } from '../types'; @@ -10,15 +15,47 @@ import { ILanguageServerOutputChannel } from '../types'; @injectable() export class JediLanguageServerAnalysisOptions extends LanguageServerAnalysisOptionsWithEnv { - // eslint-disable-next-line @typescript-eslint/no-useless-constructor + private resource: Resource | undefined; + constructor( @inject(IEnvironmentVariablesProvider) envVarsProvider: IEnvironmentVariablesProvider, @inject(ILanguageServerOutputChannel) lsOutputChannel: ILanguageServerOutputChannel, + @inject(IConfigurationService) private readonly configurationService: IConfigurationService, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, ) { super(envVarsProvider, lsOutputChannel); + this.resource = undefined; + } + + public async initialize(resource: Resource, interpreter: PythonEnvironment | undefined) { + this.resource = resource; + return super.initialize(resource, interpreter); + } + + protected getWorkspaceFolder(): WorkspaceFolder | undefined { + return this.workspace.getWorkspaceFolder(this.resource); } protected async getInitializationOptions() { + const pythonSettings = this.configurationService.getSettings(this.resource); + const workspacePath = this.getWorkspaceFolder()?.uri.fsPath; + const extraPaths = pythonSettings.autoComplete + ? pythonSettings.autoComplete.extraPaths.map((extraPath) => { + if (path.isAbsolute(extraPath)) { + return extraPath; + } + return workspacePath ? path.join(workspacePath, extraPath) : ''; + }) + : []; + + if (workspacePath) { + extraPaths.unshift(workspacePath); + } + + const distinctExtraPaths = extraPaths + .filter((value) => value.length > 0) + .filter((value, index, self) => self.indexOf(value) === index); + return { markupKindPreferred: 'markdown', completion: { @@ -31,6 +68,9 @@ export class JediLanguageServerAnalysisOptions extends LanguageServerAnalysisOpt didSave: true, didChange: true, }, + workspace: { + extraPaths: distinctExtraPaths, + }, }; } } diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index e3564a8b5e79..b76418e75940 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -1,9 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +/* eslint-disable max-classes-per-file, @typescript-eslint/no-explicit-any */ + import { ChildProcess } from 'child_process'; import * as path from 'path'; -// @ts-ignore +// @ts-ignore pidusage does not work in strict mode import * as pidusage from 'pidusage'; import { CancellationToken, CancellationTokenSource, CompletionItemKind, Disposable, SymbolKind, Uri } from 'vscode'; import '../common/extensions'; @@ -21,7 +23,7 @@ import { IServiceContainer } from '../ioc/types'; import { PythonEnvironment } from '../pythonEnvironments/info'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; -import { traceError, traceWarning } from './../common/logger'; +import { traceError, traceWarning } from '../common/logger'; const pythonVSCodeTypeMappings = new Map(); pythonVSCodeTypeMappings.set('none', CompletionItemKind.Value); @@ -153,23 +155,41 @@ type JediProxyPayload = { export class JediProxy implements Disposable { private proc?: ChildProcess; + private pythonSettings: IPythonSettings; - private cmdId: number = 0; + + private cmdId = 0; + private lastKnownPythonInterpreter: string; + private previousData = ''; + private commands = new Map>(); + private commandQueue: number[] = []; + private spawnRetryAttempts = 0; + private additionalAutoCompletePaths: string[] = []; + private workspacePath: string; + private languageServerStarted!: Deferred; + private initialized: Deferred; + private environmentVariablesProvider!: IEnvironmentVariablesProvider; - private ignoreJediMemoryFootprint: boolean = false; + + private ignoreJediMemoryFootprint = false; + private pidUsageFailures = { timer: new StopWatch(), counter: 0 }; + private lastCmdIdProcessed?: number; + private lastCmdIdProcessedForPidUsage?: number; + private readonly disposables: Disposable[] = []; + private timer?: NodeJS.Timer | number; public constructor( @@ -188,11 +208,11 @@ export class JediProxy implements Disposable { this.checkJediMemoryFootprint().ignoreErrors(); } - private static getProperty(o: object, name: string): T { - return (o as any)[name]; + private static getProperty(o: Record, name: string): T { + return o[name]; } - public dispose() { + public dispose(): void { while (this.disposables.length > 0) { const disposable = this.disposables.pop(); if (disposable) { @@ -200,6 +220,7 @@ export class JediProxy implements Disposable { } } if (this.timer) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any clearTimeout(this.timer as any); } this.killProcess(); @@ -227,7 +248,7 @@ export class JediProxy implements Disposable { this.commandQueue.push(executionCmd.id); } catch (ex) { traceError(ex); - //If 'This socket is closed.' that means process didn't start at all (at least not properly). + // If 'This socket is closed.' that means process didn't start at all (at least not properly). if (ex.message === 'This socket is closed.') { this.killProcess(); } else { @@ -247,6 +268,7 @@ export class JediProxy implements Disposable { this.handleError('spawnProcess', ex); }); } + private shouldCheckJediMemoryFootprint() { if (this.ignoreJediMemoryFootprint || this.pythonSettings.jediMemoryLimit === -1) { return false; @@ -262,6 +284,7 @@ export class JediProxy implements Disposable { } return true; } + private async checkJediMemoryFootprint() { // Check memory footprint periodically. Do not check on every request due to // the performance impact. See https://github.com/soyuka/pidusage - on Windows @@ -276,6 +299,7 @@ export class JediProxy implements Disposable { } this.timer = setTimeout(() => this.checkJediMemoryFootprint(), 15 * 1000); } + private async checkJediMemoryFootprintImpl(): Promise { if (!this.proc || this.proc.killed) { return; @@ -321,24 +345,27 @@ export class JediProxy implements Disposable { deferred.resolve(); }); + // eslint-disable-next-line consistent-return return deferred.promise; } // @debounce(1500) @swallowExceptions('JediProxy') private async environmentVariablesChangeHandler() { - const newAutoComletePaths = await this.buildAutoCompletePaths(); - if (this.additionalAutoCompletePaths.join(',') !== newAutoComletePaths.join(',')) { - this.additionalAutoCompletePaths = newAutoComletePaths; + const newAutoCompletePaths = await this.buildAutoCompletePaths(); + if (this.additionalAutoCompletePaths.join(',') !== newAutoCompletePaths.join(',')) { + this.additionalAutoCompletePaths = newAutoCompletePaths; this.restartLanguageServer().ignoreErrors(); } } + @swallowExceptions('JediProxy') private async startLanguageServer(): Promise { const newAutoComletePaths = await this.buildAutoCompletePaths(); this.additionalAutoCompletePaths = newAutoComletePaths; return this.restartLanguageServer(); } + private restartLanguageServer(): Promise { this.killProcess(); this.clearPendingRequests(); @@ -360,11 +387,14 @@ export class JediProxy implements Disposable { if (this.proc) { this.proc.kill(); } - } catch (ex) {} + } catch (ex) { + // intentionally left blank + } this.proc = undefined; } - private handleError(source: string, errorMessage: string) { + // eslint-disable-next-line class-methods-use-this + private handleError(source: string, errorMessage: string): void { traceError(`${source} jediProxy`, `Error (${source}) ${errorMessage}`); } @@ -412,7 +442,8 @@ export class JediProxy implements Disposable { const data = output.out; // Possible there was an exception in parsing the data returned, // so append the data and then parse it. - const dataStr = (this.previousData = `${this.previousData}${data}`); + this.previousData = `${this.previousData}${data}`; + const dataStr = this.previousData; let responses: any[]; try { @@ -444,7 +475,7 @@ export class JediProxy implements Disposable { return; } this.lastCmdIdProcessed = cmd.id; - if (JediProxy.getProperty(response, 'arguments')) { + if (JediProxy.getProperty(response, 'arguments')) { this.commandQueue.splice(this.commandQueue.indexOf(cmd.id), 1); return; } @@ -473,9 +504,10 @@ export class JediProxy implements Disposable { (error) => this.handleError('subscription.error', `${error}`), ); } + private getCommandHandler( command: CommandType, - ): undefined | ((command: IExecutionCommand, response: object) => void) { + ): undefined | ((command: IExecutionCommand, response: Record) => void) { switch (command) { case CommandType.Completions: return this.onCompletion; @@ -490,10 +522,11 @@ export class JediProxy implements Disposable { case CommandType.Arguments: return this.onArguments; default: - return; } + return undefined; } - private onCompletion(command: IExecutionCommand, response: object): void { + + private onCompletion(command: IExecutionCommand, response: Record): void { let results = JediProxy.getProperty(response, 'results'); results = Array.isArray(results) ? results : []; results.forEach((item) => { @@ -509,7 +542,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, completionResult); } - private onDefinition(command: IExecutionCommand, response: object): void { + private onDefinition(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); const defResult: IDefinitionResult = { requestId: command.id, @@ -537,7 +570,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResult); } - private onHover(command: IExecutionCommand, response: object): void { + private onHover(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); const defResult: IHoverResult = { requestId: command.id, @@ -554,7 +587,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResult); } - private onSymbols(command: IExecutionCommand, response: object): void { + private onSymbols(command: IExecutionCommand, response: Record): void { let defs = JediProxy.getProperty(response, 'results'); defs = Array.isArray(defs) ? defs : []; const defResults: ISymbolResult = { @@ -581,7 +614,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, defResults); } - private onUsages(command: IExecutionCommand, response: object): void { + private onUsages(command: IExecutionCommand, response: Record): void { let defs = JediProxy.getProperty(response, 'results'); defs = Array.isArray(defs) ? defs : []; const refResult: IReferenceResult = { @@ -599,7 +632,7 @@ export class JediProxy implements Disposable { this.safeResolve(command, refResult); } - private onArguments(command: IExecutionCommand, response: object): void { + private onArguments(command: IExecutionCommand, response: Record): void { const defs = JediProxy.getProperty(response, 'results'); this.safeResolve(command, { @@ -617,6 +650,7 @@ export class JediProxy implements Disposable { try { this.safeResolve(cmd1, undefined); } catch (ex) { + // Intentionally left blank } finally { this.commands.delete(id); } @@ -663,6 +697,7 @@ export class JediProxy implements Disposable { return ''; } } + private async buildAutoCompletePaths(): Promise { const filePathPromises = [ // Sysprefix. @@ -703,6 +738,7 @@ export class JediProxy implements Disposable { return []; } } + private getEnvironmentVariablesProvider() { if (!this.environmentVariablesProvider) { this.environmentVariablesProvider = this.serviceContainer.get( @@ -714,6 +750,7 @@ export class JediProxy implements Disposable { } return this.environmentVariablesProvider; } + private getConfig(): JediProxyConfig { // Add support for paths relative to workspace. const extraPaths = this.pythonSettings.autoComplete @@ -747,6 +784,7 @@ export class JediProxy implements Disposable { }; } + // eslint-disable-next-line class-methods-use-this private safeResolve( command: IExecutionCommand | undefined | null, result: ICommandResult | PromiseLike | undefined, @@ -827,6 +865,7 @@ export interface IAutoCompleteItem { kind: SymbolKind; text: string; description: string; + // eslint-disable-next-line camelcase raw_docstring: string; rightLabel: string; } @@ -865,12 +904,13 @@ export class JediProxyHandler implements Disposable { return this.jediProxy; } - public dispose() { + public dispose(): void { if (this.jediProxy) { this.jediProxy.dispose(); } } + // eslint-disable-next-line @typescript-eslint/no-unused-vars public sendCommand(cmd: ICommand, _token?: CancellationToken): Promise { const executionCmd = >cmd; executionCmd.id = executionCmd.id || this.jediProxy.getNextCommandId(); diff --git a/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts b/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts new file mode 100644 index 000000000000..e7a1f5011edb --- /dev/null +++ b/src/test/activation/jedi/jediAnalysisOptions.unit.test.ts @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as path from 'path'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { EventEmitter, Uri, WorkspaceFolder } from 'vscode'; +import { JediLanguageServerAnalysisOptions } from '../../../client/activation/jedi/analysisOptions'; +import { ILanguageServerAnalysisOptions, ILanguageServerOutputChannel } from '../../../client/activation/types'; +import { IWorkspaceService } from '../../../client/common/application/types'; +import { WorkspaceService } from '../../../client/common/application/workspace'; +import { ConfigurationService } from '../../../client/common/configuration/service'; +import { IConfigurationService } from '../../../client/common/types'; +import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; + +suite('Jedi LSP - analysis Options', () => { + const workspacePath = path.join('this', 'is', 'fake', 'workspace', 'path'); + const expectedWorkspacePath = path.sep + workspacePath; + + let envVarsProvider: IEnvironmentVariablesProvider; + let lsOutputChannel: ILanguageServerOutputChannel; + let configurationService: IConfigurationService; + let workspaceService: IWorkspaceService; + + let analysisOptions: ILanguageServerAnalysisOptions; + + class MockWorkspaceFolder implements WorkspaceFolder { + public uri: Uri; + + public name: string; + + public ownedResources = new Set(); + + constructor(folder: string, public index: number = 0) { + this.uri = Uri.file(folder); + this.name = folder; + } + } + + setup(() => { + envVarsProvider = mock(IEnvironmentVariablesProvider); + lsOutputChannel = mock(ILanguageServerOutputChannel); + configurationService = mock(ConfigurationService); + workspaceService = mock(WorkspaceService); + + const onDidChangeEnvVariables = new EventEmitter(); + when(envVarsProvider.onDidEnvironmentVariablesChange).thenReturn(onDidChangeEnvVariables.event); + + analysisOptions = new JediLanguageServerAnalysisOptions( + instance(envVarsProvider), + instance(lsOutputChannel), + instance(configurationService), + instance(workspaceService), + ); + }); + test('Without extraPaths provided and no workspace', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + when(configurationService.getSettings(anything())).thenReturn({} as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([]); + }); + + test('Without extraPaths provided', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(new MockWorkspaceFolder(workspacePath)); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + when(configurationService.getSettings(anything())).thenReturn({} as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([expectedWorkspacePath]); + }); + + test('With extraPaths provided', async () => { + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(new MockWorkspaceFolder(workspacePath)); + when(configurationService.getSettings(anything())).thenReturn({ + // We expect a distinct set of paths back, using __dirname to test absolute path + autoComplete: { extraPaths: [__dirname, 'relative/pathB', 'relative/pathB'] }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + analysisOptions.initialize(undefined, undefined); + + const result = await analysisOptions.getAnalysisOptions(); + + expect(result.initializationOptions.workspace.extraPaths).to.deep.equal([ + expectedWorkspacePath, + __dirname, + path.join(expectedWorkspacePath, 'relative/pathB'), + ]); + }); +}); From 525ec7873bf95a8e9a51ed3afdcdab412a4d56a2 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 9 Feb 2021 15:30:42 -0800 Subject: [PATCH 03/21] Ignore directories with the same name as python binaries (#15367) * Ignore directories with the same name as pyhton binaries * Use withFileTypes instead of lstat --- .../pythonEnvironments/common/commonUtils.ts | 6 +++--- .../pythonEnvironments/common/posixUtils.ts | 2 +- .../services/posixKnownPathsLocator.ts | 21 ++++++++++--------- .../posixroot/location3/python3.9/empty | 1 + 4 files changed, 16 insertions(+), 14 deletions(-) create mode 100644 src/test/pythonEnvironments/common/envlayouts/posixroot/location3/python3.9/empty diff --git a/src/client/pythonEnvironments/common/commonUtils.ts b/src/client/pythonEnvironments/common/commonUtils.ts index 825ca783bc51..95467aff687e 100644 --- a/src/client/pythonEnvironments/common/commonUtils.ts +++ b/src/client/pythonEnvironments/common/commonUtils.ts @@ -12,10 +12,10 @@ import { comparePythonVersionSpecificity } from '../base/info/env'; import { parseVersion } from '../base/info/pythonVersion'; import { getPythonVersionFromConda } from '../discovery/locators/services/conda'; import { getPythonVersionFromPyvenvCfg } from '../discovery/locators/services/virtualEnvironmentIdentifier'; -import { isPosixPythonBin } from './posixUtils'; +import { isPosixPythonBinPattern } from './posixUtils'; import { isWindowsPythonExe } from './windowsUtils'; -const matchPythonExecutable = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBin; +const matchPythonExecutable = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBinPattern; type FileFilterFunc = (filename: string) => boolean; @@ -33,7 +33,7 @@ export async function* findInterpretersInDir( ): AsyncIterableIterator { // "checkBin" is a local variable rather than global // so we can stub it out during unit testing. - const checkBin = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBin; + const checkBin = getOSType() === OSType.Windows ? isWindowsPythonExe : isPosixPythonBinPattern; const cfg = { ignoreErrors, filterSubDir, diff --git a/src/client/pythonEnvironments/common/posixUtils.ts b/src/client/pythonEnvironments/common/posixUtils.ts index 181976fddf3d..62227fbcb2c1 100644 --- a/src/client/pythonEnvironments/common/posixUtils.ts +++ b/src/client/pythonEnvironments/common/posixUtils.ts @@ -10,7 +10,7 @@ import { getSearchPathEntries } from '../../common/utils/exec'; * @param {string} interpreterPath : Path to python interpreter. * @returns {boolean} : Returns true if the path matches pattern for windows python executable. */ -export function isPosixPythonBin(interpreterPath: string): boolean { +export function isPosixPythonBinPattern(interpreterPath: string): boolean { /** * This Reg-ex matches following file names: * python diff --git a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts index 87a4e76d0b16..8dc0e1dc095b 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as fsapi from 'fs-extra'; +import * as fs from 'fs'; import * as path from 'path'; import { traceError, traceInfo } from '../../../../common/logger'; @@ -11,23 +11,24 @@ import { buildEnvInfo } from '../../../base/info/env'; import { parseVersion } from '../../../base/info/pythonVersion'; import { IPythonEnvsIterator, Locator } from '../../../base/locator'; import { getFileInfo, resolveSymbolicLink } from '../../../common/externalDependencies'; -import { commonPosixBinPaths, isPosixPythonBin } from '../../../common/posixUtils'; +import { commonPosixBinPaths, isPosixPythonBinPattern } from '../../../common/posixUtils'; async function getPythonBinFromKnownPaths(): Promise { - const knownPaths = await commonPosixBinPaths(); + const knownDirs = await commonPosixBinPaths(); const pythonBins: Set = new Set(); - for (const knownPath of knownPaths) { - const files = (await fsapi.readdir(knownPath)) - .map((filename: string) => path.join(knownPath, filename)) - .filter(isPosixPythonBin); + for (const dirname of knownDirs) { + const paths = (await fs.promises.readdir(dirname, { withFileTypes: true })) + .filter((dirent: fs.Dirent) => !dirent.isDirectory()) + .map((dirent: fs.Dirent) => path.join(dirname, dirent.name)) + .filter(isPosixPythonBinPattern); - for (const file of files) { + for (const filepath of paths) { // Ensure that we have a collection of unique global binaries by // resolving all symlinks to the target binaries. try { - const resolvedBin = await resolveSymbolicLink(file); + const resolvedBin = await resolveSymbolicLink(filepath); pythonBins.add(resolvedBin); - traceInfo(`Found: ${file} --> ${resolvedBin}`); + traceInfo(`Found: ${filepath} --> ${resolvedBin}`); } catch (ex) { traceError('Failed to resolve symbolic link: ', ex); } diff --git a/src/test/pythonEnvironments/common/envlayouts/posixroot/location3/python3.9/empty b/src/test/pythonEnvironments/common/envlayouts/posixroot/location3/python3.9/empty new file mode 100644 index 000000000000..0c6fe8957e8a --- /dev/null +++ b/src/test/pythonEnvironments/common/envlayouts/posixroot/location3/python3.9/empty @@ -0,0 +1 @@ +this is intentionally empty From 338af8951e190c2e55c0ea69c90d7c045fb976ea Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 9 Feb 2021 11:06:03 -0800 Subject: [PATCH 04/21] Correct pipenv activation provider when in discovery experiment (#15319) * Correct pipenv activation provider when in discovery experiment * Fix ESLint errors --- .eslintignore | 2 - .../pipEnvActivationProvider.ts | 40 +++++++++++-------- src/client/pythonEnvironments/legacyIOC.ts | 8 +--- .../pipEnvActivationProvider.unit.test.ts | 11 ++++- 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/.eslintignore b/.eslintignore index 55d41fe9132c..9c9908ede444 100644 --- a/.eslintignore +++ b/.eslintignore @@ -186,7 +186,6 @@ src/test/common/terminals/service.unit.test.ts src/test/common/terminals/helper.unit.test.ts src/test/common/terminals/activation.unit.test.ts src/test/common/terminals/shellDetectors/shellDetectors.unit.test.ts -src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts src/test/common/socketStream.test.ts src/test/common/configSettings.test.ts @@ -548,7 +547,6 @@ src/client/common/terminal/shellDetectors/vscEnvironmentShellDetector.ts src/client/common/terminal/shellDetectors/terminalNameShellDetector.ts src/client/common/terminal/shellDetectors/settingsShellDetector.ts src/client/common/terminal/shellDetectors/baseShellDetector.ts -src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts src/client/common/terminal/environmentActivationProviders/baseActivationProvider.ts src/client/common/terminal/environmentActivationProviders/commandPrompt.ts src/client/common/terminal/environmentActivationProviders/bash.ts diff --git a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts index 1589bd43ce54..49e60e109a03 100644 --- a/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts +++ b/src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts @@ -5,13 +5,15 @@ import { inject, injectable, named } from 'inversify'; import { Uri } from 'vscode'; -import '../../../common/extensions'; +import '../../extensions'; import { IInterpreterService } from '../../../interpreter/contracts'; +import { isPipenvEnvironmentRelatedToFolder } from '../../../pythonEnvironments/discovery/locators/services/pipEnvHelper'; import { EnvironmentType } from '../../../pythonEnvironments/info'; import { IWorkspaceService } from '../../application/types'; +import { inDiscoveryExperiment } from '../../experiments/helpers'; import { IFileSystem } from '../../platform/types'; -import { IToolExecutionPath, ToolExecutionPath } from '../../types'; -import { ITerminalActivationCommandProvider, TerminalShellType } from '../types'; +import { IExperimentService, IToolExecutionPath, ToolExecutionPath } from '../../types'; +import { ITerminalActivationCommandProvider } from '../types'; @injectable() export class PipEnvActivationCommandProvider implements ITerminalActivationCommandProvider { @@ -22,37 +24,41 @@ export class PipEnvActivationCommandProvider implements ITerminalActivationComma private readonly pipEnvExecution: IToolExecutionPath, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(IFileSystem) private readonly fs: IFileSystem, + @inject(IExperimentService) private readonly experimentService: IExperimentService, ) {} - public isShellSupported(_targetShell: TerminalShellType): boolean { + // eslint-disable-next-line class-methods-use-this + public isShellSupported(): boolean { return false; } - public async getActivationCommands(resource: Uri | undefined, _: TerminalShellType): Promise { + public async getActivationCommands(resource: Uri | undefined): Promise { const interpreter = await this.interpreterService.getActiveInterpreter(resource); if (!interpreter || interpreter.envType !== EnvironmentType.Pipenv) { - return; + return undefined; } // Activate using `pipenv shell` only if the current folder relates pipenv environment. const workspaceFolder = resource ? this.workspaceService.getWorkspaceFolder(resource) : undefined; - if ( - workspaceFolder && - interpreter.pipEnvWorkspaceFolder && - !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) - ) { - return; + if (workspaceFolder) { + if (await inDiscoveryExperiment(this.experimentService)) { + if (!(await isPipenvEnvironmentRelatedToFolder(interpreter.path, workspaceFolder?.uri.fsPath))) { + return undefined; + } + } else if ( + interpreter.pipEnvWorkspaceFolder && + !this.fs.arePathsSame(workspaceFolder.uri.fsPath, interpreter.pipEnvWorkspaceFolder) + ) { + return undefined; + } } const execName = this.pipEnvExecution.executable; return [`${execName.fileToCommandArgument()} shell`]; } - public async getActivationCommandsForInterpreter( - pythonPath: string, - _targetShell: TerminalShellType, - ): Promise { + public async getActivationCommandsForInterpreter(pythonPath: string): Promise { const interpreter = await this.interpreterService.getInterpreterDetails(pythonPath); if (!interpreter || interpreter.envType !== EnvironmentType.Pipenv) { - return; + return undefined; } const execName = this.pipEnvExecution.executable; diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index 3c1239b1ff32..e1f09632e33c 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -85,7 +85,7 @@ const convertedKinds = new Map( ); function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment { - const { name, location, executable, arch, kind, searchLocation, version, distro } = info; + const { name, location, executable, arch, kind, version, distro } = info; const { filename, sysPrefix } = executable; const env: PythonEnvironment = { sysPrefix, @@ -102,12 +102,6 @@ function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment { } // Otherwise it stays Unknown. - if (searchLocation !== undefined) { - if (kind === PythonEnvKind.Pipenv) { - env.pipEnvWorkspaceFolder = searchLocation.fsPath; - } - } - if (version !== undefined) { const { release, sysVersion } = version; if (release === undefined) { diff --git a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts index 1e0e5d506043..89bc04005eab 100644 --- a/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts +++ b/src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvider.unit.test.ts @@ -9,11 +9,12 @@ import * as TypeMoq from 'typemoq'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../../../client/common/application/types'; import { WorkspaceService } from '../../../../client/common/application/workspace'; +import { DiscoveryVariants } from '../../../../client/common/experiments/groups'; import { FileSystem } from '../../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../../client/common/platform/types'; import { PipEnvActivationCommandProvider } from '../../../../client/common/terminal/environmentActivationProviders/pipEnvActivationProvider'; import { ITerminalActivationCommandProvider, TerminalShellType } from '../../../../client/common/terminal/types'; -import { IToolExecutionPath } from '../../../../client/common/types'; +import { IExperimentService, IToolExecutionPath } from '../../../../client/common/types'; import { getNamesAndValues } from '../../../../client/common/utils/enum'; import { IInterpreterService } from '../../../../client/interpreter/contracts'; import { InterpreterService } from '../../../../client/interpreter/interpreterService'; @@ -28,17 +29,22 @@ suite('Terminals Activation - Pipenv', () => { let pipEnvExecution: TypeMoq.IMock; let workspaceService: IWorkspaceService; let fs: IFileSystem; + let experimentService: IExperimentService; setup(() => { interpreterService = mock(InterpreterService); fs = mock(FileSystem); workspaceService = mock(WorkspaceService); interpreterService = mock(InterpreterService); + experimentService = mock(); + when(experimentService.inExperiment(DiscoveryVariants.discoverWithFileWatching)).thenResolve(false); + when(experimentService.inExperiment(DiscoveryVariants.discoveryWithoutFileWatching)).thenResolve(false); pipEnvExecution = TypeMoq.Mock.ofType(); activationProvider = new PipEnvActivationCommandProvider( instance(interpreterService), pipEnvExecution.object, instance(workspaceService), instance(fs), + instance(experimentService), ); pipEnvExecution.setup((p) => p.executable).returns(() => pipenvExecFile); @@ -60,6 +66,7 @@ suite('Terminals Activation - Pipenv', () => { for (const interpreterType of nonPipInterpreterTypes) { when(interpreterService.getActiveInterpreter(resource)).thenResolve({ type: interpreterType, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) { @@ -72,6 +79,7 @@ suite('Terminals Activation - Pipenv', () => { test('pipenv shell is returned for pipenv interpeter', async () => { when(interpreterService.getActiveInterpreter(resource)).thenResolve({ envType: EnvironmentType.Pipenv, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) { @@ -84,6 +92,7 @@ suite('Terminals Activation - Pipenv', () => { pipenvExecFile = 'my pipenv'; when(interpreterService.getActiveInterpreter(resource)).thenResolve({ envType: EnvironmentType.Pipenv, + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); for (const shell of getNamesAndValues(TerminalShellType)) { From b47cac4d9943d24cc407ec58f6ef4e3f811deedf Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 9 Feb 2021 14:04:51 -0700 Subject: [PATCH 05/21] In PythonEnvsReducer.resolveEnv(), always fall back to the wrapped locator. (#15350) fixes #15118 --- .../locators/composite/environmentsReducer.ts | 6 ++++-- .../composite/environmentsReducer.unit.test.ts | 18 +++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts b/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts index 8bbb91a5b8e7..af7495490986 100644 --- a/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts +++ b/src/client/pythonEnvironments/base/locators/composite/environmentsReducer.ts @@ -22,9 +22,11 @@ export class PythonEnvsReducer implements ILocator { public async resolveEnv(env: string | PythonEnvInfo): Promise { const environments = await getEnvs(this.iterEnvs()); - const environment = environments.find((e) => areSameEnv(e, env)); + let environment: string | PythonEnvInfo | undefined = environments.find((e) => areSameEnv(e, env)); if (!environment) { - return undefined; + // It isn't one we've reduced, but fall back + // to the wrapped locator anyway. + environment = env; } return this.parentLocator.resolveEnv(environment); } diff --git a/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts index 2f1e0310e8b8..3865572ac780 100644 --- a/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/environmentsReducer.unit.test.ts @@ -248,7 +248,7 @@ suite('Python envs locator - Environments Reducer', () => { assert.deepEqual(resolved, expected); }); - test("If the reducer isn't able to resolve environment, return undefined", async () => { + test("If the reducer isn't able to resolve environment, fall back to the wrapped locator", async () => { const env1 = createNamedEnv('env', '3.8', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); const env2 = createNamedEnv('env2', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3')); const env3 = createNamedEnv('env', '3.8.1b1', PythonEnvKind.System, path.join('path', 'to', 'exec')); @@ -257,20 +257,24 @@ suite('Python envs locator - Environments Reducer', () => { const env6 = createNamedEnv('env', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec')); const environmentsToBeIterated = [env1, env2, env3, env4, env5, env6]; // env1 env3 env6 are same - const env136 = createNamedEnv('env', '3.8.1b1', PythonEnvKind.Conda, path.join('path', 'to', 'exec')); + const filename1 = path.join('resolved', 'path', 'to', 'execNeverSeenBefore'); + const filename2 = path.join('resolved', 'path', 'to', 'execAlsoNeverSeenBefore'); + const expected = createNamedEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, filename1); const parentLocator = new SimpleLocator(environmentsToBeIterated, { resolve: async (e: PythonEnvInfo) => { - if (isEqual(e, env136)) { - return createNamedEnv('resolvedEnv', '3.8.1', PythonEnvKind.Conda, 'resolved/path/to/exec'); + if (e.executable.filename === expected.executable.filename) { + return expected; } - throw new Error('Incorrect environment sent to the resolve'); + return undefined; }, }); const reducer = new PythonEnvsReducer(parentLocator); - const expected = await reducer.resolveEnv(path.join('path', 'to', 'execNeverSeenBefore')); + const resolved1 = await reducer.resolveEnv(filename1); + const resolved2 = await reducer.resolveEnv(filename2); - assert.deepEqual(expected, undefined); + assert.deepEqual(resolved1, expected); + assert.equal(resolved2, undefined); }); }); }); From 9622cd0c9cff99818dfc7be2e58c62e51d263c2f Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 10 Feb 2021 16:14:04 -0800 Subject: [PATCH 06/21] Fix issue with missing interpreter info for some cases (#15376) * Use child process apis directly. * Use raw API in process service * Handle process cleanup * Address sonar * Refactor process service by pulling the raw process APIs out of the class * Address comments --- src/client/common/process/proc.ts | 210 +-------------- src/client/common/process/rawProcessApis.ts | 251 ++++++++++++++++++ .../base/info/interpreter.ts | 6 +- .../common/externalDependencies.ts | 40 ++- .../discovery/locators/services/conda.ts | 14 +- .../info/environmentInfoService.ts | 25 +- 6 files changed, 324 insertions(+), 222 deletions(-) create mode 100644 src/client/common/process/rawProcessApis.ts diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index 667eafa5d1ee..61e06ed811d1 100644 --- a/src/client/common/process/proc.ts +++ b/src/client/common/process/proc.ts @@ -1,23 +1,17 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { exec, execSync, spawn } from 'child_process'; import { EventEmitter } from 'events'; -import { Observable } from 'rxjs/Observable'; -import { Readable } from 'stream'; import { IDisposable } from '../types'; -import { createDeferred } from '../utils/async'; import { EnvironmentVariables } from '../variables/types'; -import { DEFAULT_ENCODING } from './constants'; +import { execObservable, killPid, plainExec, shellExec } from './rawProcessApis'; import { ExecutionResult, IBufferDecoder, IProcessService, ObservableExecutionResult, - Output, ShellOptions, SpawnOptions, - StdErrError, } from './types'; export class ProcessService extends EventEmitter implements IProcessService { @@ -37,16 +31,7 @@ export class ProcessService extends EventEmitter implements IProcessService { } public static kill(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 { - // Ignore. - } + killPid(pid); } public dispose(): void { @@ -61,199 +46,18 @@ export class ProcessService extends EventEmitter implements IProcessService { } public execObservable(file: string, args: string[], options: SpawnOptions = {}): ObservableExecutionResult { - const spawnOptions = this.getDefaultOptions(options); - const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; - const proc = spawn(file, args, spawnOptions); - let procExited = false; - const disposable: IDisposable = { - dispose() { - if (proc && !proc.killed && !procExited) { - ProcessService.kill(proc.pid); - } - if (proc) { - proc.unref(); - } - }, - }; - this.processesToKill.add(disposable); - - const output = new Observable>((subscriber) => { - const disposables: 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 - disposables.push({ dispose: () => ee?.removeListener(name, fn as any) as any }); - }; - - if (options.token) { - disposables.push( - options.token.onCancellationRequested(() => { - if (!procExited && !proc.killed) { - proc.kill(); - procExited = true; - } - }), - ); - } - - const sendOutput = (source: 'stdout' | 'stderr', data: Buffer) => { - const out = this.decoder.decode([data], encoding); - if (source === 'stderr' && options.throwOnStdErr) { - subscriber.error(new StdErrError(out)); - } else { - subscriber.next({ source, out }); - } - }; - - on(proc.stdout, 'data', (data: Buffer) => sendOutput('stdout', data)); - on(proc.stderr, 'data', (data: Buffer) => sendOutput('stderr', data)); - - proc.once('close', () => { - procExited = true; - subscriber.complete(); - disposables.forEach((d) => d.dispose()); - }); - proc.once('exit', () => { - procExited = true; - subscriber.complete(); - disposables.forEach((d) => d.dispose()); - }); - proc.once('error', (ex) => { - procExited = true; - subscriber.error(ex); - disposables.forEach((d) => d.dispose()); - }); - }); - + const result = execObservable(file, args, options, this.decoder, this.env, this.processesToKill); this.emit('exec', file, args, options); - - return { - proc, - out: output, - dispose: disposable.dispose, - }; + return result; } public exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { - const spawnOptions = this.getDefaultOptions(options); - const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; - const proc = spawn(file, args, spawnOptions); - const deferred = createDeferred>(); - const disposable: IDisposable = { - dispose: () => { - if (!proc.killed && !deferred.completed) { - proc.kill(); - } - }, - }; - this.processesToKill.add(disposable); - const disposables: 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 - disposables.push({ dispose: () => ee?.removeListener(name, fn as any) as any }); - }; - - if (options.token) { - disposables.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 : this.decoder.decode(stderrBuffers, encoding); - if (stderr && stderr.length > 0 && options.throwOnStdErr) { - deferred.reject(new StdErrError(stderr)); - } else { - const stdout = this.decoder.decode(stdoutBuffers, encoding); - deferred.resolve({ stdout, stderr }); - } - disposables.forEach((d) => d.dispose()); - }); - proc.once('error', (ex) => { - deferred.reject(ex); - disposables.forEach((d) => d.dispose()); - }); - + const promise = plainExec(file, args, options, this.decoder, this.env, this.processesToKill); this.emit('exec', file, args, options); - - return deferred.promise; + return promise; } public shellExec(command: string, options: ShellOptions = {}): Promise> { - const shellOptions = this.getDefaultOptions(options); - return new Promise((resolve, reject) => { - const proc = exec(command, shellOptions, (e, stdout, stderr) => { - if (e && e !== null) { - reject(e); - } else if (shellOptions.throwOnStdErr && stderr && stderr.length) { - reject(new Error(stderr)); - } else { - // 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 }); - } - }); // NOSONAR - const disposable: IDisposable = { - dispose: () => { - if (!proc.killed) { - proc.kill(); - } - }, - }; - this.processesToKill.add(disposable); - }); - } - - private getDefaultOptions(options: T): 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 = this.env ? this.env : 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; + return shellExec(command, options, this.env, this.processesToKill); } } diff --git a/src/client/common/process/rawProcessApis.ts b/src/client/common/process/rawProcessApis.ts new file mode 100644 index 000000000000..9cb0c40b9bca --- /dev/null +++ b/src/client/common/process/rawProcessApis.ts @@ -0,0 +1,251 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { exec, execSync, spawn } from 'child_process'; +import { Readable } from 'stream'; +import { Observable } from 'rxjs/Observable'; +import { IDisposable } from '../types'; +import { createDeferred } from '../utils/async'; +import { EnvironmentVariables } from '../variables/types'; +import { DEFAULT_ENCODING } from './constants'; +import { + ExecutionResult, + IBufferDecoder, + ObservableExecutionResult, + Output, + ShellOptions, + SpawnOptions, + StdErrError, +} from './types'; + +export 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 shellExec( + 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 { + // 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 }); + } + }; + const proc = exec(command, shellOptions, callback); // NOSONAR + const disposable: IDisposable = { + dispose: () => { + if (!proc.killed) { + proc.kill(); + } + }, + }; + if (disposables) { + disposables.add(disposable); + } + }); +} + +export function plainExec( + file: string, + args: string[], + options: SpawnOptions = {}, + decoder?: IBufferDecoder, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): Promise> { + const spawnOptions = getDefaultOptions(options, defaultEnv); + const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; + const proc = spawn(file, args, spawnOptions); + const deferred = createDeferred>(); + const disposable: IDisposable = { + dispose: () => { + if (!proc.killed && !deferred.completed) { + 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 }); + }; + + 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 : decoder?.decode(stderrBuffers, encoding); + if (stderr && stderr.length > 0 && options.throwOnStdErr) { + deferred.reject(new StdErrError(stderr)); + } else { + const stdout = decoder ? decoder.decode(stdoutBuffers, encoding) : ''; + deferred.resolve({ stdout, stderr }); + } + internalDisposables.forEach((d) => d.dispose()); + }); + proc.once('error', (ex) => { + deferred.reject(ex); + internalDisposables.forEach((d) => d.dispose()); + }); + + return deferred.promise; +} + +export function execObservable( + file: string, + args: string[], + options: SpawnOptions = {}, + decoder?: IBufferDecoder, + defaultEnv?: EnvironmentVariables, + disposables?: Set, +): ObservableExecutionResult { + const spawnOptions = getDefaultOptions(options, defaultEnv); + const encoding = spawnOptions.encoding ? spawnOptions.encoding : 'utf8'; + const proc = spawn(file, args, spawnOptions); + let procExited = false; + const disposable: IDisposable = { + dispose() { + if (proc && !proc.killed && !procExited) { + killPid(proc.pid); + } + if (proc) { + proc.unref(); + } + }, + }; + disposables?.add(disposable); + + const output = new Observable>((subscriber) => { + 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 }); + }; + + if (options.token) { + internalDisposables.push( + options.token.onCancellationRequested(() => { + if (!procExited && !proc.killed) { + proc.kill(); + procExited = true; + } + }), + ); + } + + const sendOutput = (source: 'stdout' | 'stderr', data: Buffer) => { + const out = decoder ? decoder.decode([data], encoding) : ''; + if (source === 'stderr' && options.throwOnStdErr) { + subscriber.error(new StdErrError(out)); + } else { + subscriber.next({ source, out }); + } + }; + + on(proc.stdout, 'data', (data: Buffer) => sendOutput('stdout', data)); + on(proc.stderr, 'data', (data: Buffer) => sendOutput('stderr', data)); + + proc.once('close', () => { + procExited = true; + subscriber.complete(); + internalDisposables.forEach((d) => d.dispose()); + }); + proc.once('exit', () => { + procExited = true; + subscriber.complete(); + internalDisposables.forEach((d) => d.dispose()); + }); + proc.once('error', (ex) => { + procExited = true; + subscriber.error(ex); + internalDisposables.forEach((d) => d.dispose()); + }); + }); + + return { + proc, + out: output, + dispose: disposable.dispose, + }; +} + +export 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 { + // Ignore. + } +} diff --git a/src/client/pythonEnvironments/base/info/interpreter.ts b/src/client/pythonEnvironments/base/info/interpreter.ts index 8c436b6329ae..4e639fbaa4a4 100644 --- a/src/client/pythonEnvironments/base/info/interpreter.ts +++ b/src/client/pythonEnvironments/base/info/interpreter.ts @@ -6,6 +6,7 @@ import { interpreterInfo as getInterpreterInfoCommand, InterpreterInfoJson, } from '../../../common/process/internal/scripts'; +import { IDisposable } from '../../../common/types'; import { Architecture } from '../../../common/utils/platform'; import { copyPythonExecInfo, PythonExecInfo } from '../../exec'; import { parseVersion } from './pythonVersion'; @@ -45,7 +46,7 @@ type ShellExecResult = { stdout: string; stderr?: string; }; -type ShellExecFunc = (command: string, timeout: number) => Promise; +type ShellExecFunc = (command: string, timeout: number, disposables?: Set) => Promise; type Logger = { info(msg: string): void; @@ -64,6 +65,7 @@ export async function getInterpreterInfo( python: PythonExecInfo, shellExec: ShellExecFunc, logger?: Logger, + disposables?: Set, ): Promise { const [args, parse] = getInterpreterInfoCommand(); const info = copyPythonExecInfo(python, args); @@ -78,7 +80,7 @@ export async function getInterpreterInfo( // See these two bugs: // https://github.com/microsoft/vscode-python/issues/7569 // https://github.com/microsoft/vscode-python/issues/7760 - const result = await shellExec(quoted, 15000); + const result = await shellExec(quoted, 15000, disposables); if (result.stderr) { if (logger) { logger.error(`Failed to parse interpreter information for ${argv} stderr: ${result.stderr}`); diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index 219cdecac503..062a8923245c 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -4,13 +4,14 @@ import * as fsapi from 'fs-extra'; import * as path from 'path'; import * as vscode from 'vscode'; -import { ExecutionResult, IProcessServiceFactory, SpawnOptions } from '../../common/process/types'; -import { IExperimentService } from '../../common/types'; +import { ExecutionResult, SpawnOptions } from '../../common/process/types'; +import { IExperimentService, IDisposable } from '../../common/types'; import { chain, iterable } from '../../common/utils/async'; import { normalizeFilename } from '../../common/utils/filesystem'; import { getOSType, OSType } from '../../common/utils/platform'; -import { IDisposable } from '../../common/utils/resourceLifecycle'; import { IServiceContainer } from '../../ioc/types'; +import { plainExec, shellExec } from '../../common/process/rawProcessApis'; +import { BufferDecoder } from '../../common/process/decoder'; let internalServiceContainer: IServiceContainer; export function initializeExternalDependencies(serviceContainer: IServiceContainer): void { @@ -19,18 +20,31 @@ export function initializeExternalDependencies(serviceContainer: IServiceContain // processes -function getProcessFactory(): IProcessServiceFactory { - return internalServiceContainer.get(IProcessServiceFactory); -} - -export async function shellExecute(command: string, timeout: number): Promise> { - const proc = await getProcessFactory().create(); - return proc.shellExec(command, { timeout }); +/** + * Specialized version of the more generic shellExecute function to use only in + * cases where we don't need to pass custom environment variables read from env + * files or execution options. + */ +export async function shellExecute( + command: string, + timeout: number, + disposables?: Set, +): Promise> { + return shellExec(command, { timeout }, undefined, disposables); } -export async function exec(file: string, args: string[], options: SpawnOptions = {}): Promise> { - const proc = await getProcessFactory().create(); - return proc.exec(file, args, options); +/** + * Specialized version of the more generic exec function to use only in + * cases where we don't need to pass custom environment variables read from + * env files. + */ +export async function exec( + file: string, + args: string[], + options: SpawnOptions = {}, + disposables?: Set, +): Promise> { + return plainExec(file, args, options, new BufferDecoder(), undefined, disposables); } // filesystem diff --git a/src/client/pythonEnvironments/discovery/locators/services/conda.ts b/src/client/pythonEnvironments/discovery/locators/services/conda.ts index be23c549456d..537be256bd2f 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/conda.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/conda.ts @@ -9,6 +9,7 @@ import { parseVersion } from '../../../base/info/pythonVersion'; import { getRegistryInterpreters } from '../../../common/windowsUtils'; import { EnvironmentType, PythonEnvironment } from '../../../info'; +import { IDisposable } from '../../../../common/types'; export const AnacondaCompanyNames = ['Anaconda, Inc.', 'Continuum Analytics, Inc.']; @@ -326,8 +327,19 @@ export class Conda { * Corresponds to "conda info --json". */ public async getInfo(): Promise { - const result = await exec(this.command, ['info', '--json']); + const disposables = new Set(); + const result = await exec(this.command, ['info', '--json'], {}, disposables); traceVerbose(`conda info --json: ${result.stdout}`); + + // Ensure the process we started is cleaned up. + disposables.forEach((p) => { + try { + p.dispose(); + } catch { + // ignore. + } + }); + return JSON.parse(result.stdout); } diff --git a/src/client/pythonEnvironments/info/environmentInfoService.ts b/src/client/pythonEnvironments/info/environmentInfoService.ts index 09715b888b8f..675739245e58 100644 --- a/src/client/pythonEnvironments/info/environmentInfoService.ts +++ b/src/client/pythonEnvironments/info/environmentInfoService.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { traceVerbose } from '../../common/logger'; +import { IDisposable } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { createRunningWorkerPool, IWorkerPool, QueuePosition } from '../../common/utils/workerPool'; import { getInterpreterInfo, InterpreterInformation } from '../base/info/interpreter'; @@ -21,9 +23,26 @@ export interface IEnvironmentInfoService { } async function buildEnvironmentInfo(interpreterPath: string): Promise { - const interpreterInfo = await getInterpreterInfo(buildPythonExecInfo(interpreterPath), shellExecute).catch( - () => undefined, - ); + const disposables = new Set(); + const interpreterInfo = await getInterpreterInfo( + buildPythonExecInfo(interpreterPath), + shellExecute, + undefined, + disposables, + ).catch((reason) => { + traceVerbose(reason); + return undefined; + }); + + // Ensure the process we started is cleaned up. + disposables.forEach((p) => { + try { + p.dispose(); + } catch { + // ignore. + } + }); + if (interpreterInfo === undefined || interpreterInfo.version === undefined) { return undefined; } From fda25050849de768cb6379380f84a813a56553ab Mon Sep 17 00:00:00 2001 From: Jim Griesmer Date: Thu, 11 Feb 2021 09:47:16 -0800 Subject: [PATCH 07/21] Add reference to CVE-2020-16977 fixed in Oct 2020. (#15381) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93d944df5799..be0cfc9fd710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -653,6 +653,8 @@ part of! ([#14182](https://github.com/Microsoft/vscode-python/issues/14182)) 1. Fix exporting from the interactive window. ([#14210](https://github.com/Microsoft/vscode-python/issues/14210)) +1. Fix for CVE-2020-16977 + ([CVE-2020-16977](https://msrc.microsoft.com/update-guide/vulnerability/CVE-2020-16977)) ### Thanks From 3e56c969100ebc485297a1e35ca7282b212430fc Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 9 Feb 2021 11:01:15 -0800 Subject: [PATCH 08/21] Fix CI failing with the number of constructor arguments error (#15347) --- src/test/serviceRegistry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 2e84c6355335..4764653c2272 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -175,7 +175,7 @@ export class IocContainer { private disposables: Disposable[] = []; constructor() { - const cont = new Container(); + const cont = new Container({ skipBaseClassChecks: true }); this.serviceManager = new ServiceManager(cont); this.serviceContainer = new ServiceContainer(cont); From 6db5c5eb0c6f2c82b6e1f947c722dde2cd80c4a8 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 9 Feb 2021 09:59:13 -0800 Subject: [PATCH 09/21] Fix Command 'workbench.action.closeAllEditors' timed out failure on CI (#15322) --- src/test/initialize.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/initialize.ts b/src/test/initialize.ts index f1542f9c3baa..cba7529693f8 100644 --- a/src/test/initialize.ts +++ b/src/test/initialize.ts @@ -86,7 +86,7 @@ async function closeWindowsInteral() { // Lets not waste too much time. const timer = setTimeout(() => { reject(new Error("Command 'workbench.action.closeAllEditors' timed out")); - }, 2000); + }, 15000); vscode.commands.executeCommand('workbench.action.closeAllEditors').then( () => { clearTimeout(timer); From d619de6ca23961b117ec04be81816023a20976bc Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 10 Feb 2021 16:25:09 -0800 Subject: [PATCH 10/21] Ensure environment variables provider can be created using standard classes (#15377) * Ensure environment variables provider can be created using standard classes * Fix unit tests --- .../variables/environmentVariablesProvider.ts | 17 +++++--- src/client/extensionActivation.ts | 2 - src/client/extensionInit.ts | 2 + .../envVarsProvider.multiroot.test.ts | 3 -- .../environmentVariablesProvider.unit.test.ts | 41 +++++++------------ 5 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index 0a6b7a962f4a..20ddff4b9fc0 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -5,10 +5,12 @@ import { inject, injectable, optional } from 'inversify'; import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode'; import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry'; import { IWorkspaceService } from '../application/types'; +import { PythonSettings } from '../configSettings'; import { traceVerbose } from '../logger'; import { IPlatformService } from '../platform/types'; -import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types'; +import { ICurrentProcess, IDisposableRegistry } from '../types'; import { InMemoryCache } from '../utils/cacheUtils'; +import { SystemVariables } from './systemVariables'; import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; const CACHE_DURATION = 60 * 60 * 1000; @@ -25,7 +27,6 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IPlatformService) private platformService: IPlatformService, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, - @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(ICurrentProcess) private process: ICurrentProcess, @optional() private cacheDuration: number = CACHE_DURATION, ) { @@ -86,11 +87,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid return mergedVars; } public async getCustomEnvironmentVariables(resource?: Uri): Promise { - const settings = this.configurationService.getSettings(resource); + const systemVariables: SystemVariables = new SystemVariables( + undefined, + PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri?.fsPath, + this.workspaceService, + ); + const envFileSetting = this.workspaceService.getConfiguration('python', resource).get('envFile'); + const envFile = systemVariables.resolveAny(envFileSetting)!; const workspaceFolderUri = this.getWorkspaceFolderUri(resource); this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : ''); - this.createFileWatcher(settings.envFile, workspaceFolderUri); - return this.envVarsService.parseFile(settings.envFile, this.process.env); + this.createFileWatcher(envFile, workspaceFolderUri); + return this.envVarsService.parseFile(envFile, this.process.env); } public configurationChanged(e: ConfigurationChangeEvent) { this.trackedWorkspaceFolders.forEach((item) => { diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index b0d0686de25c..816a0d400333 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -30,7 +30,6 @@ import { IOutputChannel, } from './common/types'; import { noop } from './common/utils/misc'; -import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; import { DebuggerTypeName } from './debugger/constants'; import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher'; import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types'; @@ -112,7 +111,6 @@ async function activateLegacy(ext: ExtensionState): Promise { const { enableProposedApi } = applicationEnv.packageJson; serviceManager.addSingletonInstance(UseProposedApi, enableProposedApi); // Feature specific registrations. - variableRegisterTypes(serviceManager); unitTestsRegisterTypes(serviceManager); lintersRegisterTypes(serviceManager); interpretersRegisterTypes(serviceManager); diff --git a/src/client/extensionInit.ts b/src/client/extensionInit.ts index 60caffbb1617..1f825d9210b6 100644 --- a/src/client/extensionInit.ts +++ b/src/client/extensionInit.ts @@ -17,6 +17,7 @@ import { IOutputChannel, WORKSPACE_MEMENTO, } from './common/types'; +import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry'; import { OutputChannelNames } from './common/utils/localize'; import { ExtensionState } from './components'; import { ServiceContainer } from './ioc/container'; @@ -69,6 +70,7 @@ export function initializeStandard(ext: ExtensionState): void { const { serviceManager } = ext.legacyIOC; // Core registrations (non-feature specific). commonRegisterTypes(serviceManager); + variableRegisterTypes(serviceManager); platformRegisterTypes(serviceManager); processRegisterTypes(serviceManager); diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 56509cd4761a..a62dff0cb8af 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -7,7 +7,6 @@ import * as path from 'path'; import { anything, instance, mock, when } from 'ts-mockito'; import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode'; import { WorkspaceService } from '../../../client/common/application/workspace'; -import { ConfigurationService } from '../../../client/common/configuration/service'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IFileSystem } from '../../../client/common/platform/types'; import { IDisposableRegistry, IPathUtils } from '../../../client/common/types'; @@ -76,14 +75,12 @@ suite('Multiroot Environment Variables Provider', () => { const variablesService = new EnvironmentVariablesService(pathUtils, fs); const disposables = ioc.serviceContainer.get(IDisposableRegistry); ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService()); - const cfgService = new ConfigurationService(ioc.serviceContainer); const workspaceService = new WorkspaceService(); return new EnvironmentVariablesProvider( variablesService, disposables, new PlatformService(), workspaceService, - cfgService, mockProcess, ); } diff --git a/src/test/common/variables/environmentVariablesProvider.unit.test.ts b/src/test/common/variables/environmentVariablesProvider.unit.test.ts index 44cfabe35c62..f55d128e48fc 100644 --- a/src/test/common/variables/environmentVariablesProvider.unit.test.ts +++ b/src/test/common/variables/environmentVariablesProvider.unit.test.ts @@ -11,12 +11,10 @@ import * as typemoq from 'typemoq'; import { ConfigurationChangeEvent, FileSystemWatcher, Uri } from 'vscode'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; -import { PythonSettings } from '../../../client/common/configSettings'; -import { ConfigurationService } from '../../../client/common/configuration/service'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IPlatformService } from '../../../client/common/platform/types'; import { CurrentProcess } from '../../../client/common/process/currentProcess'; -import { IConfigurationService, ICurrentProcess, IPythonSettings } from '../../../client/common/types'; +import { ICurrentProcess } from '../../../client/common/types'; import { sleep } from '../../../client/common/utils/async'; import { EnvironmentVariablesService } from '../../../client/common/variables/environment'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; @@ -29,26 +27,27 @@ suite('Multiroot Environment Variables Provider', () => { let envVarsService: IEnvironmentVariablesService; let platform: IPlatformService; let workspace: IWorkspaceService; - let configuration: IConfigurationService; let currentProcess: ICurrentProcess; - let settings: IPythonSettings; + let envFile: string; setup(() => { + envFile = ''; envVarsService = mock(EnvironmentVariablesService); platform = mock(PlatformService); workspace = mock(WorkspaceService); - configuration = mock(ConfigurationService); currentProcess = mock(CurrentProcess); - settings = mock(PythonSettings); - when(configuration.getSettings(anything())).thenReturn(instance(settings)); when(workspace.onDidChangeConfiguration).thenReturn(noop as any); + when(workspace.getConfiguration('python', anything())).thenReturn({ + get: (settingName: string) => { + return settingName === 'envFile' ? envFile : ''; + }, + } as any); provider = new EnvironmentVariablesProvider( instance(envVarsService), [], instance(platform), instance(workspace), - instance(configuration), instance(currentProcess), ); @@ -198,12 +197,11 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Getting environment variables (without an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -211,20 +209,18 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual({}))).once(); verify(platform.pathVariableName).atLeast(1); assert.deepEqual(vars, {}); }); test(`Getting environment variables (with an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; const envFileVars = { MY_FILE: '1234' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH'); @@ -232,20 +228,18 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once(); verify(platform.pathVariableName).atLeast(1); assert.deepEqual(vars, envFileVars); }); test(`Getting environment variables (with an envfile, with PATH in current env, with PYTHONPATH in current env) & ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow', PATH: 'some path value', PYTHONPATH: 'some python path value' }; const envFileVars = { MY_FILE: '1234' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH'); @@ -253,7 +247,6 @@ suite('Multiroot Environment Variables Provider', () => { const vars = await provider.getEnvironmentVariables(workspaceUri); verify(currentProcess.env).atLeast(1); - verify(settings.envFile).atLeast(1); verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1); verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once(); verify(envVarsService.appendPath(deepEqual(envFileVars), currentProcEnv.PATH)).once(); @@ -263,12 +256,11 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Getting environment variables which are already cached does not reinvoke the method ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -280,7 +272,7 @@ suite('Multiroot Environment Variables Provider', () => { await provider.getEnvironmentVariables(workspaceUri); // Verify that the contents of `_getEnvironmentVariables()` method are only invoked once - verify(configuration.getSettings(anything())).once(); + verify(workspace.getConfiguration('python', anything())).once(); assert.deepEqual(vars, {}); }); @@ -290,7 +282,6 @@ suite('Multiroot Environment Variables Provider', () => { const currentProcEnv = { SOMETHING: 'wow' }; when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined); when(platform.pathVariableName).thenReturn('PATH'); @@ -300,7 +291,6 @@ suite('Multiroot Environment Variables Provider', () => { [], instance(platform), instance(workspace), - instance(configuration), instance(currentProcess), 100, ); @@ -312,14 +302,14 @@ suite('Multiroot Environment Variables Provider', () => { await provider.getEnvironmentVariables(workspaceUri); // Verify that the contents of `_getEnvironmentVariables()` method are invoked twice - verify(configuration.getSettings(anything())).twice(); + verify(workspace.getConfiguration('python', anything())).twice(); assert.deepEqual(vars, {}); }); test(`Environment variables are updated when env file changes ${workspaceTitle}`, async () => { const root = workspaceUri?.fsPath ?? ''; const sourceDir = path.join(root, 'a', 'b'); - const envFile = path.join(sourceDir, 'env.file'); + envFile = path.join(sourceDir, 'env.file'); const sourceFile = path.join(sourceDir, 'main.py'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; @@ -339,7 +329,6 @@ suite('Multiroot Environment Variables Provider', () => { when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); when(currentProcess.env).thenReturn(currentProcEnv); - when(settings.envFile).thenReturn(envFile); when(workspace.getWorkspaceFolder(anything())).thenReturn(workspaceFolder); when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars })); when(platform.pathVariableName).thenReturn('PATH'); From 18c263728516cfae642dd483a113bc523f63149e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 11 Feb 2021 00:11:18 -0800 Subject: [PATCH 11/21] Fix ESLint errors for environment variable provider (#15383) --- .eslintignore | 3 - .../variables/environmentVariablesProvider.ts | 17 ++++-- .../envVarsProvider.multiroot.test.ts | 4 +- .../environmentVariablesProvider.unit.test.ts | 59 +++++++++++++------ 4 files changed, 57 insertions(+), 26 deletions(-) diff --git a/.eslintignore b/.eslintignore index 9c9908ede444..806ca4428bef 100644 --- a/.eslintignore +++ b/.eslintignore @@ -209,8 +209,6 @@ src/test/common/featureDeprecationManager.unit.test.ts src/test/common/serviceRegistry.unit.test.ts src/test/common/extensions.unit.test.ts src/test/common/variables/envVarsService.unit.test.ts -src/test/common/variables/environmentVariablesProvider.unit.test.ts -src/test/common/variables/envVarsProvider.multiroot.test.ts src/test/common/nuget/azureBobStoreRepository.unit.test.ts src/test/common/helpers.test.ts src/test/common/application/commands/reloadCommand.unit.test.ts @@ -575,7 +573,6 @@ src/client/common/constants.ts src/client/common/variables/serviceRegistry.ts src/client/common/variables/environment.ts src/client/common/variables/types.ts -src/client/common/variables/environmentVariablesProvider.ts src/client/common/variables/sysTypes.ts src/client/common/variables/systemVariables.ts src/client/common/nuget/azureBlobStoreNugetRepository.ts diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts index 20ddff4b9fc0..2736a2dd8856 100644 --- a/src/client/common/variables/environmentVariablesProvider.ts +++ b/src/client/common/variables/environmentVariablesProvider.ts @@ -17,9 +17,13 @@ const CACHE_DURATION = 60 * 60 * 1000; @injectable() export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvider, Disposable { public trackedWorkspaceFolders = new Set(); + private fileWatchers = new Map(); + private disposables: Disposable[] = []; + private changeEventEmitter: EventEmitter; + private readonly envVarCaches = new Map>(); constructor( @@ -40,7 +44,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid return this.changeEventEmitter.event; } - public dispose() { + public dispose(): void { this.changeEventEmitter.dispose(); this.fileWatchers.forEach((watcher) => { if (watcher) { @@ -70,6 +74,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid cache.data = { ...vars }; return vars; } + public async _getEnvironmentVariables(resource?: Uri): Promise { let mergedVars = await this.getCustomEnvironmentVariables(resource); if (!mergedVars) { @@ -86,6 +91,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid } return mergedVars; } + public async getCustomEnvironmentVariables(resource?: Uri): Promise { const systemVariables: SystemVariables = new SystemVariables( undefined, @@ -99,7 +105,8 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid this.createFileWatcher(envFile, workspaceFolderUri); return this.envVarsService.parseFile(envFile, this.process.env); } - public configurationChanged(e: ConfigurationChangeEvent) { + + public configurationChanged(e: ConfigurationChangeEvent): void { this.trackedWorkspaceFolders.forEach((item) => { const uri = item && item.length > 0 ? Uri.file(item) : undefined; if (e.affectsConfiguration('python.envFile', uri)) { @@ -107,7 +114,8 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid } }); } - public createFileWatcher(envFile: string, workspaceFolderUri?: Uri) { + + public createFileWatcher(envFile: string, workspaceFolderUri?: Uri): void { if (this.fileWatchers.has(envFile)) { return; } @@ -119,9 +127,10 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(workspaceFolderUri))); } } + private getWorkspaceFolderUri(resource?: Uri): Uri | undefined { if (!resource) { - return; + return undefined; } const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource!); return workspaceFolder ? workspaceFolder.uri : undefined; diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index a62dff0cb8af..63a3dd8e411d 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -34,7 +34,7 @@ suite('Multiroot Environment Variables Provider', () => { const pathVariableName = getSearchPathEnvVarNames()[0]; suiteSetup(async function () { if (!IS_MULTI_ROOT_TEST) { - return this.skip(); + this.skip(); } await clearPythonPathInWorkspaceFolder(workspace4Path); await updateSetting('envFile', undefined, workspace4PyFile, ConfigurationTarget.WorkspaceFolder); @@ -178,7 +178,7 @@ suite('Multiroot Environment Variables Provider', () => { // this test is flaky on windows (likely the value of the path property // has incorrect path separator chars). Tracked by GH #4756 if (isOs(OSType.Windows)) { - return this.skip(); + this.skip(); } await updateSetting('envFile', '${workspaceRoot}/.env', workspace4PyFile, ConfigurationTarget.WorkspaceFolder); diff --git a/src/test/common/variables/environmentVariablesProvider.unit.test.ts b/src/test/common/variables/environmentVariablesProvider.unit.test.ts index f55d128e48fc..3ea7036e249d 100644 --- a/src/test/common/variables/environmentVariablesProvider.unit.test.ts +++ b/src/test/common/variables/environmentVariablesProvider.unit.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. @@ -65,7 +66,9 @@ suite('Multiroot Environment Variables Provider', () => { provider.trackedWorkspaceFolders.add(workspaceFolder1Uri.fsPath); provider.trackedWorkspaceFolders.add(workspaceFolder2Uri.fsPath); - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); const changedEvent: ConfigurationChangeEvent = { affectsConfiguration(setting: string, uri?: Uri) { return setting === 'python.envFile' && uri!.fsPath === workspaceFolder1Uri.fsPath; @@ -82,9 +85,11 @@ suite('Multiroot Environment Variables Provider', () => { const workspaceFolderUri = Uri.file('workspace1'); provider.trackedWorkspaceFolders.add(workspaceFolderUri.fsPath); - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); const changedEvent: ConfigurationChangeEvent = { - affectsConfiguration(_setting: string, _uri?: Uri) { + affectsConfiguration() { return false; }, }; @@ -95,9 +100,11 @@ suite('Multiroot Environment Variables Provider', () => { }); test('Event is not fired when workspace is not tracked', () => { let affectedWorkspace: Uri | undefined; - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); const changedEvent: ConfigurationChangeEvent = { - affectsConfiguration(_setting: string, _uri?: Uri) { + affectsConfiguration() { return true; }, }; @@ -110,17 +117,22 @@ suite('Multiroot Environment Variables Provider', () => { const workspaceTitle = workspaceUri ? '(with a workspace)' : '(without a workspace)'; test(`Event is fired when the environment file is modified ${workspaceTitle}`, () => { let affectedWorkspace: Uri | undefined = Uri.file('dummy value'); - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const fileSystemWatcher = typemoq.Mock.ofType(); + // eslint-disable-next-line @typescript-eslint/ban-types let onChangeHandler: undefined | ((resource?: Uri) => Function); fileSystemWatcher .setup((fs) => fs.onDidChange(typemoq.It.isAny())) - .callback((cb) => (onChangeHandler = cb)) + .callback((cb) => { + onChangeHandler = cb; + }) .verifiable(typemoq.Times.once()); when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); provider.createFileWatcher(envFile, workspaceUri); @@ -133,17 +145,22 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Event is fired when the environment file is deleted ${workspaceTitle}`, () => { let affectedWorkspace: Uri | undefined = Uri.file('dummy value'); - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const fileSystemWatcher = typemoq.Mock.ofType(); + // eslint-disable-next-line @typescript-eslint/ban-types let onDeleted: undefined | ((resource?: Uri) => Function); fileSystemWatcher .setup((fs) => fs.onDidDelete(typemoq.It.isAny())) - .callback((cb) => (onDeleted = cb)) + .callback((cb) => { + onDeleted = cb; + }) .verifiable(typemoq.Times.once()); when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); provider.createFileWatcher(envFile, workspaceUri); @@ -156,17 +173,22 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Event is fired when the environment file is created ${workspaceTitle}`, () => { let affectedWorkspace: Uri | undefined = Uri.file('dummy value'); - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const fileSystemWatcher = typemoq.Mock.ofType(); + // eslint-disable-next-line @typescript-eslint/ban-types let onCreated: undefined | ((resource?: Uri) => Function); fileSystemWatcher .setup((fs) => fs.onDidCreate(typemoq.It.isAny())) - .callback((cb) => (onCreated = cb)) + .callback((cb) => { + onCreated = cb; + }) .verifiable(typemoq.Times.once()); when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); - provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri)); + provider.onDidEnvironmentVariablesChange((uri) => { + affectedWorkspace = uri; + }); provider.createFileWatcher(envFile, workspaceUri); @@ -178,7 +200,7 @@ suite('Multiroot Environment Variables Provider', () => { assert.equal(affectedWorkspace, workspaceUri); }); test(`File system watcher event handlers are added once ${workspaceTitle}`, () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const fileSystemWatcher = typemoq.Mock.ofType(); fileSystemWatcher.setup((fs) => fs.onDidChange(typemoq.It.isAny())).verifiable(typemoq.Times.once()); @@ -277,7 +299,7 @@ suite('Multiroot Environment Variables Provider', () => { }); test(`Cache result must be cleared when cache expires ${workspaceTitle}`, async () => { - const envFile = path.join('a', 'b', 'env.file'); + envFile = path.join('a', 'b', 'env.file'); const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined; const currentProcEnv = { SOMETHING: 'wow' }; @@ -319,12 +341,15 @@ suite('Multiroot Environment Variables Provider', () => { }; const envFileVars = { MY_FILE: '1234', PYTHONPATH: `./foo${path.delimiter}./bar` }; + // eslint-disable-next-line @typescript-eslint/ban-types let onChangeHandler: undefined | ((resource?: Uri) => Function); const fileSystemWatcher = typemoq.Mock.ofType(); fileSystemWatcher .setup((fs) => fs.onDidChange(typemoq.It.isAny())) - .callback((cb) => (onChangeHandler = cb)) + .callback((cb) => { + onChangeHandler = cb; + }) .verifiable(typemoq.Times.once()); when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object); From ca8cbdfc05042c7fda24578062ca5b8d98baea8a Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 9 Feb 2021 14:21:57 -0800 Subject: [PATCH 12/21] Fix problem with notebook apis not being used. (#15366) --- news/2 Fixes/15364.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/15364.md diff --git a/news/2 Fixes/15364.md b/news/2 Fixes/15364.md new file mode 100644 index 000000000000..3d49c2ac9734 --- /dev/null +++ b/news/2 Fixes/15364.md @@ -0,0 +1 @@ +Allow support for using notebook APIs in the VS code stable build. From 55539535b1d8f6c171cfd5149202b1d20ebba6c8 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Thu, 11 Feb 2021 14:11:46 -0800 Subject: [PATCH 13/21] Show Python: Launch TensorBoard command in command palette (#15382) (#15386) * Always register Launch TensorBoard command * Put usage tracking behind experiment --- package.json | 3 +- .../tensorBoard/tensorBoardSessionProvider.ts | 43 +++++++------------ .../tensorBoard/tensorBoardUsageTracker.ts | 7 ++- .../tensorBoard/tensorBoardSession.test.ts | 5 --- .../tensorBoardUsageTracker.unit.test.ts | 13 +++++- 5 files changed, 35 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index 68e97e23b293..5e71bed1cd5a 100644 --- a/package.json +++ b/package.json @@ -488,8 +488,7 @@ }, { "command": "python.launchTensorBoard", - "category": "Python", - "when": "python.isInNativeTensorBoardExperiment" + "category": "Python" } ], "view/title": [ diff --git a/src/client/tensorBoard/tensorBoardSessionProvider.ts b/src/client/tensorBoard/tensorBoardSessionProvider.ts index 0ac4a7be3468..3067110f5b86 100644 --- a/src/client/tensorBoard/tensorBoardSessionProvider.ts +++ b/src/client/tensorBoard/tensorBoardSessionProvider.ts @@ -5,11 +5,9 @@ import { inject, injectable } from 'inversify'; import { IExtensionSingleActivationService } from '../activation/types'; import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; import { Commands } from '../common/constants'; -import { ContextKey } from '../common/contextKey'; -import { NativeTensorBoard } from '../common/experiments/groups'; import { traceError, traceInfo } from '../common/logger'; import { IProcessServiceFactory } from '../common/process/types'; -import { IDisposableRegistry, IExperimentService, IInstaller } from '../common/types'; +import { IDisposableRegistry, IInstaller } from '../common/types'; import { TensorBoard } from '../common/utils/localize'; import { IInterpreterService } from '../interpreter/contracts'; import { sendTelemetryEvent } from '../telemetry'; @@ -26,34 +24,25 @@ export class TensorBoardSessionProvider implements IExtensionSingleActivationSer @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - @inject(IExperimentService) private readonly experimentService: IExperimentService, @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, ) {} public async activate(): Promise { - this.activateInternal().ignoreErrors(); - } - - private async activateInternal() { - if (await this.experimentService.inExperiment(NativeTensorBoard.experiment)) { - this.disposables.push( - this.commandManager.registerCommand( - Commands.LaunchTensorBoard, - ( - entrypoint: TensorBoardEntrypoint = TensorBoardEntrypoint.palette, - trigger: TensorBoardEntrypointTrigger = TensorBoardEntrypointTrigger.palette, - ) => { - sendTelemetryEvent(EventName.TENSORBOARD_SESSION_LAUNCH, undefined, { - trigger, - entrypoint, - }); - return this.createNewSession(); - }, - ), - ); - const contextKey = new ContextKey('python.isInNativeTensorBoardExperiment', this.commandManager); - contextKey.set(true).ignoreErrors(); - } + this.disposables.push( + this.commandManager.registerCommand( + Commands.LaunchTensorBoard, + ( + entrypoint: TensorBoardEntrypoint = TensorBoardEntrypoint.palette, + trigger: TensorBoardEntrypointTrigger = TensorBoardEntrypointTrigger.palette, + ) => { + sendTelemetryEvent(EventName.TENSORBOARD_SESSION_LAUNCH, undefined, { + trigger, + entrypoint, + }); + return this.createNewSession(); + }, + ), + ); } private async createNewSession(): Promise { diff --git a/src/client/tensorBoard/tensorBoardUsageTracker.ts b/src/client/tensorBoard/tensorBoardUsageTracker.ts index 750e6c1e835f..90f6ef7e9b32 100644 --- a/src/client/tensorBoard/tensorBoardUsageTracker.ts +++ b/src/client/tensorBoard/tensorBoardUsageTracker.ts @@ -7,7 +7,8 @@ import { TextEditor } from 'vscode'; import { IExtensionSingleActivationService } from '../activation/types'; import { IDocumentManager } from '../common/application/types'; import { isTestExecution } from '../common/constants'; -import { IDisposableRegistry } from '../common/types'; +import { NativeTensorBoard } from '../common/experiments/groups'; +import { IDisposableRegistry, IExperimentService } from '../common/types'; import { getDocumentLines } from '../telemetry/importTracker'; import { TensorBoardEntrypointTrigger } from './constants'; import { containsTensorBoardImport } from './helpers'; @@ -23,9 +24,13 @@ export class TensorBoardUsageTracker implements IExtensionSingleActivationServic @inject(IDocumentManager) private documentManager: IDocumentManager, @inject(IDisposableRegistry) private disposables: IDisposableRegistry, @inject(TensorBoardPrompt) private prompt: TensorBoardPrompt, + @inject(IExperimentService) private experimentService: IExperimentService, ) {} public async activate(): Promise { + if (!(await this.experimentService.inExperiment(NativeTensorBoard.experiment))) { + return; + } if (testExecution) { await this.activateInternal(); } else { diff --git a/src/test/tensorBoard/tensorBoardSession.test.ts b/src/test/tensorBoard/tensorBoardSession.test.ts index b66549687c26..fbbf535839db 100644 --- a/src/test/tensorBoard/tensorBoardSession.test.ts +++ b/src/test/tensorBoard/tensorBoardSession.test.ts @@ -7,7 +7,6 @@ import { TensorBoard } from '../../client/common/utils/localize'; import { IServiceManager } from '../../client/ioc/types'; import { TensorBoardEntrypoint, TensorBoardEntrypointTrigger } from '../../client/tensorBoard/constants'; import { TensorBoardSession } from '../../client/tensorBoard/tensorBoardSession'; -import { TensorBoardSessionProvider } from '../../client/tensorBoard/tensorBoardSessionProvider'; import { closeActiveWindows, initialize } from '../initialize'; import * as ExperimentHelpers from '../../client/common/experiments/helpers'; import { IInterpreterService } from '../../client/interpreter/contracts'; @@ -31,7 +30,6 @@ suite('TensorBoard session creation', async () => { let serviceManager: IServiceManager; let errorMessageStub: Sinon.SinonStub; let sandbox: Sinon.SinonSandbox; - let provider: TensorBoardSessionProvider; let applicationShell: IApplicationShell; let commandManager: ICommandManager; @@ -59,9 +57,6 @@ suite('TensorBoard session creation', async () => { const interpreterService = serviceManager.get(IInterpreterService); sandbox.stub(interpreterService, 'getActiveInterpreter').resolves(interpreter); - // Create tensorboard session provider - provider = serviceManager.get(TensorBoardSessionProvider); - await provider.activate(); applicationShell = serviceManager.get(IApplicationShell); errorMessageStub = sandbox.stub(applicationShell, 'showErrorMessage'); commandManager = serviceManager.get(ICommandManager); diff --git a/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts b/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts index a267c5cb8569..a29af0207623 100644 --- a/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts +++ b/src/test/tensorBoard/tensorBoardUsageTracker.unit.test.ts @@ -1,21 +1,32 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; +import { mock, when, instance } from 'ts-mockito'; import { TensorBoardUsageTracker } from '../../client/tensorBoard/tensorBoardUsageTracker'; import { TensorBoardPrompt } from '../../client/tensorBoard/tensorBoardPrompt'; import { MockDocumentManager } from '../startPage/mockDocumentManager'; import { createTensorBoardPromptWithMocks } from './helpers'; +import { NativeTensorBoard } from '../../client/common/experiments/groups'; +import { ExperimentService } from '../../client/common/experiments/service'; suite('TensorBoard usage tracker', () => { let documentManager: MockDocumentManager; let tensorBoardImportTracker: TensorBoardUsageTracker; let prompt: TensorBoardPrompt; + let experimentService: ExperimentService; let showNativeTensorBoardPrompt: sinon.SinonSpy; setup(() => { documentManager = new MockDocumentManager(); prompt = createTensorBoardPromptWithMocks(); showNativeTensorBoardPrompt = sinon.spy(prompt, 'showNativeTensorBoardPrompt'); - tensorBoardImportTracker = new TensorBoardUsageTracker(documentManager, [], prompt); + experimentService = mock(ExperimentService); + when(experimentService.inExperiment(NativeTensorBoard.experiment)).thenResolve(true); + tensorBoardImportTracker = new TensorBoardUsageTracker( + documentManager, + [], + prompt, + instance(experimentService), + ); }); test('Simple tensorboard import in Python file', async () => { From 2179b3014c2bfa57f199906a202e89a195231648 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 17 Feb 2021 12:24:17 -0800 Subject: [PATCH 14/21] Cherry picks from main to release (#15421) * Do not call activate the discovery component before registering all the classes (#15379) * Do not attempt to activate discovery component before registering all the classes * Add clarification comment * Code reviews * Skip windows store and shims paths when using known path locators (#15388) * Skip windows store and shims paths when using known path locators * Clean up and comments * Tests * Handle cases where envs variables might not be set * Typo Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> * Change "Pylance not installed" prompt to allow reverting to Jedi (#15420) * Allow on suggestion refresh by default (#15430) Co-authored-by: Kartik Raj Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com> Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> --- package.nls.json | 4 +- package.nls.ru.json | 1 - package.nls.zh-cn.json | 1 - src/client/activation/activationService.ts | 2 + src/client/activation/common/activatorBase.ts | 2 +- .../common/languageServerChangeHandler.ts | 36 +++-- src/client/activation/node/activator.ts | 7 +- src/client/common/utils/localize.ts | 11 +- src/client/extensionActivation.ts | 13 +- .../lowLevel/windowsKnownPathsLocator.ts | 13 ++ .../services/posixKnownPathsLocator.ts | 7 +- .../locators/services/pyenvLocator.ts | 13 +- .../locators/services/windowsStoreLocator.ts | 88 +++++++++--- src/client/pythonEnvironments/legacyIOC.ts | 2 +- .../activation/node/activator.unit.test.ts | 27 ++-- .../languageServerChangeHandler.unit.test.ts | 131 +++++++++++++++--- .../common/environmentIdentifier.unit.test.ts | 17 ++- .../locators/pyenvLocator.unit.test.ts | 22 +++ .../locators/windowsStoreLocator.unit.test.ts | 10 ++ 19 files changed, 332 insertions(+), 75 deletions(-) diff --git a/package.nls.json b/package.nls.json index 3c903570d2b9..e86c191091a9 100644 --- a/package.nls.json +++ b/package.nls.json @@ -49,9 +49,11 @@ "Pylance.proposePylanceMessage": "Try out a new faster, feature-rich language server for Python by Microsoft, Pylance! Install the extension now.", "Pylance.tryItNow": "Try it now", "Pylance.remindMeLater": "Remind me later", - "Pylance.installPylanceMessage": "Pylance extension is not installed. Click Yes to open Pylance installation page.", "Pylance.pylanceNotInstalledMessage": "Pylance extension is not installed.", "Pylance.pylanceInstalledReloadPromptMessage": "Pylance extension is now installed. Reload window to activate?", + "Pylance.pylanceRevertToJediPrompt": "The Pylance extension is not installed but the python.languageServer value is set to \"Pylance\". Would you like to install the Pylance extension to use Pylance, or revert back to Jedi?", + "Pylance.pylanceInstallPylance": "Install Pylance", + "Pylance.pylanceRevertToJedi": "Revert to Jedi", "Experiments.inGroup": "User belongs to experiment group '{0}'", "Experiments.optedOutOf": "User opted out of experiment group '{0}'", "Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters", diff --git a/package.nls.ru.json b/package.nls.ru.json index 9b6b0a5661da..0a80260e6644 100644 --- a/package.nls.ru.json +++ b/package.nls.ru.json @@ -34,7 +34,6 @@ "Pylance.proposePylanceMessage": "Попробуйте новый языковый сервер для Python от Microsoft: Pylance! Установите расширение Pylance.", "Pylance.tryItNow": "Да, хочу", "Pylance.remindMeLater": "Напомните позже", - "Pylance.installPylanceMessage": "Расширение Pylance не установлено. Нажмите Да чтобы открыть страницу установки Pylance.", "Pylance.pylanceNotInstalledMessage": "Расширение Pylance не установлено.", "Pylance.pylanceInstalledReloadPromptMessage": "Расширение Pylance установлено. Перезагрузить окно для его активации?", "LanguageService.reloadAfterLanguageServerChange": "Пожалуйста, перезагрузите окно после смены типа языкового сервера." diff --git a/package.nls.zh-cn.json b/package.nls.zh-cn.json index 823b1d6a9268..14b9701851f8 100644 --- a/package.nls.zh-cn.json +++ b/package.nls.zh-cn.json @@ -49,7 +49,6 @@ "Pylance.proposePylanceMessage": "试试微软新的更快的、功能丰富的语言服务器 Pylance! ", "Pylance.tryItNow": "立即安装", "Pylance.remindMeLater": "稍后提醒", - "Pylance.installPylanceMessage": "Pylance 扩展未安装。点击 \"是\" 打开 Pylance 安装页面。", "Pylance.pylanceNotInstalledMessage": "Pylance 扩展未安装。", "Pylance.pylanceInstalledReloadPromptMessage": "Pylance 扩展未安装。重新加载窗口以激活？", "Experiments.inGroup": "用户属于 '{0}' 实验组", diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 82bc3eebc33b..b63721acd392 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -86,6 +86,8 @@ export class LanguageServerExtensionActivationService this.serviceContainer.get(IExtensions), this.serviceContainer.get(IApplicationShell), this.serviceContainer.get(ICommandManager), + this.serviceContainer.get(IWorkspaceService), + this.serviceContainer.get(IConfigurationService), ); disposables.push(this.languageServerChangeHandler); } diff --git a/src/client/activation/common/activatorBase.ts b/src/client/activation/common/activatorBase.ts index 7dcc80455291..3de6b41c2e98 100644 --- a/src/client/activation/common/activatorBase.ts +++ b/src/client/activation/common/activatorBase.ts @@ -42,7 +42,7 @@ export abstract class LanguageServerActivatorBase implements ILanguageServerActi protected resource?: Resource; constructor( protected readonly manager: ILanguageServerManager, - private readonly workspace: IWorkspaceService, + protected readonly workspace: IWorkspaceService, protected readonly fs: IFileSystem, protected readonly configurationService: IConfigurationService, ) {} diff --git a/src/client/activation/common/languageServerChangeHandler.ts b/src/client/activation/common/languageServerChangeHandler.ts index d81f2cfdc723..22c325b6d6e5 100644 --- a/src/client/activation/common/languageServerChangeHandler.ts +++ b/src/client/activation/common/languageServerChangeHandler.ts @@ -1,10 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { Disposable } from 'vscode'; -import { IApplicationShell, ICommandManager } from '../../common/application/types'; +import { ConfigurationTarget, Disposable } from 'vscode'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types'; import { PYLANCE_EXTENSION_ID } from '../../common/constants'; -import { IExtensions } from '../../common/types'; +import { IConfigurationService, IExtensions } from '../../common/types'; import { createDeferred } from '../../common/utils/async'; import { Common, LanguageService, Pylance } from '../../common/utils/localize'; import { LanguageServerType } from '../types'; @@ -12,16 +12,32 @@ import { LanguageServerType } from '../types'; export async function promptForPylanceInstall( appShell: IApplicationShell, commandManager: ICommandManager, + workspace: IWorkspaceService, + configService: IConfigurationService, ): Promise { - // If not installed, point user to Pylance at the store. const response = await appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ); - if (response === Common.bannerLabelYes()) { + if (response === Pylance.pylanceInstallPylance()) { commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID); + } else if (response === Pylance.pylanceRevertToJedi()) { + const inspection = workspace.getConfiguration('python').inspect('languageServer'); + + let target: ConfigurationTarget | undefined; + if (inspection?.workspaceValue) { + target = ConfigurationTarget.Workspace; + } else if (inspection?.globalValue) { + target = ConfigurationTarget.Global; + } + + if (target) { + await configService.updateSetting('languageServer', LanguageServerType.Jedi, undefined, target); + commandManager.executeCommand('workbench.action.reloadWindow'); + } } } @@ -37,6 +53,8 @@ export class LanguageServerChangeHandler implements Disposable { private readonly extensions: IExtensions, private readonly appShell: IApplicationShell, private readonly commands: ICommandManager, + private readonly workspace: IWorkspaceService, + private readonly configService: IConfigurationService, ) { this.pylanceInstalled = this.isPylanceInstalled(); this.disposables.push( @@ -70,7 +88,7 @@ export class LanguageServerChangeHandler implements Disposable { let response: string | undefined; if (lsType === LanguageServerType.Node && !this.isPylanceInstalled()) { // If not installed, point user to Pylance at the store. - await promptForPylanceInstall(this.appShell, this.commands); + await promptForPylanceInstall(this.appShell, this.commands, this.workspace, this.configService); // At this point Pylance is not yet installed. Skip reload prompt // since we are going to show it when Pylance becomes available. } else { diff --git a/src/client/activation/node/activator.ts b/src/client/activation/node/activator.ts index 1d5766c1856e..fa3a99e2df01 100644 --- a/src/client/activation/node/activator.ts +++ b/src/client/activation/node/activator.ts @@ -47,7 +47,12 @@ export class NodeLanguageServerActivator extends LanguageServerActivatorBase { // Pylance is not yet installed. Throw will cause activator to use Jedi // temporarily. Language server installation tracker will prompt for window // reload when Pylance becomes available. - await promptForPylanceInstall(this.appShell, this.commandManager); + await promptForPylanceInstall( + this.appShell, + this.commandManager, + this.workspace, + this.configurationService, + ); throw new Error(Pylance.pylanceNotInstalledMessage()); } } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 65423b36f133..ca090b39afff 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -114,10 +114,6 @@ export namespace Pylance { export const tryItNow = localize('Pylance.tryItNow', 'Try it now'); export const remindMeLater = localize('Pylance.remindMeLater', 'Remind me later'); - export const installPylanceMessage = localize( - 'Pylance.installPylanceMessage', - 'Pylance extension is not installed. Click Yes to open Pylance installation page.', - ); export const pylanceNotInstalledMessage = localize( 'Pylance.pylanceNotInstalledMessage', 'Pylance extension is not installed.', @@ -126,6 +122,13 @@ export namespace Pylance { 'Pylance.pylanceInstalledReloadPromptMessage', 'Pylance extension is now installed. Reload window to activate?', ); + + export const pylanceRevertToJediPrompt = localize( + 'Pylance.pylanceRevertToJediPrompt', + 'The Pylance extension is not installed but the python.languageServer value is set to "Pylance". Would you like to install the Pylance extension to use Pylance, or revert back to Jedi?', + ); + export const pylanceInstallPylance = localize('Pylance.pylanceInstallPylance', 'Install Pylance'); + export const pylanceRevertToJedi = localize('Pylance.pylanceRevertToJedi', 'Revert to Jedi'); } export namespace Jupyter { diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 816a0d400333..0652602250d0 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -78,12 +78,19 @@ export async function activateComponents( // `Promise.all()`, etc.) will flatten nested promises. Thus // activation resolves `ActivationResult`, which can safely wrap // the "inner" promise. + + // TODO: As of now activateLegacy() registers various classes which might + // be required while activating components. Once registration from + // activateLegacy() are moved before we activate other components, we can + // activate them parallelly with the other components. + // https://github.com/microsoft/vscode-python/issues/15380 + // These will go away eventually once everything is refactored into components. + const legacyActivationResult = await activateLegacy(ext); const promises: Promise[] = [ + // More component activations will go here pythonEnvironments.activate(components.pythonEnvs), - // These will go away eventually. - activateLegacy(ext), ]; - return Promise.all(promises); + return Promise.all([legacyActivationResult, ...promises]); } /// ////////////////////////// diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index c5f1bde4733e..d9e986db45de 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -9,6 +9,8 @@ import { Event } from 'vscode'; import { getSearchPathEntries } from '../../../../common/utils/exec'; import { Disposables, IDisposable } from '../../../../common/utils/resourceLifecycle'; import { isStandardPythonBinary } from '../../../common/commonUtils'; +import { isPyenvShimDir } from '../../../discovery/locators/services/pyenvLocator'; +import { isWindowsStoreDir } from '../../../discovery/locators/services/windowsStoreLocator'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource } from '../../info'; import { ILocator, IPythonEnvsIterator, PythonLocatorQuery } from '../../locator'; import { Locators } from '../../locators'; @@ -31,6 +33,17 @@ export class WindowsPathEnvVarLocator implements ILocator, IDisposable { constructor() { const dirLocators: (ILocator & IDisposable)[] = getSearchPathEntries() + .filter((dirname) => { + // Filter out following directories: + // 1. Windows Store app directories: We have a store app locator that handles this. The + // python.exe available in these directories might not be python. It can be a store + // install shortcut that takes you to windows store. + // + // 2. Filter out pyenv shims: They are not actual python binaries, they are used to launch + // the binaries specified in .python-version file in the cwd. We should not be reporting + // those binaries as environments. + return !isWindowsStoreDir(dirname) && !isPyenvShimDir(dirname); + }) // Build a locator for each directory. .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.Unknown)); this.disposables.push(...dirLocators); diff --git a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts index 8dc0e1dc095b..e7017089280d 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts @@ -4,7 +4,6 @@ import * as fs from 'fs'; import * as path from 'path'; import { traceError, traceInfo } from '../../../../common/logger'; - import { Architecture } from '../../../../common/utils/platform'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource, PythonReleaseLevel, PythonVersion } from '../../../base/info'; import { buildEnvInfo } from '../../../base/info/env'; @@ -12,9 +11,13 @@ import { parseVersion } from '../../../base/info/pythonVersion'; import { IPythonEnvsIterator, Locator } from '../../../base/locator'; import { getFileInfo, resolveSymbolicLink } from '../../../common/externalDependencies'; import { commonPosixBinPaths, isPosixPythonBinPattern } from '../../../common/posixUtils'; +import { isPyenvShimDir } from './pyenvLocator'; async function getPythonBinFromKnownPaths(): Promise { - const knownDirs = await commonPosixBinPaths(); + // Filter out pyenv shims. They are not actual python binaries, they are used to launch + // the binaries specified in .python-version file in the cwd. We should not be reporting + // those binaries as environments. + const knownDirs = (await commonPosixBinPaths()).filter((dirname) => !isPyenvShimDir(dirname)); const pythonBins: Set = new Set(); for (const dirname of knownDirs) { const paths = (await fs.promises.readdir(dirname, { withFileTypes: true })) diff --git a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts index 9fcbde3bd2dc..ddac1574932d 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts @@ -13,7 +13,7 @@ import { getInterpreterPathFromDir, getPythonVersionFromPath, } from '../../../common/commonUtils'; -import { getFileInfo, getSubDirs, pathExists } from '../../../common/externalDependencies'; +import { arePathsSame, getFileInfo, getSubDirs, pathExists } from '../../../common/externalDependencies'; function getPyenvDir(): string { // Check if the pyenv environment variables exist: PYENV on Windows, PYENV_ROOT on Unix. @@ -37,6 +37,17 @@ function getPyenvVersionsDir(): string { return path.join(getPyenvDir(), 'versions'); } +/** + * Checks if a given directory path is same as `pyenv` shims path. This checks + * `~/.pyenv/shims` on posix and `~/.pyenv/pyenv-win/shims` on windows. + * @param {string} dirPath: Absolute path to any directory + * @returns {boolean}: Returns true if the patch is same as `pyenv` shims directory. + */ +export function isPyenvShimDir(dirPath: string): boolean { + const shimPath = path.join(getPyenvDir(), 'shims'); + return arePathsSame(shimPath, dirPath) || arePathsSame(`${shimPath}${path.sep}`, dirPath); +} + /** * Checks if the given interpreter belongs to a pyenv based environment. * @param {string} interpreterPath: Absolute path to the python interpreter. diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts index 571c415551a2..daa46800dc0d 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts @@ -11,7 +11,7 @@ import { buildEnvInfo } from '../../../base/info/env'; import { getPythonVersionFromPath } from '../../../base/info/pythonVersion'; import { IPythonEnvsIterator } from '../../../base/locator'; import { FSWatchingLocator } from '../../../base/locators/lowLevel/fsWatchingLocator'; -import { getFileInfo } from '../../../common/externalDependencies'; +import { getFileInfo, pathExists } from '../../../common/externalDependencies'; import { PythonEnvStructure } from '../../../common/pythonBinariesWatcher'; /** @@ -26,16 +26,59 @@ export function getWindowsStoreAppsRoot(): string { /** * Checks if a given path is under the forbidden windows store directory. - * @param {string} interpreterPath : Absolute path to the python interpreter. + * @param {string} absPath : Absolute path to a file or directory. * @returns {boolean} : Returns true if `interpreterPath` is under * `%ProgramFiles%/WindowsApps`. */ -export function isForbiddenStorePath(interpreterPath: string): boolean { +export function isForbiddenStorePath(absPath: string): boolean { const programFilesStorePath = path .join(getEnvironmentVariable('ProgramFiles') || 'Program Files', 'WindowsApps') .normalize() .toUpperCase(); - return path.normalize(interpreterPath).toUpperCase().includes(programFilesStorePath); + return path.normalize(absPath).toUpperCase().includes(programFilesStorePath); +} + +/** + * Checks if a given directory is any one of the possible windows store directories, or + * its sub-directory. + * @param {string} dirPath : Absolute path to a directory. + * + * Remarks: + * These locations are tested: + * 1. %LOCALAPPDATA%/Microsoft/WindowsApps + * 2. %ProgramFiles%/WindowsApps + */ +export function isWindowsStoreDir(dirPath: string): boolean { + const storeRootPath = path.normalize(getWindowsStoreAppsRoot()).toUpperCase(); + return path.normalize(dirPath).toUpperCase().includes(storeRootPath) || isForbiddenStorePath(dirPath); +} + +/** + * Checks if store python is installed. + * @param {string} interpreterPath : Absolute path to a interpreter. + * Remarks: + * If store python was never installed then the store apps directory will not + * have idle.exe or pip.exe. We can use this as a way to identify the python.exe + * found in the store apps directory is a real python or a store install shortcut. + */ +async function isStorePythonInstalled(interpreterPath?: string): Promise { + let results = await Promise.all([ + pathExists(path.join(getWindowsStoreAppsRoot(), 'idle.exe')), + pathExists(path.join(getWindowsStoreAppsRoot(), 'pip.exe')), + ]); + + if (results.includes(true)) { + return true; + } + + if (interpreterPath) { + results = await Promise.all([ + pathExists(path.join(path.dirname(interpreterPath), 'idle.exe')), + pathExists(path.join(path.dirname(interpreterPath), 'pip.exe')), + ]); + return results.includes(true); + } + return false; } /** @@ -71,17 +114,19 @@ export function isForbiddenStorePath(interpreterPath: string): boolean { * */ export async function isWindowsStoreEnvironment(interpreterPath: string): Promise { - const pythonPathToCompare = path.normalize(interpreterPath).toUpperCase(); - const localAppDataStorePath = path.normalize(getWindowsStoreAppsRoot()).toUpperCase(); - if (pythonPathToCompare.includes(localAppDataStorePath)) { - return true; - } + if (await isStorePythonInstalled(interpreterPath)) { + const pythonPathToCompare = path.normalize(interpreterPath).toUpperCase(); + const localAppDataStorePath = path.normalize(getWindowsStoreAppsRoot()).toUpperCase(); + if (pythonPathToCompare.includes(localAppDataStorePath)) { + return true; + } - // Program Files store path is a forbidden path. Only admins and system has access this path. - // We should never have to look at this path or even execute python from this path. - if (isForbiddenStorePath(pythonPathToCompare)) { - traceWarning('isWindowsStoreEnvironment called with Program Files store path.'); - return true; + // Program Files store path is a forbidden path. Only admins and system has access this path. + // We should never have to look at this path or even execute python from this path. + if (isForbiddenStorePath(pythonPathToCompare)) { + traceWarning('isWindowsStoreEnvironment called with Program Files store path.'); + return true; + } } return false; } @@ -107,7 +152,7 @@ const pythonExeGlob = 'python3.{[0-9],[0-9][0-9]}.exe'; * @param {string} interpreterPath : Path to python interpreter. * @returns {boolean} : Returns true if the path matches pattern for windows python executable. */ -export function isWindowsStorePythonExe(interpreterPath: string): boolean { +export function isWindowsStorePythonExePattern(interpreterPath: string): boolean { return minimatch(path.basename(interpreterPath), pythonExeGlob, { nocase: true }); } @@ -128,11 +173,16 @@ export function isWindowsStorePythonExe(interpreterPath: string): boolean { * that location. */ export async function getWindowsStorePythonExes(): Promise { - const windowsAppsRoot = getWindowsStoreAppsRoot(); + if (await isStorePythonInstalled()) { + const windowsAppsRoot = getWindowsStoreAppsRoot(); - // Collect python*.exe directly under %LOCALAPPDATA%/Microsoft/WindowsApps - const files = await fsapi.readdir(windowsAppsRoot); - return files.map((filename: string) => path.join(windowsAppsRoot, filename)).filter(isWindowsStorePythonExe); + // Collect python*.exe directly under %LOCALAPPDATA%/Microsoft/WindowsApps + const files = await fsapi.readdir(windowsAppsRoot); + return files + .map((filename: string) => path.join(windowsAppsRoot, filename)) + .filter(isWindowsStorePythonExePattern); + } + return []; } export class WindowsStoreLocator extends FSWatchingLocator { diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index e1f09632e33c..c8c2d1ad736d 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -142,7 +142,7 @@ class ComponentAdapter implements IComponentAdapter, IExtensionSingleActivationS private readonly refreshed = new vscode.EventEmitter(); - private allowOnSuggestionRefresh = false; + private allowOnSuggestionRefresh = true; constructor( // The adapter only wraps one thing: the component API. diff --git a/src/test/activation/node/activator.unit.test.ts b/src/test/activation/node/activator.unit.test.ts index bfc3bfa3cb0f..1207526d1f09 100644 --- a/src/test/activation/node/activator.unit.test.ts +++ b/src/test/activation/node/activator.unit.test.ts @@ -17,7 +17,7 @@ import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; import { IConfigurationService, IExtensions, IPythonSettings } from '../../../client/common/types'; -import { Common, Pylance } from '../../../client/common/utils/localize'; +import { Pylance } from '../../../client/common/utils/localize'; suite('Pylance Language Server - Activator', () => { let activator: NodeLanguageServerActivator; @@ -92,20 +92,22 @@ suite('Pylance Language Server - Activator', () => { test('When Pylance is not installed activator should show install prompt ', async () => { when( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), - ).thenReturn(Promise.resolve(Common.bannerLabelNo())); + ).thenReturn(Promise.resolve(Pylance.remindMeLater())); try { await activator.start(undefined); } catch {} verify( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), ).once(); verify(commandManager.executeCommand('extension.open', PYLANCE_EXTENSION_ID)).never(); @@ -114,11 +116,12 @@ suite('Pylance Language Server - Activator', () => { test('When Pylance is not installed activator should open Pylance install page if users clicks Yes', async () => { when( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), - ).thenReturn(Promise.resolve(Common.bannerLabelYes())); + ).thenReturn(Promise.resolve(Pylance.pylanceInstallPylance())); try { await activator.start(undefined); diff --git a/src/test/activation/node/languageServerChangeHandler.unit.test.ts b/src/test/activation/node/languageServerChangeHandler.unit.test.ts index 09c96206708c..d66b4e574f0f 100644 --- a/src/test/activation/node/languageServerChangeHandler.unit.test.ts +++ b/src/test/activation/node/languageServerChangeHandler.unit.test.ts @@ -3,13 +3,13 @@ 'use strict'; -import { anyString, instance, mock, verify, when } from 'ts-mockito'; -import { EventEmitter, Extension } from 'vscode'; +import { anyString, instance, mock, verify, when, anything } from 'ts-mockito'; +import { ConfigurationTarget, EventEmitter, Extension, WorkspaceConfiguration } from 'vscode'; import { LanguageServerChangeHandler } from '../../../client/activation/common/languageServerChangeHandler'; import { LanguageServerType } from '../../../client/activation/types'; -import { IApplicationShell, ICommandManager } from '../../../client/common/application/types'; +import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../client/common/application/types'; import { PYLANCE_EXTENSION_ID } from '../../../client/common/constants'; -import { IExtensions } from '../../../client/common/types'; +import { IConfigurationService, IExtensions } from '../../../client/common/types'; import { Common, LanguageService, Pylance } from '../../../client/common/utils/localize'; suite('Language Server - Change Handler', () => { @@ -19,11 +19,16 @@ suite('Language Server - Change Handler', () => { let extensionsChangedEvent: EventEmitter; let handler: LanguageServerChangeHandler; + let workspace: IWorkspaceService; + let configService: IConfigurationService; + let pylanceExtension: Extension; setup(() => { extensions = mock(); appShell = mock(); commands = mock(); + workspace = mock(); + configService = mock(); pylanceExtension = mock>(); @@ -84,9 +89,10 @@ suite('Language Server - Change Handler', () => { test('Handler should prompt for install when language server changes to Pylance and Pylance is not installed', async () => { when( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), ).thenReturn(Promise.resolve(undefined)); @@ -98,9 +104,10 @@ suite('Language Server - Change Handler', () => { ).never(); verify( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), ).once(); }); @@ -108,11 +115,12 @@ suite('Language Server - Change Handler', () => { test('Handler should open Pylance store page when language server changes to Pylance, Pylance is not installed and user clicks Yes', async () => { when( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), - ).thenReturn(Promise.resolve(Common.bannerLabelYes())); + ).thenReturn(Promise.resolve(Pylance.pylanceInstallPylance())); handler = makeHandler(undefined); await handler.handleLanguageServerChange(LanguageServerType.Node); @@ -124,11 +132,12 @@ suite('Language Server - Change Handler', () => { test('Handler should not open Pylance store page when language server changes to Pylance, Pylance is not installed and user clicks No', async () => { when( appShell.showWarningMessage( - Pylance.installPylanceMessage(), - Common.bannerLabelYes(), - Common.bannerLabelNo(), + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), ), - ).thenReturn(Promise.resolve(Common.bannerLabelNo())); + ).thenReturn(Promise.resolve(Pylance.remindMeLater())); handler = makeHandler(undefined); await handler.handleLanguageServerChange(LanguageServerType.Node); @@ -171,12 +180,98 @@ suite('Language Server - Change Handler', () => { verify(commands.executeCommand('workbench.action.reloadWindow')).never(); }); + [ConfigurationTarget.Global, ConfigurationTarget.Workspace].forEach((target) => { + const targetName = target === ConfigurationTarget.Global ? 'global' : 'workspace'; + test(`Revert to Jedi with setting in ${targetName} config`, async () => { + const configuration = mock(); + + when( + appShell.showWarningMessage( + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), + ), + ).thenReturn(Promise.resolve(Pylance.pylanceRevertToJedi())); + + when(workspace.getConfiguration('python')).thenReturn(instance(configuration)); + + const inspection = { + key: 'python.languageServer', + workspaceValue: target === ConfigurationTarget.Workspace ? LanguageServerType.Node : undefined, + globalValue: target === ConfigurationTarget.Global ? LanguageServerType.Node : undefined, + }; + + when(configuration.inspect('languageServer')).thenReturn(inspection); + + handler = makeHandler(undefined); + await handler.handleLanguageServerChange(LanguageServerType.Node); + + verify( + appShell.showInformationMessage(LanguageService.reloadAfterLanguageServerChange(), Common.reload()), + ).never(); + verify( + appShell.showWarningMessage( + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), + ), + ).once(); + verify(configService.updateSetting('languageServer', LanguageServerType.Jedi, undefined, target)).once(); + }); + }); + + [ConfigurationTarget.WorkspaceFolder, undefined].forEach((target) => { + const targetName = target === ConfigurationTarget.WorkspaceFolder ? 'workspace folder' : 'missing'; + test(`Revert to Jedi with ${targetName} setting does nothing`, async () => { + const configuration = mock(); + + when( + appShell.showWarningMessage( + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), + ), + ).thenReturn(Promise.resolve(Pylance.pylanceRevertToJedi())); + + when(workspace.getConfiguration('python')).thenReturn(instance(configuration)); + + const inspection = { + key: 'python.languageServer', + workspaceFolderValue: + target === ConfigurationTarget.WorkspaceFolder ? LanguageServerType.Node : undefined, + }; + + when(configuration.inspect('languageServer')).thenReturn(inspection); + + handler = makeHandler(undefined); + await handler.handleLanguageServerChange(LanguageServerType.Node); + + verify( + appShell.showInformationMessage(LanguageService.reloadAfterLanguageServerChange(), Common.reload()), + ).never(); + verify( + appShell.showWarningMessage( + Pylance.pylanceRevertToJediPrompt(), + Pylance.pylanceInstallPylance(), + Pylance.pylanceRevertToJedi(), + Pylance.remindMeLater(), + ), + ).once(); + verify(configService.updateSetting(anything(), anything(), anything(), anything())).never(); + }); + }); + function makeHandler(initialLSType: LanguageServerType | undefined): LanguageServerChangeHandler { return new LanguageServerChangeHandler( initialLSType, instance(extensions), instance(appShell), instance(commands), + instance(workspace), + instance(configService), ); } }); diff --git a/src/test/pythonEnvironments/common/environmentIdentifier.unit.test.ts b/src/test/pythonEnvironments/common/environmentIdentifier.unit.test.ts index c258b0d04481..0a87b1331e6c 100644 --- a/src/test/pythonEnvironments/common/environmentIdentifier.unit.test.ts +++ b/src/test/pythonEnvironments/common/environmentIdentifier.unit.test.ts @@ -81,6 +81,7 @@ suite('Environment Identifier', () => { suite('Windows Store', () => { let getEnvVar: sinon.SinonStub; + let pathExists: sinon.SinonStub; const fakeLocalAppDataPath = path.join(TEST_LAYOUT_ROOT, 'storeApps'); const fakeProgramFilesPath = 'X:\\Program Files'; const executable = ['python.exe', 'python3.exe', 'python3.8.exe']; @@ -88,17 +89,23 @@ suite('Environment Identifier', () => { getEnvVar = sinon.stub(platformApis, 'getEnvironmentVariable'); getEnvVar.withArgs('LOCALAPPDATA').returns(fakeLocalAppDataPath); getEnvVar.withArgs('ProgramFiles').returns(fakeProgramFilesPath); + + pathExists = sinon.stub(externalDependencies, 'pathExists'); + pathExists.withArgs(path.join(fakeLocalAppDataPath, 'Microsoft', 'WindowsApps', 'idle.exe')).resolves(true); }); suiteTeardown(() => { getEnvVar.restore(); + pathExists.restore(); }); executable.forEach((exe) => { test(`Path to local app data windows store interpreter (${exe})`, async () => { + getEnvVar.withArgs('LOCALAPPDATA').returns(fakeLocalAppDataPath); const interpreterPath = path.join(fakeLocalAppDataPath, 'Microsoft', 'WindowsApps', exe); const envType: EnvironmentType = await identifyEnvironment(interpreterPath); assert.deepEqual(envType, EnvironmentType.WindowsStore); }); test(`Path to local app data windows store interpreter app sub-directory (${exe})`, async () => { + getEnvVar.withArgs('LOCALAPPDATA').returns(fakeLocalAppDataPath); const interpreterPath = path.join( fakeLocalAppDataPath, 'Microsoft', @@ -126,13 +133,15 @@ suite('Environment Identifier', () => { assert.deepEqual(envType, EnvironmentType.WindowsStore); }); test(`Program files app data not set (${exe})`, async () => { - getEnvVar.withArgs('ProgramFiles').returns(undefined); const interpreterPath = path.join( fakeProgramFilesPath, 'WindowsApps', 'PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0', exe, ); + getEnvVar.withArgs('ProgramFiles').returns(undefined); + pathExists.withArgs(path.join(path.dirname(interpreterPath), 'idle.exe')).resolves(true); + const envType: EnvironmentType = await identifyEnvironment(interpreterPath); assert.deepEqual(envType, EnvironmentType.WindowsStore); }); @@ -147,6 +156,12 @@ suite('Environment Identifier', () => { const interpreterPath = path .join(fakeLocalAppDataPath, 'Microsoft', 'WindowsApps', exe) .replace('\\', '/'); + pathExists.callsFake((p: string) => { + if (p.endsWith('idle.exe')) { + return Promise.resolve(true); + } + return Promise.resolve(false); + }); const envType: EnvironmentType = await identifyEnvironment(`\\\\?\\${interpreterPath}`); assert.deepEqual(envType, EnvironmentType.WindowsStore); }); diff --git a/src/test/pythonEnvironments/discovery/locators/pyenvLocator.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/pyenvLocator.unit.test.ts index 74c5b912f20f..1e6819aa1a6f 100644 --- a/src/test/pythonEnvironments/discovery/locators/pyenvLocator.unit.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/pyenvLocator.unit.test.ts @@ -9,6 +9,7 @@ import * as fileUtils from '../../../../client/pythonEnvironments/common/externa import { IPyenvVersionStrings, isPyenvEnvironment, + isPyenvShimDir, parsePyenvVersion, } from '../../../../client/pythonEnvironments/discovery/locators/services/pyenvLocator'; @@ -269,3 +270,24 @@ suite('Pyenv Versions Parser Test', () => { }); }); }); + +suite('Pyenv Shims Dir filter tests', () => { + let getEnvVariableStub: sinon.SinonStub; + const pyenvRoot = path.join('path', 'to', 'pyenv', 'root'); + + setup(() => { + getEnvVariableStub = sinon.stub(platformUtils, 'getEnvironmentVariable'); + getEnvVariableStub.withArgs('PYENV_ROOT').returns(pyenvRoot); + }); + + teardown(() => { + getEnvVariableStub.restore(); + }); + + test('isPyenvShimDir: valid case', () => { + assert.deepStrictEqual(isPyenvShimDir(path.join(pyenvRoot, 'shims')), true); + }); + test('isPyenvShimDir: invalid case', () => { + assert.deepStrictEqual(isPyenvShimDir(__dirname), false); + }); +}); diff --git a/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.unit.test.ts index fc065aa6096b..995082bb4192 100644 --- a/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.unit.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.unit.test.ts @@ -20,6 +20,7 @@ import { parseVersion } from '../../../../client/pythonEnvironments/base/info/py import * as externalDep from '../../../../client/pythonEnvironments/common/externalDependencies'; import { getWindowsStorePythonExes, + isWindowsStoreDir, WindowsStoreLocator, } from '../../../../client/pythonEnvironments/discovery/locators/services/windowsStoreLocator'; import { getEnvs } from '../../base/common'; @@ -50,6 +51,15 @@ suite('Windows Store', () => { const actual = await getWindowsStorePythonExes(); assert.deepEqual(actual, expected); }); + + test('isWindowsStoreDir: valid case', () => { + assert.deepStrictEqual(isWindowsStoreDir(testStoreAppRoot), true); + assert.deepStrictEqual(isWindowsStoreDir(testStoreAppRoot + path.sep), true); + }); + + test('isWindowsStoreDir: invalid case', () => { + assert.deepStrictEqual(isWindowsStoreDir(__dirname), false); + }); }); suite('Locator', () => { From 0eeaab4b6fb52d971bef4dfe4f1e0cf7d4778662 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 17 Feb 2021 15:38:42 -0800 Subject: [PATCH 15/21] Release final (#15433) * Version update * Change log updates * TPN update --- CHANGELOG.md | 2 +- ThirdPartyNotices-Distribution.txt | 126 ++++++++++++++--------------- news/2 Fixes/15364.md | 1 - package-lock.json | 2 +- package.json | 2 +- 5 files changed, 66 insertions(+), 67 deletions(-) delete mode 100644 news/2 Fixes/15364.md diff --git a/CHANGELOG.md b/CHANGELOG.md index be0cfc9fd710..f1a623ec4cab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 2021.2.0-rc (8 February 2021) +## 2021.2.0 (17 February 2021) ### Enhancements diff --git a/ThirdPartyNotices-Distribution.txt b/ThirdPartyNotices-Distribution.txt index 7d5d4789c92d..1d6f1f9bbb0d 100644 --- a/ThirdPartyNotices-Distribution.txt +++ b/ThirdPartyNotices-Distribution.txt @@ -3283,7 +3283,7 @@ The complete list of contributors can be found at: https://github.com/hapijs/qs/ --------------------------------------------------------- -source-map 0.6.1 - BSD-3-Clause +source-map 0.5.7 - BSD-3-Clause https://github.com/mozilla/source-map Copyright 2011 The Closure Compiler @@ -3326,7 +3326,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. --------------------------------------------------------- -source-map 0.5.7 - BSD-3-Clause +source-map 0.6.1 - BSD-3-Clause https://github.com/mozilla/source-map Copyright 2011 The Closure Compiler @@ -5396,15 +5396,15 @@ THE SOFTWARE. --------------------------------------------------------- -define-property 1.0.0 - MIT +define-property 0.2.5 - MIT https://github.com/jonschlinkert/define-property -Copyright (c) 2015, 2017, Jon Schlinkert. -Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). +Copyright (c) 2015 Jon Schlinkert +Copyright (c) 2015, Jon Schlinkert. The MIT License (MIT) -Copyright (c) 2015, 2017, Jon Schlinkert +Copyright (c) 2015, Jon Schlinkert. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -5429,15 +5429,15 @@ THE SOFTWARE. --------------------------------------------------------- -define-property 0.2.5 - MIT +define-property 2.0.2 - MIT https://github.com/jonschlinkert/define-property -Copyright (c) 2015 Jon Schlinkert -Copyright (c) 2015, Jon Schlinkert. +Copyright (c) 2015-2018, Jon Schlinkert. +Copyright (c) 2018, Jon Schlinkert (https://github.com/jonschlinkert). The MIT License (MIT) -Copyright (c) 2015, Jon Schlinkert. +Copyright (c) 2015-2018, Jon Schlinkert. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -5462,15 +5462,15 @@ THE SOFTWARE. --------------------------------------------------------- -define-property 2.0.2 - MIT +define-property 1.0.0 - MIT https://github.com/jonschlinkert/define-property -Copyright (c) 2015-2018, Jon Schlinkert. -Copyright (c) 2018, Jon Schlinkert (https://github.com/jonschlinkert). +Copyright (c) 2015, 2017, Jon Schlinkert. +Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). The MIT License (MIT) -Copyright (c) 2015-2018, Jon Schlinkert. +Copyright (c) 2015, 2017, Jon Schlinkert Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -6966,7 +6966,7 @@ THE SOFTWARE. --------------------------------------------------------- -is-descriptor 0.1.6 - MIT +is-descriptor 1.0.2 - MIT https://github.com/jonschlinkert/is-descriptor Copyright (c) 2015-2017, Jon Schlinkert. @@ -6998,7 +6998,7 @@ THE SOFTWARE. --------------------------------------------------------- -is-descriptor 1.0.2 - MIT +is-descriptor 0.1.6 - MIT https://github.com/jonschlinkert/is-descriptor Copyright (c) 2015-2017, Jon Schlinkert. @@ -7030,15 +7030,15 @@ THE SOFTWARE. --------------------------------------------------------- -is-extendable 1.0.1 - MIT +is-extendable 0.1.1 - MIT https://github.com/jonschlinkert/is-extendable -Copyright (c) 2015-2017, Jon Schlinkert. -Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). +Copyright (c) 2015 Jon Schlinkert +Copyright (c) 2015, Jon Schlinkert. The MIT License (MIT) -Copyright (c) 2015-2017, Jon Schlinkert. +Copyright (c) 2015, Jon Schlinkert. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -7058,19 +7058,20 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + --------------------------------------------------------- --------------------------------------------------------- -is-extendable 0.1.1 - MIT +is-extendable 1.0.1 - MIT https://github.com/jonschlinkert/is-extendable -Copyright (c) 2015 Jon Schlinkert -Copyright (c) 2015, Jon Schlinkert. +Copyright (c) 2015-2017, Jon Schlinkert. +Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). The MIT License (MIT) -Copyright (c) 2015, Jon Schlinkert. +Copyright (c) 2015-2017, Jon Schlinkert. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -7090,7 +7091,6 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - --------------------------------------------------------- --------------------------------------------------------- @@ -7822,7 +7822,7 @@ THE SOFTWARE --------------------------------------------------------- -kind-of 3.2.2 - MIT +kind-of 4.0.0 - MIT https://github.com/jonschlinkert/kind-of Copyright (c) 2014-2017, Jon Schlinkert @@ -7855,15 +7855,15 @@ THE SOFTWARE. --------------------------------------------------------- -kind-of 5.1.0 - MIT +kind-of 3.2.2 - MIT https://github.com/jonschlinkert/kind-of -Copyright (c) 2014-2017, Jon Schlinkert. +Copyright (c) 2014-2017, Jon Schlinkert Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). The MIT License (MIT) -Copyright (c) 2014-2017, Jon Schlinkert. +Copyright (c) 2014-2017, Jon Schlinkert Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -7888,15 +7888,15 @@ THE SOFTWARE. --------------------------------------------------------- -kind-of 4.0.0 - MIT +kind-of 5.1.0 - MIT https://github.com/jonschlinkert/kind-of -Copyright (c) 2014-2017, Jon Schlinkert +Copyright (c) 2014-2017, Jon Schlinkert. Copyright (c) 2017, Jon Schlinkert (https://github.com/jonschlinkert). The MIT License (MIT) -Copyright (c) 2014-2017, Jon Schlinkert +Copyright (c) 2014-2017, Jon Schlinkert. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -10170,37 +10170,6 @@ IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ---------------------------------------------------------- - ---------------------------------------------------------- - -string_decoder 0.10.31 - MIT -https://github.com/rvagg/string_decoder - -Copyright Joyent, Inc. and other Node contributors. - -Copyright Joyent, Inc. and other Node contributors. - -Permission is hereby granted, free of charge, to any person obtaining a -copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to permit -persons to whom the Software is furnished to do so, subject to the -following conditions: - -The above copyright notice and this permission notice shall be included -in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -USE OR OTHER DEALINGS IN THE SOFTWARE. - - --------------------------------------------------------- --------------------------------------------------------- @@ -10319,6 +10288,37 @@ IN THE SOFTWARE. +--------------------------------------------------------- + +--------------------------------------------------------- + +string_decoder 0.10.31 - MIT +https://github.com/rvagg/string_decoder + +Copyright Joyent, Inc. and other Node contributors. + +Copyright Joyent, Inc. and other Node contributors. + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the +"Software"), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to permit +persons to whom the Software is furnished to do so, subject to the +following conditions: + +The above copyright notice and this permission notice shall be included +in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +USE OR OTHER DEALINGS IN THE SOFTWARE. + + --------------------------------------------------------- --------------------------------------------------------- diff --git a/news/2 Fixes/15364.md b/news/2 Fixes/15364.md deleted file mode 100644 index 3d49c2ac9734..000000000000 --- a/news/2 Fixes/15364.md +++ /dev/null @@ -1 +0,0 @@ -Allow support for using notebook APIs in the VS code stable build. diff --git a/package-lock.json b/package-lock.json index 301be1136f72..f2227a897409 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2021.2.0-rc", + "version": "2021.2.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 5e71bed1cd5a..1229857c9ac8 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, and more.", - "version": "2021.2.0-rc", + "version": "2021.2.0", "featureFlags": { "usingNewInterpreterStorage": true }, From 492fc99ecf982d7457abe43821b201ee85f64ac0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 19 Feb 2021 11:57:32 -0800 Subject: [PATCH 16/21] Ensure pyenv virtual envs are not skipped when in discovery experiment (#15451) * Ensure pyenv virtual envs are not skipped * Add news * Clean up * Address comments --- news/2 Fixes/15439.md | 1 + .../common/externalDependencies.ts | 32 +++++++++++++------ .../locators/services/pyenvLocator.ts | 16 +++++----- 3 files changed, 31 insertions(+), 18 deletions(-) create mode 100644 news/2 Fixes/15439.md diff --git a/news/2 Fixes/15439.md b/news/2 Fixes/15439.md new file mode 100644 index 000000000000..e6e4b7429126 --- /dev/null +++ b/news/2 Fixes/15439.md @@ -0,0 +1 @@ +Fix for missing pyenv virtual environments from selectable environments. diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index 062a8923245c..059a0bdd387f 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -97,23 +97,35 @@ export async function getFileInfo(filePath: string): Promise<{ ctime: number; mt } } -export async function resolveSymbolicLink(filepath: string): Promise { - const stats = await fsapi.lstat(filepath); +export async function resolveSymbolicLink(absPath: string): Promise { + const stats = await fsapi.lstat(absPath); if (stats.isSymbolicLink()) { - const link = await fsapi.readlink(filepath); + const link = await fsapi.readlink(absPath); return resolveSymbolicLink(link); } - return filepath; + return absPath; } -export async function* getSubDirs(root: string): AsyncIterableIterator { - const dirContents = await fsapi.readdir(root); +/** + * Returns full path to sub directories of a given directory. + * @param root + * @param resolveSymlinks + */ +export async function* getSubDirs(root: string, resolveSymlinks: boolean): AsyncIterableIterator { + const dirContents = await fsapi.promises.readdir(root, { withFileTypes: true }); const generators = dirContents.map((item) => { async function* generator() { - const stat = await fsapi.lstat(path.join(root, item)); - - if (stat.isDirectory()) { - yield item; + const fullPath = path.join(root, item.name); + if (item.isDirectory()) { + yield fullPath; + } else if (resolveSymlinks && item.isSymbolicLink()) { + // The current FS item is a symlink. It can potentially be a file + // or a directory. Resolve it first and then check if it is a directory. + const resolvedPath = await resolveSymbolicLink(fullPath); + const resolvedPathStat = await fsapi.lstat(resolvedPath); + if (resolvedPathStat.isDirectory()) { + yield resolvedPath; + } } } diff --git a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts index ddac1574932d..550dffa3dafc 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts @@ -259,15 +259,15 @@ export function parsePyenvVersion(str: string): Promise { const pyenvVersionDir = getPyenvVersionsDir(); - const subDirs = getSubDirs(pyenvVersionDir); - for await (const subDir of subDirs) { - const envDir = path.join(pyenvVersionDir, subDir); - const interpreterPath = await getInterpreterPathFromDir(envDir); + const subDirs = getSubDirs(pyenvVersionDir, true); + for await (const subDirPath of subDirs) { + const envDirName = path.basename(subDirPath); + const interpreterPath = await getInterpreterPathFromDir(subDirPath); if (interpreterPath) { // The sub-directory name sometimes can contain distro and python versions. // here we attempt to extract the texts out of the name. - const versionStrings = await parsePyenvVersion(subDir); + const versionStrings = await parsePyenvVersion(envDirName); // Here we look for near by files, or config files to see if we can get python version info // without running python itself. @@ -290,7 +290,7 @@ async function* getPyenvEnvironments(): AsyncIterableIterator { // `pyenv local|global ` or `pyenv shell ` // // For the display name we are going to treat these as `pyenv` environments. - const display = `${subDir}:pyenv`; + const display = `${envDirName}:pyenv`; const org = versionStrings && versionStrings.distro ? versionStrings.distro : ''; @@ -299,14 +299,14 @@ async function* getPyenvEnvironments(): AsyncIterableIterator { const envInfo = buildEnvInfo({ kind: PythonEnvKind.Pyenv, executable: interpreterPath, - location: envDir, + location: subDirPath, version: pythonVersion, source: [PythonEnvSource.Pyenv], display, org, fileInfo, }); - envInfo.name = subDir; + envInfo.name = envDirName; yield envInfo; } From 9f7789add385a38da66fd9357527f2d3643ad557 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 19 Feb 2021 12:00:51 -0800 Subject: [PATCH 17/21] Register Jedi regardless of what language server is configured (#15452) * Register Jedi regardless of what language server is configured * News entry --- news/2 Fixes/15452.md | 1 + src/client/activation/activationService.ts | 1 + src/client/activation/serviceRegistry.ts | 11 +++++------ src/test/activation/serviceRegistry.unit.test.ts | 8 ++++++++ 4 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 news/2 Fixes/15452.md diff --git a/news/2 Fixes/15452.md b/news/2 Fixes/15452.md new file mode 100644 index 000000000000..ade93536ece8 --- /dev/null +++ b/news/2 Fixes/15452.md @@ -0,0 +1 @@ +Register Jedi regardless of what language server is configured. diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index b63721acd392..2f939da226c5 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -243,6 +243,7 @@ export class LanguageServerExtensionActivationService if (serverType === LanguageServerType.Jedi) { throw ex; } + traceError(ex); this.output.appendLine(LanguageService.lsFailedToStart()); serverType = LanguageServerType.Jedi; server = this.serviceContainer.get(ILanguageServerActivator, serverType); diff --git a/src/client/activation/serviceRegistry.ts b/src/client/activation/serviceRegistry.ts index baa10ab345cf..8245b364a9e2 100644 --- a/src/client/activation/serviceRegistry.ts +++ b/src/client/activation/serviceRegistry.ts @@ -136,12 +136,6 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp ILanguageServerFolderService, NodeLanguageServerFolderService, ); - } else if (languageServerType === LanguageServerType.Jedi) { - serviceManager.add( - ILanguageServerActivator, - JediExtensionActivator, - LanguageServerType.Jedi, - ); } else if (languageServerType === LanguageServerType.JediLSP) { serviceManager.add( ILanguageServerActivator, @@ -165,6 +159,11 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp LanguageServerType.None, ); } + serviceManager.add( + ILanguageServerActivator, + JediExtensionActivator, + LanguageServerType.Jedi, + ); // We fallback to Jedi if for some reason we're unable to use other language servers, hence register this always. serviceManager.addSingleton( IDownloadChannelRule, diff --git a/src/test/activation/serviceRegistry.unit.test.ts b/src/test/activation/serviceRegistry.unit.test.ts index bd48ef6f62ef..614fe02e6514 100644 --- a/src/test/activation/serviceRegistry.unit.test.ts +++ b/src/test/activation/serviceRegistry.unit.test.ts @@ -9,6 +9,7 @@ import { DownloadBetaChannelRule, DownloadDailyChannelRule } from '../../client/ import { LanguageServerDownloader } from '../../client/activation/common/downloader'; import { LanguageServerDownloadChannel } from '../../client/activation/common/packageRepository'; import { ExtensionSurveyPrompt } from '../../client/activation/extensionSurvey'; +import { JediExtensionActivator } from '../../client/activation/jedi'; import { DotNetLanguageServerActivator } from '../../client/activation/languageServer/activator'; import { DotNetLanguageServerAnalysisOptions } from '../../client/activation/languageServer/analysisOptions'; import { DotNetLanguageClientFactory } from '../../client/activation/languageServer/languageClientFactory'; @@ -161,6 +162,13 @@ suite('Unit Tests - Language Server Activation Service Registry', () => { LanguageServerType.Microsoft, ), ).once(); + verify( + serviceManager.add( + ILanguageServerActivator, + JediExtensionActivator, + LanguageServerType.Jedi, + ), + ).once(); verify(serviceManager.add(ILanguageServerProxy, DotNetLanguageServerProxy)).once(); verify(serviceManager.add(ILanguageServerManager, DotNetLanguageServerManager)).once(); verify( From d9c00fb3933e3a3a914b708febaa68f2083fd9e4 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 19 Feb 2021 12:03:42 -0800 Subject: [PATCH 18/21] Only call component adapter behind the discovery experiment (#15459) --- .../discovery/locators/index.ts | 23 ++----------------- .../discovery/locators/index.unit.test.ts | 6 +---- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/client/pythonEnvironments/discovery/locators/index.ts b/src/client/pythonEnvironments/discovery/locators/index.ts index 8a56ba2731f9..07c2b5a8fb66 100644 --- a/src/client/pythonEnvironments/discovery/locators/index.ts +++ b/src/client/pythonEnvironments/discovery/locators/index.ts @@ -16,7 +16,6 @@ import { CURRENT_PATH_SERVICE, GetInterpreterLocatorOptions, GLOBAL_VIRTUAL_ENV_SERVICE, - IComponentAdapter, IInterpreterLocatorHelper, IInterpreterLocatorService, KNOWN_PATH_SERVICE, @@ -182,12 +181,6 @@ export class WorkspaceLocators extends LazyResourceBasedLocator { } } -// The parts of IComponentAdapter used here. -interface IComponent { - hasInterpreters: Promise; - getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise; -} - /** * Facilitates locating Python interpreters. */ @@ -207,10 +200,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi Promise >(); - constructor( - @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(IComponentAdapter) private readonly pyenvs: IComponent, - ) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this._hasInterpreters = createDeferred(); serviceContainer.get(IDisposableRegistry).push(this); this.platform = serviceContainer.get(IPlatformService); @@ -231,12 +221,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi } public get hasInterpreters(): Promise { - return this.pyenvs.hasInterpreters.then((res) => { - if (res !== undefined) { - return res; - } - return this._hasInterpreters.completed ? this._hasInterpreters.promise : Promise.resolve(false); - }); + return this._hasInterpreters.completed ? this._hasInterpreters.promise : Promise.resolve(false); } /** @@ -256,10 +241,6 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi */ @traceDecorators.verbose('Get Interpreters') public async getInterpreters(resource?: Uri, options?: GetInterpreterLocatorOptions): Promise { - const envs = await this.pyenvs.getInterpreters(resource, options); - if (envs !== undefined) { - return envs; - } const locators = this.getLocators(options); const promises = locators.map(async (provider) => provider.getInterpreters(resource)); locators.forEach((locator) => { diff --git a/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts index 3b49f1cfeee6..34843c5d60c1 100644 --- a/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts @@ -18,7 +18,6 @@ import { CONDA_ENV_SERVICE, CURRENT_PATH_SERVICE, GLOBAL_VIRTUAL_ENV_SERVICE, - IComponentAdapter, IInterpreterLocatorHelper, IInterpreterLocatorService, KNOWN_PATH_SERVICE, @@ -785,21 +784,18 @@ suite('Interpreters - Locators Index', () => { let serviceContainer: TypeMoq.IMock; let platformSvc: TypeMoq.IMock; let helper: TypeMoq.IMock; - let pyenvs: TypeMoq.IMock; let locator: IInterpreterLocatorService; setup(() => { serviceContainer = TypeMoq.Mock.ofType(); platformSvc = TypeMoq.Mock.ofType(); helper = TypeMoq.Mock.ofType(); - pyenvs = TypeMoq.Mock.ofType(); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformSvc.object); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IComponentAdapter))).returns(() => pyenvs.object); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterLocatorHelper))) .returns(() => helper.object); - locator = new PythonInterpreterLocatorService(serviceContainer.object, pyenvs.object); + locator = new PythonInterpreterLocatorService(serviceContainer.object); }); [undefined, Uri.file('Something')].forEach((resource) => { getNamesAndValues(OSType).forEach((osType) => { From ba1dc7d70a68f9f963efc64d27950659709b2aeb Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 19 Feb 2021 13:26:19 -0800 Subject: [PATCH 19/21] Update version and change log for point release (#15463) * Version update * Update change log --- CHANGELOG.md | 63 +++++++++++++++++++++++++++++++++++++++++++ news/2 Fixes/15439.md | 1 - news/2 Fixes/15452.md | 1 - package-lock.json | 2 +- package.json | 2 +- 5 files changed, 65 insertions(+), 4 deletions(-) delete mode 100644 news/2 Fixes/15439.md delete mode 100644 news/2 Fixes/15452.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a623ec4cab..fd747e3d74ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,68 @@ # Changelog +## 2021.2.1 (19 February 2021) + +### Fixes + +1. Fix for missing pyenv virtual environments from selectable environments. + ([#15439](https://github.com/Microsoft/vscode-python/issues/15439)) +1. Register Jedi regardless of what language server is configured. + ([#15452](https://github.com/Microsoft/vscode-python/issues/15452)) + +### Thanks + +Thanks to the following projects which we fully rely on to provide some of +our features: + +- [debugpy](https://pypi.org/project/debugpy/) +- [isort](https://pypi.org/project/isort/) +- [jedi](https://pypi.org/project/jedi/) + and [parso](https://pypi.org/project/parso/) +- [Microsoft Python Language Server](https://github.com/microsoft/python-language-server) +- [Pylance](https://github.com/microsoft/pylance-release) +- [exuberant ctags](http://ctags.sourceforge.net/) (user-installed) +- [rope](https://pypi.org/project/rope/) (user-installed) + +Also thanks to the various projects we provide integrations with which help +make this extension useful: + +- Debugging support: + [Django](https://pypi.org/project/Django/), + [Flask](https://pypi.org/project/Flask/), + [gevent](https://pypi.org/project/gevent/), + [Jinja](https://pypi.org/project/Jinja/), + [Pyramid](https://pypi.org/project/pyramid/), + [PySpark](https://pypi.org/project/pyspark/), + [Scrapy](https://pypi.org/project/Scrapy/), + [Watson](https://pypi.org/project/Watson/) +- Formatting: + [autopep8](https://pypi.org/project/autopep8/), + [black](https://pypi.org/project/black/), + [yapf](https://pypi.org/project/yapf/) +- Interpreter support: + [conda](https://conda.io/), + [direnv](https://direnv.net/), + [pipenv](https://pypi.org/project/pipenv/), + [pyenv](https://github.com/pyenv/pyenv), + [venv](https://docs.python.org/3/library/venv.html#module-venv), + [virtualenv](https://pypi.org/project/virtualenv/) +- Linting: + [bandit](https://pypi.org/project/bandit/), + [flake8](https://pypi.org/project/flake8/), + [mypy](https://pypi.org/project/mypy/), + [prospector](https://pypi.org/project/prospector/), + [pylint](https://pypi.org/project/pylint/), + [pydocstyle](https://pypi.org/project/pydocstyle/), + [pylama](https://pypi.org/project/pylama/) +- Testing: + [nose](https://pypi.org/project/nose/), + [pytest](https://pypi.org/project/pytest/), + [unittest](https://docs.python.org/3/library/unittest.html#module-unittest) + +And finally thanks to the [Python](https://www.python.org/) development team and +community for creating a fantastic programming language and community to be a +part of! + ## 2021.2.0 (17 February 2021) ### Enhancements diff --git a/news/2 Fixes/15439.md b/news/2 Fixes/15439.md deleted file mode 100644 index e6e4b7429126..000000000000 --- a/news/2 Fixes/15439.md +++ /dev/null @@ -1 +0,0 @@ -Fix for missing pyenv virtual environments from selectable environments. diff --git a/news/2 Fixes/15452.md b/news/2 Fixes/15452.md deleted file mode 100644 index ade93536ece8..000000000000 --- a/news/2 Fixes/15452.md +++ /dev/null @@ -1 +0,0 @@ -Register Jedi regardless of what language server is configured. diff --git a/package-lock.json b/package-lock.json index f2227a897409..132b85bf0f31 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2021.2.0", + "version": "2021.2.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 1229857c9ac8..3120b0b0552b 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, and more.", - "version": "2021.2.0", + "version": "2021.2.1", "featureFlags": { "usingNewInterpreterStorage": true }, From ec0fb4f310a79e57f491250ae64f6165f6d141c0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 5 Mar 2021 14:04:47 -0800 Subject: [PATCH 20/21] Update start-up telemetry for Jedi LSP (#15419) (#15571) --- src/client/activation/activationService.ts | 2 +- .../activation/jedi/languageServerProxy.ts | 28 +++++-------------- src/client/activation/jedi/manager.ts | 2 +- src/client/telemetry/constants.ts | 5 ++++ src/client/telemetry/index.ts | 26 +++++++++++++++++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/client/activation/activationService.ts b/src/client/activation/activationService.ts index 2f939da226c5..6e9735fb0135 100644 --- a/src/client/activation/activationService.ts +++ b/src/client/activation/activationService.ts @@ -279,7 +279,7 @@ export class LanguageServerExtensionActivationService outputLine = LanguageService.startingNone(); break; default: - throw new Error('Unknown langauge server type in activator.'); + throw new Error('Unknown language server type in activator.'); } this.output.appendLine(outputLine); } diff --git a/src/client/activation/jedi/languageServerProxy.ts b/src/client/activation/jedi/languageServerProxy.ts index f086225481f4..33e6e31225b6 100644 --- a/src/client/activation/jedi/languageServerProxy.ts +++ b/src/client/activation/jedi/languageServerProxy.ts @@ -13,13 +13,13 @@ import { import { DeprecatePythonPath } from '../../common/experiments/groups'; import { traceDecorators, traceError } from '../../common/logger'; -import { IConfigurationService, IExperimentsManager, IInterpreterPathService, Resource } from '../../common/types'; +import { IExperimentsManager, IInterpreterPathService, Resource } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { swallowExceptions } from '../../common/utils/decorators'; import { noop } from '../../common/utils/misc'; import { LanguageServerSymbolProvider } from '../../providers/symbolProvider'; import { PythonEnvironment } from '../../pythonEnvironments/info'; -import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; +import { captureTelemetry } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { ITestManagementService } from '../../testing/types'; import { FileBasedCancellationStrategy } from '../common/cancellationUtils'; @@ -44,7 +44,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { constructor( @inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory, @inject(ITestManagementService) private readonly testManager: ITestManagementService, - @inject(IConfigurationService) private readonly configurationService: IConfigurationService, @inject(IExperimentsManager) private readonly experiments: IExperimentsManager, @inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService, ) { @@ -81,7 +80,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { @traceDecorators.error('Failed to start language server') @captureTelemetry( - EventName.LANGUAGE_SERVER_ENABLED, + EventName.JEDI_LANGUAGE_SERVER_ENABLED, undefined, true, undefined, @@ -107,7 +106,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { // late the server may have already sent a message (which leads to failures). Register // these on the state change to running to ensure they are ready soon enough. if (e.newState === State.Running) { - this.registerHandlers(resource); + this.registerHandlers(); } }); @@ -131,7 +130,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { } @captureTelemetry( - EventName.LANGUAGE_SERVER_READY, + EventName.JEDI_LANGUAGE_SERVER_READY, undefined, true, undefined, @@ -155,7 +154,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { await this.testManager.activate(new LanguageServerSymbolProvider(this.languageClient)); } - private registerHandlers(resource: Resource) { + private registerHandlers() { if (this.disposed) { // Check if it got disposed in the interim. return; @@ -167,7 +166,7 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) { this.disposables.push( this.interpreterPathService.onDidChange(() => { - // Manually send didChangeConfiguration in order to get the server to requery + // Manually send didChangeConfiguration in order to get the server to re-query // the workspace configurations (to then pick up pythonPath set in the middleware). // This is needed as interpreter changes via the interpreter path service happen // outside of VS Code's settings (which would mean VS Code sends the config updates itself). @@ -177,18 +176,5 @@ export class JediLanguageServerProxy implements ILanguageServerProxy { }), ); } - - const settings = this.configurationService.getSettings(resource); - if (settings.downloadLanguageServer) { - this.languageClient!.onTelemetry((telemetryEvent) => { - const eventName = telemetryEvent.EventName || EventName.LANGUAGE_SERVER_TELEMETRY; - const formattedProperties = { - ...telemetryEvent.Properties, - // Replace all slashes in the method name so it doesn't get scrubbed by vscode-extension-telemetry. - method: telemetryEvent.Properties.method?.replace(/\//g, '.'), - }; - sendTelemetryEvent(eventName, telemetryEvent.Measurements, formattedProperties); - }); - } } } diff --git a/src/client/activation/jedi/manager.ts b/src/client/activation/jedi/manager.ts index 75efd14e38bd..1b00061b6f82 100644 --- a/src/client/activation/jedi/manager.ts +++ b/src/client/activation/jedi/manager.ts @@ -121,7 +121,7 @@ export class JediLanguageServerManager implements ILanguageServerManager { } @captureTelemetry( - EventName.LANGUAGE_SERVER_STARTUP, + EventName.JEDI_LANGUAGE_SERVER_STARTUP, undefined, true, undefined, diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 0a5e8b893ac6..66ad81834b96 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -120,6 +120,11 @@ export enum EventName { HASHED_PACKAGE_PERF = 'HASHED_PACKAGE_PERF', JEDI_MEMORY = 'JEDI_MEMORY', + JEDI_LANGUAGE_SERVER_ENABLED = 'JEDI_LANGUAGE_SERVER.ENABLED', + JEDI_LANGUAGE_SERVER_STARTUP = 'JEDI_LANGUAGE_SERVER.STARTUP', + JEDI_LANGUAGE_SERVER_READY = 'JEDI_LANGUAGE_SERVER.READY', + JEDI_LANGUAGE_SERVER_TELEMETRY = 'JEDI_LANGUAGE_SERVER.EVENT', + JEDI_LANGUAGE_SERVER_REQUEST = 'JEDI_LANGUAGE_SERVER.REQUEST', TENSORBOARD_SESSION_LAUNCH = 'TENSORBOARD.SESSION_LAUNCH', TENSORBOARD_SESSION_DURATION = 'TENSORBOARD.SESSION_DURATION', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index dd21ca1020e9..613128ce6223 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1375,6 +1375,32 @@ export interface IEventNamePropertyMapping { */ userAction: string; }; + /** + * Telemetry event sent when Jedi Language Server is started for workspace (workspace folder in case of multi-root) + */ + [EventName.JEDI_LANGUAGE_SERVER_ENABLED]: { + lsVersion?: string; + }; + /** + * Telemetry event sent when Jedi Language Server server is ready to receive messages + */ + [EventName.JEDI_LANGUAGE_SERVER_READY]: { + lsVersion?: string; + }; + /** + * Telemetry event sent when starting Node.js server + */ + [EventName.JEDI_LANGUAGE_SERVER_STARTUP]: { + lsVersion?: string; + }; + /** + * Telemetry sent from Node.js server (details of telemetry sent can be provided by LS team) + */ + [EventName.JEDI_LANGUAGE_SERVER_TELEMETRY]: unknown; + /** + * Telemetry sent when the client makes a request to the Node.js server + */ + [EventName.JEDI_LANGUAGE_SERVER_REQUEST]: unknown; /** * Telemetry captured for enabling reload. */ From 08565f161f17f6f157db5eaf8f7f10b083c38a1d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 5 Mar 2021 14:35:42 -0800 Subject: [PATCH 21/21] Version and change log update for point release (#15572) --- CHANGELOG.md | 61 +++++++++++++++++++++++++++++++++++++++++++++++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd747e3d74ac..8f3b28798b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,66 @@ # Changelog +## 2021.2.2 (5 March 2021) + +### Fixes + +1. Fixes issue with Jedi Language Server telemetry. + ([#15419](https://github.com/microsoft/vscode-python/issues/15419)) + +### Thanks + +Thanks to the following projects which we fully rely on to provide some of +our features: + +- [debugpy](https://pypi.org/project/debugpy/) +- [isort](https://pypi.org/project/isort/) +- [jedi](https://pypi.org/project/jedi/) + and [parso](https://pypi.org/project/parso/) +- [Microsoft Python Language Server](https://github.com/microsoft/python-language-server) +- [Pylance](https://github.com/microsoft/pylance-release) +- [exuberant ctags](http://ctags.sourceforge.net/) (user-installed) +- [rope](https://pypi.org/project/rope/) (user-installed) + +Also thanks to the various projects we provide integrations with which help +make this extension useful: + +- Debugging support: + [Django](https://pypi.org/project/Django/), + [Flask](https://pypi.org/project/Flask/), + [gevent](https://pypi.org/project/gevent/), + [Jinja](https://pypi.org/project/Jinja/), + [Pyramid](https://pypi.org/project/pyramid/), + [PySpark](https://pypi.org/project/pyspark/), + [Scrapy](https://pypi.org/project/Scrapy/), + [Watson](https://pypi.org/project/Watson/) +- Formatting: + [autopep8](https://pypi.org/project/autopep8/), + [black](https://pypi.org/project/black/), + [yapf](https://pypi.org/project/yapf/) +- Interpreter support: + [conda](https://conda.io/), + [direnv](https://direnv.net/), + [pipenv](https://pypi.org/project/pipenv/), + [pyenv](https://github.com/pyenv/pyenv), + [venv](https://docs.python.org/3/library/venv.html#module-venv), + [virtualenv](https://pypi.org/project/virtualenv/) +- Linting: + [bandit](https://pypi.org/project/bandit/), + [flake8](https://pypi.org/project/flake8/), + [mypy](https://pypi.org/project/mypy/), + [prospector](https://pypi.org/project/prospector/), + [pylint](https://pypi.org/project/pylint/), + [pydocstyle](https://pypi.org/project/pydocstyle/), + [pylama](https://pypi.org/project/pylama/) +- Testing: + [nose](https://pypi.org/project/nose/), + [pytest](https://pypi.org/project/pytest/), + [unittest](https://docs.python.org/3/library/unittest.html#module-unittest) + +And finally thanks to the [Python](https://www.python.org/) development team and +community for creating a fantastic programming language and community to be a +part of! + ## 2021.2.1 (19 February 2021) ### Fixes diff --git a/package-lock.json b/package-lock.json index 132b85bf0f31..b3a5edf5aebe 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "python", - "version": "2021.2.1", + "version": "2021.2.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 3120b0b0552b..7c3d8779cf0b 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "python", "displayName": "Python", "description": "Linting, Debugging (multi-threaded, remote), Intellisense, Jupyter Notebooks, code formatting, refactoring, unit tests, and more.", - "version": "2021.2.1", + "version": "2021.2.2", "featureFlags": { "usingNewInterpreterStorage": true },