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.

Sign up for GitHub

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

Jump to bottom

[introspection][dotnet] Enable LegacyAttributes test #11055

Closed
spouliot opened this issue Mar 31, 2021 · 4 comments
Closed

[introspection][dotnet] Enable LegacyAttributes test #11055

spouliot opened this issue Mar 31, 2021 · 4 comments
Assignees
@chamons
Labels
dotnet An issue or pull request related to .NET (6) dotnet-pri3 .NET 6: wishlist for stable release test-only-issue The issue only affects tests
Milestone
.NET 6

Comments

@spouliot
Copy link
Contributor

A new test is being added, but disabled, inside PR#10580. It detects the use of the old, legacy, availability attributes. Those should not be used inside the platform assemblies shipping for dotnet.

However this requires large changes in most manual [1] binding files and is better done in several, distinct PRs.

Once the conversion is complete then the test needs to be enabled to avoid regressions.

https://github.com/xamarin/xamarin-macios/pull/10580/files#diff-0f2fc77f14b962b0c7827112e11d4af3917f549422a5504f311356a0daeaf301R394

#if IOS || TVOS
	[Ignore ("work in progress")]
#endif

The test-side conversion also needs to be removed from tests/common/PlatformInfo.cs

https://github.com/xamarin/xamarin-macios/pull/10580/files#diff-3070012c3058ef2b1ff75465e1394c3c3a03eddeaddc34e8ac35c92e2920383eR147

// FIXME - temporary, while older attributes co-exists (in manual bindings)
if (ca is AvailabilityBaseAttribute old)
	list.Add (old);

[1] the generator handles the conversion to the new dotnet attributes
@spouliot spouliot added the test-only-issue The issue only affects tests label Mar 31, 2021
@spouliot spouliot added this to the .NET 6 milestone Mar 31, 2021
@spouliot spouliot mentioned this issue Mar 31, 2021
Merged
3 tasks
spouliot added a commit that referenced this issue Apr 10, 2021
@spouliot
[generator][dotnet] Add support for `[Uns|S]upportedOSPlatformAttribu…
4e48aa2 
…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
@spouliot spouliot added dotnet An issue or pull request related to .NET (6) dotnet-pri3 .NET 6: wishlist for stable release labels Aug 5, 2021
@spouliot
Copy link
Contributor Author

spouliot commented Aug 5, 2021

Update

MacCatalyst: LegacyAttributes: 1111 API with mixed legacy availability attributes
tvOS: LegacyAttributes : 942 API with mixed legacy availability attributes
iOS: LegacyAttributes : 1236 API with mixed legacy availability attributes

The intersection between them would be quite high.

Mac introspection for dotnet is not yet part of xharness

@spouliot spouliot mentioned this issue Aug 5, 2021
Merged
@rolfbjarne
Copy link
Member

A new test is being added, but disabled, inside PR#10580. It detects the use of the old, legacy, availability attributes. Those should not be used inside the platform assemblies shipping for dotnet.

I wonder if we could rewrite the old availability attributes with a C# source generator?

@spouliot
Copy link
Contributor Author

spouliot commented Aug 6, 2021

Maybe ? 🤔 I had a look a while ago, IIRC...

Generating the new attributes was not the problem - at least with C#9, where most restrictions were removed on partial methods, but still missing on a few things

The partial modifier is not available on delegate or enumeration declarations.
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/partial-classes-and-methods

Removing the old/existing attributes is... but the linker can remove them. However it makes testing missing attributes more difficult for introspection.

@rolfbjarne
Copy link
Member

This was fixed here: e16d8fe

@ghost ghost locked as resolved and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@chamons chamons

Labels
dotnet An issue or pull request related to .NET (6) dotnet-pri3 .NET 6: wishlist for stable release test-only-issue The issue only affects tests
Projects
None yet
Milestone
.NET 6
Development

No branches or pull requests
3 participants
@rolfbjarne @spouliot @chamons