Ensure release level is set when using new environment discovery comp…

…onent (#15633) * Ensure release level is set when using new environment discovery component * Fix tests * Fix display version when not in experiment. * Address test issues and simplify * Clean up * Fix tests * Ensure we don't send any PII with version info * Fix tests
microsoft · Mar 19, 2021 · 3ca66f9 · 3ca66f9
1 parent 6955f5a
commit 3ca66f9
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 35 deletions.
diff --git a/news/2 Fixes/15462.md b/news/2 Fixes/15462.md
@@ -0,0 +1 @@
+Ensure release level is set when using new environment discovery component.
diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts
@@ -37,6 +37,7 @@ import { IVirtualEnvironmentManager } from './virtualEnvs/types';
 import { getInterpreterHash } from '../pythonEnvironments/discovery/locators/services/hashProvider';
 import { inDiscoveryExperiment } from '../common/experiments/helpers';
 import { StopWatch } from '../common/utils/stopWatch';
+import { PythonVersion } from '../pythonEnvironments/info/pythonVersion';
 
 const EXPIRY_DURATION = 24 * 60 * 60 * 1000;
 
@@ -290,6 +291,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
             undefined,
             EXPIRY_DURATION,
         );
+
         if (store.value && store.value.hash === interpreterHash && store.value.displayName) {
             this.inMemoryCacheOfDisplayNames.set(info.path!, store.value.displayName);
             return store.value.displayName;
@@ -354,12 +356,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
         const envSuffixParts: string[] = [];
 
         if (info.version) {
-            // Exclude invalid -1 filler values.
-            const versionParts = [info.version.major, info.version.minor, info.version.patch].filter(
-                (value) => value > -1,
-            );
-
-            displayNameParts.push(versionParts.join('.'));
+            displayNameParts.push(this.getVersionForDisplay(info.version));
         }
         if (info.architecture) {
             displayNameParts.push(getArchitectureDisplayName(info.architecture));
@@ -393,6 +390,37 @@ export class InterpreterService implements Disposable, IInterpreterService {
         return `${displayNameParts.join(' ')} ${envSuffix}`.trim();
     }
 
+    // eslint-disable-next-line class-methods-use-this
+    private getVersionForDisplay(version: PythonVersion): string {
+        // Exclude invalid -1 filler values.
+        const versionParts = [version.major, version.minor, version.patch].filter((value) => value > -1);
+
+        let preRelease = '';
+        if (version.prerelease.length > 0) {
+            switch (version.prerelease[0]) {
+                case 'alpha':
+                case 'a':
+                    preRelease = `a`;
+                    break;
+                case 'beta':
+                case 'b':
+                    preRelease = `b`;
+                    break;
+                case 'candidate':
+                case 'rc':
+                    preRelease = `rc`;
+                    break;
+                case 'final':
+                default:
+                    break;
+            }
+            if (preRelease !== '' && version.prerelease.length > 1) {
+                preRelease = `${preRelease}${version.prerelease.slice(1).join('')}`;
+            }
+        }
+        return `${versionParts.join('.')}${preRelease}`;
+    }
+
     private async collectInterpreterDetails(pythonPath: string, resource: Uri | undefined) {
         const interpreterHelper = this.serviceContainer.get<IInterpreterHelper>(IInterpreterHelper);
         const virtualEnvManager = this.serviceContainer.get<IVirtualEnvironmentManager>(IVirtualEnvironmentManager);

diff --git a/src/client/pythonEnvironments/base/info/interpreter.ts b/src/client/pythonEnvironments/base/info/interpreter.ts
@@ -26,7 +26,24 @@ export type InterpreterInformation = {
  * @param raw - the information returned by the `interpreterInfo.py` script
  */
 function extractInterpreterInfo(python: string, raw: InterpreterInfoJson): InterpreterInformation {
-    const rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}-${raw.versionInfo[3]}`;
+    let rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}`;
+
+    // We only need additional version details if the version is 'alpha', 'beta' or 'candidate'.
+    // This restriction is needed to avoid sending any PII if this data is used with telemetry.
+    // With custom builds of python it is possible that release level and values after that can
+    // contain PII.
+    if (raw.versionInfo[3] !== undefined && ['final', 'alpha', 'beta', 'candidate'].includes(raw.versionInfo[3])) {
+        rawVersion = `${rawVersion}-${raw.versionInfo[3]}`;
+        if (raw.versionInfo[4] !== undefined) {
+            let serial = -1;
+            try {
+                serial = parseInt(`${raw.versionInfo[4]}`, 10);
+            } catch (ex) {
+                serial = -1;
+            }
+            rawVersion = serial >= 0 ? `${rawVersion}${serial}` : rawVersion;
+        }
+    }
     return {
         arch: raw.is64Bit ? Architecture.x64 : Architecture.x86,
         executable: {

diff --git a/src/client/pythonEnvironments/base/info/pythonVersion.ts b/src/client/pythonEnvironments/base/info/pythonVersion.ts
@@ -329,3 +329,37 @@ export function copyBestVersion(version: PythonVersion, other: PythonVersion): P
     const winner = result > 0 ? other : version;
     return cloneDeep(winner);
 }
+
+/**
+ * Convert Python version to semver like version object.
+ *
+ * Remarks: primarily used to convert to old type of environment info.
+ * @deprecated
+ */
+export function toSemverLikeVersion(
+    version: PythonVersion,
+): {
+    raw: string;
+    major: number;
+    minor: number;
+    patch: number;
+    build: string[];
+    prerelease: string[];
+} {
+    const versionPrefix = basic.getVersionString(version);
+    let preRelease: string[] = [];
+    if (version.release) {
+        preRelease =
+            version.release.serial < 0
+                ? [`${version.release.level}`]
+                : [`${version.release.level}`, `${version.release.serial}`];
+    }
+    return {
+        raw: versionPrefix,
+        major: version.major,
+        minor: version.minor,
+        patch: version.micro,
+        build: [],
+        prerelease: preRelease,
+    };
+}
diff --git a/src/client/pythonEnvironments/info/interpreter.ts b/src/client/pythonEnvironments/info/interpreter.ts
@@ -1,14 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { SemVer } from 'semver';
 import { InterpreterInformation } from '.';
 import {
     interpreterInfo as getInterpreterInfoCommand,
     InterpreterInfoJson,
 } from '../../common/process/internal/scripts';
 import { Architecture } from '../../common/utils/platform';
 import { copyPythonExecInfo, PythonExecInfo } from '../exec';
-import { parsePythonVersion } from './pythonVersion';
 
 /**
  * Compose full interpreter information based on the given data.
@@ -19,11 +19,27 @@ import { parsePythonVersion } from './pythonVersion';
  * @param raw - the information returned by the `interpreterInfo.py` script
  */
 export function extractInterpreterInfo(python: string, raw: InterpreterInfoJson): InterpreterInformation {
-    const rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}-${raw.versionInfo[3]}`;
+    let rawVersion = `${raw.versionInfo.slice(0, 3).join('.')}`;
+    // We only need additional version details if the version is 'alpha', 'beta' or 'candidate'.
+    // This restriction is needed to avoid sending any PII if this data is used with telemetry.
+    // With custom builds of python it is possible that release level and values after that can
+    // contain PII.
+    if (raw.versionInfo[3] !== undefined && ['alpha', 'beta', 'candidate'].includes(raw.versionInfo[3])) {
+        rawVersion = `${rawVersion}-${raw.versionInfo[3]}`;
+        if (raw.versionInfo[4] !== undefined) {
+            let serial = -1;
+            try {
+                serial = parseInt(`${raw.versionInfo[4]}`, 10);
+            } catch (ex) {
+                serial = -1;
+            }
+            rawVersion = serial >= 0 ? `${rawVersion}${serial}` : rawVersion;
+        }
+    }
     return {
         architecture: raw.is64Bit ? Architecture.x64 : Architecture.x86,
         path: python,
-        version: parsePythonVersion(rawVersion),
+        version: new SemVer(rawVersion),
         sysVersion: raw.sysVersion,
         sysPrefix: raw.sysPrefix,
     };

diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts
@@ -9,7 +9,6 @@ import { DiscoveryVariants } from '../common/experiments/groups';
 import { traceError } from '../common/logger';
 import { FileChangeType } from '../common/platform/fileSystemWatcher';
 import { IDisposableRegistry, Resource } from '../common/types';
-import { getVersionString } from '../common/utils/version';
 import {
     CONDA_ENV_FILE_SERVICE,
     CONDA_ENV_SERVICE,
@@ -36,7 +35,7 @@ import { IPipEnvServiceHelper, IPythonInPathCommandProvider } from '../interpret
 import { VirtualEnvironmentManager } from '../interpreter/virtualEnvs';
 import { IVirtualEnvironmentManager } from '../interpreter/virtualEnvs/types';
 import { IServiceManager } from '../ioc/types';
-import { PythonEnvInfo, PythonEnvKind, PythonEnvSource, PythonReleaseLevel } from './base/info';
+import { PythonEnvInfo, PythonEnvKind, PythonEnvSource } from './base/info';
 import { buildEnvInfo } from './base/info/env';
 import { ILocator, PythonLocatorQuery } from './base/locator';
 import { isMacDefaultPythonPath } from './base/locators/lowLevel/macDefaultLocator';
@@ -68,7 +67,7 @@ import {
 import { WorkspaceVirtualEnvWatcherService } from './discovery/locators/services/workspaceVirtualEnvWatcherService';
 import { EnvironmentType, PythonEnvironment } from './info';
 import { EnvironmentsSecurity, IEnvironmentsSecurity } from './security';
-import { parseBasicVersion } from './base/info/pythonVersion';
+import { toSemverLikeVersion } from './base/info/pythonVersion';
 import { PythonVersion } from './info/pythonVersion';
 
 const convertedKinds = new Map(
@@ -106,28 +105,13 @@ function convertEnvInfo(info: PythonEnvInfo): PythonEnvironment {
 
     if (version !== undefined) {
         const { release, sysVersion } = version;
-        const versionPrefix = getVersionString(version);
-        let versionStr;
-
         if (release === undefined) {
-            versionStr = `${versionPrefix}-final`;
             env.sysVersion = '';
         } else {
-            const { level, serial } = release;
-            const releaseStr = level === PythonReleaseLevel.Final ? 'final' : `${level}${serial}`;
-            versionStr = `${versionPrefix}-${releaseStr}`;
             env.sysVersion = sysVersion;
         }
 
-        const result = parseBasicVersion(versionStr)[0];
-        const semverLikeVersion: PythonVersion = {
-            raw: versionPrefix,
-            major: result.major,
-            minor: result.minor,
-            patch: result.micro,
-            build: [],
-            prerelease: [],
-        };
+        const semverLikeVersion: PythonVersion = toSemverLikeVersion(version);
         env.version = semverLikeVersion;
     }
 

diff --git a/src/test/common/process/pythonEnvironment.unit.test.ts b/src/test/common/process/pythonEnvironment.unit.test.ts
@@ -32,7 +32,7 @@ suite('PythonEnvironment', () => {
 
     test('getInterpreterInformation should return an object if the python path is valid', async () => {
         const json = {
-            versionInfo: [3, 7, 5, 'candidate'],
+            versionInfo: [3, 7, 5, 'candidate', 1],
             sysPrefix: '/path/of/sysprefix/versions/3.7.5rc1',
             version: '3.7.5rc1 (default, Oct 18 2019, 14:48:48) \n[Clang 11.0.0 (clang-1100.0.33.8)]',
             is64Bit: true,
@@ -47,14 +47,42 @@ suite('PythonEnvironment', () => {
         const expectedResult = {
             architecture: Architecture.x64,
             path: pythonPath,
-            version: new SemVer('3.7.5-candidate'),
+            version: new SemVer('3.7.5-candidate1'),
             sysPrefix: json.sysPrefix,
             sysVersion: undefined,
         };
 
         expect(result).to.deep.equal(expectedResult, 'Incorrect value returned by getInterpreterInformation().');
     });
 
+    test('getInterpreterInformation should return an object if the version info contains less than 5 items', async () => {
+        const json = {
+            versionInfo: [3, 7, 5, 'alpha'],
+            sysPrefix: '/path/of/sysprefix/versions/3.7.5a1',
+            version: '3.7.5a1 (default, Oct 18 2019, 14:48:48) \n[Clang 11.0.0 (clang-1100.0.33.8)]',
+            is64Bit: true,
+        };
+
+        processService
+            .setup((p) => p.shellExec(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+            .returns(() => Promise.resolve({ stdout: JSON.stringify(json) }));
+        const env = createPythonEnv(pythonPath, processService.object, fileSystem.object);
+
+        const result = await env.getInterpreterInformation();
+        const expectedResult = {
+            architecture: Architecture.x64,
+            path: pythonPath,
+            version: new SemVer('3.7.5-alpha'),
+            sysPrefix: json.sysPrefix,
+            sysVersion: undefined,
+        };
+
+        expect(result).to.deep.equal(
+            expectedResult,
+            'Incorrect value returned by getInterpreterInformation() with truncated versionInfo.',
+        );
+    });
+
     test('getInterpreterInformation should return an object if the version info contains less than 4 items', async () => {
         const json = {
             versionInfo: [3, 7, 5],

diff --git a/src/test/interpreters/interpreterService.unit.test.ts b/src/test/interpreters/interpreterService.unit.test.ts
@@ -51,6 +51,7 @@ import * as hashApi from '../../client/pythonEnvironments/discovery/locators/ser
 import { EnvironmentType, PythonEnvironment } from '../../client/pythonEnvironments/info';
 import { PYTHON_PATH } from '../common';
 import { MockAutoSelectionService } from '../mocks/autoSelector';
+import { PythonVersion } from '../../client/pythonEnvironments/info/pythonVersion';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
@@ -528,7 +529,7 @@ suite('Interpreters service', () => {
     suite('Display Format (with all permutations)', () => {
         setup(setupSuite);
         [undefined, Uri.file('xyz')].forEach((resource) => {
-            [undefined, new SemVer('1.2.3-alpha')].forEach((version) => {
+            [undefined, new SemVer('3.10.0-alpha6'), new SemVer('3.10.0')].forEach((version) => {
                 // Forced cast to ignore TS warnings.
                 (EnumEx.getNamesAndValues<Architecture>(Architecture) as (
                     | { name: string; value: Architecture }
@@ -548,7 +549,9 @@ suite('Interpreters service', () => {
                                         ['', 'my pipenv name'].forEach((pipEnvName) => {
                                             const testName = [
                                                 `${resource ? 'With' : 'Without'} a workspace`,
-                                                `${version ? 'with' : 'without'} version information`,
+                                                `${version ? 'with' : 'without'} ${
+                                                    version && version.prerelease.length > 0 ? 'pre-release' : 'final'
+                                                } version information`,
                                                 `${arch ? arch.name : 'without'} architecture`,
                                                 `${pythonPath ? 'with' : 'without'} python Path`,
                                                 `${
@@ -607,13 +610,34 @@ suite('Interpreters service', () => {
                                                 expect(displayName).to.equal(expectedDisplayName);
                                             });
 
+                                            function buildVersionForDisplay(semVersion: PythonVersion): string {
+                                                let preRelease = '';
+                                                if (semVersion.prerelease.length > 0) {
+                                                    switch (semVersion.prerelease[0]) {
+                                                        case 'alpha':
+                                                            preRelease = `a`;
+                                                            break;
+                                                        case 'beta':
+                                                            preRelease = `b`;
+                                                            break;
+                                                        case 'candidate':
+                                                            preRelease = `rc`;
+                                                            break;
+                                                        case 'final':
+                                                        default:
+                                                            break;
+                                                    }
+                                                }
+                                                return `${semVersion.major}.${semVersion.minor}.${semVersion.patch}${preRelease}`;
+                                            }
+
                                             function buildDisplayName(interpreterInfo: Partial<PythonEnvironment>) {
                                                 const displayNameParts: string[] = ['Python'];
                                                 const envSuffixParts: string[] = [];
 
                                                 if (interpreterInfo.version) {
                                                     displayNameParts.push(
-                                                        `${interpreterInfo.version.major}.${interpreterInfo.version.minor}.${interpreterInfo.version.patch}`,
+                                                        buildVersionForDisplay(interpreterInfo.version),
                                                     );
                                                 }
                                                 if (interpreterInfo.architecture) {

diff --git a/src/test/pythonEnvironments/info/interpreter.unit.test.ts b/src/test/pythonEnvironments/info/interpreter.unit.test.ts
@@ -97,7 +97,7 @@ suite('getInterpreterInfo()', () => {
         const expected = {
             architecture: Architecture.x64,
             path: python.command,
-            version: new SemVer('3.7.5-candidate'),
+            version: new SemVer('3.7.5-candidate1'),
             sysPrefix: '/path/of/sysprefix/versions/3.7.5rc1',
             sysVersion: undefined,
         };
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Ensure release level is set when using new environment discovery component.