From 972649f544a6cfce45615408d80299a56bd6e9b0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 9 Feb 2021 14:25:50 -0800 Subject: [PATCH 1/2] Ignore directories with the same name as pyhton binaries --- .../pythonEnvironments/common/commonUtils.ts | 6 ++-- .../pythonEnvironments/common/posixUtils.ts | 2 +- .../services/posixKnownPathsLocator.ts | 28 +++++++++++-------- .../posixroot/location3/python3.9/empty | 1 + 4 files changed, 21 insertions(+), 16 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..572ca89548d8 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts @@ -11,25 +11,29 @@ 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 pythonBins: Set = new Set(); for (const knownPath of knownPaths) { - const files = (await fsapi.readdir(knownPath)) + const paths = (await fsapi.readdir(knownPath)) .map((filename: string) => path.join(knownPath, filename)) - .filter(isPosixPythonBin); + .filter(isPosixPythonBinPattern); - for (const file of files) { - // Ensure that we have a collection of unique global binaries by - // resolving all symlinks to the target binaries. - try { - const resolvedBin = await resolveSymbolicLink(file); - pythonBins.add(resolvedBin); - traceInfo(`Found: ${file} --> ${resolvedBin}`); - } catch (ex) { - traceError('Failed to resolve symbolic link: ', ex); + for (const filepath of paths) { + // Skip any directories + const stats = await fsapi.lstat(filepath); + if (!stats.isDirectory()) { + // Ensure that we have a collection of unique global binaries by + // resolving all symlinks to the target binaries. + try { + const resolvedBin = await resolveSymbolicLink(filepath); + pythonBins.add(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 9fcae82872720826807029f29e5208a1d0e6e79b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 9 Feb 2021 15:02:52 -0800 Subject: [PATCH 2/2] Use withFileTypes instead of lstat --- .../services/posixKnownPathsLocator.ts | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts index 572ca89548d8..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'; @@ -14,26 +14,23 @@ import { getFileInfo, resolveSymbolicLink } from '../../../common/externalDepend 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 paths = (await fsapi.readdir(knownPath)) - .map((filename: string) => path.join(knownPath, filename)) + 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 filepath of paths) { - // Skip any directories - const stats = await fsapi.lstat(filepath); - if (!stats.isDirectory()) { - // Ensure that we have a collection of unique global binaries by - // resolving all symlinks to the target binaries. - try { - const resolvedBin = await resolveSymbolicLink(filepath); - pythonBins.add(resolvedBin); - traceInfo(`Found: ${filepath} --> ${resolvedBin}`); - } catch (ex) { - traceError('Failed to resolve symbolic link: ', ex); - } + // Ensure that we have a collection of unique global binaries by + // resolving all symlinks to the target binaries. + try { + const resolvedBin = await resolveSymbolicLink(filepath); + pythonBins.add(resolvedBin); + traceInfo(`Found: ${filepath} --> ${resolvedBin}`); + } catch (ex) { + traceError('Failed to resolve symbolic link: ', ex); } } }