Consider Runtime Patch Versions in PATH API (#2008)

nagilson · web-flow · commit c86868ff5554 · 2024-11-05T22:42:31.000Z
* 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.
diff --git 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
@@ -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<boolean>
     {
         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,16 +41,15 @@ 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;
             }
             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
@@ -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,15 +78,15 @@ 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();
 }
 
 /**
  *
  * @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
@@ -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
@@ -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');
     });

Original file line number	Diff line number	Diff line change
`@@ -247,7 +247,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function()`
`247`	`247`	`}`
`248`	`248`	`else`
`249`	`249`	`{`
`250`		`- assert.equal(result, undefined, 'find path command returned no undefined if no path matches condition');`
	`250`	`+ assert.equal(result?.dotnetPath, undefined, 'find path command returned no undefined if no path matches condition');`
`251`	`251`	`}`
`252`	`252`	`}`
`253`	`253`
`@@ -379,6 +379,17 @@ suite('DotnetCoreAcquisitionExtension End to End', function()`
`379`	`379`	`}`
`380`	`380`	`}).timeout(standardTimeoutTime);`
`381`	`381`
	`382`	`+ test('Find dotnet PATH Command Unmet Runtime Patch Condition', async () => {`
	`383`	`+ // Install 8.0.{LATEST, which will be < 99}, look for 8.0.99 with accepting dotnet gr than or eq to 8.0.99`
	`384`	`+ // No tests for SDK since that's harder to replicate with a global install and different machine states`
	`385`	`+ if(os.platform() !== 'darwin')`
	`386`	`+ {`
	`387`	`+ await findPathWithRequirementAndInstall('8.0', 'runtime', os.arch(), 'greater_than_or_equal', false,`
	`388`	`+ {version : '8.0.99', mode : 'runtime', architecture : os.arch(), requestingExtensionId : requestingExtensionId}`
	`389`	`+ );`
	`390`	`+ }`
	`391`	`+ }).timeout(standardTimeoutTime);`
	`392`	`+`
`382`	`393`	`test('Install SDK Globally E2E (Requires Admin)', async () => {`
`383`	`394`	`// We only test if the process is running under ADMIN because non-admin requires user-intervention.`
`384`	`395`	`if(new FileUtilities().isElevated())`
Original file line number	Diff line number	Diff line change
`@@ -487,6 +487,10 @@ export class DotnetFeatureBandDoesNotExistError extends DotnetAcquisitionError {`
`487`	`487`	`public readonly eventName = 'DotnetFeatureBandDoesNotExistError';`
`488`	`488`	`}`
`489`	`489`
	`490`	`+export class DotnetInvalidRuntimePatchVersion extends DotnetAcquisitionError {`
	`491`	`+ public readonly eventName = 'DotnetInvalidRuntimePatchVersion';`
	`492`	`+}`
	`493`	`+`
`490`	`494`	`export class DotnetWSLSecurityError extends DotnetInstallExpectedAbort {`
`491`	`495`	`public readonly eventName = 'DotnetWSLSecurityError';`
`492`	`496`	`}`