Skip to content

Commit 5340d74

Browse files
Code cleanup prior to fixing #2266. (#2451)
Also, favor Python 3 over Python 2 when falling back to system Python.
1 parent 42ad522 commit 5340d74

15 files changed

Lines changed: 700 additions & 130 deletions

File tree

news/3 Code Health/2266.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Minor cleanup prior to fixing #2266.
2+
3+
This includes changing the default interpreter to favor Python 3
4+
over Python 2.

src/client/common/installer/condaInstaller.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,27 @@ import { ExecutionInfo, IConfigurationService } from '../types';
99
import { ModuleInstaller } from './moduleInstaller';
1010
import { IModuleInstaller } from './types';
1111

12+
/**
13+
* A Python module installer for a conda environment.
14+
*/
1215
@injectable()
1316
export class CondaInstaller extends ModuleInstaller implements IModuleInstaller {
1417
private isCondaAvailable: boolean | undefined;
18+
19+
constructor(
20+
@inject(IServiceContainer) serviceContainer: IServiceContainer
21+
) {
22+
super(serviceContainer);
23+
}
24+
1525
public get displayName() {
1626
return 'Conda';
1727
}
28+
1829
public get priority(): number {
1930
return 0;
2031
}
21-
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
22-
super(serviceContainer);
23-
}
32+
2433
/**
2534
* Checks whether we can use Conda as module installer for a given resource.
2635
* We need to perform two checks:
@@ -41,6 +50,10 @@ export class CondaInstaller extends ModuleInstaller implements IModuleInstaller
4150
// Now we need to check if the current environment is a conda environment or not.
4251
return this.isCurrentEnvironmentACondaEnvironment(resource);
4352
}
53+
54+
/**
55+
* Return the commandline args needed to install the module.
56+
*/
4457
protected async getExecutionInfo(moduleName: string, resource?: Uri): Promise<ExecutionInfo> {
4558
const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
4659
const condaFile = await condaService.getCondaFile();
@@ -64,6 +77,10 @@ export class CondaInstaller extends ModuleInstaller implements IModuleInstaller
6477
execPath: condaFile
6578
};
6679
}
80+
81+
/**
82+
* Is anaconda the current interpreter?
83+
*/
6784
private async isCurrentEnvironmentACondaEnvironment(resource?: Uri): Promise<boolean> {
6885
const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
6986
const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath;

src/client/common/platform/osinfo.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ export class OSInfo implements IOSInfo {
4343
public get is64bit(): boolean {
4444
return this.arch === 'x64';
4545
}
46+
47+
public matchPlatform(names: string): boolean {
48+
return matchPlatform(names, this);
49+
}
4650
}
4751

4852
export function getOSInfo(
@@ -163,3 +167,66 @@ export function parseVersion(raw: string): semver.SemVer {
163167
}
164168
return ver;
165169
}
170+
171+
// Match the platform string to the given OS info.
172+
export function matchPlatform(names: string, info: OSInfo = getOSInfo()): boolean {
173+
if (info.type === OSType.Unknown) {
174+
return false;
175+
}
176+
names = names.trim();
177+
if (names === '') {
178+
return true;
179+
}
180+
for (let name of names.split('|')) {
181+
name = name.trim();
182+
if (matchOnePlatform(name, info)) {
183+
return true;
184+
}
185+
}
186+
return false;
187+
}
188+
189+
function matchOnePlatform(name: string, info: OSInfo): boolean {
190+
if (name === '' || name === '-') {
191+
return false;
192+
}
193+
const negate = name[0] === '-';
194+
if (negate) {
195+
name = name.replace(/^-/, '');
196+
}
197+
198+
const [osType, distro] = identifyOS(name);
199+
if (osType === OSType.Unknown) {
200+
return false;
201+
}
202+
203+
let result = false;
204+
if (osType === info.type) {
205+
result = true;
206+
if (osType === OSType.Linux) {
207+
if (distro !== OSDistro.Unknown) {
208+
result = distro === info.distro;
209+
}
210+
}
211+
}
212+
return negate ? !result : result;
213+
}
214+
215+
function identifyOS(name: string): [OSType, OSDistro] {
216+
name = name.toLowerCase();
217+
if (/win/.test(name)) {
218+
return [OSType.Windows, OSDistro.Unknown];
219+
} else if (/darwin|mac|osx/.test(name)) {
220+
return [OSType.OSX, OSDistro.Unknown];
221+
} else if (/linux/.test(name)) {
222+
return [OSType.Linux, OSDistro.Unknown];
223+
}
224+
225+
// Try linux distros.
226+
const distro = getLinuxDistroFromName(name);
227+
if (distro !== OSDistro.Unknown) {
228+
return [OSType.Linux, distro];
229+
} else {
230+
return [OSType.Unknown, OSDistro.Unknown];
231+
}
232+
}

src/client/common/platform/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export interface IOSInfo {
3737
readonly arch: string;
3838
readonly version: semver.SemVer;
3939
readonly distro: OSDistro;
40+
41+
matchPlatform(names: string): boolean;
4042
}
4143

4244
export enum RegistryHive {

src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,25 @@ import { IPlatformService } from '../../platform/types';
1010
import { IConfigurationService } from '../../types';
1111
import { ITerminalActivationCommandProvider, TerminalShellType } from '../types';
1212

13+
/**
14+
* Support conda env activation (in the terminal).
15+
*/
1316
@injectable()
1417
export class CondaActivationCommandProvider implements ITerminalActivationCommandProvider {
15-
constructor(private readonly serviceContainer: IServiceContainer) { }
18+
constructor(
19+
private readonly serviceContainer: IServiceContainer
20+
) { }
1621

22+
/**
23+
* Is the given shell supported for activating a conda env?
24+
*/
1725
public isShellSupported(_targetShell: TerminalShellType): boolean {
1826
return true;
1927
}
28+
29+
/**
30+
* Return the command needed to activate the conda env.
31+
*/
2032
public async getActivationCommands(resource: Uri | undefined, targetShell: TerminalShellType): Promise<string[] | undefined> {
2133
const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
2234
const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath;

src/client/interpreter/locators/index.ts

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,39 @@ import {
1818
WORKSPACE_VIRTUAL_ENV_SERVICE
1919
} from '../contracts';
2020

21+
/**
22+
* Facilitates locating Python interpreters.
23+
*/
2124
@injectable()
2225
export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
2326
private readonly disposables: Disposable[] = [];
2427
private readonly platform: IPlatformService;
2528
private readonly interpreterLocatorHelper: IInterpreterLocatorHelper;
2629

27-
constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
30+
constructor(
31+
@inject(IServiceContainer) private serviceContainer: IServiceContainer
32+
) {
2833
serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
2934
this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
3035
this.interpreterLocatorHelper = serviceContainer.get<IInterpreterLocatorHelper>(IInterpreterLocatorHelper);
3136
}
32-
public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
33-
return this.getInterpretersPerResource(resource);
34-
}
37+
38+
/**
39+
* Release any held resources.
40+
*
41+
* Called by VS Code to indicate it is done with the resource.
42+
*/
3543
public dispose() {
3644
this.disposables.forEach(disposable => disposable.dispose());
3745
}
38-
private async getInterpretersPerResource(resource?: Uri): Promise<PythonInterpreter[]> {
46+
47+
/**
48+
* Return the list of known Python interpreters.
49+
*
50+
* The optional resource arg may control where locators look for
51+
* interpreters.
52+
*/
53+
public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
3954
const locators = this.getLocators();
4055
const promises = locators.map(async provider => provider.getInterpreters(resource));
4156
const listOfInterpreters = await Promise.all(promises);
@@ -45,26 +60,47 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
4560
.map(item => item!);
4661
return this.interpreterLocatorHelper.mergeInterpreters(items);
4762
}
63+
64+
/**
65+
* Return the list of applicable interpreter locators.
66+
*
67+
* The locators are pulled from the registry.
68+
*/
4869
private getLocators(): IInterpreterLocatorService[] {
49-
const locators: IInterpreterLocatorService[] = [];
5070
// The order of the services is important.
5171
// The order is important because the data sources at the bottom of the list do not contain all,
5272
// the information about the interpreters (e.g. type, environment name, etc).
5373
// This way, the items returned from the top of the list will win, when we combine the items returned.
54-
if (this.platform.isWindows) {
55-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WINDOWS_REGISTRY_SERVICE));
56-
}
57-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_SERVICE));
58-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CONDA_ENV_FILE_SERVICE));
59-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, PIPENV_SERVICE));
60-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, GLOBAL_VIRTUAL_ENV_SERVICE));
61-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, WORKSPACE_VIRTUAL_ENV_SERVICE));
74+
const keys: [string, string][] = [
75+
[WINDOWS_REGISTRY_SERVICE, 'win'],
76+
[CONDA_ENV_SERVICE, ''],
77+
[CONDA_ENV_FILE_SERVICE, ''],
78+
[PIPENV_SERVICE, ''],
79+
[GLOBAL_VIRTUAL_ENV_SERVICE, ''],
80+
[WORKSPACE_VIRTUAL_ENV_SERVICE, ''],
81+
[KNOWN_PATH_SERVICE, '-win'],
82+
[CURRENT_PATH_SERVICE, '']
83+
];
84+
return getLocators(keys, this.platform, (key) => {
85+
return this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, key);
86+
});
87+
}
88+
}
6289

63-
if (!this.platform.isWindows) {
64-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, KNOWN_PATH_SERVICE));
65-
}
66-
locators.push(this.serviceContainer.get<IInterpreterLocatorService>(IInterpreterLocatorService, CURRENT_PATH_SERVICE));
90+
type PlatformName = string;
6791

68-
return locators;
92+
function getLocators(
93+
keys: [string, PlatformName][],
94+
platform: IPlatformService,
95+
getService: (string) => IInterpreterLocatorService
96+
): IInterpreterLocatorService[] {
97+
const locators: IInterpreterLocatorService[] = [];
98+
for (const [key, platformName] of keys) {
99+
if (!platform.os.matchPlatform(platformName)) {
100+
continue;
101+
}
102+
const locator = getService(key);
103+
locators.push(locator);
69104
}
105+
return locators;
70106
}

src/client/interpreter/locators/services/KnownPathsService.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,39 @@ import { CacheableLocatorService } from './cacheableLocatorService';
1111
// tslint:disable-next-line:no-require-imports no-var-requires
1212
const untildify = require('untildify');
1313

14+
/**
15+
* Locates "known" paths.
16+
*/
1417
@injectable()
1518
export class KnownPathsService extends CacheableLocatorService {
16-
public constructor(@inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: string[],
19+
public constructor(
20+
@inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: string[],
1721
@inject(IInterpreterHelper) private helper: IInterpreterHelper,
18-
@inject(IServiceContainer) serviceContainer: IServiceContainer) {
22+
@inject(IServiceContainer) serviceContainer: IServiceContainer
23+
) {
1924
super('KnownPathsService', serviceContainer);
2025
}
26+
27+
/**
28+
* Release any held resources.
29+
*
30+
* Called by VS Code to indicate it is done with the resource.
31+
*/
2132
// tslint:disable-next-line:no-empty
2233
public dispose() { }
34+
35+
/**
36+
* Return the located interpreters.
37+
*
38+
* This is used by CacheableLocatorService.getInterpreters().
39+
*/
2340
protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
2441
return this.suggestionsFromKnownPaths();
2542
}
43+
44+
/**
45+
* Return the located interpreters.
46+
*/
2647
private suggestionsFromKnownPaths() {
2748
const promises = this.knownSearchPaths.map(dir => this.getInterpretersInDirectory(dir));
2849
return Promise.all<string[]>(promises)
@@ -32,6 +53,10 @@ export class KnownPathsService extends CacheableLocatorService {
3253
.then(interpreters => Promise.all(interpreters.map(interpreter => this.getInterpreterDetails(interpreter))))
3354
.then(interpreters => interpreters.filter(interpreter => !!interpreter).map(interpreter => interpreter!));
3455
}
56+
57+
/**
58+
* Return the information about the identified interpreter binary.
59+
*/
3560
private async getInterpreterDetails(interpreter: string) {
3661
const details = await this.helper.getInterpreterInformation(interpreter);
3762
if (!details) {
@@ -43,12 +68,19 @@ export class KnownPathsService extends CacheableLocatorService {
4368
type: InterpreterType.Unknown
4469
};
4570
}
71+
72+
/**
73+
* Return the interpreters in the given directory.
74+
*/
4675
private getInterpretersInDirectory(dir: string) {
4776
return fsExistsAsync(dir)
4877
.then(exists => exists ? lookForInterpretersInDirectory(dir) : Promise.resolve<string[]>([]));
4978
}
5079
}
5180

81+
/**
82+
* Return the paths where Python interpreters might be found.
83+
*/
5284
export function getKnownSearchPathsForInterpreters(): string[] {
5385
if (IS_WINDOWS) {
5486
return [];

0 commit comments

Comments
 (0)