Skip windows store and shims paths when using known path locators

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);

src/client/pythonEnvironments/discovery/locators/services/posixKnownPathsLocator.ts

@@ -4,17 +4,20 @@
 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';
 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<string[]> {
-    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<string> = new Set();
     for (const dirname of knownDirs) {
         const paths = (await fs.promises.readdir(dirname, { withFileTypes: true }))

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.

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<boolean> {
+    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<boolean> {
-    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<string[]> {
-    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 {

src/test/pythonEnvironments/common/environmentIdentifier.unit.test.ts

@@ -81,24 +81,31 @@ 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'];
         suiteSetup(() => {
             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);
             });

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);
+    });
+});

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