-
Notifications
You must be signed in to change notification settings - Fork 514
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
Decorate all APIs with correct SupportedOSPlatform attributes #10170
Comments
Beside the two issues above the migration means we lose the helper messages, usually the alternative API to use for obsolete/deprecated attributes. E.g. bindings [Deprecated (PlatformName.iOS, 7, 0, message : "Use 'UbiquitousItemDownloadingStatusKey' instead.")]
[Deprecated (PlatformName.MacOSX, 10, 9, message : "Use 'UbiquitousItemDownloadingStatusKey' instead.")]
[Field ("NSMetadataUbiquitousItemIsDownloadedKey")]
NSString UbiquitousItemIsDownloadedKey { get; } becomes [BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]
static NSString? _UbiquitousItemIsDownloadedKey;
[Field ("NSMetadataUbiquitousItemIsDownloadedKey", "Foundation")]
[UnsupportedOSPlatform ("ios7.0")]
[UnsupportedOSPlatform ("macos10.9")]
public static NSString UbiquitousItemIsDownloadedKey {
...
} |
You could consider using ObsoleteAttribute for that (extended in .net5). |
Unlike both We can use conditionals to work around that limit. That would look like: [SupportedOSPlatform ("macos10.12.2")]
[SupportedOSPlatform ("watchos5.0")]
#if __WATCHOS__
[Obsolete ("API obsoleted in 'watchos7.0': Removed in Xcode 12.")]
#endif
[SupportedOSPlatform ("tvos14.0")]
public unsafe partial class MPMediaEntity : NSObject, INSCoding, INSSecureCoding {
...
} The new Note that we might have case where .NET |
The attribute messages are quote with
Also having a single Before
After
even if the linker will remove all of them inside the final/release bundles. |
This moves our current/legacy attributes to the ones added in dotnet 5 [1]. Short Forms (only in bindings) | Old | New | |---------------------------------------|-------------------------------------| | [iOS (7,0)] | [SupportedOSPlatform ("ios7.0")] | | [NoIOS] | [UnsupportedOSPlatform ("ios")] | Long Forms | Old | New | |---------------------------------------|-------------------------------------| | [Introduced (PlatformName.iOS, 7,0)] | [SupportedOSPlatform ("ios7.0")] | | [Obsoleted (PlatformName.iOS, 12,1)] | [Obsolete (...)] | | [Deprecated (PlatformName.iOS, 14,3)] | [UnsupportedOSPlatform ("ios14.3")] | | [Unavailable (PlatformName.iOS)] | [UnsupportedOSPlatform ("ios")] | Other changes * `[SupportedOSPlatform]` and `[UnsupportedOSPlatform]` are not allowed on `interface` [2] which means they cannot be used for protocols. This is currently handled by inlining the existing attributes on all members. * `[ObsoletedInOSPlatform]` was removed in net5 RC. This PR is now mapping the existing attributes to `[Obsolote]`, however multiple ones cannot be added so they need to be platform specific. Still Missing * [ ] Generator deduplication of type/members attributes for interfaces (due to [2]) * [ ] Manual bindings * [ ] Introspection tests updates References * [1] xamarin#10170 * [2] dotnet/runtime#47599 * [3] dotnet/runtime#47601
…te` (#10580) This moves our current/legacy attributes to the ones added in dotnet 5 [1]. Short Forms (only in bindings) | Old | New | |---------------------------------------|-------------------------------------| | [iOS (7,0)] | [SupportedOSPlatform ("ios7.0")] | | [NoIOS] | [UnsupportedOSPlatform ("ios")] | Long Forms | Old | New | |---------------------------------------|-------------------------------------| | [Introduced (PlatformName.iOS, 7,0)] | [SupportedOSPlatform ("ios7.0")] | | [Obsoleted (PlatformName.iOS, 12,1)] | [Obsolete (...)] | | [Deprecated (PlatformName.iOS, 14,3)] | [UnsupportedOSPlatform ("ios14.3")] | | [Unavailable (PlatformName.iOS)] | [UnsupportedOSPlatform ("ios")] | Other changes * `[SupportedOSPlatform]` and `[UnsupportedOSPlatform]` are not allowed on `interface` [2] which means they cannot be used for protocols. This is currently handled by inlining the existing attributes on all members. * `[ObsoletedInOSPlatform]` was removed in net5 RC. This PR is now mapping the existing attributes to `[Obsolote]`, however multiple ones cannot be added so they need to be platform specific. Remaining work (manual bindings update) tracked in #11055 References * [1] #10170 * [2] dotnet/runtime#47599 * [3] dotnet/runtime#47601
It seems we need to generate the SupportedOSPlatform attribute if there are attributes for other platforms, because otherwise it's implied that the lack of an SupportedOSPlatform attribute means the API isn't available. Ref: https://discord.com/channels/732297728826277939/732297808148824115/859501966349565992 |
Here's the list I have so far:
|
hmm... I thought it would fall back to the assembly-level attribute from [assembly: SupportedOSPlatform ("ios@DOTNET_MIN_IOS_SDK_VERSION@")] into [assembly: SupportedOSPlatform ("ios10.0")] |
A user reported this:
which is a bit weird, because neither Ref: https://discord.com/channels/732297728826277939/732297808148824115/886792528697901057 |
@rolfbjarne not really :) it's right there [Deprecated (PlatformName.MacOSX, 10,15, message: "Use 'Write (out NSError)' instead.")]
[Deprecated (PlatformName.iOS, 13,0, message: "Use 'Write (out NSError)' instead.")]
[Deprecated (PlatformName.WatchOS, 6,0, message: "Use 'Write (out NSError)' instead.")]
[Deprecated (PlatformName.TvOS, 13,0, message: "Use 'Write (out NSError)' instead.")]
[Export ("writeData:")]
void WriteData (NSData data); That becomes an That attribute does not allow us to tell why (the string part) so that should be generated as a separate wrt [Advice ("You must use 'SavePanel' method if the application is sandboxed.")]
[Deprecated (PlatformName.MacOSX, 10, 15, message: "All save panels now run out-of-process, use 'SavePanel' method instead")]
[Export ("init")]
IntPtr Constructor (); so I guess the code is trying to instance the type, which is not something it should do (anymore), and the default .ctor gets mapped to the type (for the compiler warning). |
- The tool I wrote to convert attributes [mellite](https://github.com/chamons/mellite) does single pass through each file - This means files that define Foo and !Foo, each with availability attributes inside can not be proccessed - These 2 locations were easy to fix by moving defines inside binding definations, so I did that - About 10 other locations were not so easy, so those will be worked around by hand in the conversion process. Part of xamarin#10170
- The tool I wrote to convert attributes [mellite](https://github.com/chamons/mellite) does single pass through each file - This means files that define Foo and !Foo, each with availability attributes inside can not be proccessed - These 2 locations were easy to fix by moving defines inside binding definations, so I did that - About 10 other locations were not so easy, so those will be worked around by hand in the conversion process. Part of #10170
- Disable check "[FAIL] Both '{t}' and '{m}' are marked with `{s}`." type and member on that type in NET - Due to the behavior of NET6 style attributes, we are forced to duplicate availability attributes in many cases - See xamarin#10170 for details
- Disable check "[FAIL] Both '{t}' and '{m}' are marked with `{s}`." type and member on that type in NET - Due to the behavior of NET6 style attributes, we are forced to duplicate availability attributes in many cases - See #10170 for details
I have a new proposal: Availability attribute mapping between legacy Xamarin and .NETSuggestion:
The main problem is the following scenario:
If we want to provide a good message to the user when they use the old API (telling them about the new API in iOS 15), we currently have to use the Obsolete attribute. However, this means that the user have to do something like this to have a warning-free build: #pragma warning disable 618
CalliOS14API ();
#pragma warning restore 618 adding a version check won't change anything: if (checkForiOS15) {
CalliOS15API ();
} else {
#pragma warning disable 618
CalliOS14API ();
#pragma warning restore 618
} We need improvements in the .NET API to be able to handle this better. The suggestion above has the advantage that if .NET 7 adds additional API Ref: dotnet/runtime#47601 |
I've started a discussion about improving this in .NET 7: dotnet/runtime#68557 |
So I looked into implementing Rolf's suggestion, (though I think you flipped the advice/obsolete in the table) and I've run into a major problem:
You can only have one Advice per item. Now for our attributes, that's "fine", as I can generate a However, a number of our bindings are using Advice directly:
or indirectly:
And thus mixing a Deprecated (or Obsolete if we aren't flipped) with one of those is non-obvious a compile error. |
My vote is for changing the Advice attribute to AllowMultiple=true.
My thinking was that the Deprecated attribute is "stronger" than the Obsoleted attribute (in that if something is [Deprecated] Apple might reject your app, while [Obsoleted] is just a recommendation for the future), and in the same vein [Obsolete] is "stronger" than [Advice]. I'm fine with either way though. That said, I'm not really sure we really need the distinction between [Deprecated] and [Obsoleted], we seem to use it somewhat interchangeably in the bindings. That's probably a discussion for the future though, this is whole attribute conversion is already complicated enough. |
AFAIK the |
I'm going to finally, after like 5 months of work, call this good enough and done. There are some clean up tasks for the future, but the known work is done, and I was able to test 4 NET sample and a large MAUI app and saw no spurious availability warnings. |
What version is this in? With RC2 I'm still seeing lots of warnings in:
|
Apologies for the delay in response. This only landed 12 days ago (#14877), so I'm not surprised it didn't make it to you 5 days after landing. Let me look into when it should be available publicly. |
Sorry for the trouble, the fix won't hit public until MAUI’s stable release, which will be "soon". |
@chamons thanks for following up and sorry for being "too" eager :-) I was just really happy I could re-instate all my platforms warnings only to find I couldn't... patience... ugh 😁 |
🤞 that it's fixed for you soon @dotMorten - This was one of the most difficult fixes I've made since starting at Xamarin, and required writing custom tools and manually fixing hundreds of locations. If after MAUI stable release you are seeing warnings unnecessarily, please open a new issue. |
@chamons Glad you're getting to do challenging and VERY MUCH APPRECIATED work :-) |
As part of netcore migration, we need to correctly annotate all APIs with .NET 6 to work correctly with existing tooling (primary existing analyzers). The attributes already exist in .NET5 and could be extended if necessary.
Available attributes are listed at https://github.com/dotnet/designs/blob/main/accepted/2020/platform-checks/platform-checks.md#attributes
Required Actions
Generator: read existing attributes but generate the new ones for dotnet#if NET
and hide existing ones under#else
-> in progressTime estimate: 2 weeks.The text was updated successfully, but these errors were encountered: