From dd6b65896d4918e4a5dd8302a4e2777c34542965 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 31 Jan 2023 19:27:59 +0530 Subject: [PATCH 1/7] Remove getEnvId() code --- src/client/pythonEnvironments/base/info/env.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 9340792a4f4b..88f585ce1d81 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -73,7 +73,6 @@ export function buildEnvInfo(init?: { if (init !== undefined) { updateEnv(env, init); } - env.id = getEnvID(env.executable.filename, env.location); return env; } @@ -266,7 +265,12 @@ export function areSameEnv( const leftFilename = leftInfo.executable!.filename; const rightFilename = rightInfo.executable!.filename; - if (getEnvID(leftFilename, leftInfo.location) === getEnvID(rightFilename, rightInfo.location)) { + if ( + arePathsSame( + getEnvPath(leftFilename, leftInfo.location).path, + getEnvPath(rightFilename, rightInfo.location).path, + ) + ) { return true; } From 0c70756175e531b3fafdffd9300e21ba280b7208 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 31 Jan 2023 20:01:50 +0530 Subject: [PATCH 2/7] Simplify --- src/client/proposedApi.ts | 2 +- src/client/pythonEnvironments/base/info/env.ts | 3 ++- .../base/locators/composite/resolverUtils.ts | 17 +++++++++++++---- .../common/environmentManagers/conda.ts | 11 ++++------- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 5f40fcf263db..1b710a888c99 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -304,7 +304,7 @@ export function convertCompleteEnvInfo(env: PythonEnvInfo): ResolvedEnvironment const { path } = getEnvPath(env.executable.filename, env.location); const resolvedEnv: ResolvedEnvironment = { path, - id: getEnvID(path), + id: env.id!, executable: { uri: env.executable.filename === 'python' ? undefined : Uri.file(env.executable.filename), bitness: convertBitness(env.arch), diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 88f585ce1d81..328bee0322c7 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -73,6 +73,7 @@ export function buildEnvInfo(init?: { if (init !== undefined) { updateEnv(env, init); } + env.id = getNormCaseEnvPath(env.executable.filename, env.location); return env; } @@ -236,7 +237,7 @@ export function getEnvPath(interpreterPath: string, envFolderPath?: string): Env /** * Gets unique identifier for an environment. */ -export function getEnvID(interpreterPath: string, envFolderPath?: string): string { +export function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { return normCasePath(getEnvPath(interpreterPath, envFolderPath).path); } diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index c0506d4a06ba..ad606e4fa2fd 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -12,10 +12,15 @@ import { UNKNOWN_PYTHON_VERSION, virtualEnvKinds, } from '../../info'; -import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString, getEnvID } from '../../info/env'; +import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString } from '../../info/env'; import { getEnvironmentDirFromPath, getPythonVersionFromPath } from '../../../common/commonUtils'; -import { arePathsSame, getFileInfo, isParentPath } from '../../../common/externalDependencies'; -import { AnacondaCompanyName, Conda, isCondaEnvironment } from '../../../common/environmentManagers/conda'; +import { arePathsSame, getFileInfo, isParentPath, normCasePath } from '../../../common/externalDependencies'; +import { + AnacondaCompanyName, + Conda, + getCondaInterpreterPath, + isCondaEnvironment, +} from '../../../common/environmentManagers/conda'; import { getPyenvVersionsDir, parsePyenvVersion } from '../../../common/environmentManagers/pyenv'; import { Architecture, getOSType, OSType } from '../../../../common/utils/platform'; import { getPythonVersionFromPath as parsePythonVersionFromPath, parseVersion } from '../../info/pythonVersion'; @@ -57,7 +62,6 @@ export async function resolveBasicEnv(env: BasicEnvInfo): Promise await updateEnvUsingRegistry(resolvedEnv); } setEnvDisplayString(resolvedEnv); - resolvedEnv.id = getEnvID(resolvedEnv.executable.filename, resolvedEnv.location); const { ctime, mtime } = await getFileInfo(resolvedEnv.executable.filename); resolvedEnv.executable.ctime = ctime; resolvedEnv.executable.mtime = mtime; @@ -189,6 +193,11 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise { if (name) { info.name = name; } + if (env.envPath && env.kind === PythonEnvKind.Conda && path.basename(executable) === executable) { + // For environments without python, set ID using the predicted executable path after python is installed. + const predictedExecutable = getCondaInterpreterPath(env.envPath); + info.id = normCasePath(predictedExecutable); + } return info; } diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index bdbcb7ea3ac5..0e19c8e94801 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -226,14 +226,11 @@ export async function getPythonVersionFromConda(interpreterPath: string): Promis /** * Return the interpreter's filename for the given environment. */ -async function getInterpreterPath(condaEnvironmentPath: string): Promise { +export function getCondaInterpreterPath(condaEnvironmentPath: string): string { // where to find the Python binary within a conda env. const relativePath = getOSType() === OSType.Windows ? 'python.exe' : path.join('bin', 'python'); const filePath = path.join(condaEnvironmentPath, relativePath); - if (await pathExists(filePath)) { - return filePath; - } - return undefined; + return filePath; } // Minimum version number of conda required to be able to use 'conda run' with '--no-capture-output' flag. @@ -494,8 +491,8 @@ export class Conda { */ // eslint-disable-next-line class-methods-use-this public async getInterpreterPathForEnvironment(condaEnv: CondaEnvInfo | { prefix: string }): Promise { - const executablePath = await getInterpreterPath(condaEnv.prefix); - if (executablePath) { + const executablePath = getCondaInterpreterPath(condaEnv.prefix); + if (await pathExists(executablePath)) { traceVerbose('Found executable within conda env', JSON.stringify(condaEnv)); return executablePath; } From 4558d87f25fb00be9fdd46b3be016431e739b8fa Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 31 Jan 2023 20:04:55 +0530 Subject: [PATCH 3/7] Refactor --- src/client/pythonEnvironments/base/info/env.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 328bee0322c7..d5f31e5304ac 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -235,9 +235,9 @@ export function getEnvPath(interpreterPath: string, envFolderPath?: string): Env } /** - * Gets unique identifier for an environment. + * Gets general unique identifier for most environments. */ -export function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { +function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { return normCasePath(getEnvPath(interpreterPath, envFolderPath).path); } @@ -266,12 +266,7 @@ export function areSameEnv( const leftFilename = leftInfo.executable!.filename; const rightFilename = rightInfo.executable!.filename; - if ( - arePathsSame( - getEnvPath(leftFilename, leftInfo.location).path, - getEnvPath(rightFilename, rightInfo.location).path, - ) - ) { + if (getNormCaseEnvPath(leftFilename, leftInfo.location) === getNormCaseEnvPath(rightFilename, rightInfo.location)) { return true; } From c444aaae2a590a98a5c51ad1f947272d2e2753a1 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 31 Jan 2023 20:17:17 +0530 Subject: [PATCH 4/7] Fix test --- src/test/proposedApi.unit.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index 80db62f4814b..5dfa54492c6b 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -252,6 +252,7 @@ suite('Proposed Extension API', () => { test('environments: python found', async () => { const expectedEnvs = [ { + id: normCasePath('this/is/a/test/python/path1'), executable: { filename: 'this/is/a/test/python/path1', ctime: 1, @@ -273,6 +274,7 @@ suite('Proposed Extension API', () => { }, }, { + id: normCasePath('this/is/a/test/python/path2'), executable: { filename: 'this/is/a/test/python/path2', ctime: 1, @@ -297,6 +299,7 @@ suite('Proposed Extension API', () => { const envs = [ ...expectedEnvs, { + id: normCasePath('this/is/a/test/python/path3'), executable: { filename: 'this/is/a/test/python/path3', ctime: 1, @@ -343,6 +346,7 @@ suite('Proposed Extension API', () => { searchLocation: Uri.file(workspacePath), }), { + id: normCasePath('this/is/a/test/python/path1'), executable: { filename: 'this/is/a/test/python/path1', ctime: 1, @@ -364,6 +368,7 @@ suite('Proposed Extension API', () => { }, }, { + id: normCasePath('this/is/a/test/python/path2'), executable: { filename: 'this/is/a/test/python/path2', ctime: 1, From 275153fbc22b5faf5dc58e0bc2b0ce98d6bf42cd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 31 Jan 2023 20:23:43 +0530 Subject: [PATCH 5/7] Add comment for an alternative --- .../pythonEnvironments/base/locators/composite/resolverUtils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index ad606e4fa2fd..d7fe00a501c1 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -195,6 +195,8 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise { } if (env.envPath && env.kind === PythonEnvKind.Conda && path.basename(executable) === executable) { // For environments without python, set ID using the predicted executable path after python is installed. + // Another alternative could've been to set ID of all conda environments to the environment path, as that + // remains constant even after python installation. const predictedExecutable = getCondaInterpreterPath(env.envPath); info.id = normCasePath(predictedExecutable); } From 633a3a55d920a0bc04cc6d2b56117151295f9d81 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 1 Feb 2023 18:40:13 +0530 Subject: [PATCH 6/7] Fix bug --- src/client/common/installer/condaInstaller.ts | 5 +++-- src/client/pythonEnvironments/base/info/env.ts | 9 ++++++++- .../base/locators/composite/resolverUtils.ts | 8 ++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/client/common/installer/condaInstaller.ts b/src/client/common/installer/condaInstaller.ts index 774cade34457..a20b35e0f110 100644 --- a/src/client/common/installer/condaInstaller.ts +++ b/src/client/common/installer/condaInstaller.ts @@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify'; import { ICondaService, IComponentAdapter } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; +import { getEnvPath } from '../../pythonEnvironments/base/info/env'; import { ModuleInstallerType } from '../../pythonEnvironments/info'; import { ExecutionInfo, IConfigurationService, Product } from '../types'; import { isResource } from '../utils/misc'; @@ -79,7 +80,7 @@ export class CondaInstaller extends ModuleInstaller { const pythonPath = isResource(resource) ? this.serviceContainer.get(IConfigurationService).getSettings(resource).pythonPath - : resource.id ?? ''; + : getEnvPath(resource.path, resource.envPath).path ?? ''; const condaLocatorService = this.serviceContainer.get(IComponentAdapter); const info = await condaLocatorService.getCondaEnvironment(pythonPath); const args = [flags & ModuleInstallFlags.upgrade ? 'update' : 'install']; @@ -132,7 +133,7 @@ export class CondaInstaller extends ModuleInstaller { const condaService = this.serviceContainer.get(IComponentAdapter); const pythonPath = isResource(resource) ? this.serviceContainer.get(IConfigurationService).getSettings(resource).pythonPath - : resource.id ?? ''; + : getEnvPath(resource.path, resource.envPath).path ?? ''; return condaService.isCondaEnvironment(pythonPath); } } diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index d5f31e5304ac..04089b26dc36 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -198,6 +198,7 @@ function getMinimalPartialInfo(env: string | PythonEnvInfo | BasicEnvInfo): Part return undefined; } return { + id: '', executable: { filename: env, sysPrefix: '', @@ -208,6 +209,7 @@ function getMinimalPartialInfo(env: string | PythonEnvInfo | BasicEnvInfo): Part } if ('executablePath' in env) { return { + id: '', executable: { filename: env.executablePath, sysPrefix: '', @@ -237,7 +239,7 @@ export function getEnvPath(interpreterPath: string, envFolderPath?: string): Env /** * Gets general unique identifier for most environments. */ -function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { +export function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { return normCasePath(getEnvPath(interpreterPath, envFolderPath).path); } @@ -270,6 +272,11 @@ export function areSameEnv( return true; } + if (leftInfo.id && leftInfo.id === rightInfo.id) { + // Env path changes for conda envs after python is installed into them, so compare ids. + return true; + } + if (allowPartialMatch) { const isSameDirectory = leftFilename !== 'python' && diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index d7fe00a501c1..ecae7475283f 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -12,9 +12,9 @@ import { UNKNOWN_PYTHON_VERSION, virtualEnvKinds, } from '../../info'; -import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString } from '../../info/env'; +import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString, getNormCaseEnvPath } from '../../info/env'; import { getEnvironmentDirFromPath, getPythonVersionFromPath } from '../../../common/commonUtils'; -import { arePathsSame, getFileInfo, isParentPath, normCasePath } from '../../../common/externalDependencies'; +import { arePathsSame, getFileInfo, isParentPath } from '../../../common/externalDependencies'; import { AnacondaCompanyName, Conda, @@ -193,12 +193,12 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise { if (name) { info.name = name; } - if (env.envPath && env.kind === PythonEnvKind.Conda && path.basename(executable) === executable) { + if (env.envPath && path.basename(executable) === executable) { // For environments without python, set ID using the predicted executable path after python is installed. // Another alternative could've been to set ID of all conda environments to the environment path, as that // remains constant even after python installation. const predictedExecutable = getCondaInterpreterPath(env.envPath); - info.id = normCasePath(predictedExecutable); + info.id = getNormCaseEnvPath(predictedExecutable, env.envPath); } return info; } From 9cfad5e447af3882627b9c1370dc9532f1413910 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 1 Feb 2023 18:58:01 +0530 Subject: [PATCH 7/7] Refactor so it makes more sense --- src/client/pythonEnvironments/base/info/env.ts | 13 ++++++++----- .../base/locators/composite/resolverUtils.ts | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/client/pythonEnvironments/base/info/env.ts b/src/client/pythonEnvironments/base/info/env.ts index 04089b26dc36..2527f18202cd 100644 --- a/src/client/pythonEnvironments/base/info/env.ts +++ b/src/client/pythonEnvironments/base/info/env.ts @@ -73,7 +73,7 @@ export function buildEnvInfo(init?: { if (init !== undefined) { updateEnv(env, init); } - env.id = getNormCaseEnvPath(env.executable.filename, env.location); + env.id = getEnvID(env.executable.filename, env.location); return env; } @@ -239,7 +239,7 @@ export function getEnvPath(interpreterPath: string, envFolderPath?: string): Env /** * Gets general unique identifier for most environments. */ -export function getNormCaseEnvPath(interpreterPath: string, envFolderPath?: string): string { +export function getEnvID(interpreterPath: string, envFolderPath?: string): string { return normCasePath(getEnvPath(interpreterPath, envFolderPath).path); } @@ -268,12 +268,15 @@ export function areSameEnv( const leftFilename = leftInfo.executable!.filename; const rightFilename = rightInfo.executable!.filename; - if (getNormCaseEnvPath(leftFilename, leftInfo.location) === getNormCaseEnvPath(rightFilename, rightInfo.location)) { + if (leftInfo.id && leftInfo.id === rightInfo.id) { + // In case IDs are available, use it. return true; } - if (leftInfo.id && leftInfo.id === rightInfo.id) { - // Env path changes for conda envs after python is installed into them, so compare ids. + if (getEnvID(leftFilename, leftInfo.location) === getEnvID(rightFilename, rightInfo.location)) { + // Otherwise use ID function to get the ID. Note ID returned by function may itself change if executable of + // an environment changes, for eg. when conda installs python into the env. So only use it as a fallback if + // ID is not available. return true; } diff --git a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts index ecae7475283f..4bfcfac7fc87 100644 --- a/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts +++ b/src/client/pythonEnvironments/base/locators/composite/resolverUtils.ts @@ -12,7 +12,7 @@ import { UNKNOWN_PYTHON_VERSION, virtualEnvKinds, } from '../../info'; -import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString, getNormCaseEnvPath } from '../../info/env'; +import { buildEnvInfo, comparePythonVersionSpecificity, setEnvDisplayString, getEnvID } from '../../info/env'; import { getEnvironmentDirFromPath, getPythonVersionFromPath } from '../../../common/commonUtils'; import { arePathsSame, getFileInfo, isParentPath } from '../../../common/externalDependencies'; import { @@ -198,7 +198,7 @@ async function resolveCondaEnv(env: BasicEnvInfo): Promise { // Another alternative could've been to set ID of all conda environments to the environment path, as that // remains constant even after python installation. const predictedExecutable = getCondaInterpreterPath(env.envPath); - info.id = getNormCaseEnvPath(predictedExecutable, env.envPath); + info.id = getEnvID(predictedExecutable, env.envPath); } return info; }