Fix .NET Version Parsing for FindPath API (#2000)

* Fix Version Parsing for FindPath API

The old comparison used string comparison which is fine for versions of dotnet until dotnet 10. '10.0' < '9.0'. 😂 This fixes that. I suppose it's a bit clunky and we could dedupe / share the compare logic but it's not being used elsewhere in this same form, so I don't plan to do that right now. There are also places that depend on the output versions being strings, so that's a problem to change that now too. It probably should have been a class from the beginning but unfortunately that just isn't how it was initially done, which is understandable because the scope at that time was much more limited.

* Fix some of the code

* Fix test

* Dont comment out the other tests

* update comment

Author: nagilson

vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts

@@ -243,7 +243,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     if(shouldFind)
     {
         assert.exists(result.dotnetPath, 'find path command returned a result');
-        assert.equal(result.dotnetPath, installPath, 'The path returned by findPath is correct');
+        assert.equal(result.dotnetPath.toLowerCase(), installPath.toLowerCase(), 'The path returned by findPath is correct');
     }
     else
     {
@@ -316,13 +316,28 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     }
   }).timeout(standardTimeoutTime);
 
-  test('Find dotnet PATH Command Unmet Version Condition', async () => {
-    // Install 3.1, look for 8.0 which is not less than or equal to 3.1
-    await findPathWithRequirementAndInstall('8.0', 'runtime', os.arch(), 'less_than_or_equal', false,
+  test('Find dotnet PATH Command Met Version Condition', async () => {
+    // Install 8.0, look for 3.1 with accepting dotnet gr than or eq to 3.1
+
+    await findPathWithRequirementAndInstall('8.0', 'runtime', os.arch(), 'greater_than_or_equal', true,
         {version : '3.1', mode : 'runtime', architecture : os.arch(), requestingExtensionId : requestingExtensionId}
     );
   }).timeout(standardTimeoutTime);
 
+  test('Find dotnet PATH Command Met Version Condition with Double Digit Major', async () => {
+    await findPathWithRequirementAndInstall('9.0', 'runtime', os.arch(), 'less_than_or_equal', true,
+        {version : '11.0', mode : 'runtime', architecture : os.arch(), requestingExtensionId : requestingExtensionId}
+    );
+  }).timeout(standardTimeoutTime);
+
+
+  test('Find dotnet PATH Command Unmet Version Condition', async () => {
+    // Install 9.0, look for 90.0 which is not equal to 9.0
+    await findPathWithRequirementAndInstall('9.0', 'runtime', os.arch(), 'equal', false,
+        {version : '90.0', mode : 'runtime', architecture : os.arch(), requestingExtensionId : requestingExtensionId}
+    );
+  }).timeout(standardTimeoutTime);
+
   test('Find dotnet PATH Command Unmet Mode Condition', async () => {
     // look for 3.1 runtime but install 3.1 aspnetcore
     await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch(), 'equal', false,

vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts

@@ -31,9 +31,9 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
 
         if(availableRuntimes.some((runtime) =>
             {
-                const foundVersion = versionUtils.getMajorMinor(runtime.version, this.workerContext.eventStream, this.workerContext);
+                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(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
+                    this.stringVersionMeetsRequirement(availableVersion, requestedMajorMinor, requirement.versionSpecRequirement);
             }))
         {
             return true;
@@ -44,8 +44,8 @@ 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 foundVersion = versionUtils.getMajorMinor(sdk.version, this.workerContext.eventStream, this.workerContext);
-                    return this.stringArchitectureMeetsRequirement(hostArch, requirement.acquireContext.architecture) && this.stringVersionMeetsRequirement(foundVersion, requestedMajorMinor, requirement.versionSpecRequirement);
+                    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 true;
@@ -136,22 +136,38 @@ 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(foundVersion : string, requiredVersion : string, requirement : DotnetVersionSpecRequirement) : boolean
+    private stringVersionMeetsRequirement(availableVersion : string, requestedVersion : string, requirement : DotnetVersionSpecRequirement) : boolean
     {
-        if(requirement === 'equal')
-        {
-            return foundVersion === requiredVersion;
-        }
-        else if(requirement === 'greater_than_or_equal')
+        const availableMajor = Number(versionUtils.getMajor(availableVersion, this.workerContext.eventStream, this.workerContext));
+        const requestedMajor = Number(versionUtils.getMajor(requestedVersion, this.workerContext.eventStream, this.workerContext));
+
+        if(availableMajor === requestedMajor)
         {
-            return foundVersion >= requiredVersion;
+            const availableMinor = Number(versionUtils.getMinor(availableVersion, this.workerContext.eventStream, this.workerContext));
+            const requestedMinor = Number(versionUtils.getMinor(requestedVersion, this.workerContext.eventStream, this.workerContext));
+
+            switch(requirement)
+            {
+                case 'equal':
+                    return availableMinor === requestedMinor;
+                case 'greater_than_or_equal':
+                    return availableMinor >= requestedMinor;
+                case 'less_than_or_equal':
+                    return availableMinor <= requestedMinor;
+            }
         }
-        else if(requirement === 'less_than_or_equal')
+        else
         {
-            return foundVersion <= requiredVersion;
+            switch(requirement)
+            {
+                case 'equal':
+                    return false;
+                case 'greater_than_or_equal':
+                    return availableMajor >= requestedMajor;
+                case 'less_than_or_equal':
+                    return availableMajor <= requestedMajor;
+            }
         }
-
-        return false;
     }
 
     private stringArchitectureMeetsRequirement(outputArchitecture : string, requiredArchitecture : string | null | undefined) : boolean

vscode-dotnet-runtime-library/src/Acquisition/VersionUtilities.ts

@@ -13,14 +13,26 @@ const invalidFeatureBandErrorString = `A feature band couldn't be determined for
 /**
  *
  * @param fullySpecifiedVersion the fully specified version of the sdk, e.g. 7.0.301 to get the major from.
- * @returns the major.minor in the form of '3', etc.
+ * @returns the major in the form of '3', etc.
  */
 export function getMajor(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string
 {
     // The called function will check that we can do the split, so we don't need to check again.
     return getMajorMinor(fullySpecifiedVersion, eventStream, context).split('.')[0];
 }
 
+/**
+ *
+ * @param fullySpecifiedVersion the fully specified version of the sdk, e.g. 7.0.301 to get the minor from.
+ * @returns the major.minor in the form of '0', etc.
+ */
+export function getMinor(fullySpecifiedVersion : string, eventStream : IEventStream, context : IAcquisitionWorkerContext) : string
+{
+    // The called function will check that we can do the split, so we don't need to check again.
+    return getMajorMinor(fullySpecifiedVersion, eventStream, context).split('.')[1];
+}
+
+
 /**
  *
  * @param fullySpecifiedVersion the fully specified version, e.g. 7.0.301 to get the major minor from.

vscode-dotnet-runtime-library/src/DotnetVersionSpecRequirement.ts

@@ -2,6 +2,11 @@
 *  Licensed to the .NET Foundation under one or more agreements.
 *  The .NET Foundation licenses this file to you under the MIT license.
 *--------------------------------------------------------------------------------------------*/
-
+/**
+ * @remarks A condition to be met when searching for .NET. This refers to the major.minor of .NET versions.
+ * When this condition is used, the available version is compared to the required version.
+ * For example, if the request is made looking for 8.0 and allowing 'greater_than_or_equal', then 10.0 would be accepted,
+ * because 10.0 >= 8.0.
+ */
 export type DotnetVersionSpecRequirement = 'equal' | 'greater_than_or_equal' | 'less_than_or_equal';
 

vscode-dotnet-runtime-library/src/test/unit/VersionUtilities.test.ts

@@ -32,6 +32,12 @@ suite('Version Utilities Unit Tests', () => {
         assert.equal(resolver.getMajor(twoDigitMajorVersion, mockEventStream, mockCtx), '10');
     });
 
+    test('Get Minor from SDK Version', async () => {
+        assert.equal(resolver.getMinor(fullySpecifiedVersion, mockEventStream, mockCtx), '0');
+        assert.equal(resolver.getMinor(uniqueMajorMinorVersion, mockEventStream, mockCtx), '1');
+        assert.equal(resolver.getMinor(twoDigitMajorVersion, mockEventStream, mockCtx), '0');
+    });
+
     test('Get Major.Minor from SDK Version', async () => {
         assert.equal(resolver.getMajorMinor(fullySpecifiedVersion, mockEventStream, mockCtx), '7.0');
         assert.equal(resolver.getMajorMinor(featureBandVersion, mockEventStream, mockCtx), '7.0');