Skip to content

Commit 3314bfd

Browse files
DonJayamanneDonJayamanne
authored
Ensure Windows Store install of Python is detected correctly (#6980)
1 parent 57da8ba commit 3314bfd

20 files changed

Lines changed: 184 additions & 59 deletions

‎news/2 Fixes/5926.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure Windows Store install of Python is displyed in the statusbar.

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import { inject, injectable } from 'inversify';
44

55
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
6+
import { WindowsStoreInterpreter } from '../../interpreter/locators/services/windowsStoreInterpreter';
7+
import { IWindowsStoreInterpreter } from '../../interpreter/locators/types';
68
import { IServiceContainer } from '../../ioc/types';
79
import { sendTelemetryEvent } from '../../telemetry';
810
import { EventName } from '../../telemetry/constants';
@@ -19,20 +21,26 @@ import {
1921
IPythonExecutionFactory,
2022
IPythonExecutionService
2123
} from './types';
24+
import { WindowsStorePythonProcess } from './windowsStorePythonProcess';
2225

2326
@injectable()
2427
export class PythonExecutionFactory implements IPythonExecutionFactory {
25-
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
28+
constructor(
29+
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
2630
@inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
2731
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
2832
@inject(IConfigurationService) private readonly configService: IConfigurationService,
29-
@inject(IBufferDecoder) private readonly decoder: IBufferDecoder) {
30-
}
33+
@inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
34+
@inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter
35+
) {}
3136
public async create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService> {
3237
const pythonPath = options.pythonPath ? options.pythonPath : this.configService.getSettings(options.resource).pythonPath;
3338
const processService: IProcessService = await this.processServiceFactory.create(options.resource);
3439
const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
3540
processService.on('exec', processLogger.logProcess.bind(processLogger));
41+
if (this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)) {
42+
return new WindowsStorePythonProcess(this.serviceContainer, processService, pythonPath, this.windowsStoreInterpreter);
43+
}
3644
return new PythonExecutionService(this.serviceContainer, processService, pythonPath);
3745
}
3846
public async createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise<IPythonExecutionService> {

‎src/client/common/process/pythonProcess.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export class PythonExecutionService implements IPythonExecutionService {
2020
constructor(
2121
serviceContainer: IServiceContainer,
2222
private readonly procService: IProcessService,
23-
private readonly pythonPath: string
23+
protected readonly pythonPath: string
2424
) {
2525
this.fileSystem = serviceContainer.get<IFileSystem>(IFileSystem);
2626
}

‎src/client/common/process/windowsStorePythonProcess.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { IWindowsStoreInterpreter } from '../../interpreter/locators/types';
7+
import { IServiceContainer } from '../../ioc/types';
8+
import { PythonExecutionService } from './pythonProcess';
9+
import { IProcessService } from './types';
10+
11+
export class WindowsStorePythonProcess extends PythonExecutionService {
12+
constructor(serviceContainer: IServiceContainer, procService: IProcessService, pythonPath: string, private readonly windowsStoreInterpreter: IWindowsStoreInterpreter) {
13+
super(serviceContainer, procService, pythonPath);
14+
}
15+
/**
16+
* With windows store python apps, we have generally use the symlinked python executable.
17+
* The actual file is not accessible by the user due to permission issues (& rest of exension fails when using that executable).
18+
* Hence lets not resolve the executable using sys.executable for windows store python interpreters.
19+
*
20+
* @returns {Promise<string>}
21+
* @memberof WindowsStorePythonProcess
22+
*/
23+
public async getExecutablePath(): Promise<string> {
24+
return this.windowsStoreInterpreter.isWindowsStoreInterpreter(this.pythonPath) ? this.pythonPath : super.getExecutablePath();
25+
}
26+
}

‎src/client/interpreter/helpers.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ import { compare } from 'semver';
33
import { ConfigurationTarget } from 'vscode';
44
import { IDocumentManager, IWorkspaceService } from '../common/application/types';
55
import { traceError } from '../common/logger';
6-
import { IFileSystem } from '../common/platform/types';
76
import { InterpreterInfomation, IPythonExecutionFactory } from '../common/process/types';
87
import { IPersistentStateFactory, Resource } from '../common/types';
98
import { IServiceContainer } from '../ioc/types';
109
import { IInterpreterHelper, InterpreterType, PythonInterpreter, WorkspacePythonPath } from './contracts';
10+
import { InterpeterHashProviderFactory } from './locators/services/hashProviderFactory';
11+
import { IInterpreterHashProviderFactory } from './locators/types';
1112

1213
const EXPITY_DURATION = 24 * 60 * 60 * 1000;
1314
type CachedPythonInterpreter = Partial<PythonInterpreter> & { fileHash: string };
@@ -22,11 +23,10 @@ export function getFirstNonEmptyLineFromMultilineString(stdout: string) {
2223

2324
@injectable()
2425
export class InterpreterHelper implements IInterpreterHelper {
25-
private readonly fs: IFileSystem;
2626
private readonly persistentFactory: IPersistentStateFactory;
27-
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
27+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
28+
@inject(InterpeterHashProviderFactory) private readonly hashProviderFactory: IInterpreterHashProviderFactory) {
2829
this.persistentFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
29-
this.fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
3030
}
3131
public getActiveWorkspaceUri(resource: Resource): WorkspacePythonPath | undefined {
3232
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
@@ -53,8 +53,12 @@ export class InterpreterHelper implements IInterpreterHelper {
5353
}
5454
}
5555
public async getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonInterpreter>> {
56-
let fileHash = await this.fs.getFileHash(pythonPath).catch(() => '');
57-
fileHash = fileHash ? fileHash : '';
56+
const fileHash = await this.hashProviderFactory.create({pythonPath})
57+
.then(provider => provider.getInterpreterHash(pythonPath))
58+
.catch(ex => {
59+
traceError(`Failed to create File hash for interpreter ${pythonPath}`, ex);
60+
return '';
61+
});
5862
const store = this.persistentFactory.createGlobalPersistentState<CachedPythonInterpreter>(`${pythonPath}.v3`, undefined, EXPITY_DURATION);
5963
if (store.value && fileHash && store.value.fileHash === fileHash) {
6064
return store.value;

‎src/client/interpreter/interpreterService.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ import {
1717
IInterpreterDisplay, IInterpreterHelper, IInterpreterLocatorService,
1818
IInterpreterService, INTERPRETER_LOCATOR_SERVICE,
1919
InterpreterType, PythonInterpreter} from './contracts';
20+
import { InterpeterHashProviderFactory } from './locators/services/hashProviderFactory';
21+
import { IInterpreterHashProviderFactory } from './locators/types';
2022
import { IVirtualEnvironmentManager } from './virtualEnvs/types';
2123

2224
const EXPITY_DURATION = 24 * 60 * 60 * 1000;
2325

2426
@injectable()
2527
export class InterpreterService implements Disposable, IInterpreterService {
2628
private readonly locator: IInterpreterLocatorService;
27-
private readonly fs: IFileSystem;
2829
private readonly persistentStateFactory: IPersistentStateFactory;
2930
private readonly configService: IConfigurationService;
3031
private readonly didChangeInterpreterEmitter = new EventEmitter<void>();
@@ -33,9 +34,9 @@ export class InterpreterService implements Disposable, IInterpreterService {
3334
private readonly updatedInterpreters = new Set<string>();
3435
private pythonPathSetting: string = '';
3536

36-
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
37+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
38+
@inject(InterpeterHashProviderFactory) private readonly hashProviderFactory: IInterpreterHashProviderFactory) {
3739
this.locator = serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, INTERPRETER_LOCATOR_SERVICE);
38-
this.fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
3940
this.persistentStateFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
4041
this.configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
4142
}
@@ -175,7 +176,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
175176
if (!info.cachedEntry && info.path && this.inMemoryCacheOfDisplayNames.has(info.path)) {
176177
return this.inMemoryCacheOfDisplayNames.get(info.path)!;
177178
}
178-
const fileHash = (info.path ? await this.fs.getFileHash(info.path).catch(() => '') : '') || '';
179+
const fileHash = (info.path ? await this.getInterepreterFileHash(info.path).catch(() => '') : '') || '';
179180
// Do not include dipslay name into hash as that changes.
180181
const interpreterHash = `${fileHash}-${md5(JSON.stringify({ ...info, displayName: '' }))}`;
181182
const store = this.persistentStateFactory.createGlobalPersistentState<{ hash: string; displayName: string }>(`${info.path}.interpreter.displayName.v7`, undefined, EXPITY_DURATION);
@@ -195,13 +196,17 @@ export class InterpreterService implements Disposable, IInterpreterService {
195196
return displayName;
196197
}
197198
public async getInterpreterCache(pythonPath: string): Promise<IPersistentState<{ fileHash: string; info?: PythonInterpreter }>> {
198-
const fileHash = (pythonPath ? await this.fs.getFileHash(pythonPath).catch(() => '') : '') || '';
199+
const fileHash = (pythonPath ? await this.getInterepreterFileHash(pythonPath).catch(() => '') : '') || '';
199200
const store = this.persistentStateFactory.createGlobalPersistentState<{ fileHash: string; info?: PythonInterpreter }>(`${pythonPath}.interpreter.Details.v7`, undefined, EXPITY_DURATION);
200201
if (!store.value || store.value.fileHash !== fileHash) {
201202
await store.updateValue({ fileHash });
202203
}
203204
return store;
204205
}
206+
protected async getInterepreterFileHash(pythonPath: string): Promise<string>{
207+
return this.hashProviderFactory.create({pythonPath})
208+
.then(provider => provider.getInterpreterHash(pythonPath));
209+
}
205210
protected async updateCachedInterpreterInformation(info: PythonInterpreter, resource: Resource): Promise<void>{
206211
const key = JSON.stringify(info);
207212
if (this.updatedInterpreters.has(key)) {

‎src/client/interpreter/locators/index.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {
1919
WINDOWS_REGISTRY_SERVICE,
2020
WORKSPACE_VIRTUAL_ENV_SERVICE
2121
} from '../contracts';
22+
import { InterpreterFilter } from './services/interpreterFilter';
23+
import { IInterpreterFilter } from './types';
2224
// tslint:disable-next-line:no-require-imports no-var-requires
2325
const flatten = require('lodash/flatten') as typeof import('lodash/flatten');
2426

@@ -31,9 +33,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
3133
private readonly platform: IPlatformService;
3234
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
3335
private readonly _hasInterpreters: Deferred<boolean>;
34-
constructor(
35-
@inject(IServiceContainer) private serviceContainer: IServiceContainer
36-
) {
36+
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(InterpreterFilter) private readonly interpreterFilter: IInterpreterFilter) {
3737
this._hasInterpreters = createDeferred<boolean>();
3838
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
3939
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
@@ -74,17 +74,20 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
7474
const locators = this.getLocators();
7575
const promises = locators.map(async provider => provider.getInterpreters(resource));
7676
locators.forEach(locator => {
77-
locator.hasInterpreters.then(found => {
78-
if (found) {
79-
this._hasInterpreters.resolve(true);
80-
}
81-
}).ignoreErrors();
77+
locator.hasInterpreters
78+
.then(found => {
79+
if (found) {
80+
this._hasInterpreters.resolve(true);
81+
}
82+
})
83+
.ignoreErrors();
8284
});
8385
const listOfInterpreters = await Promise.all(promises);
8486

8587
const items = flatten(listOfInterpreters)
8688
.filter(item => !!item)
87-
.map(item => item!);
89+
.map(item => item!)
90+
.filter(item => !this.interpreterFilter.isHiddenInterpreter(item));
8891
this._hasInterpreters.resolve(items.length > 0);
8992
return this.interpreterLocatorHelper.mergeInterpreters(items);
9093
}

‎src/client/interpreter/locators/services/interpreterFilter.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { inject, injectable } from 'inversify';
7+
import { PythonInterpreter } from '../../contracts';
8+
import { IInterpreterFilter, IWindowsStoreInterpreter } from '../types';
9+
import { WindowsStoreInterpreter } from './windowsStoreInterpreter';
10+
11+
@injectable()
12+
export class InterpreterFilter implements IInterpreterFilter {
13+
constructor(@inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter) {}
14+
public isHiddenInterpreter(interpreter: PythonInterpreter): boolean {
15+
return this.windowsStoreInterpreter.isHiddenInterpreter(interpreter.path);
16+
}
17+
}

‎src/client/interpreter/locators/services/windowsStoreInterpreter.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
'use strict';
55

66
import { inject, injectable } from 'inversify';
7+
import * as path from 'path';
78
import { traceDecorators } from '../../../common/logger';
89
import { IFileSystem } from '../../../common/platform/types';
910
import { IPythonExecutionFactory } from '../../../common/process/types';
@@ -99,7 +100,9 @@ export class WindowsStoreInterpreter implements IWindowsStoreInterpreter, IInter
99100
const executionFactory = this.serviceContainer.get<IPythonExecutionFactory>(IPythonExecutionFactory);
100101
const pythonService = await executionFactory.create({ pythonPath });
101102
const executablePath = await pythonService.getExecutablePath();
102-
const hash = await this.fs.getFileHash(executablePath);
103+
// If we are unable to get file hash of executable, then get hash of parent directory.
104+
// Its likely it will fail for the executable (fails during development, but try nevertheless - in case things start working).
105+
const hash = await this.fs.getFileHash(executablePath).catch(() => this.fs.getFileHash(path.dirname(executablePath)));
103106
await stateStore.updateValue(hash);
104107

105108
return hash;

‎src/client/interpreter/locators/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
'use strict';
55

66
import { Uri } from 'vscode';
7+
import { PythonInterpreter } from '../contracts';
78

89
export const IPythonInPathCommandProvider = Symbol('IPythonInPathCommandProvider');
910
export interface IPythonInPathCommandProvider {
@@ -62,3 +63,7 @@ export interface IWindowsStoreInterpreter {
6263
*/
6364
isHiddenInterpreter(pythonPath: string): boolean;
6465
}
66+
67+
export interface IInterpreterFilter {
68+
isHiddenInterpreter(interpreter: PythonInterpreter): boolean;
69+
}

0 commit comments

Comments
 (0)