Clean up minimum Windows version checks

[ladipro]

Context

Per https://docs.microsoft.com/en-us/visualstudio/releases/2022/system-requirements, Visual Studio 2022 is not supported on pre-Windows 10 OS.

Changes Made

Updated and removed Windows version checks accordingly.

Testing

Build and smoke test on Windows 10.

[rainersigwald]

I'm not sure this is ok for .NET Core; .NET 6 is supported on Win7 and 8.1: https://github.com/dotnet/core/blob/main/release-notes/6.0/supported-os.md

[ladipro]

True, but FEATURE_OSVERSION is defined only in .NET Framework builds so the entry point version check + error message should be OK to update. Generally we can assume >= Win7 everywhere in the codebase which led to some dead code removal opportunities.

[Forgind]

Few things:
Can't you also check if Process.GetCurrentProcess().MainModule.FileName is msbuild.exe to know if we're hosted in something else?
We could be hosted in another process but spin out msbuild.exe worker nodes; wouldn't this mean they have the wrong idea?

[ladipro]

Can't you also check if Process.GetCurrentProcess().MainModule.FileName is msbuild.exe to know if we're hosted in something else?

That would also work, with the small downside of potential false positives as anyone can name their process "MSBuild" but it is slower that setting/checking a flag. The host could be calling the Main method to circumvent the check but that's not something we support as a public API so I wouldn't worry about it much.

We could be hosted in another process but spin out msbuild.exe worker nodes; wouldn't this mean they have the wrong idea?

MSBuild.exe has the correct manifest making Windows 10 (and later) not lie about its version, so if we're hosted and spin out an MSBuild.exe process, it's fine to be checking for (10, 0) there.

[Forgind]

Just because VS stops supporting old windows doesn't mean we have to stop as well. Have we posted that we're not supporting old windows either?

[ladipro]

My understanding is that Framework MSBuild doesn't officially ship outside of VS. It looks like in VS 2022 the "Build Tools" SKU doesn't have different minimum system requirements from the full installation either - which was the case in previous versions.

So technically I think it is true that the Framework build supports only 10 and newer. Doesn't of course mean that it doesn't work on Win7 and customers don't depend on such untested configurations.

I agree that the check for minimum Windows version may be too aggressive. Especially since Core still supports Win7 and we don't seem to get much value from restricting Framework to >=10. On the other hand, we definitely don't support Windows 2000 which is what the error message currently says.

How about we unify the minimum version to Win7 everywhere for now? That would get around the ugliness with running in non-manifested exe's and still let us delete some of the ancient code.

[rainersigwald]

My understanding is that Framework MSBuild doesn't officially ship outside of VS.

That's correct, and we should follow the VS system requirements if it's helpful to do so.

How about we unify the minimum version to Win7 everywhere for now?

This is fine by me.

[Forgind]

It looks like this variable is set once and used once—in a function that executes once.

Instead of a new variable, what if we inlined it? In Main:

InitializationException.VerifyThrow(Environement.OSVersion.Platform == PlatformID.Win32NT && Environment.OSVersion.Version.Major >= 10, "UnsupportedOS");

and in Execute:

InitializationException.VerifyThrow(Environement.OSVersion.Platform == PlatformID.Win32NT && (Environment.OSVersion.Version.Major > 6 || (Environment.OSVersion.Version.Major == 6 && Environment.OSVersion.Version.Minor >= 2), "UnsupportedOS");

[ladipro]

I didn't want to move the check to not break something. What you're suggesting would be fine I think, thank you!

I have removed the variable and made us use Win7 as the minimum version, per the rationale above.