sdk
sdk copied to clipboard
Fix detection of torn SDK
If I understand correctly, the SDK is torn in case we have an old msbuild and new SDK. So e.g., if $(_MSBuildVersionMajorMinor) is 17.10 and $(_BundledMSBuildVersionMajorMinor) is 17.11 then $(_IsDisjointMSBuildVersion) should be true.
Part of https://github.com/dotnet/sdk/issues/41791.
@jaredpar shouldn't we set this whenever the two values are different? I know the SDK being newer is the majority of the issues but I believe you can still have issues the other way and wouldn't it be easier to do it whenever they were different?
shouldn't we set this whenever the two values are different?
The issue is only with old msbuild + new SDK. Then we get an error like
CSC : warning CS9057: The analyzer assembly '..\dotnet\sdk\8.0.200\Sdks\Microsoft.NET.Sdk.Razor\source- generators\Microsoft.CodeAnalysis.Razor.Compiler.SourceGenerators.dll' references version '4.9.0.0' of the compiler, which is newer than the currently running version '4.8.0.0'.
We don't get that the other way around - with new msbuild and old SDK - that should work fine (the new compiler should be able to load the old generators/analyzers, it's backwards-compatible). It also seems better to use the latest compiler with hopefully fewer bugs than the old one.
It also seems better to use the latest compiler with hopefully fewer bugs than the old one.
True but part of the rationale for this proposal is to ensure that msbuild and dotnet build have a consistent experience. Including bug fixes, even well meaning ones, would mean that we fall right back into them having differing experiences. That is why I prefer saying a torn state is when the versions differ (up or down).
This should probably be merged after https://github.com/dotnet/sdk/pull/41951, because it looks like the current Toolset.Framework installation is broken even in simple cases. I created a new blazor app and trying to build it with msbuild 17.10 and SDK 9p5 (and this disjoint detection fixed), I'm getting the Toolset.Framework PackageReference without a Version specified - so it gets installed as 4.6.0 (the lowest available version) causing even more trouble (more errors about analyzers referencing newer compiler) than without it.
@jaredpar I need a signoff here so I can merge it after https://github.com/dotnet/sdk/pull/41951