Change Environment.OSVersion to use RtlGetVersion

[stephentoub]

Fixes #33643
cc: @jkotas, @GrabYourPitchforks

[stephentoub]

@davidsh, what is the breaking change concern here? We're worried someone is relying on this returning the wrong version? I'm fine documenting it as such if we think it is, but I'd be interested in examples where we think it's realistically an issue.

[davidsh]

@davidsh, what is the breaking change concern here? We're worried someone is relying on this returning the wrong version

Yes. It's quite possible that someone is using Environment.OSVersion and it will be unexpected to suddenly get different values after upgrading to .NET 5. So, at the very least, this breaking change needs to be documented as such.

[davidsh]

@karelz @danmosemsft

[scalablecory]

GetVersionEx lies to allow forward-compat with apps doing broken version detection. Do we believe Environment.OSVersion is used differently enough that we would avoid the same issue? May want to reach out to Windows team for data.

[stephentoub]

dotnet.exe is manifested to support Windows 10, so if someone is using dotnet app.dll or dotnet run, Environment.OSVersion is already going to return the same value this will. Same goes if they've added an appropriate manifest to their app.

I believe your concerns are the same as David's, and as this is a major release of .NET, even if it is technically breaking, I think it's the right trade-off.

A ton of code is doing its own P/Invoke to get a better version, since the version that Environment.OSVersion returns is often either wrong or inconsistent based on how the app is run. We already use RtlGetVersion in RuntimeInformation:

runtime/src/libraries/System.Runtime.InteropServices.RuntimeInformation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Windows.cs

Line 22 in 0d19a5e

public static string OSDescription => s_osDescription ??= Interop.NtDll.RtlGetVersion();

.NET Framework's mscorlib uses it:
https://referencesource.microsoft.com/#mscorlib/microsoft/win32/win32native.cs,2798
Windows Forms uses it:
https://github.com/dotnet/winforms/blob/084afd290719163557de1b78d85ade45c3aa0e59/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Internals/OsVersion.cs#L19
A bazillion uses of it show up in arbitrary code on GitHub:
https://github.com/search?l=C%23&q=RtlGetVersion&type=Code
Even our public samples use it:
https://github.com/dotnet/aspnetcore/blob/37399bc2175cc253e83bd2d7b2a66e613daad38a/src/MusicStore/samples/MusicStore/Platform.cs#L43
I think it's much better if we just fix Environment.OSVersion to actually be meaningful.

[ericstj]

@stephentoub can you please file a breaking change doc? https://github.com/dotnet/docs/issues/new?template=dotnet-breaking-change.md

[stephentoub]

can you please file a breaking change doc?

Going through my backlog from while I was out, and I see you already took care of this in dotnet/docs#19684. Thanks.