sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Fix detection of torn SDK

Open jjonescz opened this issue 1 year ago • 1 comments

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.

jjonescz avatar Jul 02 '24 13:07 jjonescz

@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?

marcpopMSFT avatar Jul 02 '24 20:07 marcpopMSFT

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.

jjonescz avatar Jul 03 '24 07:07 jjonescz

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).

jaredpar avatar Jul 08 '24 03:07 jaredpar

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.

jjonescz avatar Jul 08 '24 07:07 jjonescz

@jaredpar I need a signoff here so I can merge it after https://github.com/dotnet/sdk/pull/41951

jjonescz avatar Jul 11 '24 19:07 jjonescz