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

.NET 6.0 TFMs #174

Merged
merged 24 commits into from
Feb 19, 2021
Merged

.NET 6.0 TFMs #174

merged 24 commits into from
Feb 19, 2021

Conversation

terrajobst
Copy link
Member

No description provided.

@richlander
Copy link
Member

With the context I have, LGTM.

accepted/2021/net6.0-tfms/net6.0-tfms.md Outdated Show resolved Hide resolved
accepted/2021/net6.0-tfms/net6.0-tfms.md Show resolved Hide resolved
accepted/2021/net6.0-tfms/net6.0-tfms.md Outdated Show resolved Hide resolved
accepted/2021/net6.0-tfms/net6.0-tfms.md Outdated Show resolved Hide resolved
accepted/2021/net6.0-tfms/net6.0-tfms.md Outdated Show resolved Hide resolved
@terrajobst terrajobst mentioned this pull request Feb 8, 2021
5 tasks
@terrajobst terrajobst marked this pull request as ready for review February 8, 2021 18:27
@terrajobst terrajobst changed the title Add initial draft for .NET 6.0 TFMs .NET 6.0 TFMs Feb 8, 2021

* It should prefer `xamarin.ios` assets over `netcoreapp` and `netstandard`
assets.
- However, it should still prefer `net5.0`/`net6.0` assets over

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong to me but it could be because I don't understand net5/6. Why would you ever want a less platform specific implementation over a more specific implementation? I would be very surprised to reference a library and find out it can't present UI because the net5/6 was chosen over ios version.

Copy link
Member Author

@terrajobst terrajobst Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xamarin.ios is the old Xamarin TFM. The assumption here is that if a package is updated, it should target iOS/Catalyst using the new TFMs (net6.0-ios and net6.0-maccatalyst respectively). One could argue that net5.0 should be handled like netcoreapp/netstandard but it seems easier to explain in terms of old world/new world and we generally treat net5.0 as that point.

@marek-safar @Redth what do you think?

Copy link

@praeclarum praeclarum Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so the requirement is that if someone updates to net5/6 then they should update their iOS TFMs too? Logical, but not obvious. Gonna have to spread that tip to the community.

Copy link
Member Author

@terrajobst terrajobst Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so the requirement is that if someone updates to net5/6 then they should update their iOS TFMs too? Logical but no obvious.

My assumption, and I could totally be wrong here, is that we probably won't support producing the old Xamarin TFMs from the new tooling. So I'd think once you upgrade the tools to .NET 6, you're naturally already on the new TFMs anyway, which means the confusion would disappear.

Of course, this might not be true for folks hand authoring their NuSpec.

Copy link

@praeclarum praeclarum Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this migration is going to be much slower than you think. I anticipate a lot of my apps will be mixed for years and I would prefer nugets to keep working. I'm thinking there will be a lot of nugets out there with ALL these TFMs. That said, as long as the tooling doesn't regress on the old TFMs, that old code will keep working and it's not a problem.

Edit: @terrajobst I hand author all my nuspecs. 😅

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m pretty satisfied with the arguments here. It’s logical and @Redth seems to have the bases covered.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that there are arguments for preferring platform specific over generational compatibility, but another perspective is why is a fallback, something that might work, being preferred over something that will work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that there are arguments for preferring platform specific over generational compatibility, but another perspective is why is a fallback, something that might work, being preferred over something that will work.

In this case the "something that will work" while technically correct from an API availability perspective, is not really true in practice since the net5.0 assets would not contain the platform specific 'switch' implementation that is required to be useful for these platforms. So, yes, maybe things compile, but at runtime they will throw or otherwise be completely unusable. In many cases in the Xamarin ecosystem, if there are xamarinios assets in a package, it is not just adding value on top of the netstandard assets, but rather it's a full bait and switch where the netstandard assets are really just considered a reference assembly with the public API contracts and stub implementations which throw at runtime.

My recommendations:

  • net6.0-maccatalyst prefers fallback path (in order) of: net6.0, xamarinios, netstandard
  • net6.0-ios prefers the fallback path (in order) of net6.0, xamarinios, netstandard
  • The fallback paths explicitly exclude net5.0 assets

Since Xamarin was never compatible in the net5 generation of TFM's and we're already seeing some packages (such as ReactiveUI) which ship assets for both net5.0 and xamarinios today, it's important that we don't prefer net5.0 assets over xamarinios. If you have a xamarinios project today you'll get xamarinios assets instead of net5.0 assets, and changing this for projects upgraded to net6.0-ios would break/change the expected behaviour.

Copy link
Member Author

@terrajobst terrajobst Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • net6.0-maccatalyst prefers fallback path (in order) of: net6.0, xamarinios, netstandard

This makes sense. I have updated the spec accordingly.

  • net6.0-ios prefers the fallback path (in order) of net6.0, xamarinios, netstandard

That doesn't make sense to me. Why would we want to prefer net6.0 over xamarin.ios? I thought we'd want to treat xamarin.ios as the previous version of net6.0-ios, which is basically how netcoreapp relates to net5.0.

Copy link

@nkolev92 nkolev92 Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably a bit pedantic, but we should avoid using the term fallback here.
Fallback is usually correlated with AssetTargetFallback, and this isn't that. Sounds like the ask is to have explicit compatibility between these frameworks.

`xamarin.ios`.
- It also shouldn't be compatible with any other existing Xamarin TFM (such
as `xamarin.mac`)
* Generate NuGet warning [NU1701] when a `xamarin.ios` asset is being used
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a NuGet perspective this would need to be hard-coded into the product, one of the designs of NET5 was for the SDK & runtime to be able to add new platform implementations without NuGet's support, but that can't be the case here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. As I said on email, I'm not particularly fond of this either. But I can get behind this because this doesn't introduce new crazy rules between the new TFMs but between a new TFM and an old TFM. I can justify that with our obligation to existing customers.

Copy link

@nkolev92 nkolev92 Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If xamarin.ios is preferred over netcoreapp, I think that the warning is not ideal. Same reasoning as above applies, it's not really a fallback, it's just how compatibility is in this case.

Copy link
Member Author

@terrajobst terrajobst Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I see is this:

  1. Silently using the .NET Core assets is unlikely to result in a workable application due to bait & switch
  2. Silently using xamarin.ios, while having a higher chance of working, might not work either because Mac Catalyst isn't 100% compatible with iOS.

So it seems to me the precedence rules + warning is best user experience we can deliver.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given 1, why not warn in the case of .NET Core assets older than 3.1 as well?

The thing that stands out to me is the following (actual names abstracted)

Given A:

A - works
A' - works
B - works (with a warning)
C - not likely to work (no warning).

I know it's an oversimplification, but just curious :)

Copy link
Member Author

@terrajobst terrajobst Feb 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not that netcoreapp/netstandard assets wouldn't work in Mac Catalyst; it's that if a package offers both xamarin.ios and netcoreapp/netstandard then the likelyhood of netcoreapp/netstandard being the right one is low. IOW, its probability isn't just a function of the project TFM and a given offered TFM, it's about which TFM to pick given a set of TFMs. And that's what the precedence rules try to accommodate.

Does that make more sense?


* It should prefer `xamarin.ios` assets over `netcoreapp` and `netstandard`
assets.
- However, it should still prefer `net5.0`/`net6.0` assets over
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that there are arguments for preferring platform specific over generational compatibility, but another perspective is why is a fallback, something that might work, being preferred over something that will work.

TFM automatically compatible with:

1. Itself
2. Earlier versions of itself
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make it clear this list order does not correspond to precedence?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is the default precedence for a regular netX.Y-platform, no?


## Platform support annotations

It seems the Xamarin team wants to have a single assembly with bindings for the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clarify the purpose of having a single assembly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point I was trying to make was less about a single assembly vs multiple but a having a combined set of bindings for Mac-based platforms. Is that not accurate?

@terrajobst terrajobst merged commit 38bffe6 into dotnet:main Feb 19, 2021
@terrajobst terrajobst deleted the net6.0-tfms branch February 19, 2021 17:47
Copy link

@mw-developers mw-developers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of #174

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.