From 8b847fa0fad51502990832acf040432643b11892 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 9 Sep 2021 16:52:05 -0700 Subject: [PATCH 1/4] Identify globally installed envs correctly --- .../common/environmentIdentifier.ts | 2 + .../globalInstalledEnvs.ts | 52 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts diff --git a/src/client/pythonEnvironments/common/environmentIdentifier.ts b/src/client/pythonEnvironments/common/environmentIdentifier.ts index 0dcdbd37e68c..e6956a326394 100644 --- a/src/client/pythonEnvironments/common/environmentIdentifier.ts +++ b/src/client/pythonEnvironments/common/environmentIdentifier.ts @@ -4,6 +4,7 @@ import { PythonEnvKind } from '../base/info'; import { getPrioritizedEnvKinds } from '../base/info/envKind'; import { isCondaEnvironment } from './environmentManagers/conda'; +import { isGloballyInstalledEnv } from './environmentManagers/globalEnvs'; import { isPipenvEnvironment } from './environmentManagers/pipenv'; import { isPoetryEnvironment } from './environmentManagers/poetry'; import { isPyenvEnvironment } from './environmentManagers/pyenv'; @@ -31,6 +32,7 @@ function getIdentifiers(): Map Promise identifier.set(PythonEnvKind.VirtualEnvWrapper, isVirtualEnvWrapperEnvironment); identifier.set(PythonEnvKind.VirtualEnv, isVirtualEnvEnvironment); identifier.set(PythonEnvKind.Unknown, defaultTrue); + identifier.set(PythonEnvKind.OtherGlobal, isGloballyInstalledEnv); return identifier; } diff --git a/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts b/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts new file mode 100644 index 000000000000..3877b0a852c6 --- /dev/null +++ b/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { getSearchPathEntries } from '../../../common/utils/exec'; +import { getOSType, OSType } from '../../../common/utils/platform'; +import { arePathsSame, isParentPath } from '../externalDependencies'; +import { commonPosixBinPaths } from '../posixUtils'; +import { getRegistryInterpreters } from '../windowsUtils'; +import { isPyenvShimDir } from './pyenv'; + +/** + * Checks if the given interpreter belongs to known globally installed types. + * @param {string} interpreterPath: Absolute path to the python interpreter. + * @returns {boolean} : Returns true if the interpreter belongs to a venv environment. + */ +export async function isGloballyInstalledEnv(executablePath: string): Promise { + if (getOSType() === OSType.Windows) { + if (await isFoundInWindowsRegistry(executablePath)) { + return true; + } + } + return isFoundInPathEnvVar(executablePath); +} + +async function isFoundInWindowsRegistry(executablePath: string): Promise { + const interpreters = await getRegistryInterpreters(); + for (const interpreter of interpreters) { + if (arePathsSame(executablePath, interpreter.interpreterPath)) { + return true; + } + } + return false; +} + +async function isFoundInPathEnvVar(executablePath: string): Promise { + let searchPathEntries: string[] = []; + if (getOSType() === OSType.Windows) { + searchPathEntries = getSearchPathEntries(); + } else { + searchPathEntries = 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. + searchPathEntries = searchPathEntries.filter((dirname) => !isPyenvShimDir(dirname)); + for (const searchPath of searchPathEntries) { + if (isParentPath(executablePath, searchPath)) { + return true; + } + } + return false; +} From 5eba0b44d3aa51956020549982edc7906e80947f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 9 Sep 2021 17:31:53 -0700 Subject: [PATCH 2/4] Add to cache --- .../base/locators/composite/envsCollectionService.ts | 6 +++++- .../pythonEnvironments/common/environmentIdentifier.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index 0b9189376003..79d9f713fb68 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -51,7 +51,11 @@ export class EnvsCollectionService extends PythonEnvsWatcher Date: Mon, 13 Sep 2021 12:49:42 -0700 Subject: [PATCH 3/4] Add comments --- .../globalInstalledEnvs.ts | 30 ++++++++----------- src/client/startupTelemetry.ts | 12 ++++++-- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts b/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts index 3877b0a852c6..eb52668a0c65 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/globalInstalledEnvs.ts @@ -3,35 +3,29 @@ import { getSearchPathEntries } from '../../../common/utils/exec'; import { getOSType, OSType } from '../../../common/utils/platform'; -import { arePathsSame, isParentPath } from '../externalDependencies'; +import { isParentPath } from '../externalDependencies'; import { commonPosixBinPaths } from '../posixUtils'; -import { getRegistryInterpreters } from '../windowsUtils'; import { isPyenvShimDir } from './pyenv'; /** - * Checks if the given interpreter belongs to known globally installed types. + * Checks if the given interpreter belongs to known globally installed types. If an global + * executable is discoverable, we consider it as global type. * @param {string} interpreterPath: Absolute path to the python interpreter. * @returns {boolean} : Returns true if the interpreter belongs to a venv environment. */ export async function isGloballyInstalledEnv(executablePath: string): Promise { - if (getOSType() === OSType.Windows) { - if (await isFoundInWindowsRegistry(executablePath)) { - return true; - } - } + // Identifying this type is not important, as the extension treats `Global` and `Unknown` + // types the same way. This is only required for telemetry. As windows registry is known + // to be slow, we do not want to unnecessarily block on that by default, hence skip this + // step. + // if (getOSType() === OSType.Windows) { + // if (await isFoundInWindowsRegistry(executablePath)) { + // return true; + // } + // } return isFoundInPathEnvVar(executablePath); } -async function isFoundInWindowsRegistry(executablePath: string): Promise { - const interpreters = await getRegistryInterpreters(); - for (const interpreter of interpreters) { - if (arePathsSame(executablePath, interpreter.interpreterPath)) { - return true; - } - } - return false; -} - async function isFoundInPathEnvVar(executablePath: string): Promise { let searchPathEntries: string[] = []; if (getOSType() === OSType.Windows) { diff --git a/src/client/startupTelemetry.ts b/src/client/startupTelemetry.ts index bccc081fdb62..6cdf9e2b7c0c 100644 --- a/src/client/startupTelemetry.ts +++ b/src/client/startupTelemetry.ts @@ -105,15 +105,23 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer): ? workspaceService.workspaceFolders![0].uri : undefined; const settings = configurationService.getSettings(mainWorkspaceUri); - const [condaVersion, interpreter, hasPython3] = await Promise.all([ + const [condaVersion, hasPython3] = await Promise.all([ condaLocator .getCondaVersion() .then((ver) => (ver ? ver.raw : '')) .catch(() => ''), - interpreterService.getActiveInterpreter().catch(() => undefined), interpreterService.hasInterpreters(async (item) => item.version?.major === 3), ]); const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0; + // If an unknown type environment can be found from windows registry or path env var, + // consider them as global type instead of unknown. Such types can only be known after + // windows registry is queried. So wait for the refresh of windows registry locator to + // finish. API getActiveInterpreter() does not block on windows registry by default as + // it is slow. + await interpreterService.refreshPromise; + const interpreter = await interpreterService + .getActiveInterpreter() + .catch(() => undefined); const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined; const interpreterType = interpreter ? interpreter.envType : undefined; const usingUserDefinedInterpreter = hasUserDefinedPythonPath(mainWorkspaceUri, serviceContainer); From 9e22110c6a64f5665b0996297e4fe5d8e68655fc Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 13 Sep 2021 13:23:41 -0700 Subject: [PATCH 4/4] News entry --- news/2 Fixes/17362.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/17362.md diff --git a/news/2 Fixes/17362.md b/news/2 Fixes/17362.md new file mode 100644 index 000000000000..11b1552569f2 --- /dev/null +++ b/news/2 Fixes/17362.md @@ -0,0 +1 @@ +Ensure we correctly evaluate Unknown type before sending startup telemetry.