From 484c8e4ae050d205dddd19b4a1df293f89ae1d22 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 18 Feb 2021 13:33:32 -0800 Subject: [PATCH 1/4] Ensure pyenv virtual envs are not skipped --- .../common/externalDependencies.ts | 17 ++++++++++------- .../discovery/locators/services/pyenvLocator.ts | 14 +++++++------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index 062a8923245c..d0e4f9f7301c 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -97,23 +97,26 @@ 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 { +export async function* getSubDirs(root: string, resolveSymlinks: boolean): AsyncIterableIterator { const dirContents = await fsapi.readdir(root); const generators = dirContents.map((item) => { async function* generator() { - const stat = await fsapi.lstat(path.join(root, item)); + const fullPath = path.join(root, item); + const stat = await fsapi.lstat(fullPath); if (stat.isDirectory()) { - yield item; + yield fullPath; + } else if (resolveSymlinks && stat.isSymbolicLink()) { + yield await resolveSymbolicLink(fullPath); } } diff --git a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts index ddac1574932d..0328988ac8a8 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); + const subDirs = getSubDirs(pyenvVersionDir, true); for await (const subDir of subDirs) { - const envDir = path.join(pyenvVersionDir, subDir); - const interpreterPath = await getInterpreterPathFromDir(envDir); + const envDirName = path.basename(subDir); + const interpreterPath = await getInterpreterPathFromDir(subDir); 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: subDir, version: pythonVersion, source: [PythonEnvSource.Pyenv], display, org, fileInfo, }); - envInfo.name = subDir; + envInfo.name = envDirName; yield envInfo; } From 6fdfb553887fbaed4b5b75094ded9e5abfbef1c3 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 18 Feb 2021 13:46:56 -0800 Subject: [PATCH 2/4] Add news --- news/2 Fixes/15439.md | 1 + 1 file changed, 1 insertion(+) 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..7efb6bff04e2 --- /dev/null +++ b/news/2 Fixes/15439.md @@ -0,0 +1 @@ +Fix for missing pyenv virutal environments from selectable environments. From 63d3de536723697aeb2a78f6bab152e3e945b08d Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 18 Feb 2021 14:06:13 -0800 Subject: [PATCH 3/4] Clean up --- news/2 Fixes/15439.md | 2 +- .../discovery/locators/services/pyenvLocator.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/news/2 Fixes/15439.md b/news/2 Fixes/15439.md index 7efb6bff04e2..e6e4b7429126 100644 --- a/news/2 Fixes/15439.md +++ b/news/2 Fixes/15439.md @@ -1 +1 @@ -Fix for missing pyenv virutal environments from selectable environments. +Fix for missing pyenv virtual environments from selectable environments. diff --git a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts index 0328988ac8a8..550dffa3dafc 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/pyenvLocator.ts @@ -260,9 +260,9 @@ async function* getPyenvEnvironments(): AsyncIterableIterator { const pyenvVersionDir = getPyenvVersionsDir(); const subDirs = getSubDirs(pyenvVersionDir, true); - for await (const subDir of subDirs) { - const envDirName = path.basename(subDir); - const interpreterPath = await getInterpreterPathFromDir(subDir); + 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. @@ -299,7 +299,7 @@ async function* getPyenvEnvironments(): AsyncIterableIterator { const envInfo = buildEnvInfo({ kind: PythonEnvKind.Pyenv, executable: interpreterPath, - location: subDir, + location: subDirPath, version: pythonVersion, source: [PythonEnvSource.Pyenv], display, From 69c71e8645285f20c95ad688277e9fef3a8264ac Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 19 Feb 2021 11:28:18 -0800 Subject: [PATCH 4/4] Address comments --- .../common/externalDependencies.ts | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/client/pythonEnvironments/common/externalDependencies.ts b/src/client/pythonEnvironments/common/externalDependencies.ts index d0e4f9f7301c..059a0bdd387f 100644 --- a/src/client/pythonEnvironments/common/externalDependencies.ts +++ b/src/client/pythonEnvironments/common/externalDependencies.ts @@ -106,17 +106,26 @@ export async function resolveSymbolicLink(absPath: string): Promise { return absPath; } +/** + * 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.readdir(root); + const dirContents = await fsapi.promises.readdir(root, { withFileTypes: true }); const generators = dirContents.map((item) => { async function* generator() { - const fullPath = path.join(root, item); - const stat = await fsapi.lstat(fullPath); - - if (stat.isDirectory()) { + const fullPath = path.join(root, item.name); + if (item.isDirectory()) { yield fullPath; - } else if (resolveSymlinks && stat.isSymbolicLink()) { - yield await resolveSymbolicLink(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; + } } }