Add new API `dotnet.findPath` to find the .NET host to use if one already exists that meets a requirement. #1954

nagilson · 2024-09-18T22:15:51Z

Helps resolve:
microsoft/vscode-dotnettools#1466
microsoft/vscode-dotnettools#1243
microsoft/vscode-dotnettools#1094

(CDK and C# will need to adopt this API.)

Adds the command dotnet.findPath to return the path to a dotnet executable or dotnet file to run that is set on the system. This will only be returned if it meets the requirement. This API is meant to search for machine install or runtimes set via environment variables such as PATH or DOTNET_ROOT.

This API takes in a IDotnetFindPathContext which has an Acquire Context. This allows us to edit this API without having to change the acquire API, which uses the IDotnetAcquireContext. This has-a approach is very common in .js.

Callers will add the 'requirement' which allows a .NET version that is 'greater_than_or_equal', 'equal', or 'less_than_or_equal' for major.minor versions that they request.

Here is the algorithm / control flow for the PATH lookup that we decided on with partners. Hopefully this helps explain the code behavior and what it should do.

This will also fix the issue where the vscode path setting dotnet.existingDotnetPath is used by the extensions which will call into this API as they previously had their own path lookup logic which ignored our setting.

The architecture search is not implemented any longer because it is not possible to do it correctly with the existing dotnet host information. We will need to talk to them about this.

I've tested this with various installs, such as snap installs. These installs have a 'polymorphic executable', which means the realpath of which dotnet is actually the snap executable, so the realpath doesn't work. However, there are installs such as the microsoft packages install which installs a symlink path, and that path doesn't work until the realpath is called. This is why we have various path lookup mechanisms, as various installs use different mechanisms.

…scode-dotnet-runtime into nagilson-PATH-setting

…d installation docs, and only in the last resort mention PATH munging.

C# DevKit never uses the path returned by our installation. This means users would think this path would change the sdk that this extension uses but that is not the case. This path to dotnet.exe is meant to be the path for the runtime for extensions to run on, and not the SDK path. It's confusing that the setting was used for both and a misstep in a way. DevKit is the main caller of this API so we think we can change this with minimal breakage.

The setting must be accessed earlier. This means vscode will need to be restarted. We also update the readme and messaging a bit so its more publicly clear in all places what the setting is for.

dotnet --list-runtimes and more need to be called in more places. This is a separate task so it should be done. I did not change the code in any way except for adding the requirement clause type.

… 7.0 SDK on them :zany:

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

sample/package.json

vscode-dotnet-runtime-extension/src/extension.ts

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

joeloff · 2024-09-27T18:22:59Z

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

+    private async getHostArchitecture(hostPath : string) : Promise<string>
+    {
+        return '';
+        /* The host architecture can be inaccurate. Imagine a local runtime install with no host. There is no way to tell the architecture of that runtime,


the host is always included with the runtime, doesn't that make the installation incomplete and therefore invalid?

That's true, this comment (the one in the code) is wrong. However, the host can be there but not return an architecture if it's just a local runtime and a host. I should update this comment.

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

joeloff · 2024-09-27T18:46:41Z

You could probably read the machine type information from the host (COFF or ELF depending on the OS). For the runtimes themselves, reading the COFF header of the DLLs should be sufficient.

I wouldn't use the .version file because not everyone writes the same information. Microsoft.NETCore.App doesn't write architecture information, but the SDK does, so it's not reliable

nagilson · 2024-09-30T20:51:39Z

The architecture suggestion is a good one -- we should add this in the next release along with improvements to parsing the PATH with chinese characters due to a nodejs bug.

@LakshanF

Resolves dotnet#1958 The old application insights key was created by @LakshanF, when we migrated to the new vscode-extension-telemetry service, their API had a breaking change to require a connection string instead of an insights key. dotnet#1948 The connection key can be public and is hard coded. Our existing key has been in our open source, source code for many years. Here is what their guidance says: https://www.npmjs.com/package/@vscode/extension-telemetry > Follow guide to set up Application Insights in Azure and get your connection string. Don't worry about hardcoding it, it is not sensitive.

MiYanni · 2024-09-30T21:07:58Z

sample/src/extension.ts

+                placeHolder: '8.0',
+                value: '8.0',
+                prompt: 'The .NET runtime version.',


nit: extra indentation

MiYanni · 2024-09-30T21:27:55Z

vscode-dotnet-runtime-extension/src/extension.ts

+        const dotnetsOnPATH = await finder.findRawPathEnvironmentSetting();
+        for(const dotnetPath of dotnetsOnPATH ?? [])
+        {
+            const validatedPATH = await getPathIfValid(dotnetPath, validator, commandContext);
+            if(validatedPATH)
+            {
+                outputChannel.show(true);
+                return validatedPATH;
+            }
+        }
+
+        const dotnetOnRealPATH = await finder.findRealPathEnvironmentSetting();
+        for(const dotnetPath of dotnetsOnPATH ?? [])
+        {
+            const validatedRealPATH = await getPathIfValid(dotnetPath, validator, commandContext);
+            if(validatedRealPATH)
+            {
+                outputChannel.show(true);
+                return validatedRealPATH;
+            }
+        }


These both have the same for-loop with only a const named differently. But the calls above the for-loops to findRawPathEnvironmentSetting and findRealPathEnvironmentSetting don't seem to directly influence the loop. Is there something I'm missing?

Ooo youre right, that's a very subtle bug. :) This is why redundant code is bad but it didn't feel like enough to make its own function. Will fix. Thank you.

MiYanni · 2024-09-30T21:35:24Z

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

+    if(shouldFind)
+    {
+        assert.exists(result, 'find path command returned a result');
+        assert.equal(result,  installPath, 'The path returned by findPath is correct');


nit: extra space after the comma

MiYanni · 2024-09-30T21:38:14Z

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

+  /*
+  test('Find dotnet PATH Command Unmet Arch Condition', async () => {
+    // look for a different architecture of 3.1
+    if(os.platform() !== 'darwin')
+    {
+        // The CI Machines are running on ARM64 for OS X.
+        // They also have an x64 HOST. We can't set DOTNET_MULTILEVEL_LOOKUP to 0 because it will break the ability to find the host on --info
+        // As our runtime installs have no host. So the architecture will read as x64 even though it's not.
+        //
+        // This is not fixable until the runtime team releases a better way to get the architecture of a particular dotnet installation.
+        await findPathWithRequirementAndInstall('3.1', 'runtime', os.arch() == 'arm64' ? 'x64' : os.arch(), 'greater_than_or_equal', false,
+            {version : '3.1', mode : 'runtime', architecture : 'arm64', requestingExtensionId : requestingExtensionId}
+        );
+    }
+  }).timeout(standardTimeoutTime);
+  */


Commented out test...?

Yes. This test will work and should be re-added when architecture check is added.

MiYanni · 2024-09-30T21:41:32Z

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

+        ... as the Host will not print its architecture in dotnet info.
+        Return '' for now to pass all arch checks.
+
+        Need to get an issue from the runtime team. See https://github.com/dotnet/sdk/issues/33697 and https://github.com/dotnet/runtime/issues/98735*/


nit: the second link has a * in it, which makes it invalid

nagilson and others added 27 commits September 10, 2024 16:42

update PATH setting description to make it more clear

d028fd6

add very prototype code

b3bf76d

add tests and get runtime check working

f8fa947

Merge branch 'main' into nagilson-PATH-setting

14f2930

Fix some bugs

63f943d

Merge branch 'nagilson-PATH-setting' of https://github.com/nagilson/v…

2e890ee

…scode-dotnet-runtime into nagilson-PATH-setting

Consider that the .NET SDK can also satisfy the Runtime

ccaf4fe

fix typo

baaf910

fix the tests

5ad07f3

fix

1ad0921

fix another test

4e1b2c6

Fix tests even more

e3c23a7

undo yarn changes

c66895b

fix the final test hopefully

61911be

undo yarn changes

78fa558

Dont use dotnet.exe as its not platform agnostic

058f209

use "markdownDescription" for nicer rendering in VSCode

0989525

Tweak call to action to use VSCode mechanisms first, then our standar…

81aa6a0

…d installation docs, and only in the last resort mention PATH munging.

Fix URLs

f119def

Move to version 2.1.7

efc75a5

add basic command'

b43b737

Remove warning setting and fix invalid path setting

a16491a

The setting must be accessed earlier. This means vscode will need to be restarted. We also update the readme and messaging a bit so its more publicly clear in all places what the setting is for.

Fix test

b43a96a

Merge remote-tracking branch 'upstream/main' into nagilson-PATH-setting

57f52ce

Merge branch 'nagilson-PATH-setting' into nagilson-PATH-api

3c385b1

Refactor code out into a Validator for Conditions

2918dfe

dotnet --list-runtimes and more need to be called in more places. This is a separate task so it should be done. I did not change the code in any way except for adding the requirement clause type.

nagilson mentioned this pull request Sep 18, 2024

Update existingDotnetPath to only take affect if it works. #1946

Merged

nagilson added 2 commits September 18, 2024 16:48

Prepare code to validate the path

ac7fa96

Merge remote-tracking branch 'upstream/main' into nagilson-PATH-api

f18bca3

nagilson added 12 commits September 25, 2024 15:14

Restore the env var so we dont edit it for other processes

c502c6b

Uncomment the remaining tests

3428c75

Respond to lint

baf5760

fix callback

2801e1b

Fix path to be os-gnostic

76a553d

Only search for where on windows and also search for which

24cf0e2

provide env to the find command so /usr/bin/whcih can be used

9386062

Call which which instead of which so the correct command can be found

9b803ef

Install 3.1 instead of 7.0 because the DTL CI machines seem to have a…

f34c963

… 7.0 SDK on them :zany:

give up on arch check for now because it is inaccurate, see comment

4289ff9

Add github issue in comment for context

183d0de

make linter happy

1aee50e

nagilson requested a review from a team September 27, 2024 17:40

nagilson commented Sep 27, 2024

View reviewed changes

vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts Show resolved Hide resolved

nagilson commented Sep 27, 2024

View reviewed changes

sample/package.json Show resolved Hide resolved

joeloff reviewed Sep 27, 2024

View reviewed changes

vscode-dotnet-runtime-extension/src/extension.ts Show resolved Hide resolved

joeloff reviewed Sep 27, 2024

View reviewed changes

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

joeloff reviewed Sep 27, 2024

View reviewed changes

vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts Show resolved Hide resolved

Respond to PR feedback

3ea71a7

MiYanni approved these changes Sep 30, 2024

View reviewed changes

Respond to PR Feedback

31c0d5a

nagilson enabled auto-merge (squash) September 30, 2024 22:19

nagilson merged commit 40d1a8b into dotnet:main Sep 30, 2024
9 checks passed

nagilson mentioned this pull request Oct 2, 2024

[Feature Request] Dotnet Host Should Support Programmatic Installation Architecture Scans dotnet/runtime#108503

Closed

nagilson mentioned this pull request Jul 18, 2025

Add dotnetSDKPath Setting for User SDKs and Projects #2354

Open

nagilson mentioned this pull request Aug 4, 2025

Support local SDK Path Setting #2378

Open

Add new API dotnet.findPath to find the .NET host to use if one already exists that meets a requirement. #1954

Add new API dotnet.findPath to find the .NET host to use if one already exists that meets a requirement. #1954

Uh oh!

Conversation

nagilson commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joeloff commented Sep 27, 2024

Uh oh!

nagilson commented Sep 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add new API `dotnet.findPath` to find the .NET host to use if one already exists that meets a requirement. #1954

Add new API `dotnet.findPath` to find the .NET host to use if one already exists that meets a requirement. #1954

nagilson commented Sep 18, 2024 •

edited

Loading