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

Fixed Wasdk breaking on net7+ #125

Merged
merged 5 commits into from
Aug 28, 2023
Merged

Fixed Wasdk breaking on net7+ #125

merged 5 commits into from
Aug 28, 2023

Conversation

Arlodotexe
Copy link
Member

@Arlodotexe Arlodotexe commented Aug 25, 2023

This PR ensures compatibility for net7.0-windows10.0.19041.0 consumers when using the WinAppSDK head, addressing potential issues they might encounter due to TargetFramework mismatches.

Background

NuGet's handling of TargetFrameworks for packages can lead to situations where, despite compatibility, the desired TFM might not be selected by the package manager.

For instance, while a project targeting net7.0-windows10.0.19041.0 can use packages targeting net6.0-windows10.0.19041.0, NuGet's TFM precedence might pick net7.0 over net6.0-windows10.0.19041.0 when the exact match isn't available.

Previously, it was believed that the solution might require downgrading some Uno libraries from net7.0 to netstandard2.0. However, further investigations highlighted that this wouldn't be necessary.

Changes made in this PR:

  • Addition of multi-targeting for WinAppSDK components, allowing them to target both net6.0-windows10.0.19041.0 and net7.0-windows10.0.19041.0.
  • Refactoring of certain conditions to correctly identify and work with the specified target frameworks.

Additional info

Using this tool, you can see that:

  • For tfm compatibility (see here), net7.0-windows10.0.19041.0 projects can use net6.0-windows10.0.19041.0 packages
  • For tfm precedence (see here), the very next candidate after net7.0-windows10.0.19041.0 is net7.0.

@Arlodotexe
Copy link
Member Author

Related - CommunityToolkit/Windows#186

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Aug 25, 2023

Looks like we'll have to wait for WinAppSDK to ship support for net7-windows-* first. We can target net6-windows, but if left as-is our net7.0 Uno tfm will be being picked up instead by consuming projects, if they have Uno.

This arises from how NuGet packages pick TFMs. When the consuming Wasdk head uses net7-windows* but sees net6-windows*, instead of selecting the next lowest platform-specific TFM, it drops the platform and goes for net7.0 instead.

This can only be fixed by waiting for the WinAppSDK to release explicit support for net7-windows*, or explicitly adding support ourselves.

@Arlodotexe Arlodotexe marked this pull request as ready for review August 25, 2023 21:27
@michael-hawker
Copy link
Member

Looks like we'll have to wait for WinAppSDK to ship support for net7-windows-* first. We can target net6-windows, but if left as-is our net7.0 Uno tfm will be being picked up instead by consuming projects, if they have Uno.

Looks like this change is targeting both net6.0-windows* and net7.0-windows* thought that didn't work? The main issue is the conflict from WASM between net7.0 which is why the change to netstandard2.0 should resolve that side. Or is the build only breaking when you have the mixed net6.0-windows*;net7.0-windows* for WASDK AND the net7.0 for WASM?

I'm a bit confused based on what wasn't working for you earlier with our offline conversation...

@Arlodotexe Arlodotexe changed the title Added net7 as Wasdk Tfm Fixed Wasdk breaking in multi-platform solutions Aug 26, 2023
@Arlodotexe
Copy link
Member Author

Arlodotexe commented Aug 26, 2023

Looks like this change is targeting both net6.0-windows* and net7.0-windows* thought that didn't work?

This was removed in the second commit.

We're still using net6.0-windows* for Wasdk here. If we change it to net7.0-windows*, it breaks our build (wasdk doesn't explicitly support this TFM). Same behavior if we try to use both.

For the rest, I've updated the original post with our findings.

@Arlodotexe Arlodotexe changed the title Fixed Wasdk breaking in multi-platform solutions Fixed Wasdk breaking on net7+ Aug 26, 2023
@Arlodotexe
Copy link
Member Author

Thanks @nickrandolph for working with us to find the source of the issue in the first commit so we can ship this in time.

The issue in the first commit was this line. We're checking if the current TargetFramework is contained in the WinAppSdkTargetFramework. The check is too greedy, and picks up net7.0 before it can pick up net7.0-windows*.

I've fixed the issue in efd95db

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Tested 8.0.230828-pull-194.636 packages from CommunityToolkit/Windows#194

Worked like a charm now on Windows/WinUI3, WASM, and even Android!

Still not 100% sure if we can just add to the single class library in the Uno project vs. everything, but swapping it out from my last setup worked fine.

ProjectHeads/Head.WinAppSdk.props Show resolved Hide resolved
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.

2 participants