From c86868ff5554496bc5692b649fd48c53f14fc09a Mon Sep 17 00:00:00 2001 From: Noah Gilson Date: Tue, 5 Nov 2024 14:42:31 -0800 Subject: [PATCH] Consider Runtime Patch Versions in PATH API (#2008) * Consider Runtime Patch Versions in PATH API The C# extension will now fail if the runtime is not 8.0.10 or higher on mac so we need to support patch version lookup. Sadly we cannot migrate to semver because semver rejects strings such as 8.0 which are allowable in many parts of our code and APIs. This also adds code for SDK lookup but no test since that's harder to test, like remarked in the code. * Fix test * Fix the version logic to not only parse major.minor version We now parse all of the version, dont need to do this logic. --- .../DotnetCoreAcquisitionExtension.test.ts | 13 ++++- .../Acquisition/DotnetConditionValidator.ts | 50 +++++++++++++------ .../src/Acquisition/VersionUtilities.ts | 40 +++++++++++++-- .../src/EventStream/EventStreamEvents.ts | 4 ++ .../src/test/unit/VersionUtilities.test.ts | 13 +++++ 5 files changed, 98 insertions(+), 22 deletions(-) diff --git a/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts b/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts index ecbdb323..142edf16 100644 --- a/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts +++ b/vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts @@ -247,7 +247,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function() } else { - assert.equal(result, undefined, 'find path command returned no undefined if no path matches condition'); + assert.equal(result?.dotnetPath, undefined, 'find path command returned no undefined if no path matches condition'); } } @@ -379,6 +379,17 @@ suite('DotnetCoreAcquisitionExtension End to End', function() } }).timeout(standardTimeoutTime); + test('Find dotnet PATH Command Unmet Runtime Patch Condition', async () => { + // Install 8.0.{LATEST, which will be < 99}, look for 8.0.99 with accepting dotnet gr than or eq to 8.0.99 + // No tests for SDK since that's harder to replicate with a global install and different machine states + if(os.platform() !== 'darwin') + { + await findPathWithRequirementAndInstall('8.0', 'runtime', os.arch(), 'greater_than_or_equal', false, + {version : '8.0.99', mode : 'runtime', architecture : os.arch(), requestingExtensionId : requestingExtensionId} + ); + } + }).timeout(standardTimeoutTime); + test('Install SDK Globally E2E (Requires Admin)', async () => { // We only test if the process is running under ADMIN because non-admin requires user-intervention. if(new FileUtilities().isElevated()) diff --git a/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts b/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts index 98a2a8ac..73f44b36 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts @@ -2,7 +2,6 @@ * Licensed to the .NET Foundation under one or more agreements. * The .NET Foundation licenses this file to you under the MIT license. *--------------------------------------------------------------------------------------------*/ -import { DotnetVersionSpecRequirement } from '../DotnetVersionSpecRequirement'; import { IDotnetFindPathContext } from '../IDotnetFindPathContext'; import { CommandExecutor } from '../Utils/CommandExecutor'; import { ICommandExecutor } from '../Utils/ICommandExecutor'; @@ -26,14 +25,12 @@ export class DotnetConditionValidator implements IDotnetConditionValidator public async dotnetMeetsRequirement(dotnetExecutablePath: string, requirement : IDotnetFindPathContext) : Promise { const availableRuntimes = await this.getRuntimes(dotnetExecutablePath); - const requestedMajorMinor = versionUtils.getMajorMinor(requirement.acquireContext.version, this.workerContext.eventStream, this.workerContext); const hostArch = await this.getHostArchitecture(dotnetExecutablePath, requirement); if(availableRuntimes.some((runtime) => { - const availableVersion = versionUtils.getMajorMinor(runtime.version, this.workerContext.eventStream, this.workerContext); return runtime.mode === requirement.acquireContext.mode && this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && - this.stringVersionMeetsRequirement(availableVersion, requestedMajorMinor, requirement.versionSpecRequirement); + this.stringVersionMeetsRequirement(runtime.version, requirement.acquireContext.version, requirement); })) { return true; @@ -44,8 +41,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator if(availableSDKs.some((sdk) => { // The SDK includes the Runtime, ASP.NET Core Runtime, and Windows Desktop Runtime. So, we don't need to check the mode. - const availableVersion = versionUtils.getMajorMinor(sdk.version, this.workerContext.eventStream, this.workerContext); - return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && this.stringVersionMeetsRequirement(availableVersion, requestedMajorMinor, requirement.versionSpecRequirement); + return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && this.stringVersionMeetsRequirement(sdk.version, requirement.acquireContext.version, requirement); })) { return true; @@ -53,7 +49,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator else { this.workerContext.eventStream.post(new DotnetFindPathDidNotMeetCondition(`${dotnetExecutablePath} did NOT satisfy the conditions: hostArch: ${hostArch}, requiredArch: ${requirement.acquireContext.architecture}, - required version: ${requestedMajorMinor}`)); + required version: ${requirement.acquireContext.version}, required mode: ${requirement.acquireContext.mode}`)); } } @@ -136,29 +132,51 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement return os.platform() === 'win32' ? (await this.executor!.tryFindWorkingCommand([CommandExecutor.makeCommand('chcp', ['65001'])])) !== null : false; } - private stringVersionMeetsRequirement(availableVersion : string, requestedVersion : string, requirement : DotnetVersionSpecRequirement) : boolean + private stringVersionMeetsRequirement(availableVersion : string, requestedVersion : string, requirement : IDotnetFindPathContext) : boolean { const availableMajor = Number(versionUtils.getMajor(availableVersion, this.workerContext.eventStream, this.workerContext)); const requestedMajor = Number(versionUtils.getMajor(requestedVersion, this.workerContext.eventStream, this.workerContext)); + const requestedPatchStr : string | null = requirement.acquireContext.mode !== 'sdk' ? versionUtils.getRuntimePatchVersionString(requestedVersion, this.workerContext.eventStream, this.workerContext) + : versionUtils.getSDKCompleteBandAndPatchVersionString(requestedVersion, this.workerContext.eventStream, this.workerContext); + const requestedPatch = requestedPatchStr ? Number(requestedPatchStr) : null; if(availableMajor === requestedMajor) { const availableMinor = Number(versionUtils.getMinor(availableVersion, this.workerContext.eventStream, this.workerContext)); const requestedMinor = Number(versionUtils.getMinor(requestedVersion, this.workerContext.eventStream, this.workerContext)); - switch(requirement) + if(availableMinor === requestedMinor && requestedPatch) { - case 'equal': - return availableMinor === requestedMinor; - case 'greater_than_or_equal': - return availableMinor >= requestedMinor; - case 'less_than_or_equal': - return availableMinor <= requestedMinor; + const availablePatchStr : string | null = requirement.acquireContext.mode !== 'sdk' ? versionUtils.getRuntimePatchVersionString(availableVersion, this.workerContext.eventStream, this.workerContext) + : versionUtils.getSDKCompleteBandAndPatchVersionString(availableVersion, this.workerContext.eventStream, this.workerContext); + const availablePatch = availablePatchStr ? Number(availablePatchStr) : null; + switch(requirement.versionSpecRequirement) + { + case 'equal': + return availablePatch === requestedPatch; + case 'greater_than_or_equal': + // the 'availablePatch' must exist, since the version is from --list-runtimes or --list-sdks. + return availablePatch! >= requestedPatch; + case 'less_than_or_equal': + return availablePatch! <= requestedPatch; + } + } + else + { + switch(requirement.versionSpecRequirement) + { + case 'equal': + return availableMinor === requestedMinor; + case 'greater_than_or_equal': + return availableMinor >= requestedMinor; + case 'less_than_or_equal': + return availableMinor <= requestedMinor; + } } } else { - switch(requirement) + switch(requirement.versionSpecRequirement) { case 'equal': return false; diff --git a/vscode-dotnet-runtime-library/src/Acquisition/VersionUtilities.ts b/vscode-dotnet-runtime-library/src/Acquisition/VersionUtilities.ts index 19423bce..8eaf7397 100644 --- a/vscode-dotnet-runtime-library/src/Acquisition/VersionUtilities.ts +++ b/vscode-dotnet-runtime-library/src/Acquisition/VersionUtilities.ts @@ -5,7 +5,7 @@ import { IAcquisitionWorkerContext } from './IAcquisitionWorkerContext'; import { IEventStream } from '../EventStream/EventStream'; -import { DotnetFeatureBandDoesNotExistError, DotnetVersionParseEvent, DotnetVersionResolutionError, EventCancellationError } from '../EventStream/EventStreamEvents'; +import { DotnetFeatureBandDoesNotExistError, DotnetInvalidRuntimePatchVersion, DotnetVersionParseEvent, DotnetVersionResolutionError, EventCancellationError } from '../EventStream/EventStreamEvents'; import { getInstallFromContext } from '../Utils/InstallIdUtilities'; const invalidFeatureBandErrorString = `A feature band couldn't be determined for the requested version: `; @@ -78,7 +78,7 @@ export function getFeatureBandFromVersion(fullySpecifiedVersion : string, eventS */ export function getFeatureBandPatchVersion(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string { - return Number(getPatchVersionString(fullySpecifiedVersion, eventStream, context)).toString(); + return Number(getSDKPatchVersionString(fullySpecifiedVersion, eventStream, context)).toString(); } /** @@ -86,7 +86,7 @@ export function getFeatureBandPatchVersion(fullySpecifiedVersion : string, event * @remarks the logic for getFeatureBandPatchVersion, except that it returns '01' or '00' instead of the patch number. * Not meant for public use. */ -export function getPatchVersionString(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string +export function getSDKPatchVersionString(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string { const patch : string | undefined = fullySpecifiedVersion.split('.')?.at(2)?.substring(1)?.split('-')?.at(0); if(patch === undefined || !isNumber(patch)) @@ -100,6 +100,36 @@ export function getPatchVersionString(fullySpecifiedVersion : string, eventStrea return patch } +/** + * + * @param fullySpecifiedVersion the version of the sdk, either fully specified or not, but containing a band definition. + * @returns a single string representing the band and patch version, e.g. 312 in 7.0.312. + */ +export function getSDKCompleteBandAndPatchVersionString(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string +{ + return `${getFeatureBandFromVersion(fullySpecifiedVersion, eventStream, context)}${getSDKPatchVersionString(fullySpecifiedVersion, eventStream, context)}`; +} + +/** + * The runtime version doesn't have a feature band, unlike the SDK. We need to get the patch version from the runtime version. + * It can contain any amount of text after the patch, such as 9.0.0-rc.2.24473.5. We don't process any of that extra text and should ignore it. + * Needs to handle 8, 8.0, etc. This is why we don't use semver. We don't error if there isn't a patch but we should if the patch is invalid. + * Returns null if no patch is in the string (e.g. 8.0). + */ +export function getRuntimePatchVersionString(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string | null +{ + const patch : string | undefined = fullySpecifiedVersion.split('.')?.at(2)?.split('-')?.at(0); + if(patch && !isNumber(patch)) + { + const event = new DotnetInvalidRuntimePatchVersion(new EventCancellationError('DotnetInvalidRuntimePatchVersion', + `The runtime patch version ${patch} from ${fullySpecifiedVersion} is NaN.`), + getInstallFromContext(context)); + eventStream.post(event); + throw event.error; + } + return patch ? patch : null; +} + /** * * @param fullySpecifiedVersion the requested version to analyze. @@ -113,8 +143,8 @@ export function isValidLongFormVersionFormat(fullySpecifiedVersion : string, eve { if(isNonSpecificFeatureBandedVersion(fullySpecifiedVersion) || ( - getPatchVersionString(fullySpecifiedVersion, eventStream, context).length <= 2 && - getPatchVersionString(fullySpecifiedVersion, eventStream, context).length > 1 + getSDKPatchVersionString(fullySpecifiedVersion, eventStream, context).length <= 2 && + getSDKPatchVersionString(fullySpecifiedVersion, eventStream, context).length > 1 ) ) { diff --git a/vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts b/vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts index 393851a3..8008c77c 100644 --- a/vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts +++ b/vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts @@ -487,6 +487,10 @@ export class DotnetFeatureBandDoesNotExistError extends DotnetAcquisitionError { public readonly eventName = 'DotnetFeatureBandDoesNotExistError'; } +export class DotnetInvalidRuntimePatchVersion extends DotnetAcquisitionError { + public readonly eventName = 'DotnetInvalidRuntimePatchVersion'; +} + export class DotnetWSLSecurityError extends DotnetInstallExpectedAbort { public readonly eventName = 'DotnetWSLSecurityError'; } diff --git a/vscode-dotnet-runtime-library/src/test/unit/VersionUtilities.test.ts b/vscode-dotnet-runtime-library/src/test/unit/VersionUtilities.test.ts index 1b6f1d6e..fbcd95fd 100644 --- a/vscode-dotnet-runtime-library/src/test/unit/VersionUtilities.test.ts +++ b/vscode-dotnet-runtime-library/src/test/unit/VersionUtilities.test.ts @@ -59,6 +59,19 @@ suite('Version Utilities Unit Tests', () => { assert.equal(resolver.getFeatureBandPatchVersion(twoDigitPatchVersion, mockEventStream, mockCtx), '21'); }); + test('Get Band+Patch from SDK Version', async () => { + assert.equal(resolver.getSDKCompleteBandAndPatchVersionString(fullySpecifiedVersion, mockEventStream, mockCtx), '201'); + assert.equal(resolver.getSDKCompleteBandAndPatchVersionString(uniqueMajorMinorVersion, mockEventStream, mockCtx), '300'); + assert.equal(resolver.getSDKCompleteBandAndPatchVersionString(twoDigitMajorVersion, mockEventStream, mockCtx), '102'); + assert.equal(resolver.getSDKCompleteBandAndPatchVersionString(twoDigitPatchVersion, mockEventStream, mockCtx), '221'); + }); + + test('Get Patch from Runtime Version', async () => { + assert.equal(resolver.getRuntimePatchVersionString(majorMinorOnly, mockEventStream, mockCtx), null); + assert.equal(resolver.getRuntimePatchVersionString('8.0.10', mockEventStream, mockCtx), '10'); + assert.equal(resolver.getRuntimePatchVersionString('8.0.9-rc.2.24502.A', mockEventStream, mockCtx), '9'); + }); + test('Get Patch from SDK Preview Version', async () => { assert.equal(resolver.getFeatureBandPatchVersion('8.0.400-preview.0.24324.5', mockEventStream, mockCtx), '0'); });