Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using vscode-dotnet-runtime #89

Closed
tintoy opened this issue Nov 13, 2021 · 11 comments · Fixed by #112 or #141
Closed

Consider using vscode-dotnet-runtime #89

tintoy opened this issue Nov 13, 2021 · 11 comments · Fixed by #112 or #141

Comments

@tintoy
Copy link
Owner

tintoy commented Nov 13, 2021

The vscode-dotnet-runtime extension for VS Code provides support to third-party extensions for downloading and installing various version of the .NET runtime (e.g. to run language servers).

This would enable MSBuild Project tools to transparently change its runtime version dependency without making end-users manually locate, download, and install the required version of the .NET runtime.

@DoctorKrolic
Copy link
Collaborator

Reopening since this work was reverted as a hotfix. We should investigate whether it is possible to isolate extension's runtime while also keeping the ability to reliably check information about .NET on the host and either bring this change back or provide a proof that this is not possible

@DoctorKrolic DoctorKrolic reopened this Nov 25, 2023
@DoctorKrolic DoctorKrolic assigned DoctorKrolic and unassigned tintoy Nov 25, 2023
@tintoy
Copy link
Owner Author

tintoy commented Nov 25, 2023

Check out the 2 links I posted in that other issue - it’s possible to configure the vscode-dotnet extension to use the global .net instance for a given extension:

#137 (comment)

@DoctorKrolic
Copy link
Collaborator

TBH this is the opposite of what I want to achieve with this. My idea originally was to completely isolate .NET runtime, which runs language server, so for the end user there is no impact on his global .NET environment. + it seems that the thing you've suggested works on "user level", meaning that as a user I can configure vscode dotnet runtime extension to provide existing .NET installation for a specific extension id I choose, but it isn't clear whether it is possible to do this through code inside another extension.
I never expected that implementing this will break detection of host .NET though

@tintoy
Copy link
Owner Author

tintoy commented Nov 25, 2023

Oh, I see. Yeah, in that case I am not entirely sure whether this idea is practical; the official .NET SDK discovery logic (and most of its underlying machinery and tooling) uses built-in knowledge from the current runtime to discover SDKs (especially each SDK’s associated MSBuild instance).

I suspect we would be swimming against the tide to try using our own isolated runtime but their global SDK (which we would need to do in order to load their project with correct behaviour and metadata).

I can see why you’d want to do that for a “normal”extension but for one that has to utilise the correct SDK tooling and runtime metadata to mimic what they would see if running stuff from the command line it would be very hard to be sure you’d got it right (and wouldn’t break if they changed undocumented behaviour).

@tintoy
Copy link
Owner Author

tintoy commented Nov 25, 2023

(most of the complexity comes from having to load the correct version of the MSBuild engine and any custom SDKs or tasks they have configured)

@DoctorKrolic
Copy link
Collaborator

Ok, I've done a research which seems to have positive results. My idea is to use 2 well-known environment variables: DOTNET_HOST_PATH and DOTNET_ROOT. The good thing is that these variables are also respected by other components of .NET, most importantly for us is that by setting them we can override default location where MSBuildLocator searches for MSBuild. This is the opposite of what I've suggested doing in tintoy/msbuild-project-tools-server#72, but in this case I think it is a good solution, since all shenanigans with .NET's path are our internal implementation detail and are not in user-controlled settings space. Moreover, this will give potential language server users (tintoy/msbuild-project-tools-server#33) more flexibility since they will be able to choose whether they want to use user's .NET or an isolated runtime with DOTNET_HOST_PATH environment variable pointing to user's distribution of .NET. The server side PR is tintoy/msbuild-project-tools-server#86. Client-side PR is expected to come next

@tintoy
Copy link
Owner Author

tintoy commented Dec 23, 2023

Nice idea! Thanks, will have a look at this tomorrow morning 🙂

@tintoy
Copy link
Owner Author

tintoy commented Dec 24, 2023

I think if we are going to go down this route then we will, at minimum, need to have a working CI pipeline which runs integration-type tests with various SDK / runtime combinations (in containers, for example).

Maybe using TestContainers?

@tintoy
Copy link
Owner Author

tintoy commented Dec 24, 2023

I'm happy to look into the testing side if that would be helpful.

@tintoy
Copy link
Owner Author

tintoy commented Dec 24, 2023

BTW, I can't remember if I showed you before, but our CI runs here:

https://dev.azure.com/tintoy-dev/msbuild-project-tools-vscode/_build?definitionId=2

@tintoy
Copy link
Owner Author

tintoy commented Dec 24, 2023

Ok CI is now running for both language server and extension package:

https://dev.azure.com/tintoy-dev/msbuild-project-tools-vscode/_build/results?buildId=136&view=artifacts&pathAsName=false&type=publishedArtifacts

Later, I'd like to improve extension-package CI to get the language-server artifact from the appropriate GitHub release but this will do for now.

tintoy added a commit that referenced this issue Dec 27, 2023
Note: currently, the extension loads successfully when opening the dev container, but fails to launch the language service.

#141
#89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants