Skip to content

Commit cdf2313

Browse files
DonJayamanneDonJayamanne
authored
Refactor how version information is stored against interpreter information (microsoft#3812)
For microsoft#3369
1 parent 209edb6 commit cdf2313

32 files changed

Lines changed: 273 additions & 272 deletions

‎src/client/common/installer/moduleInstaller.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export abstract class ModuleInstaller {
5656
// If installing pylint on python 2.x, then use pylint~=1.9.0
5757
const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
5858
const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
59-
if (currentInterpreter && currentInterpreter.version_info && currentInterpreter.version_info[0] === 2) {
59+
if (currentInterpreter && currentInterpreter.version && currentInterpreter.version.major === 2) {
6060
const newArgs = [...args];
6161
// This command could be sent to the terminal, hence '<' needs to be escaped for UNIX.
6262
newArgs[indexOfPylint] = '"pylint<2.0.0"';

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

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
1010
import { traceError } from '../logger';
1111
import { IFileSystem } from '../platform/types';
1212
import { Architecture } from '../utils/platform';
13+
import { convertPythonVersionToSemver } from '../utils/version';
1314
import { ExecutionResult, InterpreterInfomation, IProcessService, IPythonExecutionService, ObservableExecutionResult, PythonVersionInfo, SpawnOptions } from './types';
1415

1516
@injectable()
@@ -27,12 +28,8 @@ export class PythonExecutionService implements IPythonExecutionService {
2728
public async getInterpreterInformation(): Promise<InterpreterInfomation | undefined> {
2829
const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'interpreterInfo.py');
2930
try {
30-
const [version, jsonValue] = await Promise.all([
31-
this.procService.exec(this.pythonPath, ['--version'], { mergeStdOutErr: true })
32-
.then(output => output.stdout.trim()),
33-
this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true })
34-
.then(output => output.stdout.trim())
35-
]);
31+
const jsonValue = await this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true })
32+
.then(output => output.stdout.trim());
3633

3734
let json: { versionInfo: PythonVersionInfo; sysPrefix: string; sysVersion: string; is64Bit: boolean };
3835
try {
@@ -41,22 +38,12 @@ export class PythonExecutionService implements IPythonExecutionService {
4138
traceError(`Failed to parse interpreter information for '${this.pythonPath}' with JSON ${jsonValue}`, ex);
4239
return;
4340
}
44-
const version_info = json.versionInfo;
45-
// Exclude PII from `version_info` to ensure we don't send this up via telemetry.
46-
for (let index = 0; index < 3; index += 1) {
47-
if (typeof version_info[index] !== 'number') {
48-
version_info[index] = 0;
49-
}
50-
}
51-
if (['alpha', 'beta', 'candidate', 'final'].indexOf(version_info[3]) === -1) {
52-
version_info[3] = 'unknown';
53-
}
41+
const versionValue = json.versionInfo.length === 4 ? `${json.versionInfo.slice(0, 3).join('.')}-${json.versionInfo[3]}` : json.versionInfo.join('.');
5442
return {
5543
architecture: json.is64Bit ? Architecture.x64 : Architecture.x86,
5644
path: this.pythonPath,
57-
version,
45+
version: convertPythonVersionToSemver(versionValue),
5846
sysVersion: json.sysVersion,
59-
version_info: json.versionInfo,
6047
sysPrefix: json.sysPrefix
6148
};
6249
} catch (ex) {

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } f
44
import { Observable } from 'rxjs/Observable';
55
import { CancellationToken, Uri } from 'vscode';
66

7+
import { SemVer } from 'semver';
78
import { ExecutionInfo } from '../types';
89
import { Architecture } from '../utils/platform';
910
import { EnvironmentVariables } from '../variables/types';
@@ -20,7 +21,7 @@ export type Output<T extends string | Buffer> = {
2021
export type ObservableExecutionResult<T extends string | Buffer> = {
2122
proc: ChildProcess | undefined;
2223
out: Observable<Output<T>>;
23-
dispose() : void;
24+
dispose(): void;
2425
};
2526

2627
// tslint:disable-next-line:interface-name
@@ -32,7 +33,7 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
3233
};
3334

3435
// tslint:disable-next-line:interface-name
35-
export type ShellOptions = ExecOptions & { throwOnStdErr? : boolean };
36+
export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };
3637

3738
export type ExecutionResult<T extends string | Buffer> = {
3839
stdout: T;
@@ -60,14 +61,12 @@ export interface IPythonExecutionFactory {
6061
create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService>;
6162
}
6263
export type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown';
63-
// tslint:disable-next-line:interface-name
6464
export type PythonVersionInfo = [number, number, number, ReleaseLevel];
6565
export type InterpreterInfomation = {
6666
path: string;
67-
version: string;
67+
version?: SemVer;
6868
sysVersion: string;
6969
architecture: Architecture;
70-
version_info: PythonVersionInfo;
7170
sysPrefix: string;
7271
};
7372
export const IPythonExecutionService = Symbol('IPythonExecutionService');

‎src/client/common/terminal/service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export class TerminalService implements ITerminalService, Disposable {
8181
private async sendTelemetry() {
8282
const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(this.resource).pythonPath;
8383
const interpreterInfo = await this.serviceContainer.get<IInterpreterService>(IInterpreterService).getInterpreterDetails(pythonPath);
84-
const pythonVersion = (interpreterInfo && interpreterInfo.version_info) ? interpreterInfo.version_info.join('.') : undefined;
84+
const pythonVersion = (interpreterInfo && interpreterInfo.version) ? interpreterInfo.version.raw : undefined;
8585
const interpreterType = interpreterInfo ? interpreterInfo.type : undefined;
8686
captureTelemetry(TERMINAL_CREATE, { terminal: this.terminalShellType, pythonVersion, interpreterType });
8787
}

‎src/client/common/utils/version.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,33 @@ export function convertToSemver(version: string) {
2424
return versionParts.join('.');
2525
}
2626

27+
export function convertPythonVersionToSemver(version: string): semver.SemVer | undefined {
28+
if (!version || version.trim().length === 0) {
29+
return;
30+
}
31+
const versionParts = (version || '')
32+
.split('.')
33+
.map(item => item.trim())
34+
.filter(item => item.length > 0)
35+
.filter((_, index) => index < 4);
36+
37+
if (versionParts.length > 0 && versionParts[versionParts.length - 1].indexOf('-') > 0) {
38+
const lastPart = versionParts[versionParts.length - 1];
39+
versionParts[versionParts.length - 1] = lastPart.split('-')[0].trim();
40+
versionParts.push(lastPart.split('-')[1].trim());
41+
}
42+
while (versionParts.length < 4) {
43+
versionParts.push('');
44+
}
45+
// Exclude PII from `version_info` to ensure we don't send this up via telemetry.
46+
for (let index = 0; index < 3; index += 1) {
47+
versionParts[index] = /^\d+$/.test(versionParts[index]) ? versionParts[index] : '0';
48+
}
49+
versionParts[3] = ['alpha', 'beta', 'candidate', 'final'].indexOf(versionParts[3]) === -1 ? 'unknown' : versionParts[3];
50+
51+
return new semver.SemVer(`${versionParts[0]}.${versionParts[1]}.${versionParts[2]}-${versionParts[3]}`);
52+
}
53+
2754
export function compareVersion(versionA: string, versionB: string) {
2855
try {
2956
versionA = convertToSemver(versionA);

‎src/client/datascience/jupyterExecution.ts‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ class JupyterCommand {
9292

9393
public mainVersion = async (): Promise<number> => {
9494
const interpreter = await this.interpreterPromise;
95-
if (interpreter) {
96-
return interpreter.version_info[0];
95+
if (interpreter && interpreter.version) {
96+
return interpreter.version.major;
9797
} else {
9898
return this.execVersion();
9999
}
@@ -417,14 +417,18 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
417417
score += 1;
418418

419419
// Then start matching based on version
420-
if (list[i].version_info[0] === active.version_info[0]) {
421-
score += 32;
422-
if (list[i].version_info[1] === active.version_info[1]) {
423-
score += 16;
424-
if (list[i].version_info[2] === active.version_info[2]) {
425-
score += 8;
426-
if (list[i].version_info[3] === active.version_info[3]) {
427-
score += 4;
420+
const listVersion = list[i].version;
421+
const activeVersion = active.version;
422+
if (listVersion && activeVersion) {
423+
if (listVersion.major === activeVersion.major) {
424+
score += 32;
425+
if (listVersion.minor === activeVersion.minor) {
426+
score += 16;
427+
if (listVersion.patch === activeVersion.patch) {
428+
score += 8;
429+
if (listVersion.raw === activeVersion.raw) {
430+
score += 4;
431+
}
428432
}
429433
}
430434
}
@@ -632,33 +636,33 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
632636
score += 1;
633637

634638
// See if the version is the same
635-
if (info && info.version_info && specDetails[i]) {
639+
if (info && info.version && specDetails[i]) {
636640
const details = specDetails[i];
637-
if (details && details.version_info) {
638-
if (details.version_info[0] === info.version_info[0]) {
641+
if (details && details.version) {
642+
if (details.version.major === info.version.major) {
639643
// Major version match
640644
score += 4;
641645

642-
if (details.version_info[1] === info.version_info[1]) {
646+
if (details.version.minor === info.version.minor) {
643647
// Minor version match
644648
score += 2;
645649

646-
if (details.version_info[2] === info.version_info[2]) {
650+
if (details.version.patch === info.version.patch) {
647651
// Minor version match
648652
score += 1;
649653
}
650654
}
651655
}
652656
}
653-
} else if (info && info.version_info && spec && spec.path && spec.path.toLocaleLowerCase() === 'python' && spec.name) {
657+
} else if (info && info.version && spec && spec.path && spec.path.toLocaleLowerCase() === 'python' && spec.name) {
654658
// This should be our current python.
655659

656660
// Search for a digit on the end of the name. It should match our major version
657661
const match = /\D+(\d+)/.exec(spec.name);
658662
if (match && match !== null && match.length > 0) {
659663
// See if the version number matches
660664
const nameVersion = parseInt(match[0], 10);
661-
if (nameVersion && nameVersion === info.version_info[0]) {
665+
if (nameVersion && nameVersion === info.version.major) {
662666
score += 4;
663667
}
664668
}

‎src/client/datascience/jupyterExporter.ts‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { CellState, ICell, IJupyterExecution, INotebookExporter, ISysInfo } from
1919
export class JupyterExporter implements INotebookExporter {
2020

2121
constructor(
22-
@inject(IJupyterExecution) private jupyterExecution : IJupyterExecution,
22+
@inject(IJupyterExecution) private jupyterExecution: IJupyterExecution,
2323
@inject(ILogger) private logger: ILogger,
2424
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
2525
@inject(IFileSystem) private fileSystem: IFileSystem) {
@@ -29,7 +29,7 @@ export class JupyterExporter implements INotebookExporter {
2929
noop();
3030
}
3131

32-
public async translateToNotebook(cells: ICell[], changeDirectory?: string) : Promise<nbformat.INotebookContent | undefined> {
32+
public async translateToNotebook(cells: ICell[], changeDirectory?: string): Promise<nbformat.INotebookContent | undefined> {
3333
// If requested, add in a change directory cell to fix relative paths
3434
if (changeDirectory) {
3535
cells = await this.addDirectoryChangeCell(cells, changeDirectory);
@@ -95,18 +95,17 @@ export class JupyterExporter implements INotebookExporter {
9595
// When we export we want to our change directory back to the first real file that we saw run from any workspace folder
9696
private firstWorkspaceFolder = async (cells: ICell[]): Promise<string | undefined> => {
9797
for (const cell of cells) {
98-
const filename = cell.file;
98+
const filename = cell.file;
9999

100-
// First check that this is an absolute file that exists (we add in temp files to run system cell)
101-
if (path.isAbsolute(filename) && await this.fileSystem.fileExists(filename)) {
100+
// First check that this is an absolute file that exists (we add in temp files to run system cell)
101+
if (path.isAbsolute(filename) && await this.fileSystem.fileExists(filename)) {
102102
// We've already check that workspace folders above
103103
for (const folder of this.workspaceService.workspaceFolders!) {
104-
if (filename.toLowerCase().startsWith(folder.uri.fsPath.toLowerCase()))
105-
{
104+
if (filename.toLowerCase().startsWith(folder.uri.fsPath.toLowerCase())) {
106105
return folder.uri.fsPath;
107106
}
108107
}
109-
}
108+
}
110109
}
111110

112111
return undefined;
@@ -134,22 +133,22 @@ export class JupyterExporter implements INotebookExporter {
134133
}
135134
}
136135

137-
private pruneCells = (cells : ICell[]) : nbformat.IBaseCell[] => {
136+
private pruneCells = (cells: ICell[]): nbformat.IBaseCell[] => {
138137
// First filter out sys info cells. Jupyter doesn't understand these
139138
return cells.filter(c => c.data.cell_type !== 'sys_info')
140139
// Then prune each cell down to just the cell data.
141140
.map(this.pruneCell);
142141
}
143142

144-
private pruneCell = (cell : ICell) : nbformat.IBaseCell => {
143+
private pruneCell = (cell: ICell): nbformat.IBaseCell => {
145144
// Remove the #%% of the top of the source if there is any. We don't need
146145
// this to end up in the exported ipynb file.
147-
const copy = {...cell.data};
146+
const copy = { ...cell.data };
148147
copy.source = this.pruneSource(cell.data.source);
149148
return copy;
150149
}
151150

152-
private pruneSource = (source : nbformat.MultilineString) : nbformat.MultilineString => {
151+
private pruneSource = (source: nbformat.MultilineString): nbformat.MultilineString => {
153152

154153
if (Array.isArray(source) && source.length > 0) {
155154
if (RegExpValues.PythonCellMarker.test(source[0])) {
@@ -168,7 +167,7 @@ export class JupyterExporter implements INotebookExporter {
168167
private extractPythonMainVersion = async (cells: ICell[]): Promise<number> => {
169168
let pythonVersion;
170169
const sysInfoCells = cells.filter((targetCell: ICell) => {
171-
return targetCell.data.cell_type === 'sys_info';
170+
return targetCell.data.cell_type === 'sys_info';
172171
});
173172

174173
if (sysInfoCells.length > 0) {
@@ -184,6 +183,6 @@ export class JupyterExporter implements INotebookExporter {
184183

185184
// In this case, let's check the version on the active interpreter
186185
const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython();
187-
return usableInterpreter ? usableInterpreter.version_info[0] : 3;
186+
return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3;
188187
}
189188
}

‎src/client/extension.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,10 @@ async function sendStartupTelemetry(activatedPromise: Promise<any>, serviceConta
297297
interpreterService.getInterpreters(mainWorkspaceUri).catch<PythonInterpreter[]>(() => [])
298298
]);
299299
const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0;
300-
const pythonVersion = interpreter ? interpreter.version_info.join('.') : undefined;
300+
const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined;
301301
const interpreterType = interpreter ? interpreter.type : undefined;
302302
const hasPython3 = interpreters
303-
.filter(item => item && Array.isArray(item.version_info) ? item.version_info[0] === 3 : false)
303+
.filter(item => item && item.version ? item.version.major === 3 : false)
304304
.length > 0;
305305

306306
const props = { condaVersion, terminal: terminalShellType, pythonVersion, interpreterType, workspaceFolderCount, hasPython3 };

‎src/client/interpreter/configuration/interpreterComparer.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ export class InterpreterComparer implements IInterpreterComparer {
2929
// * Architecture
3030
// * Interpreter Type
3131
// * Environment name
32-
if (info.version_info && info.version_info.length > 0) {
33-
sortNameParts.push(info.version_info.slice(0, 3).join('.'));
32+
if (info.version) {
33+
sortNameParts.push(info.version.raw);
3434
}
3535
if (info.architecture) {
3636
sortNameParts.push(getArchitectureDisplayName(info.architecture));

‎src/client/interpreter/configuration/pythonPathUpdaterService.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ export class PythonPathUpdaterService implements IPythonPathUpdaterServiceManage
4949
.then(value => value.length === 0 ? undefined : value)
5050
.catch<string>(() => '');
5151
const [info, pipVersion] = await Promise.all([infoPromise, pipVersionPromise]);
52-
if (info) {
53-
telemtryProperties.pythonVersion = info.version_info.join('.');
52+
if (info && info.version) {
53+
telemtryProperties.pythonVersion = info.version.raw;
5454
}
5555
if (pipVersion) {
5656
telemtryProperties.pipVersion = pipVersion;

0 commit comments

Comments
 (0)