Fix IsOSVersionAtLeast when build or revision are not provided

[kotlarmilos]

Description

This PR fixes #108694 by updating IsOSVersionAtLeast to treat unspecified build or revision components as zero.

[am11]

LGTM 👍
It's not a regression so not sure if it is eligible for backport.

[kotlarmilos]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kotlarmilos]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kotlarmilos]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jkotas]

Do the version checks in https://github.com/dotnet/roslyn-analyzers/blob/main/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs need updating to match?

[kotlarmilos]

The current changes ensure that an unspecified build or revision component is treated as zero and introduce additional test cases. I still need to confirm whether ToString includes the build and revision components. Based on that, we may consider applying the same normalization to MacCatalyst for build component to maintain identical behavior with iOS.

[buyaa-n]

Do the version checks in https://github.com/dotnet/roslyn-analyzers/blob/main/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs need updating to match?

I don't think so, not provided version parts always defaulted to 0 for PlatformCompatibilityAnalyzer

[kotlarmilos]

I confirmed that normalizing the build component on MacCatalys will affect the value of Environment.OSVersion.Version.ToString() (18.0 vs. 18.0.0), which would require filling a breaking change notice.

[kotlarmilos]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[filipnavara]

I confirmed that normalizing the build component on MacCatalys will affect the value of Environment.OSVersion.Version.ToString() (18.0 vs. 18.0.0), which would require filling a breaking change notice.

I still think that's a change that we should make, but I don't feel too strongly about it.

The rationale is that it would align the behavior between iOS and Mac Catalyst. The apps are often built from the same source tree just with different TFM, so I would expect the APIs to behave as close as possible. Internally, LLVM version checks are also normalizing the values but I cannot think of any way that would be user exposed.

[kotlarmilos]

I still think that's a change that we should make, but I don't feel too strongly about it.

@jkotas What is the cost of filling a breaking change notice?

Not related to this PR, but something that seems a bit inconsistent. When using the full constructor overload, it requires the build and revision components to be >= 0, otherwise they are unspecified.

This causes an issue on MacCatalyst with the following code:

Version current = Environment.OSVersion.Version;
Version newVersion = new Version(current.Major, current.Minor, current.Build, current.Revision); // This will throw ArgumentOutOfRange exception

[am11]

This causes an issue on MacCatalyst with the following code:

Version current = Environment.OSVersion.Version;
Version newVersion = new Version(current.Major, current.Minor, current.Build, current.Revision); // This will throw ArgumentOutOfRange exception

I think user should check the values in this case, or they can use current.Clone(); to get a copy.

[jkotas]

@jkotas What is the cost of filling a breaking change notice?

https://github.com/dotnet/runtime/blob/main/docs/project/breaking-change-process.md has the instructions

[jkotas]

The rationale is that it would align the behavior between iOS and Mac Catalyst.

Why is the behavior between iOS and Mac Catalyst different at the OS level?

[filipnavara]

Why is the behavior between iOS and Mac Catalyst different at the OS level?

It’s really not different but the version values are retrieved in a different way. Both iOS and macOS use version numbers with 2 or 3 components and we were always normalizing that to 3 (by retrieving the version as NSOperatingSystemVersion and then constructing System.Version from 3 individual numbers).

Mac Catalyst is just macOS app like any other calling the same macOS libraries like a normal Mac app would and having the same structure. The principal difference is that the SDK exposes different parts of the API surface but at the end of the day it still calls the same libraries. There are some quirk flags to make some APIs behave like iOS but that mostly appeared when Apple added support for running unmodified iOS apps on ARM macOS.

When it comes to Mac Catalyst versioning there is a mapping between a macOS version and the supported iOS API surface version. Retrieving NSOperatingSystemVersion would return the macOS version, not the Mac Catalyst version. The iOS support version is stored in an XML PList file where we read it from (same as LLVM does in the support libraries for OS version checks). We get the version as a string and naively pass it to Version.Parse. We just never normalized it to 3 components in this case. If the PList files has version with 2 components then it's parsed as 2.

[dotnet-policy-service]

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.