From 37c7beff76031589274917bcb7fba1963e74c5d2 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 7 Jul 2020 14:32:39 -0700 Subject: [PATCH] Simplify interpreter filter (#12792) --- src/client/interpreter/locators/types.ts | 5 -- .../discovery/locators/index.ts | 10 +--- .../locators/services/interpreterFilter.ts | 13 ++--- .../services/windowsStoreInterpreter.ts | 31 +++++++--- src/client/pythonEnvironments/legacyIOC.ts | 2 - .../discovery/locators/index.unit.test.ts | 5 +- .../locators/interpreterFilter.unit.test.ts | 58 +++++++++++++++++++ ...in.unit.test.ts => legacyIOC.unit.test.ts} | 0 8 files changed, 89 insertions(+), 35 deletions(-) create mode 100644 src/test/pythonEnvironments/discovery/locators/interpreterFilter.unit.test.ts rename src/test/pythonEnvironments/{main.unit.test.ts => legacyIOC.unit.test.ts} (100%) diff --git a/src/client/interpreter/locators/types.ts b/src/client/interpreter/locators/types.ts index 2229a2cc8252..f39589800074 100644 --- a/src/client/interpreter/locators/types.ts +++ b/src/client/interpreter/locators/types.ts @@ -4,7 +4,6 @@ 'use strict'; import { Uri } from 'vscode'; -import { PythonInterpreter } from '../../pythonEnvironments/info'; export const IPythonInPathCommandProvider = Symbol('IPythonInPathCommandProvider'); export interface IPythonInPathCommandProvider { @@ -63,7 +62,3 @@ export interface IWindowsStoreInterpreter { */ isHiddenInterpreter(pythonPath: string): boolean; } - -export interface IInterpreterFilter { - isHiddenInterpreter(interpreter: PythonInterpreter): boolean; -} diff --git a/src/client/pythonEnvironments/discovery/locators/index.ts b/src/client/pythonEnvironments/discovery/locators/index.ts index fd6a69dbfdbc..4b2a5188c235 100644 --- a/src/client/pythonEnvironments/discovery/locators/index.ts +++ b/src/client/pythonEnvironments/discovery/locators/index.ts @@ -17,10 +17,9 @@ import { WINDOWS_REGISTRY_SERVICE, WORKSPACE_VIRTUAL_ENV_SERVICE } from '../../../interpreter/contracts'; -import { IInterpreterFilter } from '../../../interpreter/locators/types'; import { IServiceContainer } from '../../../ioc/types'; import { PythonInterpreter } from '../../info'; -import { InterpreterFilter } from './services/interpreterFilter'; +import { isHiddenInterpreter } from './services/interpreterFilter'; import { GetInterpreterLocatorOptions } from './types'; // tslint:disable-next-line:no-require-imports no-var-requires @@ -38,10 +37,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi private readonly interpreterLocatorHelper: IInterpreterLocatorHelper; private readonly _hasInterpreters: Deferred; - constructor( - @inject(IServiceContainer) private serviceContainer: IServiceContainer, - @inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter - ) { + constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this._hasInterpreters = createDeferred(); serviceContainer.get(IDisposableRegistry).push(this); this.platform = serviceContainer.get(IPlatformService); @@ -96,7 +92,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi const items = flatten(listOfInterpreters) .filter((item) => !!item) .map((item) => item!) - .filter((item) => !this.interpreterFilter.isHiddenInterpreter(item)); + .filter((item) => !isHiddenInterpreter(item)); this._hasInterpreters.resolve(items.length > 0); return this.interpreterLocatorHelper.mergeInterpreters(items); } diff --git a/src/client/pythonEnvironments/discovery/locators/services/interpreterFilter.ts b/src/client/pythonEnvironments/discovery/locators/services/interpreterFilter.ts index f82175d69a98..f80b6f359d56 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/interpreterFilter.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/interpreterFilter.ts @@ -3,15 +3,10 @@ 'use strict'; -import { inject, injectable } from 'inversify'; -import { IInterpreterFilter, IWindowsStoreInterpreter } from '../../../../interpreter/locators/types'; import { PythonInterpreter } from '../../../info'; -import { WindowsStoreInterpreter } from './windowsStoreInterpreter'; +import { isRestrictedWindowsStoreInterpreterPath } from './windowsStoreInterpreter'; -@injectable() -export class InterpreterFilter implements IInterpreterFilter { - constructor(@inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter) {} - public isHiddenInterpreter(interpreter: PythonInterpreter): boolean { - return this.windowsStoreInterpreter.isHiddenInterpreter(interpreter.path); - } +export function isHiddenInterpreter(interpreter: PythonInterpreter): boolean { + // Any set of rules to hide interpreters should go here + return isRestrictedWindowsStoreInterpreterPath(interpreter.path); } diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter.ts index c7554e7cf3e0..e28a3c320ff5 100644 --- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter.ts +++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreInterpreter.ts @@ -12,20 +12,39 @@ import { IPersistentStateFactory } from '../../../../common/types'; import { IInterpreterHashProvider, IWindowsStoreInterpreter } from '../../../../interpreter/locators/types'; import { IServiceContainer } from '../../../../ioc/types'; +/** + * When using Windows Store interpreter the path that should be used is under + * %USERPROFILE%\AppData\Local\Microsoft\WindowsApps\python*.exe. The python.exe path + * under ProgramFiles\WindowsApps should not be used at all. Execute permissions on + * that instance of the store interpreter are restricted to system. Paths under + * %USERPROFILE%\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation* are also ok + * to use. But currently this results in duplicate entries. + * + * @param {string} pythonPath + * @returns {boolean} + */ +export function isRestrictedWindowsStoreInterpreterPath(pythonPath: string): boolean { + const pythonPathToCompare = pythonPath.toUpperCase().replace(/\//g, '\\'); + return ( + pythonPathToCompare.includes('\\Program Files\\WindowsApps\\'.toUpperCase()) || + pythonPathToCompare.includes('\\Microsoft\\WindowsApps\\PythonSoftwareFoundation'.toUpperCase()) + ); +} + /** * The default location of Windows apps are `%ProgramFiles%\WindowsApps`. * (https://www.samlogic.net/articles/windows-8-windowsapps-folder.htm) * When users access Python interpreter it is installed in `\Microsoft\WindowsApps` * Based on our testing this is where Python interpreters installed from Windows Store is always installed. - * (unforutnately couldn't find any documentation on this). + * (unfortunately couldn't find any documentation on this). * What we've identified is the fact that: * - The Python interpreter in Microsoft\WIndowsApps\python.exe is a symbolic link to files located in: * - Program Files\WindowsApps\ & Microsoft\WindowsApps\PythonSoftwareFoundation * - I.e. they all point to the same place. - * However when the user lauches the executabale, its located in `Microsoft\WindowsAapps\python.exe` + * However when the user launches the executable, its located in `Microsoft\WindowsApps\python.exe` * Hence for all intensive purposes that's the main executable, that's what the user uses. * As a result: - * - We'll only display what the user has access to, that being `Microsoft\WindowsAapps\python.exe` + * - We'll only display what the user has access to, that being `Microsoft\WindowsApps\python.exe` * - Others are hidden. * * Details can be found here (original issue https://github.com/microsoft/vscode-python/issues/5926). @@ -66,11 +85,7 @@ export class WindowsStoreInterpreter implements IWindowsStoreInterpreter, IInter * @memberof IInterpreterHelper */ public isHiddenInterpreter(pythonPath: string): boolean { - const pythonPathToCompare = pythonPath.toUpperCase().replace(/\//g, '\\'); - return ( - pythonPathToCompare.includes('\\Program Files\\WindowsApps\\'.toUpperCase()) || - pythonPathToCompare.includes('\\Microsoft\\WindowsApps\\PythonSoftwareFoundation'.toUpperCase()) - ); + return isRestrictedWindowsStoreInterpreterPath(pythonPath); } /** * Gets the hash of the Python interpreter (installed from the windows store). diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index d2382287a24b..5f4517bc7a21 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -35,7 +35,6 @@ import { } from './discovery/locators/services/globalVirtualEnvService'; import { InterpreterHashProvider } from './discovery/locators/services/hashProvider'; import { InterpeterHashProviderFactory } from './discovery/locators/services/hashProviderFactory'; -import { InterpreterFilter } from './discovery/locators/services/interpreterFilter'; import { InterpreterWatcherBuilder } from './discovery/locators/services/interpreterWatcherBuilder'; import { KnownPathsService, KnownSearchPathsForInterpreters } from './discovery/locators/services/KnownPathsService'; import { PipEnvService } from './discovery/locators/services/pipEnvService'; @@ -114,7 +113,6 @@ export function registerForIOC(serviceManager: IServiceManager) { InterpeterHashProviderFactory, InterpeterHashProviderFactory ); - serviceManager.addSingleton(InterpreterFilter, InterpreterFilter); serviceManager.addSingleton( IVirtualEnvironmentsSearchPathProvider, GlobalVirtualEnvironmentsSearchPathProvider, diff --git a/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts index 00fae76cd52c..55893d823eb2 100644 --- a/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/index.unit.test.ts @@ -25,7 +25,6 @@ import { WINDOWS_REGISTRY_SERVICE, WORKSPACE_VIRTUAL_ENV_SERVICE } from '../../../../client/interpreter/contracts'; -import { IInterpreterFilter } from '../../../../client/interpreter/locators/types'; import { IServiceContainer } from '../../../../client/ioc/types'; import { PythonInterpreterLocatorService } from '../../../../client/pythonEnvironments/discovery/locators'; import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/info'; @@ -35,19 +34,17 @@ suite('Interpreters - Locators Index', () => { let platformSvc: TypeMoq.IMock; let helper: TypeMoq.IMock; let locator: IInterpreterLocatorService; - let filter: TypeMoq.IMock; setup(() => { serviceContainer = TypeMoq.Mock.ofType(); platformSvc = TypeMoq.Mock.ofType(); helper = TypeMoq.Mock.ofType(); - filter = TypeMoq.Mock.ofType(); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDisposableRegistry))).returns(() => []); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformSvc.object); serviceContainer .setup((c) => c.get(TypeMoq.It.isValue(IInterpreterLocatorHelper))) .returns(() => helper.object); - locator = new PythonInterpreterLocatorService(serviceContainer.object, filter.object); + locator = new PythonInterpreterLocatorService(serviceContainer.object); }); [undefined, Uri.file('Something')].forEach((resource) => { getNamesAndValues(OSType).forEach((osType) => { diff --git a/src/test/pythonEnvironments/discovery/locators/interpreterFilter.unit.test.ts b/src/test/pythonEnvironments/discovery/locators/interpreterFilter.unit.test.ts new file mode 100644 index 000000000000..efdb582c402b --- /dev/null +++ b/src/test/pythonEnvironments/discovery/locators/interpreterFilter.unit.test.ts @@ -0,0 +1,58 @@ +import { expect } from 'chai'; +import { isHiddenInterpreter } from '../../../../client/pythonEnvironments/discovery/locators/services/interpreterFilter'; +import { InterpreterType, PythonInterpreter } from '../../../../client/pythonEnvironments/info'; + +// tslint:disable:no-unused-expression + +suite('Interpreters - Filter', () => { + const doNotHideThesePaths = [ + 'python', + 'python.exe', + 'python2', + 'python2.exe', + 'python38', + 'python3.8.exe', + 'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe', + '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\python.exe', + '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\python.exe' + ]; + const hideThesePaths = [ + '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe', + 'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe', + '%USERPROFILE%\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe', + 'C:\\Users\\SomeUser\\AppData\\Local\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe', + '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe', + '%LOCALAPPDATA%\\Microsoft\\WindowsApps\\PythonSoftwareFoundation\\python.exe', + 'C:\\Program Files\\WindowsApps\\python.exe', + 'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation.Python.3.8_qbz5n2kfra8p0\\python.exe', + 'C:\\Program Files\\WindowsApps\\PythonSoftwareFoundation\\python.exe' + ]; + + function getInterpreterFromPath(interpreterPath: string): PythonInterpreter { + return { + path: interpreterPath, + sysVersion: '', + sysPrefix: '', + architecture: 1, + companyDisplayName: '', + displayName: 'python', + type: InterpreterType.WindowsStore, + envName: '', + envPath: '', + cachedEntry: false + }; + } + + doNotHideThesePaths.forEach((interpreterPath) => { + test(`Interpreter path should NOT be hidden - ${interpreterPath}`, () => { + const interpreter: PythonInterpreter = getInterpreterFromPath(interpreterPath); + expect(isHiddenInterpreter(interpreter), `${interpreterPath} should NOT be treated as hidden.`).to.be.false; + }); + }); + hideThesePaths.forEach((interpreterPath) => { + test(`Interpreter path should be hidden - ${interpreterPath}`, () => { + const interpreter: PythonInterpreter = getInterpreterFromPath(interpreterPath); + expect(isHiddenInterpreter(interpreter), `${interpreterPath} should be treated as hidden.`).to.be.true; + }); + }); +}); diff --git a/src/test/pythonEnvironments/main.unit.test.ts b/src/test/pythonEnvironments/legacyIOC.unit.test.ts similarity index 100% rename from src/test/pythonEnvironments/main.unit.test.ts rename to src/test/pythonEnvironments/legacyIOC.unit.test.ts