Switch the BuildHost to build with net6.0 again #71881
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -9,6 +9,7 @@ The roslyn repository produces components for a number of different products tha | |||
| - Source build: requires us to ship `$(NetCurrent)` and `$(NetPrevious)` in workspaces and below (presently `net9.0` and `net8.0` respectively) | |||
| - Visual Studio: requires us to ship `net472` for base IDE components and `$(NetVisualStudio)` (presently `net8.0`) for private runtime components. | |||
| - Visual Studio Code: expects us to ship against the same runtime as DevKit (presently `net7.0`) to avoid two runtime downloads. | |||
| - MSBuildWorkspace: requires to ship a process that must be usable on the lowest supported SDK (presently `net6.0`) | |||
There was a problem hiding this comment.
I guess the expiration date for this Nov 12th 2024?
https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core#lifecycle
There was a problem hiding this comment.
Well, depends on whether we support officially unsupported things. Which is often case in the tooling world...
There was a problem hiding this comment.
This reminds me that I need to pin an O# announcement about dropping .NET 6/7 support in November.
jasonmalinowski
force-pushed
the
switch-back-to-net6.0-for-buildhost
branch
2 times, most recently
from
03d0fbc to
daaadfc
Compare
jasonmalinowski
force-pushed
the
switch-back-to-net6.0-for-buildhost
branch
from
daaadfc to
6fe03b2
Compare
jasonmalinowski
force-pushed
the
switch-back-to-net6.0-for-buildhost
branch
from
6fe03b2 to
0bba456
Compare
| #if NET472 || NET6_0 // If we're compiling against net472 or net6.0, we get our MemberNotNull from the workspaces assembly. It has it in the net6.0 case since we're consuming the netstandard2.0 version of Workspaces. | ||
| [workspaces::System.Diagnostics.CodeAnalysis.MemberNotNull(nameof(_buildManager))] | ||
| #else // If we're compiling against net7.0 or higher, then we're getting it staright from the framework. |
There was a problem hiding this comment.
I absolutely hate this. If I don't qualify this, then we end up with conflicts since this exists in both the Workspaces DLL (visible via IVT) and also the System.Runtime reference that was getting pulled in some way. I wish there was a way for me to say just-pick-one::MemberNotNull() and the compiler picks one arbitrarily. @jaredpar told me that if I mention this to @333fred and @chsienki that this might make them sad enough to want to address this.
Longer term, the plan is to remove the reference on the Workspaces DLL entirely, since it's mostly just being used for attributes like this, or for our Contract type. Removing the reference entirely would slim down our deploy size and also just generally remove tangles like this. Getting rid of the compiler layer reference too ensures that this is just a simple isolated process with absolutely no references to get messed up with something else.
When we launch the BuildHost we generally use the dotnet.exe on the path, which the assumption that's the one the user is using for their development and that has the right SDKs installed. This means we can run into some fun version issues with VS Code though, where we target the language server on net7.0 (of this writing); in the case that the user has only a 6.0 SDK + runtime installed globally, we'll download a 7.0 runtime, but that's only used for the language server process. The BuildHost will still run with the regular dotnet.exe which needs to support 6.0 runtimes + SDKs. This PR downgrades us to use net6.0 for just the build host. The core change there is trivial but it did result in a few other changes: 1. The Microsoft.Build packages only support net7.0 or higher, so we switch back to the version that we're using for source build which are old enough. Since we only use them as reference assemblies, it doesn't really matter what we pick. 2. 6.0 builds are then missing some attributes we have on higher versions, so we need to start declaring them again.
jasonmalinowski
dismissed
dibarbet’s stale review
A bunch of changes were made.
jasonmalinowski
force-pushed
the
switch-back-to-net6.0-for-buildhost
branch
from
0bba456 to
ad1135e
Compare
| <_NetFrameworkBuildHostProjectReference Include="..\..\..\Workspaces\Core\MSBuild.BuildHost\Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.csproj"> | ||
| <TargetFramework>$(NetVSCode)</TargetFramework> | ||
| <ContentFolderName>BuildHost-netcore</ContentFolderName> | ||
| </_NetFrameworkBuildHostProjectReference> |
There was a problem hiding this comment.
The only difference between this and the next one was the TFM, but now that we have a single variable conditioned elsewhere, we can just remove it from the condition block.
| @@ -4,7 +4,7 @@ | |||
| <PropertyGroup> | |||
| <OutputType>Exe</OutputType> | |||
| <RootNamespace>Microsoft.CodeAnalysis</RootNamespace> | |||
| <TargetFrameworks>$(NetRoslynSourceBuild);net472</TargetFrameworks> | |||
| <TargetFrameworks>$(NetRoslynBuildHostNetCoreVersion);net472</TargetFrameworks> | |||
There was a problem hiding this comment.
This means though that the .NET core build host will be a net6.0 binary which pulls in all other Roslyn components at netstandard2.0. Want to make sure that is understood here.
There was a problem hiding this comment.
Yes, this is understood and fine since we only use a very tiny bit of those other assemblies. #70945 tracks removing the ProjectReferences entirely since we end up using only a tiny bit of code but end up deploying a lot of binaries in the process. I think severing the references too will also help with maintenance here simply because it means the cross-TFM adventures go away and this is a very isolated project.
ghost
added this to the 
When we launch the BuildHost we generally use the dotnet.exe on the path, which the assumption that's the one the user is using for their development and that has the right SDKs installed. This means we can run into some fun version issues with VS Code though, where we target the language server on net7.0 (of this writing); in the case that the user has only a 6.0 SDK + runtime installed globally, we'll download a 7.0 runtime, but that's only used for the language server process. The BuildHost will still run with the regular dotnet.exe which needs to support 6.0 runtimes + SDKs.
This PR downgrades us to use net6.0 for just the build host. The core change there is trivial but it did result in a few other changes: