Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore using System.OperatingSystem for API checks #461

Closed
hartez opened this issue Mar 8, 2021 · 7 comments · Fixed by #4908
Closed

Explore using System.OperatingSystem for API checks #461

hartez opened this issue Mar 8, 2021 · 7 comments · Fixed by #4908
Assignees
Labels
area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) fixed-in-6.0.200-preview.14.2 Look for this fix in 6.0.200-preview.14.2! t/app-size Application Size / Trimming t/enhancement ☀️ New feature or request

Comments

@hartez
Copy link
Contributor

hartez commented Mar 8, 2021

We've created some nicer APIs for checking for API levels, OS versions, and API support in MAUI. But those are ported from our Forms APIs. It's possible we should be leveraging System.OperatingSystem, and that NativeVersion should be wrapping the stuff in that namespace.

@hartez hartez added this to Other Work To Define in Stable 1 Backlog Mar 8, 2021
@Redth Redth added this to the .NET 7 milestone Jul 8, 2021
@jsuarezruiz jsuarezruiz added the t/enhancement ☀️ New feature or request label Oct 25, 2021
@jsuarezruiz jsuarezruiz added this to Under consideration in Enhancements Oct 25, 2021
@mattleibow
Copy link
Member

Not only that, the new APIs are linker friendly and will remove impossible code paths. This is probably something we should do sooner than later for linker things - such as app size.

@jonathanpeppers I know you are working on linker things and trimming fixes, have you seen many cases of actual places to benefit?

@mattleibow mattleibow added the t/app-size Application Size / Trimming label Nov 5, 2021
@jonathanpeppers
Copy link
Member

jonathanpeppers commented Nov 5, 2021

Yes, supposedly if you have something like:

if (OperatingSystem.IsAndroidAtLeast(24))
{
    //New APIs
}
else
{
    //Old APIs
}

If your app has SupportedOSPlatformVersion=24, then the linker will remove the else block. Same for all the platforms.

We've got to get all the assemblies trimmable first, though. Otherwise, the linker doesn't trim any Maui assemblies.

Also we can avoid some JNI calls, if we don't use Build.VERSION.SdkInt.

@hartez
Copy link
Contributor Author

hartez commented Dec 29, 2021

We need to verify the minimum versions for each platform. Probably 10 for iOS?

@marek-safar
Copy link

I didn't see the APIs but this sounds like the duplicated effort. There are APIs and tooling included in .NET nowadays that help developers to work with platform-specific APIs, including versioning support on the same platform. You can read about it for example at https://github.com/dotnet/designs/blob/main/accepted/2020/platform-checks/platform-checks.md

@hartez
Copy link
Contributor Author

hartez commented Dec 30, 2021

@marek-safar The point of this issue is to have MAUI use the APIs discussed in the doc you linked.

@hartez hartez moved this from Other Work To Define to Done in Stable 1 Backlog Jan 4, 2022
@Redth Redth removed this from the 6.0.200-preview.13 milestone Jan 30, 2022
@pjcollins pjcollins self-assigned this Feb 10, 2022
@pjcollins
Copy link
Member

@jonathanpeppers you may have already done most (if not all) of this work for Android already? I'm working on some related changes for iOS so I'll add myself to this for now.

@jonathanpeppers
Copy link
Member

@pjcollins yes, Android should be good after we got: 34e83cf

Might be worth checking if any new usage of Build.VERSION.SdkInt appeared, hopefully not?

pjcollins added a commit to pjcollins/maui that referenced this issue Feb 25, 2022
Context: dotnet#461

Checks for macOS High Sierra 10.13 and lower have been removed.  The
macOS Mojave check has been updated to use `IsMacCatalystVersionAtLeast`
or `IsMacOSVersionAtLeast`.

Some other checks for iOS 10 and lower have also been removed as they
should not be needed.
pjcollins added a commit to pjcollins/maui that referenced this issue Feb 25, 2022
Context: dotnet#461

Checks for macOS High Sierra 10.13 and lower have been removed.  The
macOS Mojave check has been updated to use `IsMacOSVersionAtLeast`.

Some other checks for iOS 10 and lower have also been removed as they
should not be needed.
pjcollins added a commit to pjcollins/maui that referenced this issue Feb 25, 2022
Fixes: dotnet#461

Commit 158ffbd accidentially introduced what was likely a breaking
change in `Platform.ios.tvos.watchos.HasOSVersion`.  The call to
`UIDevice.CurrentDevice.CheckSystemVersion` was replaced by
`OperatingSystem.IsIOSVersionAtLeast`, however this function was used
on platforms other than iOS.

Fix this oversight by removing `HasOSVersion` entirely, and use the
correct `Is$(Platform)VersionAtLeast` variant at all call sites instead.
Redth pushed a commit that referenced this issue Feb 25, 2022
Context: #461

Checks for macOS High Sierra 10.13 and lower have been removed.  The
macOS Mojave check has been updated to use `IsMacOSVersionAtLeast`.

Some other checks for iOS 10 and lower have also been removed as they
should not be needed.
@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2022
@samhouts samhouts added the fixed-in-6.0.200-preview.14.2 Look for this fix in 6.0.200-preview.14.2! label Jul 20, 2023
@Eilon Eilon added the area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-publishing Issues with the app packaging/publishing process (ipk/apk/msix/trimming) fixed-in-6.0.200-preview.14.2 Look for this fix in 6.0.200-preview.14.2! t/app-size Application Size / Trimming t/enhancement ☀️ New feature or request
Projects
Enhancements
Under consideration
Development

Successfully merging a pull request may close this issue.

10 participants