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/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', () => {