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

Support using versions of MSBuild shipped with SDK Previews #144

Closed
baronfel opened this issue Jan 22, 2024 · 20 comments · Fixed by #152
Closed

Support using versions of MSBuild shipped with SDK Previews #144

baronfel opened this issue Jan 22, 2024 · 20 comments · Fixed by #152

Comments

@baronfel
Copy link

baronfel commented Jan 22, 2024

Hello, thanks for this awesome tooling! I work on the .NET SDK and as such have a lot of preview SDK versions installed, including .NET 9 previews. When I run in this scenario, the MSBuild location code throws an exception with the following message:

System.Exception: Cannot locate MSBuild engine for .NET SDK v9.0.100. This probably means that MSBuild Project Tools cannot find the MSBuild for the current project instance. It did find the following version(s), though: [8.0.200, 8.0.101, 7.0.400, 7.0.312, 7.0.115, 6.0.418].
   at MSBuildProjectTools.LanguageServer.Utilities.MSBuildHelper.DiscoverMSBuildEngine(String baseDirectory, ILogger logger)
   at MSBuildProjectTools.LanguageServer.Program.Main()
[Error - 4:13:16 PM] Connection to server got closed. Server will not be restarted.
[Error - 4:13:16 PM] MSBuild Language Service client: couldn't create connection to server.
  Message: Pending response rejected since connection got disposed
  Code: -32097 
[Error - 4:13:16 PM] Server process exited with code 1.

It would be awesome if prereleases could be opted into somehow for this tool!

@tintoy tintoy self-assigned this Jan 22, 2024
@tintoy
Copy link
Owner

tintoy commented Jan 22, 2024

Hi - let me have a look at how we drive the MSBuild discovery and get back to you 🙂

@tintoy
Copy link
Owner

tintoy commented Jan 22, 2024

I suspect the constraint may be a result of how MSBuildLocator works; we basically just ask it for a list of SDKs and try to find the one that matches the current runtime/SDK (as returned by dotnet --info when run in the target directory).

https://github.com/tintoy/msbuild-project-tools-server/blob/e0e84cdd3c929f8f5df02c90bdfdc53d82e4cb7d/src/LanguageServer.Common/Utilities/MSBuildHelper.cs#L90C13-L114C109

If the 9.x SDK doesn't appear in the list of SDKs from that error message, then MSBuildLocator isn't seeing it for some reason.

@tintoy
Copy link
Owner

tintoy commented Jan 22, 2024

It seems unlikely, but I wonder if this could be a result of recent changes we made to use Microsoft's .NET runtime acquisition extension for VS Code...

CC: @DoctorKrolic

@DoctorKrolic
Copy link
Collaborator

This is a limitation of MSBuildLocator - it can only locate SDKs which are compatible with current runtime. Since language server runs on .NET 8, .NET 9 SDKs are not located (and this is clear from exception message - all 8.0.x SDKs and lower are there).
@baronfel Do you have SDK 9.0.100 explicitly specified somewhere (e.g. in a global.json)?
If you do, I don't think there is much we can do on our end to unblock you (there is no public .NET 9 preview yet, so we cannot upgrade to it to unlock preview SDK discovery). In such case you can try overriding which .NET runtime to use and force using your local .NET (extension id is tintoy.msbuild-project-tools).
If you don't have SDK 9.0.100 pinned it's probably our task to select not the latest available SDK, but the latest one MSBuildLocator can discover. I cannot test this myself (again, because no public preview of .NET 9 yet), so I might ask you to test some things if this is the case.

@baronfel
Copy link
Author

Luckily I have some ability to influence what the MSBuild team works on, and we're already looking at loosening the restrictions of the MSBuildLocator APIs - this request would seem to be one more kind of loosening.

@baronfel
Copy link
Author

Also! We make public nightly builds of the SDK installers and zips/tarballs available in a table form on the dotnet/installer repo - that's how many tooling authors get early access to do validation before previews, etc. Feel free to try those if you want to do any smoke tests, that's how we find lots of issues!

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

I suppose we might be able to support overriding the target runtime we request via extension option (providing the .net runtime acquisition extension supports prerelease versions)…

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

(although, as @DoctorKrolic suggests, this can already be overridden in settings for the acquisition extension…)

@baronfel
Copy link
Author

@tintoy the vscode extension doesn't directly support that, no. it's focused on the 'get a stable runtime' scenario (for better or worse). It works fine for truly independent extensions, but this extension IMO is more like Dev Kit and Ionide - you want to bind to the SDK the user has brought so you evaluate code similarly to the MSBuild in the SDK.

We've actually got a new mode of using the .NET install extension that acquires SDKs via the global installers (MSI, pkg, or system package manager on Linux) instead of extension-local installs. That might be a useful avenue to go down. The Dev Kit extension will be onboarding onto that over the next month or so.

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

acquires SDKs via the global installers

Hmm, does that mean something like this?

@DoctorKrolic
Copy link
Collaborator

Embracing .NET runtime extension was mostly pushed by me. The primary motivation was to become more independent from user's .NET. Because previously every time we upgraded language server to a new runtime it could be breaking change for people, thus it limited us from changing it too often (ideally, we needed to stay at the earliest supported runtime). Well, it is not a problem for us now, e.g. I bumped server to .NET 8 without any complaints. But now we break people who use cutting edge SDKs, so we basically exchanged one problem to another. BTW, did you try to force your local runtime as I suggested above? If it worked, maybe there isn't much need to change anything in our side? Because realistically there are very few people who use this extension with cutting edge versions of SDKs compared to the whole potential user base.

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

Either way, if the upstream functionality (for overriding the runtime / SDK / MSBuild version) winds up being implemented I think we’d probably be happy to expose it (perhaps as opt-in behaviour, initially 🙂).

@baronfel
Copy link
Author

baronfel commented Jan 23, 2024

I'm aware of the setting to override the user-local runtime installs (I'm the PM for the VSCode SDK Install extension), I just fundamentally think it's the wrong approach for development-time tools (not to mention we made it a pain in the butt for users that use a bunch of different .NET-based extensions).

I think development-time tools (which I broadly categorize IDE services, this extension, decompilers, etc) should be binding to the users' SDK and runtimes as much as possible, but it seems like at least in this case we have made that difficult (via the MSBuildLocator issue). I think this is 'just' a previews-time issue, but it's one that will continue to recur yearly, so we'll try to make this use case at least possible. Once that happens I'd like to advocate for more extensions moving away from the runtime acquisition extension.

@tintoy the spec for the new global-sdk-install feature is here if you want to take a look! The feature is released in the 2.0.0 version of the extension, but Dev Kit is still working on their implementation.

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

Thanks! That looks interesting; I’ll take a proper look later this morning 🙂

And yes, for what it’s worth I agree that there are issues with the way it works at the moment; as a bit of background for how we arrived at our current design (which does certainly have its limitations), the actual issue we used to see was that we’d just use the SDK from global.json only to find that, on some new preview runtime, either the language server or one of the SDK’s built-in tasks (or similar) would fail due to some API change somewhere between what the language server was built against and what the target SDK was built against…

Still, having the ability to at least try out new runtimes/SDKs would be nice :)

One of my goals this year for MSBuild Project Tools is to set up proper CI (including automated integration tests that target various runtime/SDK combinations) so I’ll be keeping a close eye on this issue.

@baronfel
Copy link
Author

baronfel commented Jan 23, 2024

Totally fair points (and I appreciate the links to the past issues!). Especially now with Dev Kit being A Thing and binding to a user-delivered SDK, MSBuild and the SDK are having to take a more opt-in/conservative stance. Please do yell at us when you see these kinds of issues pop up - you can ping me on specific issues, or email me at chethusk at microsoft dot com if you like.

I also maintain F# IDE Tooling for VSCode and have run into many of these same issues (including setting up multi-version CI matrixes) so I get that it's a giant pain in the butt :)

@tintoy
Copy link
Owner

tintoy commented Jan 23, 2024

Thanks! I appreciate you reaching out to us like this; in some ways this stuff is a lot better than it used to be (which is why most of those issues I linked are pretty old now) and I suspect it’s because folks like you have started doing so.

@baronfel
Copy link
Author

In the latest version of MSBuildLocator there's a static boolean property you can use that should allow the .NET 9 preview runtime to appear in the instances list. Here's an example in a quick F# Interactive session showing the difference (this is on 8.0.200):

Microsoft (R) F# Interactive version 12.8.200.0 for F# 8.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> #r "nuget: Microsoft.Build.Locator, 1.7.1";;
[Loading C:\Users\chusk\.packagemanagement\nuget\Cache\1d6aa12a38294f9f9e823cf4a5a22a2e7e0ab9748dc03420f032c23b079a432a.fsx]
module FSI_0002.
       1d6aa12a38294f9f9e823cf4a5a22a2e7e0ab9748dc03420f032c23b079a432a

> open Microsoft.Build.Locator;;
>  MSBuildLocator.QueryVisualStudioInstances();;
val it: System.Collections.Generic.IEnumerable<VisualStudioInstance> =
  seq
    [Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\8.0.200";
        Name = ".NET Core SDK";
        Version = 8.0.200;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\8.0.200";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\8.0.102";
        Name = ".NET Core SDK";
        Version = 8.0.102;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\8.0.102";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\7.0.400";
        Name = ".NET Core SDK";
        Version = 7.0.400;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\7.0.400";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\7.0.313";
        Name = ".NET Core SDK";
        Version = 7.0.313;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\7.0.313";}; ...]

> MSBuildLocator.AllowQueryAllRuntimeVersions <- true;;
val it: unit = ()

>  MSBuildLocator.QueryVisualStudioInstances();;
val it: System.Collections.Generic.IEnumerable<VisualStudioInstance> =
  seq
    [Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\9.0.100-preview.1.24101.2";
        Name = ".NET Core SDK";
        Version = 9.0.100;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\9.0.100-preview.1.24101.2";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\9.0.100-alpha.1.24067.4";
        Name = ".NET Core SDK";
        Version = 9.0.100;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\9.0.100-alpha.1.24067.4";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\8.0.200";
        Name = ".NET Core SDK";
        Version = 8.0.200;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\8.0.200";};
     Microsoft.Build.Locator.VisualStudioInstance
       {DiscoveryType = DotNetSdk;
        MSBuildPath = "C:\Program Files\dotnet\sdk\8.0.102";
        Name = ".NET Core SDK";
        Version = 8.0.102;
        VisualStudioRootPath = "C:\Program Files\dotnet\sdk\8.0.102";}; ...]

So you can see as soon as I toggled this boolean, this made the preview versions work. Since your server app is a .NET 8 app it's probably pretty safe to allow roll-forward - you could consider some sort of message in this case that could tell the user "hey, we don't have explicit support for this, but if you click this button I can restart the server in a mode that should work. If they clicked that, then you could spawn your server with the following additional env vars:

DOTNET_ROLL_FORWARD=LatestMajor
DOTNET_ROLL_FORWARD_TO_PRERELEASE=1

and things should pretty much work, without requiring you to build with/target .NET 9.

@tintoy
Copy link
Owner

tintoy commented Feb 14, 2024

Thanks - that will be useful!

@DoctorKrolic
Copy link
Collaborator

I finally got some time to try the AllowQueryAllRuntimeVersions flag and unfortunatelly it didn't help much. Yes, the server now starts, but then MSBuild tries to load other related assemblies and fails since these assemblies are built against that newer version of the runtime. In the end this shuts down even such basic features as hover, for instance. So this is not a solution.

the spec for the new global-sdk-install feature is here if you want to take a look!

I am worried about fully global mode. You see, that seems reasonable from our perspective as developers of these tools, but it is our implementation detail, which might not be obvious to the end users. In fact, people might get frustrated if extension, which goal is to provide MSBuild-related tooling, will download and install a global .NET SDK just "because it needs it". Moreover, this can theoretically break some people if they don't pin their SDK version in global.json, so a newer SDK is used, when extension installs it, which might contain breaking changes (although in practice this would be very rarely I guess). I wolud say, "the best of both worlds" behavior can sound like this: it user has globally installed SDK of the requested version or above, use it, otherwise install a local SDK of the requested version. As far as I know, C# extension uses this strategy, but it seems to be a utility behavior, which I expect to be present in the runtime extension itself.

@DoctorKrolic DoctorKrolic transferred this issue from tintoy/msbuild-project-tools-server Mar 11, 2024
@DoctorKrolic
Copy link
Collaborator

I wolud say, "the best of both worlds" behavior can sound like this: it user has globally installed SDK of the requested version or above, use it, otherwise install a local SDK of the requested version. As far as I know, C# extension uses this strategy, but it seems to be a utility behavior, which I expect to be present in the runtime extension itself.

Found this logic in C# extension. They have a whole class responsible for managing runtimes. As I already said, I think this is a utility behavior, so it might make sence to put this logic into the runtime extension itself.
@baronfel What's the vision of runtime extension team on this?

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.

3 participants