-
Notifications
You must be signed in to change notification settings - Fork 635
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
Restore WAP solution and project for bundle publishing #1349
Conversation
@niels9001, @karkarl I had to revert #1339 because it introduced a new dependency on WinUI 2.7. The Microsoft.UI.Xaml.dll in WinUI 2.7 then overrides the one from WinAppSDK and does not export XamlCheckProcessRequirements, causing a crash at startup. Note that this only occurs with the wapproj build (which had been removed but is now restored in this PR), and not in the single-project build. |
@Scottj1s there shouldn't be any change to the WinUI dependency with the package changes... We've been referencing 2.7 for months: The app should override this right? An app using WinUI 2.8 should be able to use a library built against 2.7? Oh, this is the WinUI 3 gallery, so this is the bug again about multi-targeting for UWP and Windows App SDK in the same package? That was fixed in WASDK 1.3 I thought? This was the internal link for this before related to this issue we had. Maybe this regressed in 1.4, though we just tried a 1.4 app today and didn't have issues. Since this was fixed in 1.3, I re-enabled multi-targeting and we hadn't seen issues elsewhere yet. |
The last commit to this PR is the revert, to make it easy to compare. If you 'nuget restore -force WinUIGallery.DesktopWap.sln' with and without that commit, you see in .\WinUIGallery\obj\WinUIGallery.DesktopWap.Package\project.assets.json that all the new WCT package references now bring in "Microsoft.UI.Xaml": "2.7.0". This isn't a multi-targeting issue - it's a collision of two incompatible microsoft.ui.xaml.dlls. |
@Scottj1s I'm aware that's what happening, I'm saying is that we saw that before pre-1.3 too. It was because our package multitargeted That was supposedly fixed in 1.3, so we added the multi-targeting to our package back in which the latest rc has but not the older labs packages. We haven't seen this issue elsewhere though with the WASDK again with the new packages... |
Note to reviewers: we'll wait until WCT is updated with a fix for the regression, which should be later today, and then update this PR in turn to use that. |
The TFMs look good on the new packages. @niels9001 I think I forgot to ask if you tested them in your wapproj test you had. Think I confused it with the other test we were doing. @Scottj1s would it be best for us to open an issue in the WASDK GitHub for the wapproj problem in general for the future? Also, we decided we should release in the PT AM, so polishing up everything this afternoon and packages should be up on Nuget.org tomorrow with build number |
Tested in a standalone WAP proj, and the changes seem to work :-)! I'll add a PR to re-revert the SettingsCard changes and test it once we have everything on Nuget.org! |
I tested in WinUI 3 Gallery, and the changes work there as well. Yes, let's open an issue in WASDK github to protect against inadvertently combining WinUI 2.7 and WinAppSDK. |
029471c
to
776b8c0
Compare
@niels9001 thanks for the fix! |
/azp run |
(and remove win32)