dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.42k stars 4.76k forks source link

[MacCatalyst] OperatingSystem is currently not working #108694

Open rolfbjarne opened 1 month ago

rolfbjarne commented 1 month ago

From @PureWeen on Tue, 08 Oct 2024 20:22:20 GMT

Description

It looks like on MacCatalyst the Build on the Environment.OSVersion.Version is set to "-1" so when you call

if (OperatingSystem.IsMacCatalystVersionAtLeast(18,0))

it resolves to false. This works fine on iOS just not MacCatalyst

Reproduction

aoperatingsystem.zip

Just run the app and I've set a label on the screen to output the value of OSVersion.Version. I've also included some if statements in the main page you can break point

Catalyst

image

iOS

image

Copied from original issue xamarin/xamarin-macios#21390

rolfbjarne commented 1 month ago

From @drasticactions on Wed, 09 Oct 2024 05:31:59 GMT

TL;DR macOS reports 18.0, SDK parses it as a Version which is where the -1 comes from, Runtime check will always report it as below.

https://github.com/drasticactions/MauiRepoRedux/tree/MacCatalystVersion

https://github.com/dotnet/runtime/blob/abde3f9498f09b5016a7a0d7d2e1b81ce5c1b614/src/libraries/System.Private.CoreLib/src/System/OperatingSystem.cs#L227

スクリーンショット 2024-10-09 14 19 32

https://github.com/dotnet/runtime/blob/abde3f9498f09b5016a7a0d7d2e1b81ce5c1b614/src/libraries/Common/src/Interop/OSX/System.Native/Interop.iOSSupportVersion.cs#L11

https://github.com/dotnet/runtime/blob/abde3f9498f09b5016a7a0d7d2e1b81ce5c1b614/src/libraries/System.Private.CoreLib/src/System/OperatingSystem.cs#L328-L341

https://github.com/xamarin/xamarin-macios/blob/0db186c75896a516b409274b45136714230d60ab/src/ObjCRuntime/Runtime.cs#L2688-L2702

image

https://github.com/xamarin/xamarin-macios/blob/0db186c75896a516b409274b45136714230d60ab/tests/common/PlatformInfo.cs#L82

That value is coming from SystemVersion.plist, which is 18.0, hence why it would get -1, since it's Version.Parseed and it doesn't have the Build number that it may have had in the past.

https://github.com/dotnet/runtime/blob/abde3f9498f09b5016a7a0d7d2e1b81ce5c1b614/src/libraries/System.Private.CoreLib/src/System/OperatingSystem.cs#L344

It may be that this needed that the -1 check needs to also be added to build for the same reason as revision, since it can't be assumed that build will be there. Or the SDK code above that fetches the version should always include the build even if the system doesn't provide it.

To work around it, checking against18 0 -1 should work

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

am11 commented 1 month ago

cc @akoeplinger, perhaps we can simplify the parsing something like this https://github.com/dotnet/runtime/compare/main...am11:runtime:patch-15, i.e. only require major version and make other optional (base on -1 val)?

akoeplinger commented 1 month ago

@am11 I think we should fix this in Environment.OSVersion.MacCatalyst.cs instead and default the patch/build version to 0. We already do that for iOS/macOS in GetOperatingSystemVersion in Interop.libobjc.cs

That said, I wonder how this ever worked since e.g. I also don't have a three-part version in /System/Library/CoreServices/SystemVersion.plist on my macOS 14.7 machine and from some quick googling it seems to always have been a two-part string?

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to 'os-maccatalyst': @vitek-karas, @kotlarmilos, @ivanpovazan, @steveisok, @akoeplinger See info in area-owners.md if you want to be subscribed.

akoeplinger commented 1 month ago

@vitek-karas we probably need to backport this.

am11 commented 1 month ago

Yup, whichever solution makes sense. If it's a bit more general, may just cover more platforms. Version.Parse requires two parts (major and minor) at minimum, everything else is optional and result in -1.

kotlarmilos commented 4 weeks ago

The IsOSVersionAtLeast​ fix will be backported to .NET 9 in a servicing release without the breaking change once the release/9.0-staging becomes available.