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

[Announcement] Breaking changes in .NET 6 for iOS, tvOS and macOS #13087

Closed
rolfbjarne opened this issue Oct 22, 2021 · 42 comments
Closed

[Announcement] Breaking changes in .NET 6 for iOS, tvOS and macOS #13087

rolfbjarne opened this issue Oct 22, 2021 · 42 comments
Assignees
Labels
breaking-change If an issue or a pull request represents a breaking change documentation The issue or pull request is about documentation dotnet An issue or pull request related to .NET (6)
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Oct 22, 2021

In order to fully support the new native types in C# (nint, nuint), we’ve unfortunately realized that we can’t keep compatibility with existing code or assemblies built for Xamarin.iOS or Xamarin.Mac.

The consequences are as follows when upgrading a project to .NET 6:

  1. All code must be recompiled to support .NET 6. Existing assemblies (such as NuGets built for the old TargetFrameworkIdentifier xamarinios10) won’t work and won’t be supported. Non-xamarin specific assets for net4.x, netstandard, netcoreapp, net5.0+, etc. will work fine however.
  2. Existing code might need to be modified. We’ll post a more detailed document with examples of code changes that might be required at a later point.

In addition, we’re taking the opportunity to fix a few mistakes we’ve made in the past, that we’ve been unable able to address because they were breaking changes.

These are the main changes:

  1. Removal of the System.nint and System.nuint types. We’re going to use the nint and nuint types as defined by C# (they are System.IntPtr and System.UIntPtr under the hood).
  2. Removal of the System.nfloat type. We're going to use the System.Runtime.InteropServices.NFloat type instead. For the System.nfloat type we’re probably going to keep the type [1], but move it to a different namespace.
  3. Use a different type than System.IntPtr for handles (the NSObject.Handle property): ObjCRuntime.NativeHandle. This is to avoid confusion between nint/nuint and handles.
  4. Move all the types in the OpenTK namespace elsewhere.

We also have a list of minor changes we’ve wanted to do for a long time: #5107. We’ll be going through this list and implement the ones that make sense and that we have time to complete.

The first preview with these breaking changes will be Preview 11.

Reference: #10508
Reference: dotnet/designs#252
Reference: NuGet/Home#11338

[1]: System.Runtime.InteropServices.NFloat is not a good replacement for System.nfloat, because it’s a very basic type intended only for interop scenarios. For instance, it does not have any operators, so code like “NFloat + NFloat” does not compile. The operators were implemented and back ported to .NET 6, so the concerns with System.Runtime.InteropServices.NFloat went away.

@rolfbjarne rolfbjarne added breaking-change If an issue or a pull request represents a breaking change documentation The issue or pull request is about documentation dotnet An issue or pull request related to .NET (6) labels Oct 22, 2021
@rolfbjarne rolfbjarne added this to the .NET 6 milestone Oct 22, 2021
@rolfbjarne rolfbjarne pinned this issue Oct 22, 2021
@marek-safar
Copy link
Contributor

This also means that we should change precedence rules for net6-ios/tvos/macos TFMs as were designed in https://github.com/dotnet/designs/blob/main/accepted/2021/net6.0-tfms/net6.0-tfms.md.

/cc @mhutch

rolfbjarne added a commit to rolfbjarne/designs that referenced this issue Oct 22, 2021
Context: xamarin/xamarin-macios#13087

We've decided to ship breaking changes with .NET 6 for iOS, macOS and tvOS,
and as such we won't be compatible with existing TFMs. All binaries will have
be recompiled for .NET 6.

I've modified compatibility scenarios that involve iOS or Mac Catalyst to use
Android instead (because the compatibility rules for Android won't change).
@rolfbjarne
Copy link
Member Author

@marek-safar I have a PR for that: dotnet/designs#252

@mattleibow
Copy link
Contributor

If NFloat is missing things, maybe operators should be added to that? What other features need to be added to that to make it work?

Not sure if it is going to have a weird scenario next year where half the stuff is xamarin nfloat and the other is dotnet nfloat.

@jkotas
Copy link

jkotas commented Oct 22, 2021

Can nfloat be replace with double in the public surface to keep things simple? nfloat does not have much advantage anymore now that almost all Apple devices are 64-bit.

@NogginBops
Copy link

This is the time to finally fix the OpenTK issue!
You are doing a binary breaking change here which was the only reason stated for why the OpenTK assembly types where not completely removed from Xamarin when this issue was raised before.
This has been an issue for more than 6 years and it would be soooooo good to finally have it fixed.
It's in the list from #5107 as Move OpenTK types out of platform assemblies.

@Perksey
Copy link

Perksey commented Oct 23, 2021

Thanks for this. To clarify for those unfamiliar with the Xamarin project planning/schedule, where on the .NET 6 shipping schedule does Xamarin "Preview 11" fall?

@Perksey
Copy link

Perksey commented Oct 23, 2021

@NogginBops I believe this is point 3 in the issue description?

@rolfbjarne
Copy link
Member Author

@jkotas

Can nfloat be replace with double in the public surface to keep things simple? nfloat does not have much advantage anymore now that almost all Apple devices are 64-bit.

Yes, it's technically possible to expose double instead of nfloat, but there are still 32-bit iOS devices out there (which we support), and if we add (back) support for the Apple Watch in the future, we'll be running in 32-bit mode there too (there's no 64-bit Apple Watch).

Our feeling is that exposing double instead of nfloat would mean that we're not accurately mirroring the underlying platform, which can lead to unexpected behavior. One example would be if code set a double property on a 32-bit platform, and then read it back, you might get a different value back.

A more complex example would be a matrix of nfloats:

https://github.com/xamarin/xamarin-macios/blob/main/src/CoreGraphics/CGAffineTransform.cs#L39-L45

with numerous math operations defined on it, and where you might get different behavior whether we implement something in managed code:

public CGPoint TransformPoint (CGPoint point)
{
return new CGPoint (xx * point.X + xy * point.Y + x0,
yx * point.X + yy * point.Y + y0);
}

or we P/Invoke into native code:

[DllImport (Constants.CoreGraphicsLibrary)]
extern static CGSize CGSizeApplyAffineTransform (CGSize rect, CGAffineTransform t);
public CGSize TransformSize (CGSize size)
{
return CGSizeApplyAffineTransform (size, this);
}

@rolfbjarne
Copy link
Member Author

@mattleibow

If NFloat is missing things, maybe operators should be added to that? What other features need to be added to that to make it work?

NFloat was designed that way, so I feel it's unlikely to change: dotnet/runtime#13788

Not sure if it is going to have a weird scenario next year where half the stuff is xamarin nfloat and the other is dotnet nfloat.

The fact that it was designed to only show up in the interop layer, and not public API, makes me hope that won't happen.

Additionally we're moving our nfloat type out of the System namespace to lessen the impact if it were to happen.

@rolfbjarne
Copy link
Member Author

@NogginBops

This is the time to finally fix the OpenTK issue!

Correct, that's planned.

@rolfbjarne
Copy link
Member Author

@Perksey

Thanks for this. To clarify for those unfamiliar with the Xamarin project planning/schedule, where on the .NET 6 shipping schedule does Xamarin "Preview 11" fall?

I can't give any specific dates, but Preview 10 will be released around the same time .NET 6 is launched (.NET Conf Nov 9th-11th), so it's the preview after that, and this fall we've been having a new preview every months or so.

@Perksey
Copy link

Perksey commented Oct 25, 2021

Ok cool. So I guess my last question is: will the net6.0-ios (and friends) TFM remain preview for a little while after .NET 6 GA?

@cleardemon
Copy link

Does this mean all the APIs are also being moved from usages of double (not even System.nfloat) to the new C# nfloat? Similarly for int types. I'd imagine there's quite a few overrides that need changing, if that's the case.

there's no 64-bit Apple Watch

Not strictly true, S4 and up are at least dual-core 64-bit ARM SoC (wiki). Series 3 is still 32-bit, though, and supported.

@dalexsoto
Copy link
Member

Ok cool. So I guess my last question is: will the net6.0-ios (and friends) TFM remain preview for a little while after .NET 6 GA?

Correct

@rolfbjarne
Copy link
Member Author

Does this mean all the APIs are also being moved from usages of double (not even System.nfloat) to the new C# nfloat?

No, we're not moving any double or float usages to any variety of nfloat [1] - note that there's no C# nfloat type (like there are C# nint and nuint types), there's only a System.Runtime.InteropServices.NFloat struct in .NET 6, which, as mentioned in the description, isn't a good replacement for our System.nfloat type.

Similarly for int types. I'd imagine there's quite a few overrides that need changing, if that's the case.

Yes, any code that currently uses System.nint and System.nuint will have to migrate to use the C# nint and nuint types.

However, if your code just uses nint and nuint, it should compile just fine without any changes.

there's no 64-bit Apple Watch

Not strictly true, S4 and up are at least dual-core 64-bit ARM SoC (wiki). Series 3 is still 32-bit, though, and supported.

Strictly speaking you're correct. It's a 64-bit CPU, but it runs in a weird 32-bit mode. Applications are basically 32-bit, and as such, we have to treat it as a 32-bit architecture.

[1]: unless it happened to be a mistake to not use nfloat in the first place, but that's quite rare, we've been able to fix most of such mistakes already

@jkotas
Copy link

jkotas commented Oct 25, 2021

Our feeling is that exposing double instead of nfloat would mean that we're not accurately mirroring the underlying platform, which can lead to unexpected behavior. One example would be if code set a double property on a 32-bit platform, and then read it back, you might get a different value back.

Yes, it is a trade-off. We typically opt for more native .NET look-and-feel than to precisely expose platform corner cases in .NET APIs. It is not unusual to see manual casts from double to nfloat in Xamarin code that is not a native .NET look-and-feel. Do you think it is the right trade-off to make people type these manual casts and keep learning about the special nfloat type going forward just to avoid hitting corner cases on watch? (I am fine with you saying that it is the right tradeoff, just making sure that we are asking the right questions.)

@spouliot
Copy link
Contributor

In theory you could use System.Runtime.InteropServices.NFloat in the internal interop code and expose double in the public API. For 32bits support that would require the generator (or manual bindings) to add the extra cast (from float to double). It's likely more work today, but less work in the future (for the team). OTOH it would be easier for consumer from day 1.

WRT Handle you could try to changes NSObject to subclass SafeHandle (so only one class needs to be instantiated per native object). Dealing with non-NSObject INativeObject types (and related runtime assuptions) will be harder... but your move toward using NativeObject subclasses should help (and that could also subclass SafeHandle).

Anyway have fun breaking things :-)

@Therzok
Copy link
Contributor

Therzok commented Oct 25, 2021

Move all the types in the OpenTK namespace elsewhere.

@rolfbjarne Does this mean that OpenTK will have its own registrar, possibly opening up the per-dll-registrar discussion?

If so, having a dll-per-framework model would be great. Currently, there is no way to have only Foundation.dll or AppKit.dll, which would define whether the library can be run as headless, considering the ObjCRuntime is initialized, enabling proper layering of an application on top of Xamarin.Mac.

@rolfbjarne
Copy link
Member Author

@Therzok

Move all the types in the OpenTK namespace elsewhere.

@rolfbjarne Does this mean that OpenTK will have its own registrar, possibly opening up the per-dll-registrar discussion?

No, not in the short term. The per-dll-registrar feature is unrelated to this (and rather unlikely to be completed for a while due to time constraints).

If so, having a dll-per-framework model would be great. Currently, there is no way to have only Foundation.dll or AppKit.dll, which would define whether the library can be run as headless, considering the ObjCRuntime is initialized, enabling proper layering of an application on top of Xamarin.Mac.

A dll per framework is something we've discussed internally in the past, but there are circular references between Apple's frameworks, which makes the implementation rather non-trivial, and the cost was deemed not worth it (which doesn't mean that it can't be brought up again at some point).

@Therzok
Copy link
Contributor

Therzok commented Oct 25, 2021

No, not in the short term. The per-dll-registrar feature is unrelated to this (and rather unlikely to be completed for a while due to time constraints).

👍 I thought it was not going to be slotted for net6, but considered asking, since OpenTK being moved elsewhere means it has to receive static registrar support somehow.

A dll per framework is something we've discussed internally in the past, but there are circular references between Apple's frameworks, which makes the implementation rather non-trivial, and the cost was deemed not worth it (which doesn't mean that it can't be brought up again at some point).

Understood, should I file an issue for this? The split doesn't have to be per-framework, merged assemblies that need AppKit/UIKit and those that don't might be a good compromise.

@rolfbjarne
Copy link
Member Author

@Therzok

A dll per framework is something we've discussed internally in the past, but there are circular references between Apple's frameworks, which makes the implementation rather non-trivial, and the cost was deemed not worth it (which doesn't mean that it can't be brought up again at some point).

Understood, should I file an issue for this? The split doesn't have to be per-framework, merged assemblies that need AppKit/UIKit and those that don't might be a good compromise.

Yes, that would be great.

@tannergooding
Copy link

tannergooding commented Jan 14, 2022

That was a mistake in the sample code

Awesome and I did notice that when looking at the existing surface area.

I've opened dotnet/runtime#63801 to track this and left a couple annotations. Notably there were just conversions missing to/from nuint (UIntPtr), the nint/nuint conversions are "inconsistent" with explicit/implicitness of int/long and uint/ulong, we've previously opted to not extend new types with System.IConvertible, and Xamarin is currently using static readonly where-as static properties likely provide better perf and is the general "goto".

I also called out that we should likely make them inline with Half/Double/Single and have it extend the generic math interfaces that will be moving to "stable" for .NET 7. This would also expose all the math APIs like Sqrt, Min, Max, etc (what looks to be currently covered by NMath in Xamarin).

@bgavrilMS
Copy link

@rolfbjarne - what does this change imply for SDKs which currently target Xamarin.iOS10 ? Will you have any guidance on how to fix compatibility issues?

@rolfbjarne
Copy link
Member Author

@bgavrilMS that will continue to work as-is, there are no breaking changes if you want to target Xamarin.iOS10. The only breaking changes are if you upgrade to .NET.

@bgavrilMS
Copy link

@rolfbjarne - my team owns the identity SDK. We currently target xamarin.ios10 but our customers will want to target NET6.

@rolfbjarne
Copy link
Member Author

@bgavrilMS it's possible to target both at the same time, and ship a single NuGet that supports both.

@bgavrilMS
Copy link

I understand that, and we already target android9.0 and android10.0; I was hoping that there will be a guide that shows us what breaking changes there are and how to mitigate them. I am sure many SDKs will require this.

@rolfbjarne
Copy link
Member Author

@bgavrilMS yes, we're working on a document to explain the changes and how to migrate existing code.

@tannergooding
Copy link

@rolfbjarne, I still have to write tests but dotnet/runtime#64234 implements the necessary surface area and can be pulled down/built locally if you want to do any prototyping.

@mattleibow
Copy link
Contributor

@rolfbjarne due to the changes in the type used for handles, I now get this error: #13867

@rolfbjarne
Copy link
Member Author

@rolfbjarne, I still have to write tests but dotnet/runtime#64234 implements the necessary surface area and can be pulled down/built locally if you want to do any prototyping.

Thanks!

rolfbjarne added a commit that referenced this issue Feb 24, 2022
…pServices.NFloat. (#14197)

* Remove ObjCRuntime.nfloat (in favor of   System.Runtime.InteropServices.NFloat).
* Automatically add a reference to the System.Runtime.InteropServices.Internal
  package, so that developers get the new NFloat API (with operators) we've
  added post .NET 6 (but don't do this for .NET 7).
* Automatically add a global using alias for
  System.Runtime.InteropServices.NFloat -> nfloat. This is not behind the
  usual `ImplicitUsings` condition our other implicit usings are, because
  they're off by default for existing projects, and the main target for the
  global using alias for nfloat is upgraded projects.
* Automatically generate a global using alias (like above) in the generator
  for all code the generator compiles.
* Update xtro entries to reference System.Runtime.InteropServices.NFloat
  instead of ObjCRuntime.nfloat.
* Add a workaround for a hopefully temporary issue with .NET/CoreCLR where the
  wrong runtime pack is selected otherwise (without the new NFloat API, so
  nothing works at runtime).

Ref: #13087
@rolfbjarne
Copy link
Member Author

All the changes we wanted to do have been implemented, so I'm closing this.

The upcoming preview 14 will have all these changes.

Barring extreme circumstances, we won't be doing any more breaking changes for subsequent preview/rc releases.

Regarding nfloat, we were able to add the operators that were missing in System.Runtime.InteropServices.NFloat into .NET 6, so we're going to remove our System.nfloat in favor of System.Runtime.InteropServices.NFloat. Most code will compile without changes, because we're automatically adding a global using to make nfloat mean System.Runtime.InteropServices.NFloat (the most common piece of code that won't compile would be any code that references nfloat using the full typename with namespace, such as: System.nfloat number = 123;)

tj-devel709 pushed a commit to tj-devel709/xamarin-macios that referenced this issue Mar 8, 2022
…pServices.NFloat. (xamarin#14197)

* Remove ObjCRuntime.nfloat (in favor of   System.Runtime.InteropServices.NFloat).
* Automatically add a reference to the System.Runtime.InteropServices.Internal
  package, so that developers get the new NFloat API (with operators) we've
  added post .NET 6 (but don't do this for .NET 7).
* Automatically add a global using alias for
  System.Runtime.InteropServices.NFloat -> nfloat. This is not behind the
  usual `ImplicitUsings` condition our other implicit usings are, because
  they're off by default for existing projects, and the main target for the
  global using alias for nfloat is upgraded projects.
* Automatically generate a global using alias (like above) in the generator
  for all code the generator compiles.
* Update xtro entries to reference System.Runtime.InteropServices.NFloat
  instead of ObjCRuntime.nfloat.
* Add a workaround for a hopefully temporary issue with .NET/CoreCLR where the
  wrong runtime pack is selected otherwise (without the new NFloat API, so
  nothing works at runtime).

Ref: xamarin#13087
@ghost ghost locked as resolved and limited conversation to collaborators Apr 27, 2022
@chamons chamons unpinned this issue May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change If an issue or a pull request represents a breaking change documentation The issue or pull request is about documentation dotnet An issue or pull request related to .NET (6)
Projects
None yet
Development

No branches or pull requests